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

Update packages which need include subdirectories #10773

Merged
merged 1 commit into from
Mar 4, 2019

Conversation

scheibelp
Copy link
Member

@scheibelp scheibelp commented Mar 1, 2019

Fixes #10769
Closes #10751 (EDIT: see #10773 (comment))

@HadrienG2 @davydden @jslee02 @alalazo

#10623 updated the default behavior of .headers.directories to exclude subdirectories (since this can cause clashes with system headers). This broke some packages which depended on the old behavior of .headers.directories: for example if you had <package-prefix>/include/subdir/ex1.h, .headers.directories would include <package-prefix>/include/subdir.

This updates the .headers property to include subdirectories for Python and Eigen (as is recommended by these packages).

In hindsight, it was a mistake not to request documentation on #10623. So that should be added here or soon.

@scheibelp
Copy link
Member Author

@jslee02 regarding #10751 I wanted to confirm myself that this fixes but didn't actually see packages using .headers for the eigen package.

If this works for you, it would be the ideal way of including subdirectories (over modifying CPATH). Are you using a custom package which calls Eigen.headers (no problem with that, I just wanted to make sure I wasn't overlooking an example of that access in an existing Spack package).

@HadrienG2
Copy link
Contributor

Thanks, I finally understood why #10623 broke python in spite of it already overriding headers. Turns out one needs to override both headers AND to manually reset the HeaderList's directories property.

I think extra documentation would definitely be helpful here. Not sure where it would be most visible to those who need it, though.

In any case, if you haven't tested this with boost+python already, I will give it a try myself next week 😉

@HadrienG2
Copy link
Contributor

Works for me 👍

@alalazo
Copy link
Member

alalazo commented Mar 4, 2019

@scheibelp If you prefer having a separate PR for docs I'd merge this.

@jslee02
Copy link
Contributor

jslee02 commented Mar 4, 2019

@scheibelp I use scons as a buildsystem for my project and run spack module loads <packages> to let scons find the package location using the environment variables (e.g., CPATH). Could you please elaborate how this PR fixes #10751 without modifying CPATH?

@scheibelp scheibelp merged commit af4a36c into spack:develop Mar 4, 2019
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.

Installation issue: boost+python
4 participants