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

All CMakePackages depend on CMake #2466

Closed
adamjstewart opened this issue Dec 2, 2016 · 6 comments · Fixed by #2623
Closed

All CMakePackages depend on CMake #2466

adamjstewart opened this issue Dec 2, 2016 · 6 comments · Fixed by #2623

Comments

@adamjstewart
Copy link
Member

@alalazo Is there any reason we can't add depends_on('cmake', type='build') to the CMakePackage base class? It seems silly to have to add it to every package that extends it.

@tgamblin
Copy link
Member

tgamblin commented Dec 2, 2016

We might need to adjust the way directives store metadata for this, but I think it's a good idea.

@alalazo
Copy link
Member

alalazo commented Dec 3, 2016

@adamjstewart I didn't do that because of what @tgamblin said, see here.

I think I may have an idea anyhow how to do that: instead of using inspect.stack we may:

  1. register at class construction time all the directives in a global attribute within the directives module
  2. flush them into the class being constructed using PackageMeta

Does that make sense to you @tgamblin ?

@citibeth
Copy link
Member

citibeth commented Dec 5, 2016

I thought of this too. But I'm doubtful for two reasons:

  1. Different packages have different CMake version requirements. So there is no one-size-fits-all depends('cmake') command.

  2. We continue to go down the path of making things more implicitly automagic in order to save a few lines in the package.py files. I continue to think that is a dangerous idea --- at least something we should have a serious discussion about. So far, we have not had that discussion.

@alalazo
Copy link
Member

alalazo commented Dec 6, 2016

We continue to go down the path of making things more implicitly automagic in order to save a few lines in the package.py files. I continue to think that is a dangerous idea --- at least something we should have a serious discussion about. So far, we have not had that discussion.

What I propose above involves substituting the use of inspect.stack with meta-classes. It's not additional machinery, just a refactoring of the existing one that permits to inherit across different module files (currently you can't).

@citibeth
Copy link
Member

citibeth commented Dec 6, 2016 via email

@adamjstewart
Copy link
Member Author

Different packages have different CMake version requirements. So there is no one-size-fits-all depends('cmake') command.

@citibeth This won't be a problem. You can always add depends_on('cmake@1.2.3:') to your package to make a more specific version constraint. Spack won't complain that you used depends_on('cmake') twice.

alalazo added a commit to epfl-scitas/spack that referenced this issue Dec 18, 2016
…ming from directives into packages + lazy directives

* _dep_types -> dependency_types
* using a meta-class to inject directives into packages
* directives are lazy

fixes spack#2466
alalazo added a commit to epfl-scitas/spack that referenced this issue Dec 28, 2016
…ming from directives into packages + lazy directives

* _dep_types -> dependency_types
* using a meta-class to inject directives into packages
* directives are lazy

fixes spack#2466
tgamblin pushed a commit that referenced this issue Dec 28, 2016
* inheritance of directives: using meta-classes to inject attributes coming from directives into packages + lazy directives

* _dep_types -> dependency_types
* using a meta-class to inject directives into packages
* directives are lazy

fixes #2466

* directives.py: allows for multiple inheritance. Added blank lines as suggested by @tgamblin

* directives.py: added a test for simple inheritance of directives

* Minor improvement requested by @tgamblin

CMakePackage: importing names from spack.directives
directives: wrap __new__ to respect pep8

* Refactoring requested by @tgamblin

directives: removed global variables in favor of class variables. Simplified the interface for directives (they return a callable on a package or a list of them).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants