root-event and root-options tests are failing with ROOT6

[Jochem Snuverink]

Copied from issue #171 as reported by @AnneSch:

Start 64: root-options

64: Test command: /space/vol1/root/bin/root "-b" "-q" "-l" "optionsTest.C"
64: Test timeout computed to be: 1500
64: 
64: Processing optionsTest.C...
64: Loading BDSIM Libraries 
64: Loading libbdsimRootEvent 
64: Loading librebdsimLib 
64: BDISM libraries loading complete
64: In file included from OptionsDict dictionary payload:10:
64: /space/vol2/bdsim/analysis/Options.hh:6:10: fatal error: 'BDSOutputROOTEventOptions.hh' file not found
64: #include "BDSOutputROOTEventOptions.hh" 
64: 
64: Error in <TInterpreter::AutoParse>: Error parsing payload code for class Options with content:
64: 
64: #line 1 "OptionsDict dictionary payload"
64: 
64: #ifndef GVECTOR_HAS_CLASS_ITERATOR 
64: #define GVECTOR_HAS_CLASS_ITERATOR 1 
64: #endif 
64: #ifndef ROOTBUILD 
64: #define ROOTBUILD 1 
64: #endif 
64:
64: #define _BACKWARD_BACKWARD_WARNING_H 
64: #include "/space/vol2/bdsim/analysis/Options.hh" 
64:
64: #undef _BACKWARD_BACKWARD_WARNING_H 
64:
64: /space/vol2/bdsim-build/examples/features/analysis/0_output/./createOptions.C:3:20: error: allocation of incomplete type 'Options' 
64: Options o = new Options(); 
64: 
64: OptionsDict dictionary forward declarations' payload:5:90: note: forward declaration of 'Options' 
64: class attribute((annotate("$clingAutoload$/space/vol2/bdsim/analysis/Options.hh"))) Options; 
64:
64: /space/vol2/bdsim-build/examples/features/analysis/0_output/./createOptions.C:4:3: warning: deleting pointer to incomplete type 'Options' may cause undefined behavior [-Wdelete-incomplete] 
64: delete o; 
64: 
64: OptionsDict dictionary forward declarations' payload:5:90: note: forward declaration of 'Options' 
64: class attribute((annotate("$clingAutoload$/space/vol2/bdsim/analysis/Options.hh"))) Options; 
64:
1/1 Test #
64: root-options .....................**Failed Error regular expression found in output. Regex=[Error] 0.68 sec

0% tests passed, 1 tests failed out of 1

Total Test time (real) = 0.87 sec

The following tests FAILED: 64 - root-options (Failed) Errors while running CTest

The header file it is looking for "fatal error: 'BDSOutputROOTEventOptions.hh' file not found" is here:

../bdsim/include/BDSOutputROOTEventOptions.hh

I compiled and run it with ROOT 6.06/04

[Jochem Snuverink]

We also see these tests failing on our test server (just never had looked into it properly):

http://abp-cdash.web.cern.ch/abp-cdash/testDetails.php?test=156084&build=32296

http://abp-cdash.web.cern.ch/abp-cdash/testDetails.php?test=156085&build=32296

My guess is that an includePath needs to be added to the loadLibraries.in file so that BDSOutputROOTEventOptions.hh can be found.

[Laurie Nevay]

I've patched this just now in develop, but I'm leaving this open for now. Our farm build spews out lots and lots of errors related to finding headers with ROOT at run time. This is because it's looking the build dir for the headers at run time.

Stewart made a patch for this to develop that should've fixed this, but it causes run time issues elsewhere. I've undone this for now in analysis/CMakeLists.txt.

[Laurie Nevay]

Assigning to Stewart, but we'll all work on it.

[Laurie Nevay]

As of ROOT V6, ROOT provides CMAKE macros for dictionary generation. Perhaps this would prove more maintainable.

https://root.cern.ch/how/integrate-root-my-project-cmake

All of the developers and most of our known users use ROOT 6 now. Our farm building system is suitable for deployment on many sites, not just at RHUL, and includes ROOT6. If we switch to the cmake functionality provided by root 6, could we drop root 5 support? I'd need to check exactly what cmake support is provided in the latest root 5 release.

[Laurie Nevay]

You can see the source of this macro in your root-install-dir/cmake/modules/RootNewMacros.cmake : line ~330.

[Jochem Snuverink]

Personal comments:

• It would certainly be good to migrate eventually to the ROOT CMake macros instead of our own. Indeed more maintainable. These exist also for ROOT5.
• I don't use ROOT6 myself yet.
• I would prefer it if ROOT5 support is not dropped for a while. We don't have nightly test servers where ROOT6 runs without problems. Also PSI doesn't provide ROOT6 officially, and I suspect there are other labs too.

[Laurie Nevay]

Ok, from a build point of view we'll retain root 5 support.

Stewart mentioned there is some ability to extend compiled classes with user macros in only root6 for analysis purposes that we may use, which would therefore only be available in root6.

[Jochem Snuverink]

I changed my mind a bit regarding ROOT5. I thought this was still well supported but it seems not (still no official new patch that compiles with gcc6 for example). I have moved to ROOT6 now as well. So once we have a working ROOT6 nightly and if there are indeed no users with ROOT5 we could drop support from my point of view.

[Jochem Snuverink]

What is the status on this one by the way? Could we just not add an include path to loadLibraries.C.in:

gSystem->AddIncludePath("-I@CMAKE_SOURCE_DIR@/include");

I imagine that would solve the error originally reported (haven't checked), or is this not desirable?

[Jochem Snuverink]

adding these two lines to loadLibraries.C.in solves the error in the original case:

  gInterpreter->AddIncludePath("-I@CMAKE_SOURCE_DIR@/include");
  gInterpreter->AddIncludePath("-I@CMAKE_SOURCE_DIR@");

Note the gInterpreter instead of the gSystem. Let me know if I should push the changes (also adding the noIncludePaths again).

[Laurie Nevay]

Is this still a problem or can we close the issue?

[Jochem Snuverink]

The tests are fixed yes. But I understood the change you made to fix them was temporary? So far I haven't pushed any of the changes I mentioned above.

[Jochem Snuverink]

On advice of @nevay I have pushed my changes to the branch "issue173" so that @stewartboogert can test them on the new farm build.

[Laurie Nevay]

I've tested it on the EasyBuild build and this didn't fix it in the end, but is a general improvement. I've merged it into develop now as it's been a while and the original problem is fixed. In future we can move to the root cmake macros.

The warnings persist, but do not prevent any functionality.

Also interesting is that the warning points to a header in the build directory.

[Jochem Snuverink]

Maybe we can open another issue for those warnings and close this one?

[Jochem Snuverink]

For reference adding link to relevant commit 8d743b4cb