Instantiating a ROOT.vector adds the build directory to cling's include paths #7108
Comments
|
I can't reproduce on my laptop (Fedora 32) with the following 4 config:
Python2 and ROOT release Python2 and ROOT debug Python3 and ROOT release Python3 and ROOT debug |
|
I can reproduce with Enrico's reproducer, but not with: import ROOT
print(ROOT.gSystem.GetIncludePath())
ROOT.gInterpreter.ProcessLine("""
auto v = std::vector<int>();
""")
print(ROOT.gSystem.GetIncludePath())So it could be related to the use of the TCling interfaces. |
|
Instantiating the template is enough to reproduce (no need to create the object): import ROOT
print(ROOT.gSystem.GetIncludePath())
ROOT.vector('int')
print(ROOT.gSystem.GetIncludePath()) |
|
Using cppyy only has the extra path both before and after: import cppyy
print(cppyy.gbl.gSystem.GetIncludePath())
cppyy.gbl.std.vector('int')
print(cppyy.gbl.gSystem.GetIncludePath()) |
|
I guess that means that the path is added by |
|
Yes, |
|
@vepadulano can you try with makefiles instead of ninja? I don't see any other difference |
|
More info: it's not a vector thing, it also happens if you instantiate e.g. a map or list or deque. |
|
This line in but still does not explain why/where the instantiation adds the path. |
|
the wrong include path is added here: |
|
cling-only repro (PyROOT is off the hook :) ): |
|
Looks like it's a problem in the dictionary generation. The following snippet comes from static const char* includePaths[] = {
"/home/eguiraud/ROOT/build_dbg_includepaths/include",
"/home/eguiraud/ROOT/build_dbg_includepaths/include",
0
}; |
Just finished compiling with |
|
As for the In In In |
|
As far as I can tell those includes end up in the dictionary because they are passed to
I don't see a way this could not happen, so figuring out what's different in Vincenzo's case should be interesting. |
|
@vgvassilev @Axel-Naumann ping! |
|
It may make sense to register such include paths as "private" and adjust the interface of In principle we do not need this injected include path when runtime_cxxmodules are on and we might just drop that part from dictionary generation. There might be some code out there that was relying on this gray area include path. I'd defer that to @Axel-Naumann. |
|
We've been trying to figure out why Vincenzo doesn't see this. It turns it's root/core/dictgen/src/rootcling_impl.cxx Lines 4097 to 4115 in 730d24b We think the fix should be put in root/cmake/modules/RootMacros.cmake Line 329 in 68ffb0b and root/cmake/modules/RootMacros.cmake Line 463 in 68ffb0b |
|
Adding |
|
Some insight into when/where this actually causes a problem. Take for example a recent CMSSW release where we indeed end up with a bunch of stuff in the include path starting with "/data/cmsbld/jenkins/...". If I try to do anything with gSystem->CompileMacro then things are fine if /data/cmsbld doesn't exist on the machine where I'm running, but if I create that directory and set permissions to make it inaccessible then I get a permission denied error when compiling a macro. This also explains why I was having problems with this specifically in my singularity images, because in this case the build path was in /root/root_build, where /root still exists but is inaccessible by normal users in the final singularity image. |
Libraries are free to use the interpreter during their static initialization after being loaded. In that case this library decided to add an extra include path. If we do not need this include path (eg there is no useful header to resolve from there). In fact we ignore such include paths here. What is |
|
Hi @vgvassilev , I don't have the repro set up anymore, sorry Ah but yes, As Josh mentions above adding |
Isn't the recent nlohmann_json issue contradicting this? These headers must be found at runtime, and that's happening either because they are found where they were during build time, or due to |
|
@Axel-Naumann, yeah, "this" means "ROOT build directory". I suspect that we did not capture properly which is the build directory. I remember some discussion about this with @linev where he reported he could not drop the |
|
I do not remember all details, but one reason why For instance - |
|
But maybe one can resolve this - |
|
Ping :) it would be nice to fix this for v6.26 |
…clude). We do not want to remember the build directory, see issue root-project#7108.
…clude). We do not want to remember the build directory, see issue #7108.
…clude). We do not want to remember the build directory, see issue root-project#7108. (cherry picked from commit c2f028f)
|
Should be fixed in master and v6-26-patches. Can you confirm? Is this also needed in 6.24? |
|
this doesn't fix it for me unfortunately |
|
Uh, disappointing. My repro at the top of the issue seems to be fixed: (no build directory in the include paths anymore) |
|
I think you can easily reproduce my error by keeping the build directory present but making it inaccessible by permissions. |
|
I can't, sorry. I did the following: As root user: As my user: And still: |
|
I also can't reproduce this in a standalone build, only happens when I install using the system package manager (in this case archlinux in a podman image). Maybe it has to do with $ROOTSYS being set vs not. Let me see if I can provoke it in a more standalone manner. On the other hand if I grep for the build directory, it does still appear in the final .so and .pcm files. |
…clude). We do not want to remember the build directory, see issue root-project#7108.
That might or might not be a problem, I guess? With the original reproducer fixed I'm not sure how to make progress here :/ |
|
Closing since the original reproducer has been fixed. |
Describe the bug
A reproducer:
prints
Note that the second line contains
-I"/home/eguiraud/ROOT/build_dbg_includepaths/include", which wasn't there before aROOT.vectorwas instantiated. That's problematic because the user might not have permissions to access the build directory (while it can access the install directory) leading to cling errors.This is with ROOT master@028fcca0fa , compiling ROOT with
cmake -DCMAKE_BUILD_TYPE=Debug -Dccache=ON -Ddev=ON -Droofit=OFF -Dtmva=OFF -Dtesting=ON -Droottest=ON ../root.The text was updated successfully, but these errors were encountered: