Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RootReader symbols in podioDict #290

Closed
vvolkl opened this issue May 27, 2022 · 3 comments · Fixed by #307
Closed

RootReader symbols in podioDict #290

vvolkl opened this issue May 27, 2022 · 3 comments · Fixed by #307
Assignees

Comments

@vvolkl
Copy link
Contributor

vvolkl commented May 27, 2022

When using the podio master branch running anything that is linked to podio::podioDict prints the following warnings (errors?):

~$ python
Python 3.9.10 (main, Mar 29 2022, 00:07:05) 
[GCC 11.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import ROOT
Warning in <TInterpreter::ReadRootmapFile>: class  edm4hep::ObjectID found in libedm4hepDict.so  is already in libedm4drDict.so 
>>> ROOT.gSystem.Load('libpodioDict')
cling::DynamicLibraryManager::loadLibrary(): /afs/cern.ch/user/v/vavolkl/repo/podio/install/lib64/libpodioDict.so: undefined symbol: _ZTVN5podio10ROOTReaderE
-1
>>> 

Can be fixed by linking to podio::podioRootIO as well, but should probably not happen in the first place.

Must have been introduced after v00-14-01, as that runs the above without issues.

@tmadlener
Copy link
Collaborator

The edm4hep::ObjectID warning is due to it being defined a second time in the dual-readout EDM: https://github.com/HEP-FCC/dual-readout/blob/master/edm4dr.yaml#L9 This should be fairly safe to ignore, and eventually be handled with something that fixes #263

The missing ROOTReader symbol in the podio dictionary seems to be a bug. I will have to check which change introduced that.

@tmadlener tmadlener self-assigned this May 31, 2022
@tmadlener
Copy link
Collaborator

The behavior was introduced in #254. Specifically what seems to trigger this is defining the ~ROOTReader destructor in the ROOTReader.h header file, either via ~ROOTReader() = default or via ~ROOTReader() {}. Only declaring it in the header file and defining an empty destructor in the ROOTReader.cc file fixes this, i.e.:

diff --git a/include/podio/ROOTReader.h b/include/podio/ROOTReader.h
index e91bde3..b9afe9f 100644
--- a/include/podio/ROOTReader.h
+++ b/include/podio/ROOTReader.h
@@ -35,7 +35,7 @@ class ROOTReader : public IReader {
 
 public:
   ROOTReader() = default;
-  ~ROOTReader() = default;
+  ~ROOTReader();
 
   // non-copyable
   ROOTReader(const ROOTReader&) = delete;
diff --git a/src/ROOTReader.cc b/src/ROOTReader.cc
index 8c1a294..436df53 100644
--- a/src/ROOTReader.cc
+++ b/src/ROOTReader.cc
@@ -16,6 +16,9 @@
 
 namespace podio {
 
+ROOTReader::~ROOTReader() {
+}
+
 std::pair<TTree*, unsigned> ROOTReader::getLocalTreeAndEntry(const std::string& treename) {
   auto localEntry = m_chain->LoadTree(m_eventNumber);
   auto* tree = static_cast<TTree*>(m_chain->GetFile()->Get(treename.c_str()));

I am not entirely sure why that makes a difference for ROOT. I will try and see if there is a way around this or if actually linking against libpodioRootIO is the right thing to do.

vvolkl added a commit to vvolkl/podio that referenced this issue Jun 22, 2022
make sure that there are no unknown symbols in podioDict
@vvolkl
Copy link
Contributor Author

vvolkl commented Jun 22, 2022

Created the most straightforward fix here, but I think either linking podioRootIO to the dict or adding all the root sources to it should not be a problem - ROOT is required in any case to create the dict in the first place.

@tmadlener tmadlener linked a pull request Jun 22, 2022 that will close this issue
tmadlener pushed a commit that referenced this issue Jun 22, 2022
* hotfix for #290

make sure that there are no unknown symbols in podioDict
tmadlener pushed a commit that referenced this issue Jun 24, 2022
* hotfix for #290

make sure that there are no unknown symbols in podioDict
tmadlener pushed a commit that referenced this issue Jun 24, 2022
* hotfix for #290

make sure that there are no unknown symbols in podioDict
tmadlener added a commit that referenced this issue Jun 24, 2022
soumilbaldota pushed a commit to soumilbaldota/podio that referenced this issue Jun 28, 2022
* hotfix for AIDASoft#290

make sure that there are no unknown symbols in podioDict
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants