podio build on MacOS broken about podio

[andresailer]

It works on linux because -Wl,-no-undefined isn't enabled.

If the event reader have to be part of the dictionary the undefined function implementations can be moved to the header file for example.

Or maybe creating a second dictionary for the readers that is then part of podioRootIO

[gaede]

Thank, Andre,
I guess enabling -Wl,-no-undefined is not really an option, as it would then crash at run time,
wouldn't it ?
In #79 I have moved all virtual functions to ROOTReader.h, which works but is a but ugly.
Maybe in the long run we should consider re-organizing the libraries properly.
Also, I had to add ROOT includes and library to TestDataModel.

[andresailer]

No, the flag is just for the linker, but on clang with a different linker it is a different flag

[petricm]

Try

  if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "AppleClang")
    SET ( CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -Wl,-undefined,error")
  elseif ( ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang") OR ( "${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU" ))
    SET ( CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -Wl,--no-undefined")
  else()
    MESSAGE( WARNING "We do not test with the ${CMAKE_CXX_COMPILER_ID} compiler, use at your own discretion" )
  endif()

[gaede]

This is happily ignored by the linker:

cd /Users/gaede/podio/podio/build/src && /usr/local/Cellar/cmake/3.15.1/bin/cmake -E cmake_link_script CMakeFiles/podioDict.dir/link.txt --verbose=1
/Library/Developer/CommandLineTools/usr/bin/c++  -std=c++14 -stdlib=libc++ -O2 -g -DNDEBUG -dynamiclib -Wl,-headerpad_max_install_names  -Wl,-undefined,error -o libpodioDict.so -install_name @rpath/libpodioDict.so CMakeFiles/podioDict.dir/podioDict.cxx.o -Wl,-rpath,/Users/gaede/podio/podio/build/src -Wl,-rpath,/data/ilcsoft/root/6.18.04/lib libpodio.so /data/ilcsoft/root/6.18.04/lib/libTree.so /data/ilcsoft/root/6.18.04/lib/libImt.so /data/ilcsoft/root/6.18.04/lib/libNet.so /data/ilcsoft/root/6.18.04/lib/libRIO.so /data/ilcsoft/root/6.18.04/lib/libThread.so /data/ilcsoft/root/6.18.04/lib/libCore.so 
Undefined symbols for architecture x86_64:
  "podio::ROOTReader::~ROOTReader()", referenced from:
      ROOT::delete_podiocLcLPythonEventStore(void*) in podioDict.cxx.o
      ROOT::deleteArray_podiocLcLPythonEventStore(void*) in podioDict.cxx.o
      ROOT::destruct_podiocLcLPythonEventStore(void*) in podioDict.cxx.o
ld: symbol(s) not found for architecture x86_64

[petricm]

Did you add the code sniped I posted?

[gaede]

Yes - as you can see in my post: -Wl,-undefined,error is there ...

[andresailer]

That flag enables the error (or rather makes it explicit since error is the default value for the llvm/clang/macos linker), so with the current podio master this would be the expected output. To disable the error use -Wl,-undefined,warning (https://stackoverflow.com/a/26989988)

[andresailer]

Miss-clicked

[gaede]

This did not work either - finally -Wl,-undefined,dynamic_lookup did the trick, see #81.
With warning I got:

ld: can't use -undefined warning or suppress with -twolevel_namespace
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Anyways, I think that dynamic_lookup is what we really want and what we use on linux.

[petricm]

I think we had a long discussion once in DD4hep and agreed that no-undefined is what we really want :)

[gaede]

I remember that we had this discussion for DD4hep and that we had concluded that we want no-undefined but this does not work with podio (and the split of libraries) - see #81.
My understanding is that -Wl,-undefined,dynamic_lookup causes the same behavior as the default on linux: allow the symbol to not be there when linking the library. It then later needs to be available at run time from some other library...

[gaede]

... and this is the case with our current tests - in particular the python reading....