-
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
document git commit versions #25981
document git commit versions #25981
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it can be used anywhere that a package and version are parsed. This PR adds a reference to it in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No that's sufficient. You make several good points. |
||
|
||
The ``spack develop`` command allows one to develop Spack packages in | ||
an environment. It requires a spec containing a concrete version, and | ||
will configure Spack to install the package from local source. By | ||
default, it will also clone the package to a subdirectory in the | ||
environment. This package will have a special variant ``dev_path`` | ||
set, and Spack will ensure the package and its dependents are rebuilt | ||
any time the environment is installed if the package's local source | ||
code has been modified. Spack ensures that all instances of a | ||
developed package in the environment are concretized to match the | ||
version (and other constraints) passed as the spec argument to the | ||
``spack develop`` command. | ||
|
||
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
latest version of the package, and ``spack install`` will install from | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so "latest version" in this case follows the rules below? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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:
FWIW, in Julia they differentiate between version numbers (using $ 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 |
||
|
||
Further information on git commits as versions can be found in the | ||
API documentation for the ``Version`` class. | ||
|
||
^^^^^^^ | ||
Loading | ||
^^^^^^^ | ||
|
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.
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?
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.
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.