Fixing compilation and packaging issues #137

mexanick · 2020-02-12T15:21:48Z

Description

Fixing gcc compilation flags and changing the dependencies (current way might be not optimal, however it at least compiles and being packed, which will allow testing.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.

lpetre-ulb

I'm not sure removing dependencies is the right call since the new Makefiles have been tailored for them. It also might be cause of the issues experienced with the templated RPC modules.

lpetre-ulb · 2020-02-13T21:28:42Z

xhal/src/common/utils/XHALXMLParser.cpp

+  //std::auto_ptr<log4cplus::Layout> myLayout = std::auto_ptr<log4cplus::Layout>(new log4cplus::TTCCLayout());
+  //myAppender->setLayout( myLayout );


lpetre-ulb · 2020-02-13T21:30:14Z

xhal/src/client/XHALInterface.cpp

+  //std::auto_ptr<log4cplus::Layout> myLayout = std::auto_ptr<log4cplus::Layout>(new log4cplus::TTCCLayout());
+  //myAppender->setLayout( myLayout );
+#if LOG4CPLUS_VERSION < LOG4CPLUS_MAKE_VERSION(2, 0, 0)
+  myAppender->setLayout(std::auto_ptr<log4cplus::Layout>(new log4cplus::TTCCLayout()));
+#else
+  myAppender->setLayout(std::unique_ptr<log4cplus::Layout>(new log4cplus::TTCCLayout()));
+#endif


I guess these lines can be removed altogether instead of commented. I don't know and when the branching was done, but this compilation issue is fixed in develop by #117.

yeah, that was probably an artifact of a rebase on my part

lpetre-ulb · 2020-02-13T21:33:00Z

xhal/include/packageinfo.h

-#define XHAL_REQUIRED_PACKAGE_LIST reedmuller,xerces-c
-#define XHAL_BASE_REQUIRED_PACKAGE_LIST reedmuller,xerces-c
-#define XHAL_CLIENT_REQUIRED_PACKAGE_LIST xhal-base,reedmuller,xerces-c
-#define XHAL_SERVER_REQUIRED_PACKAGE_LIST xhal-base,reedmuller,xerces-c,lmdb
+#define XHAL_REQUIRED_PACKAGE_LIST 
+#define XHAL_BASE_REQUIRED_PACKAGE_LIST 
+#define XHAL_CLIENT_REQUIRED_PACKAGE_LIST xhal
+#define XHAL_SERVER_REQUIRED_PACKAGE_LIST xhal

-#define XHAL_BUILD_REQUIRED_PACKAGE_LIST reedmuller-devel,gem-peta-stage-ctp7
-#define XHAL_BASE_BUILD_REQUIRED_PACKAGE_LIST reedmuller-devel,gem-peta-stage-ctp7
-#define XHAL_CLIENT_BUILD_REQUIRED_PACKAGE_LIST reedmuller-devel,gem-peta-stage-ctp7
-#define XHAL_SERVER_BUILD_REQUIRED_PACKAGE_LIST reedmuller-devel,lmdb-devel,gem-peta-stage-ctp7
+#define XHAL_BUILD_REQUIRED_PACKAGE_LIST gem-peta-stage-ctp7
+#define XHAL_BASE_BUILD_REQUIRED_PACKAGE_LIST gem-peta-stage-ctp7
+#define XHAL_CLIENT_BUILD_REQUIRED_PACKAGE_LIST gem-peta-stage-ctp7
+#define XHAL_SERVER_BUILD_REQUIRED_PACKAGE_LIST gem-peta-stage-ctp7


I believe that the errors you had earlier are due to the incomplete dependency list. If the new and devel packages are not installed, the headers and libraries won't be at the right location for proper compilation and linking.

The problem is that as is the way the dependencies were specified pulled in some xerces-c package (rather than recognizing that the xdaq bundled version was available), though possibly the lmdb dependency can be re-added.
reedmuller is definitely not should not be a dependency of the updated xhal.
The library already declares a dependency on libxerces, which the xdaq provided package declares a Provides for, this list was to have the dependency get resolved at install time, so if you have a trick to have it pull the non-xdaq version only when the xdaq version isn't installed, that's fine, but the Makefile would still expect that the library were provided in the xdaq tree except for the non-PC builds

That is to say, installation of the runtime package will fail if the xdaq version is not installed, but probably the devel package will install fine, because the xdaq code doesn't use sonames and thus the devel RPM can't declare that it provides the devel library, which is a lever to depend on the devel package without explicitly declaring it.
I suppose this could be explicitly changed to declare a dependency on the xdaq provided packages for the xhal-client* packages and leave things as they were for the xhal-server* packages, but it leaves open the xhal-base dependency, which is maybe OK, considering that it's unlikely that it would be needed anywhere on its own...

Right, my comment was about issues at run-time on the CTP7 with the modified packageinfo.h file (discussed in private). I guess it all goes down to the non state of the art xDAQ packages and the inherent limitations of the Makefile-based build framework (which is not smart enough to find dependencies at configure-time).

Or we also allow the auto dependency on libxerces cause the CTP7 install to fail if it can't find a package which declares that it provides libxerces.so (we've disabled this because it's onerous with the way the packages are declared in the image).
But this failure would indicate a problem with the base image itself (because the image is supposed to provide these libraries for us), rather than anything else...

Since rpmbuild doesn't support cross-compilation and, AFAIK, the rpm tool is not used by Peta Linux, but only used by CMS packages, I believe that building RPM packages for the CTP7 will be complicated in any case. The auto dependency is probably never going to work fine on the CTP7.

The proposal to use a single Git repository for all the GEM backend software and to statically compile the executables and RPC module shared objects deeply simplifies all these issues. In fact, if the whole source code is contained in a single repository, it becomes useless to create packages for cross-compilation development files. xhal could still be reused by other groups and provided as a Git submodule; this distribution method is modern and makes a lot of sense for a mostly header library.

Just a few points:

Building the RPMs for the Zynq is already done (has been for a while)

Installing the RPMs locally was tested and worked

Installing the RPMs from a remotely hosted repo was tested and worked

Dependencies between GEM packages will work, because those are the ones that will be explicitly specified, and those packages will be discoverable by the package manager

Indeed, I remembered these points. I tried to install an RPM on the ULB CTP7 and it went smoothly (with the right RPM...)

However, I did not manage to get the RPM to be persistent across reboots; smart query and rpm -qa both do not report the newly installed RPM after a reboot. Note that obviously the files installed in /mnt/persistent persisted across reboots.

@mexanick, we talked about the reboot persistency concern in one of the latest SW developer meetings. However, I don't remember whether you explicitly tested it or not.

jsturdy · 2020-02-28T15:02:51Z

@mexanick, @lpetre-ulb, was there anything left outstanding from this?

lpetre-ulb · 2020-02-28T23:02:46Z

I'm running into other issues with the updated build system for which I will open issues and/or PR. The way the dependencies are fixed here clearly does not fix the issue, but just remove the functionality. So, I believe this PR should be closed.

mexanick · 2020-02-29T06:13:14Z

superceeded by #139

mexanick added 2 commits February 12, 2020 14:43

Fixing log4cplus (forgotten fix) and compilation flags

370cf33

temporarily fixing dependencies

e25da5f

mexanick added Priority: Critical Type: Bugfix labels Feb 12, 2020

mexanick requested review from jsturdy, cgalloni and lpetre-ulb February 12, 2020 15:21

mexanick self-assigned this Feb 12, 2020

lpetre-ulb reviewed Feb 13, 2020

View reviewed changes

lpetre-ulb mentioned this pull request Feb 29, 2020

Fix compilation issues in the updated build system #139

Merged

3 tasks

mexanick closed this Feb 29, 2020

mexanick deleted the xhal_templated branch February 29, 2020 06:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixing compilation and packaging issues #137

Fixing compilation and packaging issues #137

mexanick commented Feb 12, 2020

lpetre-ulb left a comment

lpetre-ulb Feb 13, 2020

lpetre-ulb Feb 13, 2020

jsturdy Feb 14, 2020

lpetre-ulb Feb 13, 2020

jsturdy Feb 14, 2020

jsturdy Feb 14, 2020

lpetre-ulb Feb 14, 2020 •

edited

Loading

jsturdy Feb 14, 2020

lpetre-ulb Feb 16, 2020

jsturdy Feb 17, 2020

lpetre-ulb Feb 17, 2020

jsturdy commented Feb 28, 2020

lpetre-ulb commented Feb 28, 2020

mexanick commented Feb 29, 2020

		//std::auto_ptr<log4cplus::Layout> myLayout = std::auto_ptr<log4cplus::Layout>(new log4cplus::TTCCLayout());
		//myAppender->setLayout( myLayout );

Fixing compilation and packaging issues #137

Fixing compilation and packaging issues #137

Conversation

mexanick commented Feb 12, 2020

Description

Types of changes

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Checklist:

lpetre-ulb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lpetre-ulb Feb 14, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsturdy commented Feb 28, 2020

lpetre-ulb commented Feb 28, 2020

mexanick commented Feb 29, 2020

lpetre-ulb Feb 14, 2020 •

edited

Loading