Feature/external by mamelara · Pull Request #2507 · spack/spack

mamelara · 2016-12-07T16:07:05Z

First working design for new spack external add/rm/list command. Would like some feedback on it.

Usage works as follows:
spack external add [package_spec] [path_or_module_for_external_pkg]
spack external rm [package_spec]
spack external list
All take an optional scope flag

external add
The external add command takes in a package spec (with or without a package version) and either a path or module name. This information is passed to the ExternalPackage class which then validates the path or module and attempts to detect a version. If a version is not detected and the user did not specify a version in the spec, a version is spit out. Otherwise, that version is appended to the spec. An external package object is created and is used to add an entry to packages.yaml.

example:

└┤[16:53] ➜ spack external add [email protected]%[email protected] cray-mpich/7.4.4
==> Added [email protected]%[email protected] to /global/homes/m/mamelara/.spack/cray/packages.yaml

external rm
The external rm command also takes as input a package_spec. It will remove the specified spec entry from a package. If that package entry is empty (no specs) then that entire package entry is removed.

example:

┌[mamelara::spack
└┤[16:54] ➜ spack external rm [email protected]%[email protected]
==> Removed package: [email protected]%[email protected]
==> Removed mpich

external list
Outputs the contents of a list. Similar to compiler list.

output:

┌[mamelara::spack
└┤[16:56] ➜ spack external list
==> Available external packages
-- petsc --------------------------------------------------------
[email protected]%[email protected]   cray-petsc/3.6.1.0

-- mkl ----------------------------------------------------------
[email protected]%[email protected]   /opt/intel/compilers_and_libraries_2016.3.210/linux/mkl

-- netcdf -------------------------------------------------------
[email protected]%[email protected]   cray-netcdf/4.4.1
[email protected]%[email protected]          cray-netcdf/4.4.1

-- hdf5 ---------------------------------------------------------
[email protected]%[email protected]          cray-hdf5/1.8.16
[email protected]%[email protected]   cray-hdf5/1.8.14
[email protected]%[email protected]   cray-hdf5/1.8.16

So far I've tested this on Cori and on my Mac. Please feel free to comment on it and I'm open to all kinds of suggestions and looking to improve functionality and design. Thanks!

Note: I'm not too excited about how I have been detecting versions from packages. Currently, I just look through the path for something that looks like a version and stick it on to the spec string. I also parse the output of module avail but not sure if my way is too NERSC specific.

Another thing, I added more tests and a mock modulefiles path to test modulecmd with fake modulefiles. Hopefully people will find that useful.

mamelara · 2016-12-07T16:08:38Z

how do I label WIP?

adamjstewart · 2016-12-07T16:09:35Z

@mamelara Add [WIP] to the start of the title and click on the gear next to labels to add WIP. If you have access that is. I'm not sure whether or not everyone can do the latter.

adamjstewart · 2016-12-07T16:12:54Z

This is very interesting! I just got done ranting about how difficult it is to manually add external packages: #2504 (comment)

davydden · 2016-12-07T16:20:10Z

First working design for new spack external add/rm/list command.

nice, it should certainly ease the pain of setting up external packages while still being precise in what is needed for each build! 👍

davydden · 2016-12-07T16:21:46Z

how do I label WIP?

i added the label

citibeth · 2016-12-07T16:39:03Z

I like the general idea of this PR... Once the basic machinery is set up, I think the big challenge will be in detecting external packages and versions. That info could come from:

Module files
System repos (yum, apt-get, etc)
Ad-hoc from the user

The next step would be a command that rifles through your system and finds all the external packages it can. This brings up questions of course. Spack would ideally want to construct a full spec for a package including variants and dependencies, not just its version number. Even just the version number could be tricky, a full spec even trickier...

And stuff that rifles through your system and finds versions (or even deduces versions from something the user gives) will be OS-specific. I would like to see this functionality broken out into a separate file with a clear API. Then we can start hanging more complexity into that file.

becker33 · 2016-12-08T23:56:13Z

@citibeth I agree that eventually this should be refactored into something that includes automatic external registration in Spack. I don't think that should hold up integration of this PR though. It's a useful feature, and I don't know what the timeframe will be for the fuller feature (it could come along with the full rework of compilers that models them as dependencies on virtual deps).

mamelara · 2016-12-16T00:56:58Z

Could I get a review requested label on this? Thank you!

davydden · 2016-12-16T08:09:52Z

@tgamblin @alalazo @adamjstewart ping.

alalazo · 2016-12-16T08:38:19Z

@mamelara I added @tgamblin to the list of requested reviewers. Let me know if you want somebody more added to it.

alalazo · 2016-12-16T08:43:28Z

@mamelara @tgamblin After reading the description I also added myself: I really like the PR and would like to check it in the details. Don't consider my review "blocking" if this needs to be merged quickly.

adamjstewart

This looks like a great start! We'll eventually want to add more tests for determining names and versions automatically (see #3181), but I think it would be good to merge this first and integrate that later.

P.S. A few files are missing licenses at the top. Other than that I couldn't see anything obviously broken.

adamjstewart · 2017-02-18T14:48:40Z

Oh, documentation would be nice as well.

adamjstewart · 2017-02-18T20:02:44Z

lib/spack/spack/test/external_package.py

+from spack.environment import EnvironmentModifications
+from spack.external_package import ExternalPackage, SpecVersionMisMatch
+import spack.spec
+import spack.test.mock_packages_test as mock_test


This line is causing the unit tests to fail.

adamjstewart · 2017-02-18T20:02:55Z

lib/spack/spack/test/cmd/external.py

@@ -0,0 +1,123 @@
+import spack.architecture
+import spack.test.mock_packages_test as mock_test


This line is causing the unit tests to fail.

Add new spack commands. Usage is spack external [cmd-option]. So far the options include add, remove and list.

Committing work to update branch to llnl develop

Change attribute names, got rid of some property methods that are not necessary.

Moved tty.msg output to cmd/external renamed some variables

Updated external_rm tests to follow new design and also updated external_package.

Reworked external_rm so that it can display message when a spec satisfies multiple entries. Similar to compiler rm, it will notify user that multilple matches were found and tell them to use the optional -a, --all flag. Changed some var names in external_package.

Made some flake8 changes and corrected some tests that failed on TravisCI. Fix sphinx doc tests.

Changed self.assertRaises tests to not use context manager.

Update tests to pytest Squash commits to change tests Squashing commits where I changed the tests to be updated to pytest

Decided it was better design to have both add and remove functions rather than using the external cmd have the main logic for removing.

Tests were failing and changed external remove also used external package constructor instead of classmethod function.

alalazo · 2017-10-19T08:11:03Z

@mamelara Are there any plans for this PR ? I think externals lack a decent CLI, so let me know if I can help.

mamelara · 2017-10-19T14:49:51Z

This PR has grown stale. I would need to rebase and then maybe look through and adjust some of the methods since Spack has changed a lot since this was last worked on.

I was hoping to get some help with the design aspect of the code and also maybe improving the functionality. I was also working on detecting some defaults though I think that should be a separate PR and this should focus on the CLI interface instead. Any help is welcome!

I'll see if I can update this soon.

pramodskumbhar · 2017-10-19T22:02:05Z

Wasn't aware of this PR. This will be of great help!

Oximore · 2018-02-02T13:30:13Z

This PR is really what we need to an easy use of Spack on cluster platforms.
As build openmpi, and lower libraries make is a poor choice in such envrionment.

I may be a little out topic but :

How it integrate with module ?
For external package detection have you think about using something like Find*.cmake ?

Have we an expected date for this PR to be merged ?

amklinv · 2018-02-08T16:39:24Z

I am also interested in this PR and whether it will be merged.

luigi-calori · 2018-02-08T17:32:46Z

I would also be interested, would be good to at least rebase and resolve conflicts, so to ease the testing activity

becker33 · 2018-02-08T20:57:25Z

@Oximore @amklinv @luigi-calori This PR will be merged eventually. @mamelara and I will probably need to tweak the interface.

Eventually this should go along with automatic detection, but we don't have the technical solutions in place for that yet and it's a very hard problem.

pramodk · 2018-09-23T21:45:14Z

@mamelara : This will be very helpful! I was looking for the functionality to "export" installed packages to packages.yaml. Is there any other way?

tldahlgren · 2019-08-29T23:47:08Z

Closing to re-open to refresh check details since they are not available. (At least I was under the impression that would work but I guess not.)

mamelara changed the title ~~Feature/external~~ [WIP] Feature/external Dec 7, 2016

davydden added the WIP label Dec 7, 2016

davydden mentioned this pull request Dec 7, 2016

Satisfy Python build dependency with Spack's Python #2504

Closed

mamelara force-pushed the feature/external branch from 61ed1d9 to e2bde54 Compare December 8, 2016 23:14

davydden added the RFC label Dec 16, 2016

davydden changed the title ~~[WIP] Feature/external~~ Feature/external Dec 16, 2016

alalazo requested a review from tgamblin December 16, 2016 08:38

alalazo self-requested a review December 16, 2016 08:41

This was referenced Dec 31, 2016

Some adjustments in a few CEED-related packages. #2697

Merged

Spack should complain if entries in packages.yaml are virtual packages #2069

Closed

adamjstewart mentioned this pull request Jan 20, 2017

Losing Users: Perception that Spack is Too Complicated #2873

Closed

mamelara force-pushed the feature/external branch 2 times, most recently from e559c9e to a34f9f8 Compare February 15, 2017 22:24

alalazo mentioned this pull request Feb 18, 2017

Generating potential packages.yaml file? #3181

Closed

adamjstewart approved these changes Feb 18, 2017

View reviewed changes

adamjstewart reviewed Feb 18, 2017

View reviewed changes

mamelara force-pushed the feature/external branch from a34f9f8 to 67cd029 Compare March 27, 2017 19:15

alalazo self-assigned this Apr 19, 2017

mamelara added 13 commits May 22, 2017 12:05

Add new commands add, remove, and list

de73aa1

Add new spack commands. Usage is spack external [cmd-option]. So far the options include add, remove and list.

Commit work to update branch to develop

0d9608b

Committing work to update branch to llnl develop

Minor refactors

916253a

Change attribute names, got rid of some property methods that are not necessary.

Move tty message to external_add and external_rm

da0f20a

Move tty.msg and rename some vars

1de21bf

Moved tty.msg output to cmd/external renamed some variables

Change tests in external and external_package

2e256fb

Updated external_rm tests to follow new design and also updated external_package.

Flake8 changes and correct errors.

ab3f693

Made some flake8 changes and corrected some tests that failed on TravisCI. Fix sphinx doc tests.

Make tests python2.6 friendly

caaa1f4

Changed self.assertRaises tests to not use context manager.

Update tests for pytest

88d9d76

Update tests to pytest Squash commits to change tests Squashing commits where I changed the tests to be updated to pytest

First try at implementing cray detection

d9cb040

Update tests for pytest

07a51fe

Update tests to pytest Squash commits to change tests Squashing commits where I changed the tests to be updated to pytest

Moved external remove to it's own fxn

aa0b09d

Decided it was better design to have both add and remove functions rather than using the external cmd have the main logic for removing.

mamelara force-pushed the feature/external branch from 67cd029 to aa0b09d Compare May 22, 2017 20:37

mamelara added 3 commits May 25, 2017 16:21

Fix remove package entry functionality

7a381df

Tests were failing and changed external remove also used external package constructor instead of classmethod function.

Add skipif for Cray packages

42e611d

Add description, section, and level

4c98546

tldahlgren closed this Aug 29, 2019

tldahlgren reopened this Aug 29, 2019

scheibelp mentioned this pull request Feb 22, 2020

Automatically find externals #15158

Merged

tgamblin closed this in #15158 May 6, 2020

		@@ -0,0 +1,123 @@
		import spack.architecture
		import spack.test.mock_packages_test as mock_test

Conversation

mamelara commented Dec 7, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mamelara commented Dec 7, 2016

Uh oh!

adamjstewart commented Dec 7, 2016

Uh oh!

adamjstewart commented Dec 7, 2016

Uh oh!

davydden commented Dec 7, 2016

Uh oh!

davydden commented Dec 7, 2016

Uh oh!

citibeth commented Dec 7, 2016

Uh oh!

becker33 commented Dec 8, 2016

Uh oh!

mamelara commented Dec 16, 2016

Uh oh!

davydden commented Dec 16, 2016

Uh oh!

alalazo commented Dec 16, 2016

Uh oh!

alalazo commented Dec 16, 2016

Uh oh!

adamjstewart left a comment

Choose a reason for hiding this comment

Uh oh!

adamjstewart commented Feb 18, 2017

Uh oh!

adamjstewart Feb 18, 2017

Choose a reason for hiding this comment

Uh oh!

adamjstewart Feb 18, 2017

Choose a reason for hiding this comment

Uh oh!

alalazo commented Oct 19, 2017

Uh oh!

mamelara commented Oct 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pramodskumbhar commented Oct 19, 2017

Uh oh!

Oximore commented Feb 2, 2018

Uh oh!

amklinv commented Feb 8, 2018

Uh oh!

luigi-calori commented Feb 8, 2018

Uh oh!

becker33 commented Feb 8, 2018

Uh oh!

pramodk commented Sep 23, 2018

Uh oh!

tldahlgren commented Aug 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

14 participants

mamelara commented Dec 7, 2016 •

edited

Loading

mamelara commented Oct 19, 2017 •

edited

Loading

tldahlgren commented Aug 29, 2019 •

edited

Loading