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

[cmake] Stop searching headers in the default paths #948

Conversation

Teemperor
Copy link
Contributor

find_file without NO_DEFAULT_PATH searches pretty much the whole
file system for the given file. As we are looking for a root
header here that we then turn into a dictionary, copy and install
it we should probably limit us to just the ROOT directories here.

@Teemperor Teemperor requested a review from peremato as a code owner September 6, 2017 07:43
@phsft-bot
Copy link
Collaborator

Starting build on centos7/gcc49, mac1012/native, slc6/gcc49, slc6/gcc62, ubuntu14/native with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

@phsft-bot
Copy link
Collaborator

@amadio
Copy link
Member

amadio commented Sep 6, 2017

Hi Raphael, could you please rebase and push again? The failing test should be fixed now. Thanks!

@Teemperor Teemperor force-pushed the StopSearchingHeadersEverywhere branch from c8a507e to 81af2b5 Compare September 7, 2017 06:53
@phsft-bot
Copy link
Collaborator

Starting build on centos7/gcc49, mac1012/native, slc6/gcc49, slc6/gcc62, ubuntu14/native with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

@phsft-bot
Copy link
Collaborator

Build failed on slc6/gcc49.
See console output.

Failing tests:

find_file without NO_DEFAULT_PATH searches pretty much the whole
file system for the given file. As we are looking for a root
header here that we then turn into a dictionary, copy and install
it we should probably limit us to just the ROOT directories here.
@Teemperor Teemperor force-pushed the StopSearchingHeadersEverywhere branch from 81af2b5 to 5e3b0f4 Compare September 7, 2017 18:16
@phsft-bot
Copy link
Collaborator

Starting build on centos7/gcc49, mac1012/native, slc6/gcc49, slc6/gcc62, ubuntu14/native with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

@dpiparo dpiparo assigned peremato and unassigned dpiparo Sep 8, 2017
Copy link
Member

@peremato peremato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second find_file() is only executed if the first one has not succeeded. This change was to fix ROOT-8964. You forgot also that this macro is used by clients of ROOT and therefore are not only ROOT headers that are being searched.

@Teemperor
Copy link
Contributor Author

So this fixes https://root-forum.cern.ch/t/f26-v6-10-00-patches-invalid-preprocessing-directive/26197 but if experiments rely on this line, then probably we are stuck on this side.

We could at least try to make it less aggressive when searching (so, forbid some of the binary directories and CMake internal directories) to prevent these random configuration failures due to conflicting file names in the future.

@Teemperor
Copy link
Contributor Author

See also #980

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 this pull request may close these issues.

5 participants