iLCSoft / LCIO

Linear Collider I/O
BSD 3-Clause "New" or "Revised" License
17 stars 34 forks source link

ROOTDICT build fails with huge error (conflicting --std specification) #109

Closed misho104 closed 3 years ago

misho104 commented 4 years ago

Hi all, I'll submit an issue report.

reproduction

An easy way to reproduce this issue is to compile LCIO on the ROOT Docker images: docker pull rootproject/root:6.22.02-ubuntu20.04, enter the docker image, and compile the LCIO as

LCIO_VERSION=02-15-02
wget https://github.com/iLCSoft/LCIO/archive/v${LCIO_VERSION}.tar.gz .

rm -rf L* 
tar -xzvf v${LCIO_VERSION}.tar.gz
cd LCIO-${LCIO_VERSION}
mkdir build
cd build
source ${ROOTSYS}/bin/thisroot.sh
cmake -D BUILD_ROOTDICT=ON -D CMAKE_CXX_STANDARD=17 ..
make

Then the build will fail around /opt/LCIO-02-15-02/build/lib/UTIL.cxx with huge (>10000 lines) error message.

diagonosing

The fail is in fact at the command

/usr/bin/c++  -DlcioDict_EXPORTS -I/opt/LCIO-02-15-02/src/cpp/include -I/opt/LCIO-02-15-02/src/cpp/include/pre-generated -I/opt/LCIO-02-15-02/src/cpp -I/opt/LCIO-02-15-02/build/lib -isystem /opt/root/include -isystem /opt/LCIO-02-15-02/sio/include  -fdiagnostics-color=auto -Wl,-no-undefined -Wno-non-virtual-dtor -Wuninitialized -Wno-long-long -pedantic   -std=c++11 -pipe -fsigned-char -pthread -O2 -g -DNDEBUG -fPIC   -std=gnu++17 -o CMakeFiles/lcioDict.dir/__/lib/UTIL.cxx.o -c /opt/LCIO-02-15-02/build/lib/UTIL.cxx

and you can find the std options are conflicting.

Actually, cmake automatically put -std=cxxNN reading /opt/root/cmake/ROOTUseFile.cmake, during the part

IF( BUILD_ROOTDICT )
    FIND_PACKAGE( ROOT 6.04 REQUIRED ) # look for ROOT versions >= 5.27.06
    INCLUDE(${ROOT_USE_FILE})
ENDIF()

in CMakeLists.txt, but then it is overridden by the hand-specified -D CMAKE_CXX_STANDARD=17, which is incompatible with the ROOT libraries.

I'm not sure what is the intention of this specification but if I make it by

cmake -D BUILD_ROOTDICT=ON  ..

the build goes successfully.

Note

This issue can be seen on CVMFS-based system: with the setting

.           /cvmfs/sft.cern.ch/lcg/releases/gcc/9.3.0-6991b/x86_64-centos7/setup.sh
export PATH=/cvmfs/sft.cern.ch/lcg/contrib/CMake/latest/Linux-x86_64/bin:$PATH
.           /cvmfs/sft.cern.ch/lcg/releases/LCG_98python3/ROOT/v6.22.00/x86_64-centos7-gcc9-opt/bin/thisroot.sh

you will see that

cmake -DBUILD_ROOTDICT=ON -DCMAKE_CXX_STANDARD=17 ..
cat ./lcio/CMakeFiles/lcioDict.dir/flags.makecat ./lcio/CMakeFiles/lcioDict.dir/flags.make

will give CXX_FLAGS = -fdiagnostics-color=auto -Wl,-no-undefined -Wno-non-virtual-dtor -Wuninitialized -Wno-long-long -pedantic -Weffc++ -Wshadow -Wextra -Wall -std=c++17 -pipe -fsigned-char -pthread -O2 -g -DNDEBUG -fPIC -std=gnu++17, i.e., -std is doubly specified; the first from ROOT config and the second from the hand specification.

rete commented 4 years ago

Hi @misho104 thanks for reporting and sorry for the late response. I guess the problem is in ROOTUseFile.cmake where the CMAKE_CXX_FLAGS are set by hand, including the CXX standard. This should normally be done using the CMAKE_CXX_STANDARD variable. This is a ROOT issue. Can you please open an issue in ROOT JIRA?

misho104 commented 4 years ago

