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

enhancement proposal : boolean support for when=<arg> #299

Merged
merged 4 commits into from
Feb 25, 2016

Conversation

alalazo
Copy link
Member

@alalazo alalazo commented Jan 2, 2016

Rationale

This PR extends the when=<arg> keyword to Boolean arguments in both multi-methods and depends_on directives. This opens the possibility to use predicates that can be evaluated at package definition time to conditionally enable a dependency.

Modifications
  • depends_on directive and when type objects can now manage Boolean arguments
  • fixed a typo at line 196 in multimethod.py (class declaration repeated twice)
Example of use : OS related dependencies

I started this PR as an alternative solution to what is proposed in #268 and similar PRs. You may refer there for the details of the discussion, but to make it short the idea is to use sys.platform to discriminate among different operative systems and provide a less invasive way to build packages on OSX. The benefits of this approach are that:

  • nobody will be forced to use a given name for his architecture
  • inconsistencies among similar systems will be hopefully avoided (=linux won't be satisfied by =chaos_5_x86_64_ib or by =linux-x86_64, etc.)
  • the set of possible values to be used in the predicates is part of python documentation (and as such we don't have to agree on them)

@tgamblin , @eschnett , @nrichart , @trws : any comment is appreciated. Also, I don't have access to a MAC so I was not able to try an installation of OpenSSL on darwin. I tried though to switch the two install methods (having the darwin one as default and using @when(arch.os_is_in('linux2')) on the other) and that worked for me.

@alalazo alalazo changed the title proposal for enhancement : boolean support for when=<arg> enhancement proposal : boolean support for when=<arg> Jan 2, 2016
@eschnett
Copy link
Contributor

eschnett commented Jan 3, 2016

This introduces a new function os_is_in that compares the value to sys.platform. I don't think it's a good idea to hard-code things this way; the value of os should be configurable, so that one can override sys.platform if necessary. Also, just "darwin" may not suffice, as this doesn't distinguish between 32-bit and 64-bit ABIs.

Another issue I see is that the value of os is not encoded in the package name. For example, there might be a common file system for Darwin and Linux systems, and they will need to build packages differently. There needs to be some syntax (like the current =) to make this possible.

Finally, I wonder what this means for "architecture": Why should the default value for "architecture" be something like "linux-i686" if architecture can be set to anything and is unrelated to the OS?

I think you have the need for a certain Spack feature, and being able to freely change the architecture setting is working well for you. I don't think this is how the architecture was intended to be used -- look at the default values. Instead of defining a new entity that will, ultimately, be designed quite identically to the way "architecture" is currently designed, you could describe exactly what feature you need, and then have it added to Spack.

Let's call it "features" instead of architecture for the moment. Here are some questions:

  • Do you want "features" to describe just hardware? Or also software installed on a node?
  • Should package build scripts depend on features, or would this be a bad idea? You argued against it in the past.
  • Should features be encoded in package specification?
  • Should they be encoded in package md5sums?
  • Should there be a unique, agreed-upon list of features, or should this be up to system administrators?
  • Should each node define its own features, or should Spack maintain a map from host name to feature sets?
  • Should features be strings? sets of strings? multi-sets? dictionaries?

@mathstuf
Copy link
Contributor

mathstuf commented Jan 7, 2016

The OpenSSL part of this has already been fixed, but the rest looks useful. There are some dependencies which are platform specific (Qt shouldn't depend on D-Bus or GTK on OS X since neither is really there to integrate with anyways).

@alalazo
Copy link
Member Author

alalazo commented Jan 28, 2016

@mathstuf , @tgamblin : I removed the outdated changes and left here only the core functionality

becker33 added a commit that referenced this pull request Feb 25, 2016
enhancement proposal : boolean support for when=<arg>
@becker33 becker33 merged commit 7176e5e into spack:develop Feb 25, 2016
@becker33
Copy link
Member

merged #299.

@alalazo alalazo deleted the enhancement/os_detection branch February 25, 2016 22:38
matz-e added a commit to matz-e/spack that referenced this pull request Apr 27, 2020
climbfuji added a commit to climbfuji/spack that referenced this pull request Aug 14, 2023
Add w3emc 2.10.0 and update variants, run env logic
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.

None yet

5 participants