-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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 hwloc; don't require libpciaccess on OS X #257
Conversation
eschnett
commented
Dec 21, 2015
- hwloc 1.11.2 is available.
- libpciaccess is not supported on OS X; don't require it there.
version('1.11.1', '002742efd3a8431f98d6315365a2b543', | ||
url='http://www.open-mpi.org/software/hwloc/v1.11/downloads/hwloc-1.11.1.tar.bz2') | ||
version('1.9', '1f9f9155682fe8946a97c08896109508') | ||
|
||
depends_on('libpciaccess') | ||
# libpciaccess is not supported on OS X | ||
depends_on('libpciaccess', when='=linux') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering why not a variant for libpciaccess. My concern is that this approach will force every user to name the architecture linux
if they want to activate the dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
libpciaccess should be the default, except on OS X. Is this possible with variants?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not at the moment. You can do this:
variant('libpciaccess', default=True, 'Enable PCI access library')
depends_on('libpciaccess', when='+libpciaccess')
But then OS X users will need to know to build with -libpciaccess
, which isn't ideal. I would say to add the variant for now, possibly keeping the hard-coded setting in your own branch, and we could work on a way to add predicates instead of just True
/False
for the value of default
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it can be automated completely ( @tgamblin correct me if I am wrong ). Something like:
variant('libpciaccess', default=True, description='Activates dependency on libpciaccess')
...
depends_on('libpciaccess', when='+libpciaccess')
would require to explicitly type:
spack install hwloc~libpciaccess
on OS X.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's so complicated to not use a particular package on certain systems, then I think it might be easier to change libpciaccess to skip the configure/make/install steps on OS X, leaving Spack to think that this package actually exists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me see what i can add -- one reason this is complicated is that the spec
syntax doesn't really have a way to say "not" for architectures right now -- Specs
are really match objects and they only match in the positive.
A simple way to do this would be something like:
depends_on('libpciaccess', when='!=darwin-x86_64')
that would be much clearer than the above proposals. I would need to think through how the spec syntax should be modified to support this, but it is something we need.
The other option would be to just add "when_not", which might be easier short-term. That would look like this:
depends_on('libpciaccess', when_not='=darwin-x86_64')
You could test either of these in install
by just writing:
if 'libpciaccess' in spec:
...
Would either of those work for you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either of these dependency specification would work fine.
- hwloc 1.11.2 is available. - libpciaccess is not supported on OS X; don't require it there.
b31f7bd
to
6831ee6
Compare
I've modified the patch:
|
This seems reasonable for now -- thanks for updating! I can try to work in my proposed better solution (just don't depend on pciaccess on OS X) soon. |
Update hwloc; don't require libpciaccess on OS X
…orkflows/style/isort-5.13.1 build(deps): bump isort from 5.13.0 to 5.13.1 in /.github/workflows/style