-
Notifications
You must be signed in to change notification settings - Fork 181
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
ROOT 6.22 compatibility (CMSSW 11_2) #255
Comments
By chance I was also looking at this today, you can try the just-pushed branch 112x (#256). It compiles ok for me in CMSSW_11_2_4, but I didn't validate if everything is working ok yet. |
Perfect, thanks a lot! I will try it out later today. |
hi @ajgilbert, I saw the commit in the branch and I have an important heads up. In ROOT 6.22, which for the first time introduces the possibility to build with multiple Python versions at the same time, we had to separate TPython from the bindings. Now, the bindings (i.e., PyROOT) are built for the highest Python 3 and 2 versions that CMake finds, while TPython is (unfortunately) built for the highest version between the two of them (meaning that if we build for e.g. Python 2.7 and Python 3.7, the TPython library will be linked against the Python 3.7 libraries). For more considerations, I'm pinging @smuzaffar (since I honestly don't remember right now how it was decided to deal with this TPython situation in CMSSW) and @etejedor. Anyways, I updated the commit in my branch adding explicitly |
@maxgalli , Adding LDFLAGS works too but we prefer to have toolfile which instruct scram what to do when someone requires it. I can add a
in your BuildFile to avoid explicitly setting LDFLAGS |
Hi @maxgalli thanks for the heads up, so if I understand correctly, as per your branch we don't actually need to link against TPython here, but can rely on libcppyy2_7 directly, and everything should be consistent at execution time? If so, then yes @smuzaffar, I think it will be useful to have the |
@ajgilbert yes exactly, relying on libcppyy2_7 should make everything consistent at execution time when we use Python 2. @smuzaffar great, thanks! |
@maxgalli do you now have a working 112x/combine compatible branch? |
@andrzejnovak yes, this branch compiles correctly and I didn't notice anything suspicious (so far) in the few commands that I tried. It still sets explicitly |
Added a note to the README.md that 11_2_X can be used, but not on the master branch. |
Sorry it took me a bit of time to get back to this. Are there actual dependencies for this from CMSSW? Could it be built just with make instead like |
In theory yes, but we do depend on parts of combine (headers & linking), so any makefile would need to handle that. To be honest I'm not that interested in supporting standalone compilation with this - it just makes more work out of providing user support. |
Sure makes sense. I thought the motivation would be the same as for standalone |
Right, but with CMSSW I know the exact environment users have - so when I have to debug a problem for someone I need only confirm they are using the recommended CMSSW release. I also don't really want to implement (and maintain) a new build system that tracks the dependency between this repo and combine, when scram handles this just fine. If it becomes clear there is heavy demand for this in CMS then we can reconsider. Until then, of course anyone is free to fork this package, implement a standalone version, and even advertise to the collaboration - as long as they are happy to provide the long-term user support. |
After seeing that this PR in Combine was merged in this branch, I started doing the same kind of work for CombineHarvester (here).
Adapting to ROOT 6.22 seems to be limited to just a couple of
TPython
methods that have been moved inCPyCppyy
and changed names (TPython::ObjectProxy_FromVoidPtr
becameCPyCppyy::Instance_FromVoidPtr
andTPython::ObjectProxy_AsVoidPtr
becameCPyCppyy::Instance_AsVoidPtr
) in the transition from "old" PyROOT to new PyROOT.However, probably due to my lack of experience with
scram
, I haven't been able to succeed in correctly linking againstlibcppyy2_7.so
inCMSSW_11_2_0_pre10
.To reproduce:
Running this returns a bunch of undefined references to the changed methods, like e.g.:
What I tried:
root-config --libs
returnsThe
lib
directory printed, correctly containslibcppyy2_7.so
, butld
does not seem to look there:Also appending the path to
LD_LIBRARY_PATH
does not seem to work.Do anyone have any suggestion concerning what to try and how to correctly implement it in a build process?
(ping also for @nsmith- and @andrzejnovak who worked on adapting Combine to ROOT 6.22).
The text was updated successfully, but these errors were encountered: