document git commit versions #25981

becker33 · 2021-09-15T21:57:13Z

Adds documentation for git commit support added in #24639

becker33 · 2021-09-15T21:57:30Z

@tgamblin @haampie @vvolkl

sethrj

Nice!

lib/spack/docs/environments.rst

haampie · 2021-09-16T11:46:01Z

lib/spack/docs/environments.rst

+latest version of the package, and ``spack install`` will install from
+that source if ``foo`` is in the environment. Further development on
+``foo`` can be tested by reinstalling the environment, and eventually
+committed and pushed to the upstream git repo.


As a general feedback, I like the feature, but there's a couple concerns, considering this feature seems to be targeted to development, not spack install in general:

I don't ever think about commits, I work with branches

I often fork repos when developing them (i can of course do git clone myself and then spack develop -p path/to/clone, so not a big issue...)

When you do development, you make new commits; the "version" number in the spack.yaml however remains fixed? (This is why I find it a bit surprising to see that this feature targets spack develop, since for non-develop spack install xyz@<commit hash> it does not happen, and there it's still useful if spack can make an educated guess about the best matching spack version)

FWIW, in Julia they differentiate between version numbers (using @) and git refs (using #), which I think makes a whole lot of sense (note that add in julia is more like install in spack, not develop, for develop they don't have to worry, since the sources contain a Project.toml which specifies the version & dependencies):

$ julia -q (@v1.6) pkg> activate --temp Activating new environment at `/tmp/jl_Tl967c/Project.toml` (jl_Tl967c) pkg> add ArnoldiMethod#master Updating git-repo `https://github.com/haampie/ArnoldiMethod.jl.git` Updating registry at `~/.julia/registries/General` Resolving package versions... Updating `/tmp/jl_Tl967c/Project.toml` [ec485272] + ArnoldiMethod v0.2.0 `https://github.com/haampie/ArnoldiMethod.jl.git#master` Updating `/tmp/jl_Tl967c/Manifest.toml` [ec485272] + ArnoldiMethod v0.2.0 `https://github.com/haampie/ArnoldiMethod.jl.git#master` [90137ffa] + StaticArrays v1.2.12 [8f399da3] + Libdl [37e2e46d] + LinearAlgebra [9a3f8284] + Random [9e88b42a] + Serialization [2f01184e] + SparseArrays [10745b16] + Statistics (jl_Tl967c) pkg> add ArnoldiMethod@0.2.0 Resolving package versions... Updating `/tmp/jl_Tl967c/Project.toml` [ec485272] ~ ArnoldiMethod v0.2.0 `https://github.com/haampie/ArnoldiMethod.jl.git#master` ⇒ v0.2.0 Updating `/tmp/jl_Tl967c/Manifest.toml` [ec485272] ~ ArnoldiMethod v0.2.0 `https://github.com/haampie/ArnoldiMethod.jl.git#master` ⇒ v0.2.0 (jl_Tl967c) pkg> add ArnoldiMethod#1ea8e64648a2d84e4ca973b44c4e08fef0645038 Resolving package versions... Updating `/tmp/jl_Tl967c/Project.toml` [ec485272] ~ ArnoldiMethod v0.2.0 ⇒ v0.2.0 `https://github.com/haampie/ArnoldiMethod.jl.git#1ea8e64` Updating `/tmp/jl_Tl967c/Manifest.toml` [ec485272] ~ ArnoldiMethod v0.2.0 ⇒ v0.2.0 `https://github.com/haampie/ArnoldiMethod.jl.git#1ea8e64`

IMHO this is very convenient and also more correct in case of Spack, since spack version "numbers" may be anything, and there are even packages that do version('develop', branch='master')...

haampie · 2021-09-16T12:43:03Z

I'm still thinking about #22087 (develop specs not packages). The documented feature in this PR seems to reinforce that the develop section of an environment influences concretization, which I think is a mistake, since I often want to compare a developed version versus a stable version in the same environment, so I rather want to select what spec to build from local sources.

I wonder what you think about a workflow like this:

spack develop pkg git=https://github.com/haampie/pkg.git ref=feature/mkl-support

clones a fork, checks out a particular branch, figures out the best matching Spack version (say @x.y.z) and updates the spack.yaml develop section:

develop:
- spec: pkg@x.y.z
  url: https://github.com/haampie/pkg.git
  ref: feature/mkl-support
  path: ./pkg

Then I can update my specs section accordingly (okay, still not amazing syntax maybe, cause :8 and x.y.z can concretize to the same thing, so you'd have to somehow avoid that...):

spack:
  specs:
  - pkg@x.y.z-develop +x y=z ^intel-mkl
  - pkg@:8 +x y=z ^openblas
  develop:
  - spec: pkg@x.y.z-develop
    url: https://github.com/haampie/pkg.git
    ref: feature/mkl-support
    path: ./pkg

and only then concretize, where concretization can ignore the develop section altogether. Only after concretization, we can map the develop spec to the first entry and set dev_path=/path/to/env/pkg.

psakievich

I'm glad to see this feature so thanks to all those who worked on it. I think my primary use cases will be in package definitions and to avoid/workaround breaking commits from my package's dependencies while I refactor. I agree with @haampie 's assessment of the spack develop feature. I use spack develop daily and it has become my primary method for setting up a development environment.

Specifically, I agree that for develop work I'm most focused on forks and branches than versions and commits. I really like the syntax he presented where we specify we the git source and branch/ref.

I also feel like one area we are falling short in is the combinatoric nature of spack and stacks in the develop environments.
For example, spack is really well posed to allow me to develop for multiple variants of my package off the same source changes i.e. +/~cuda +/~mpi +/~fftw etc.
Moving develop to something like what is outlined in #22087 would allow me to build and test my code changes for all these variations in one environment, and the parallel build options in spack (i.e. srun -N #NodesAllocated -n #VariantsCombinations spack install) could allow me to scale out my development in a unique way and high impact way.

psakievich · 2021-09-17T15:53:08Z

lib/spack/docs/environments.rst

+For packages with ``git`` attributes, 40-character commit hashes are
+also valid versions. This means that for a package ``foo`` whose
+latest commit is ``abcdef1234abcdef1234abcdef1234abcdef1234``, ``spack
+develop foo@abcdef1234abcdef1234abcdef1234abcdef1234`` will clone the


is this still going to do a shallow clone of git with just this commit? This is one thing I find really frustrating about the current develop clone. This makes sense for production installs but for development work I always want the history.

For individual commits, we always do a full clone.

When we expand this feature to branches and tags, it should also cover making full clones for branches and tags in the spack develop feature.

psakievich · 2021-09-17T16:09:19Z

lib/spack/docs/environments.rst

+also valid versions. This means that for a package ``foo`` whose
+latest commit is ``abcdef1234abcdef1234abcdef1234abcdef1234``, ``spack
+develop foo@abcdef1234abcdef1234abcdef1234abcdef1234`` will clone the
+latest version of the package, and ``spack install`` will install from


so "latest version" in this case follows the rules below? develop > main> master>trunk. or is it going to follow the tag check and clone off the "closest version" as outlined in the Version class documentation? for a develop spec I think the former would be appropriate. It might be good to clarify that here and/or provide a direct link to the Version source documentation.

In this case, it's the "latest" because in the previous clause, I mentioned that this happens to be the latest commit of the package, and we know that as developers of the package. In general, a commit version would slot after develop/main/master/head/trunk and ahead of any known versions whose git tags are ancestors of that commit.

psakievich · 2021-09-17T16:10:57Z

lib/spack/docs/basic_usage.rst

@@ -1060,14 +1060,19 @@ set of arbitrary versions, such as ``@1.0,1.5,1.7`` (``1.0``, ``1.5``,
 or ``1.7``).  When you supply such a specifier to ``spack install``,
 it constrains the set of versions that Spack will install.

+For packages with a ``git`` attribute, a 40-character git commit sha


More of a functional question here. Does it have to be the full 40 character sha? The native git client allows for an abbreviated sha. Will it throw if I give 12 characters?

Currently it will throw if you give it 12 characters. That's another thing we want to improve as future work, but it made sense to get the feature in with the 40-char constraint first.

psakievich · 2021-09-17T16:15:19Z

lib/spack/docs/environments.rst

@@ -368,6 +368,34 @@ from being added again.  At the same time, a spec that already exists in the
 environment, but only as a dependency, will be added to the environment as a
 root spec without the ``--no-add`` option.

+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+Developing Packages in a Spack Environment
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^


Can this feature be used in package definitions as well? I'm assuming yes, but it would be good to mention that or add some documentation in the packaging guide. For me that is where I most want to use this. I have a dependency on some packages that don't have formal releases or tags. We just work off one branch/version they have and I'd like to snapshot with the git hash it for our own releases via spack.

Yes, it can be used anywhere that a package and version are parsed. This PR adds a reference to it in the basic_usage section to address that, do you think I need to add it somewhere else?

From a packaging perspective, I would strongly discourage using this to specify a dependency. I might be convinced otherwise by a compelling use case, but my instinct would be not to accept a PR that uses this feature in a depends_on directive.

No that's sufficient. You make several good points.

Co-authored-by: Harmen Stoppels <harmenstoppels@gmail.com>

psakievich · 2022-01-04T03:02:18Z

Bump. I was looking for this documentation today.

document git commit versions

cdf6b44

spackbot-app bot added documentation versions labels Sep 15, 2021

tgamblin self-requested a review September 15, 2021 22:17

sethrj previously approved these changes Sep 16, 2021

View reviewed changes

haampie requested changes Sep 16, 2021

View reviewed changes

tldahlgren assigned haampie and sethrj Sep 16, 2021

psakievich reviewed Sep 17, 2021

View reviewed changes

fix variant name in git commit version docs

1779016

Co-authored-by: Harmen Stoppels <harmenstoppels@gmail.com>

becker33 dismissed sethrj’s stale review via 1779016 September 20, 2021 16:04

sethrj removed their assignment Sep 24, 2021

alalazo added this to In progress in Spack v0.18.0 release via automation Jan 4, 2022

becker33 removed this from In progress in Spack v0.18.0 release May 23, 2022

becker33 added this to In progress in Spack v0.18.1 release via automation May 23, 2022

alalazo removed this from In progress in Spack v0.18.1 release Jul 4, 2022

alalazo added this to In progress in Spack v0.19.0 release via automation Jul 4, 2022

psakievich mentioned this pull request Aug 3, 2022

Add documentation for git refs as versions #31914

Merged

becker33 closed this in #31914 Aug 4, 2022

Spack v0.19.0 release automation moved this from In progress to Done Aug 4, 2022

alalazo removed this from Done in Spack v0.19.0 release Aug 4, 2022

haampie deleted the docs/git-commit-versions branch February 20, 2023 18:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

document git commit versions #25981

document git commit versions #25981

becker33 commented Sep 15, 2021 •

edited by tgamblin

becker33 commented Sep 15, 2021

sethrj left a comment

haampie Sep 16, 2021 •

edited

haampie commented Sep 16, 2021 •

edited

psakievich left a comment •

edited

psakievich Sep 17, 2021

becker33 Sep 20, 2021

psakievich Sep 17, 2021

becker33 Sep 20, 2021

psakievich Sep 17, 2021

becker33 Sep 20, 2021

psakievich Sep 17, 2021

becker33 Sep 20, 2021

becker33 Sep 20, 2021

psakievich Sep 21, 2021

psakievich commented Jan 4, 2022

document git commit versions #25981

document git commit versions #25981

Conversation

becker33 commented Sep 15, 2021 • edited by tgamblin

becker33 commented Sep 15, 2021

sethrj left a comment

Choose a reason for hiding this comment

haampie Sep 16, 2021 • edited

Choose a reason for hiding this comment

haampie commented Sep 16, 2021 • edited

psakievich left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

psakievich commented Jan 4, 2022

becker33 commented Sep 15, 2021 •

edited by tgamblin

haampie Sep 16, 2021 •

edited

haampie commented Sep 16, 2021 •

edited

psakievich left a comment •

edited