Thanks, that sounds reasonable.

As ROOT-JIRA says they are now using GitHub Issues, I submit an issue there: https://github.com/root-project/root/issues/6384 so it's nice if you amend my report in case you found some flaw in my report.

andresailer commented 4 years ago

The problem is trying to compile LCIO Root dictionaries with c++17 against a root that wasn't build with that standard. Then things fail when the string_view is encountered (and maybe also because of other things).

In file included from /opt/root/include/TString.h:28,
                 from /opt/root/include/TNamed.h:26,
                 from /opt/root/include/TDictionary.h:44,
                 from /opt/root/include/TClass.h:23,
                 from /opt/LCIO-02-15-02/build/lib/UTIL.cxx:14:
/opt/root/include/ROOT/RStringView.hxx:32:10: error: conflicting declaration of template ‘template<class _CharT, class _Traits> using basic_string_view = std::experimental::__ROOT::basic_string_view<_CharT, _Traits>’
   32 |    using basic_string_view = ::std::experimental::basic_string_view<_CharT,_Traits>;
      |          ^~~~~~~~~~~~~~~~~
In file included from /usr/include/c++/9/bits/basic_string.h:48,
                 from /usr/include/c++/9/string:55,
                 from /opt/root/include/TSchemaHelper.h:17,
                 from /opt/root/include/TGenericClassInfo.h:22,
                 from /opt/root/include/Rtypes.h:189,
                 from /opt/root/include/TObject.h:17,
                 from /opt/root/include/TNamed.h:25,
                 from /opt/root/include/TDictionary.h:44,
                 from /opt/root/include/TClass.h:23,
                 from /opt/LCIO-02-15-02/build/lib/UTIL.cxx:14:
/usr/include/c++/9/string_view:90:11: note: previous declaration ‘template<class _CharT, class _Traits> class std::basic_string_view’
   90 |     class basic_string_view
      |           ^~~~~~~~~~~~~~~~~

Usually gcc will use the latest appearance of an option or flag, so even if there were just a single -std in the command this would still fail.

cmake -D BUILD_ROOTDICT=ON ..

I guess you mean OFF ?

misho104 commented 4 years ago

I guess you mean OFF ?

No. cmake -D BUILD_ROOTDICT=ON .. works because root built with (say) c++14 will automatically give -std=c++14. If we further add by-hand specification -DCMAKE_CXX_STANDARD=17, it causes this problem because stds are doubly specified.

rete pointed out that this is because the way ROOT gives -std is not the "standard" way, where the "standard" way is to specify CMAKE_CXX_STANDARD.

cmake -D BUILD_ROOTDICT=OFF .. works (of course) successfully.

misho104 commented 4 years ago

By the way, even if ROOT follows the standard way, the std set by ROOT will be overridden by -DCMAKE_CXX_STANDARD=17 and I guess it fails in the same problem. So I am guessing that that won't solve the problem. @rete, what do you think? Do we have to use cxx17 to build LCIO? (then ROOT must be built with 17 standard)

rete commented 4 years ago

@misho104 In any case, compiling LCIO with a different standard than ROOT is a very bad idea and will probably fail most of the time as @andresailer explained. If ROOT sets the CXX standard using set( CMAKE_CXX_STANDARD ... ) then all targets created after setting this variable will get the same CXX standard guessed by CMake. This can be c++17 or gnu++17 or anything else depending on compiler features. This is not the case if the CXX standard is part of the CMAKE_CXX_FLAGS... Edit: https://cmake.org/cmake/help/v3.1/variable/CMAKE_CXX_STANDARD.html

misho104 commented 4 years ago

@rete Thanks, so, TL;DR, if ROOT transits to use CMAKE_CXX_STANDARD, LCIO builds will go successfully; currently we can avoid the issue by using

cmake -D BUILD_ROOTDICT=ON  ..

instead of cmake -D BUILD_ROOTDICT=ON -D CMAKE_CXX_STANDARD=17 ...

rete commented 4 years ago

You can also try to modify the ROOT CMake script manually to test whether this works:

and re-run the machinery

rete commented 3 years ago

Hi @misho104 . Any news on this issue ?

misho104 commented 3 years ago

Hi @misho104 . Any news on this issue ?

As far as I know, a PR to fix root-project/root#6384 seems to be merged soon.