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

document git commit versions #25981

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 7 additions & 2 deletions lib/spack/docs/basic_usage.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

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?

Copy link
Member Author

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.

may be specified instead of a numerical version. Spack will inspect
the tags in the git repo to determine the nearest previous version
known to Spack, and use that as the basis for comparison. Details
about how versions are compared and how Spack determines if one
version is less than another are discussed in the developer guide.

If the version spec is not provided, then Spack will choose one
according to policies set for the particular spack installation. If
the spec is ambiguous, i.e. it could match multiple versions, Spack
will choose a version within the spec's constraints according to
policies set for the particular Spack installation.

Details about how versions are compared and how Spack determines if
one version is less than another are discussed in the developer guide.

^^^^^^^^^^^^^^^^^^
Compiler specifier
Expand Down
28 changes: 28 additions & 0 deletions lib/spack/docs/environments.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Member Author

Choose a reason for hiding this comment

The 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 basic_usage section to address that, do you think I need to add it somewhere else?

Copy link
Member Author

Choose a reason for hiding this comment

The 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 depends_on directive.

Copy link
Contributor

Choose a reason for hiding this comment

The 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
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Member Author

Choose a reason for hiding this comment

The 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 spack develop feature.

latest version of the package, and ``spack install`` will install from
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The 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 develop/main/master/head/trunk and ahead of any known versions whose git tags are ancestors of that commit.

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.
Copy link
Member

@haampie haampie Sep 16, 2021

Choose a reason for hiding this comment

The 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:

  • 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')...


Further information on git commits as versions can be found in the
API documentation for the ``Version`` class.

^^^^^^^
Loading
^^^^^^^
Expand Down
68 changes: 67 additions & 1 deletion lib/spack/spack/version.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,73 @@ def __gt__(self, other):


class Version(object):
"""Class to represent versions"""
"""Class to represent versions

Versions are compared by converting to a tuple and comparing
lexicographically.

The most common Versions are numeric, and are parsed from strings
such as ``2.3.0`` or ``1.2-5``. These Versions are represented by
the tuples ``(2, 3, 0)`` and ``(1, 2, 5)`` respectively.

Some Spack versions involve slight extensions of numeric syntax;
for example, ``0.1.3a``. Versions are split on ``.``, ``-``, and
``_`` characters, as well as any point at which they switch from
numeric to alphabetical or vice-versa. For example, the version
``0.1.3a`` is represented by the tuple ``(0, 1, 3, 'a') and the
version ``a-5b`` is represented by the tuple ``('a', 5, 'b')``.

Spack versions may also be arbitrary non-numeric strings or git
commit SHAs. The following are the full rules for comparing
versions.

1. If the version represents a git commit, see the note below.

2. The version is split into fields based on the delimiters ``.``,
``-``, and ``_``, as well as alphabetic-numeric boundaries.

3. The following develop-like strings are greater (newer) than all
numbers and are ordered as ``develop > main > master > head >
trunk``.

4. All other non-numeric versions are less than numeric versions,
and are sorted alphabetically.

These rules can be summarized as follows:

``develop > main > master > head > trunk > 999 > 0 > 'zzz' > 'a' >
''``

A note on git commit versions:

Git commit versions are handled separately from all other version
comparisons. When Spack identifies a git commit version, it
associates a ``CommitLookup`` object with the version. This object
handles caching of information from the git repo. When executing
comparisons with a git commit version, Spack queries the
``CommitLookup`` for the most recent version previous to this
commit, as well as the distance between them expressed as a number
of commits. If the previous version is ``X.Y.Z`` and the distance
is ``D``, the git commit version is represented by the tuple ``(X,
Y, Z, '', D)``. The component ``''`` cannot be parsed as part of
any valid version, but is a valid component. This allows a git
commit version to be less than (older than) every Version newer
than its previous version, but still newer than its previous
version.

To find the previous version from a git commit version, Spack
queries the git repo for its tags. Any tag that matches a version
known to Spack is associated with that version, as is any tag that
is a known version prepended with the character ``v`` (i.e., a tag
``v1.0`` is associated with the known version
``1.0``). Additionally, any tag that represents a semver version
(X.Y.Z with X, Y, Z all integers) is associated with the version
it represents, even if that version is not known to Spack. Each
tag is then queried in git to see whether it is an ancestor of the
commit in question, and if so the distance between the two. The
previous version is the version that is an ancestor with the least
distance from the commit in question.
"""
__slots__ = ['version', 'separators', 'string', 'commit_lookup']

def __init__(self, string):
Expand Down