Spack: How to handle versions like develop and master

Created on 8 Oct 2016  Â·  72Comments  Â·  Source: spack/spack

@davydden @tgamblin
This Issue is related to: #1924 #1940 #1561

Currently, the @develop version is special, in that it is larger than all numeric versions, whereas all other non-numeric versions are smaller. Although this does serve a purpose, it feels like a special-case "hack" where a more general (and transparent) approach could serve us better. This is a proposal for comparing versions that is more general.

A version consists of a series of strings separated by dots. For example: '1', '1.2', 'master', '1.2.develop', etc. Versions are compared by:

  1. Split on the dots. Eg: '1.2.develop' --> ('1', '2', 'develop')
  2. Compare the tuples element-by-element using the following rules:
    a) An element is numeric if it consists of all 0-9 digits (plus other stuff, like 2a).
    b) Any element starting '<' is always less than 0; any element starting with '>' is greater than 9.
    c) All other elements will be sorted in alphabetical order, and placed less than elements starting with '<'

In practice, two things would need to change from current use:

  1. Replace '@develop' with '@>develop'
  2. Use quotes when specifying these versions in the bash shell. (Unless we think of different symbols to use for < and >. The symbols can be more than one character if we like. Maybe '~' and '+' since those are already used elsewhere in Spack).

Thoughts?

[This proposal has been updated slightly from original]

discussion revisit versions
Source
picture citibeth

Most helpful comment

Hi all,

Sorry I haven't been in on this discussion. It's a good one! It think I should clarify a few things.

Ok, on develop:

  1. I'm not a huge fan of the develop special case, but I think it's a decent workaround for the problem that we don't currently _know_ how to compare versions for unreleased things fetched out of repos.
  2. I actually _like_ @davydden's and @alalazo's idea to make develop function the same way as a version component as it does as a standalone version (sorry if I'm getting the credit wrong here).

On the larger problem:

  1. The Version class as initially designed was based on how RPM and, to some extent, Python's LooseVersion does version comparisons, with a couple things in mind:

    1. When people say they want "version 1.2" of something, they mean 1.2.whatever, PROBABLY 1.2.latest. That's the rationale for the way the : works and why the ranges are inclusive and not exclusive.

    2. The scheme was designed to be simple and understandable: split on alpha/numeric boundaries and any special characters, then compare lexicographically.

    3. There is no one true versioning scheme, and packaging systems should _not_ make packagers conform to any particular scheme. Uniform rules that can be applied across all packages are better than special cases that address shortcomings in particular packages' versioning schemes. If a package ships with unsortable versions, too bad. That's _their_ fault.

So, I think special cases complicate things. I _really_ don't want to add more than one special case because I think we're going to end up with something like the PHP type comparison table. I just really don't think I could live with having my name on something like that.

Stuff I do like:

  1. I would like a way to handle exclusive ranges, !=, and to compare OS version numbers. For the OS version, I think that we should probably not use things like elcapitan and sierra to identify Mac OS versions, and we could keep it simply by just using numeric designators. This is probably my bad for going with what Homebrew does. More on this later.
  2. I think there's a strong case for better syntax. I think it would actually be pretty easy to modify VersionList and VersionRange to support the setuptools syntax along with the : syntax.

Ok now about the special cases. I think there are a couple of major points missing from the discussion around develop. They are:

  1. All the rules suggested for comparing develop above are _correct_ ONLY at the time of installation, and they quickly become _wrong_ as the installed package ages.
  2. develop is a moving target. We need a way to convert it to a fixed target.

These, IMHO, are the biggest problem with develop, why I don't really like the special case, and why I don't think there IS a good, unified set of rules for comparison.

(1) causes problems for concretization because if you sort things, an installed foo@develop will be greater than [email protected] EVEN if foo@develop was installed 2 years ago. If we want Spack to be able to reason about installed packages as well as to-be-installed packages, we need to fix this.

(2) I think many of the approaches outlined above are trying to make the version system more complicated to address (1), when really we should be focusing on (2). I think you can address (2) with smarter fetchers. develop and friends really only arise when you pull something out of a VCS repo. If you can do that, you can also figure out what version it _really_ is, and install it with the _real_ version.

So, e.g., for git, if you install something @develop, you could query the commit id and the most recent tag in the history, and have develop concretize to, e.g., 1.2.1f34ced14 (the latest tag here was 1.2). That version would age reasonably well and would not screw up concretization later. The commit hash might conflict with other numeric versions so you could add a designator for things derived from commits or VCS revisions, e.g. 1.2.dev-1f34ced14 -- I haven't fully thought that part through.

@citibeth mentioned version aliases and I actually think this is a similar concept. Basically you need to separate how you refer to things (develop) from the way you store provenance about them. develop is an abstract thing that corresponds to a concrete version _right now_. Same for things like elcapitan, sierra, etc. I think we could handle a lot of the version mayhem with some simple aliases, _maybe_ a designator for VCS-derived versions, and smarter fetchers. All while keeping the versioning system relatively simple.

Thoughts?

picture tgamblin on 10 Oct 2016
👍3

All 72 comments

If I understand you right, I see it as a choice between

(1) usability and the current feature set with only one single special version name (current state)
(2) a more general approach with two special characters to control comparison in Version class and with worse usability both for Spack users and package developers (proposed approach)

In what circumstances do you envision (2) would be beneficial, especially given that you would anyway need to have a special treatment for two strings/characters/symbols?

p.s. there are a lot of regression tests for Version class which also include cases like 2.0.1a, xyz.4, 20101121 , 10a2 among others, see https://github.com/LLNL/spack/blob/develop/lib/spack/spack/test/versions.py .

@adamjstewart @alalazo ping.

davydden picture davydden on 8 Oct 2016

Although this does serve a purpose, it feels like a special-case "hack" where a more general (and transparent) approach could serve us better.

It doesn't. It maps perfectly on the set of natural numbers extended with +Inf.

alalazo picture alalazo on 8 Oct 2016
👍1

I don't get why we would want to remove @develop as a special case. The develop branch is always newer than the master branch, which is always newer than the latest release. And by always I mean most of the time.

As for our current numbering scheme, I don't see anything wrong with it. It matches Python sequencing, so that makes sense. But if we're going to make a big change like that, I kind of like the scheme that setuptools uses. For example, here are the dependencies of flake8:

    install_requires=[                                                          
        "pyflakes >= 0.8.1, < 1.1",                                             
        "pep8 >= 1.5.7, != 1.6.0, != 1.6.1, != 1.6.2",                          
        "mccabe >= 0.2.1, < 0.5",                                               
    ],

Right now we have no means of differentiating between > and >=. We also don't have any way to specify !=. The colon slicing is great, but certainly not as intuitive as <= or !=. I've had to answer a lot of questions about whether colon slicing is inclusive or exclusive.

The only downside is that presumably we would want to keep the same CLI interface, so you would install something like [email protected], and it could be confusing if the package.py files used a different numbering scheme. But I think the trade off of flexibility and readability would be worth it. Thoughts?

adamjstewart picture adamjstewart on 8 Oct 2016
👍1

I don't get why we would want to remove @develop as a special case.

Because it's (a) bad human interface design, and (b) inflexible.

Bad Human Interface Design

Put yourself in the position of a new Spack user; remember that users do not read manuals. Nobody knows everything, but things work best when you know what you know.

Looking around some packages, you see '@develop'. You start to think about version numbers and how they might sort. It's reasonable to assume that numeric and non-numeric version numbers might be treated differently. It's _completely surprising_ to learn that '@develop' is treated differently from '@master'. In this case, you will THINK you know things that are actually false: you don't know what you know.

Even more confusing is that @develop is greater than any other version, but it's not selected by default in the concretization algorithm, which nominally chooses the largest version. (This Issue does not solve that problem; it needs an upgraded Spack security model to solve, or maybe something else).

Now suppose you see '@>develop' instead. You don't know what the '>' means, but you assume it must mean something. You know what you don't know, which is good. So you look it up in the manual and you discover that '@>develop' is greater than any numeric version. Every time in the future that you see '@>develop', you will remember that THIS version is greater than all numeric versions. The '>' symbol will remind you again and again.

Inflexible

On simple projects that use a git branch named develop, the current system matches exactly. But git schemes vary. Some people might call their development branch dev, and resent that they cannot name the version dev as well.

A more serious concern... some projects have several development branches, which the current scheme cannot handle at all. Suppose you create a branch for each major release, and then cherry-pick bug fixes onto that branch, tagging minor releases. For example, the v1 branch leads to v1.0 and v1.2 tags, and the v2 branch leads to v2.0, v2.1 and v2.2. Meanwhile, the develop branch has the latest/greatest, off of which the release branches are derived. This is not at all hypothetical.

You now have three git branches with different versioning requirements:

  1. The v1 branch is greater than any 1.x version, but less than any 2.x version
  2. The v2 branch is greater than any 2.x version, but less than any (forthcoming) 3.x version
  3. The develop branch is greater than anything.

These facts of life can be perfectly encapsulated in the Spack versions @1.>release, @2.>release and @>develop.

Pattern Matching

Right now we have no means of differentiating between > and >=. We also don't have any way to specify !=. The colon slicing is great, but certainly not as intuitive as <= or !=. I've had to answer a lot of questions about whether colon slicing is inclusive or exclusive.

This is a pattern matching problem, which is outside the scope of this Issue. To summarize:

  1. This issue is about defining a flexible and non-surprising collation order for version numbers.
  2. Pattern matching is about... given that the user specified something (a pattern), which versions match that pattern? Patterns are similar to versions, but not exactly the same. For example, '@1.1:2.3', '@:2.3' and '@1:' are not versions but they are patterns. I agree, we need improvements in the pattern syntax and semantics.
citibeth picture citibeth on 8 Oct 2016
👎2

p.s. there are a lot of regression tests for Version class which also include cases like 2.0.1a, xyz.4, 20101121 , 10a2 among others, see https://github.com/LLNL/spack/blob/develop/lib/spack/spack/test/versions.py .

That's a wrinkle that doesn't affect the main thrust of this suggestion.

citibeth picture citibeth on 8 Oct 2016

If I was a new user to Spack, without any knowledge of its version handling, I would assume 2 things:

  1. Spack is smart enough to know which versions are newer and which are older.
  2. Spack is smart enough to choose the right version for me if I don't specify one.

And that is all I would assume.

If package A depends on [email protected]: and I noticed that the latest release of B is version 2.3, that's all fine and dandy. But if I decide I want the bleeding edge and try to install A ^B@develop, and all of a sudden Spack tells me that develop isn't newer than 2.1, I would immediately file a bug report. Because that's a bug.

These facts of life can be perfectly encapsulated in the Spack versions @1.>release, @2.>release and @>develop.

Even after reading your post I still don't know what these mean. And confusing users to the point that they have no idea what something means isn't an effective way to prevent them from being confused.

So, how do we keep users from being confused? We need to make sure the following conditions are met:

  1. Spack is smart enough to know which versions are newer and which are older.
  2. Spack is smart enough to choose the right version for me if I don't specify one.

Condition 1 is easy to meet. develop and dev should be _newer_ than master which is _newer_ than any other number scheme. Because that's how things work in the real world.

Condition 2 is also easy to meet. I don't think users will be shocked to learn that Spack chooses the newest release by default, because it's stable and safe.

adamjstewart picture adamjstewart on 8 Oct 2016

I think from all practical purposes in the context of HPC/package management having a single infinity-like version (@develop) is all is needed.

davydden picture davydden on 8 Oct 2016

as for bad human design, we can argue about the best name for develop, for example you can call it infty or infinity, then it's clear how it compare to any other version. But I don't really see the reasons to do so. We just need to use develop everywhere for git-like versions and that's it. How many packages among the current ~1000 like would have more than 1 git/svn version? Probably not many.

davydden picture davydden on 8 Oct 2016

A more serious concern... some projects have several development branches, which the current scheme cannot handle at all. Suppose you create a branch for each major release, and then cherry-pick bug fixes onto that branch, tagging minor releases. For example, the v1 branch leads to v1.0 and v1.2 tags, and the v2 branch leads to v2.0, v2.1 and v2.2. Meanwhile, the develop branch has the latest/greatest, off of which the release branches are derived. This is not at all hypothetical.

You now have three git branches with different versioning requirements:

The v1 branch is greater than any 1.x version, but less than any 2.x version
The v2 branch is greater than any 2.x version, but less than any (forthcoming) 3.x version
The develop branch is greater than anything.
These facts of life can be perfectly encapsulated in the Spack versions @1.>release, @2.>release and @>develop.

this will already work just fine with 1.0 version attached to the first git branch, 2.0 attached to the second and develop attached to the latest-greatest.

davydden picture davydden on 8 Oct 2016

Two observations:

  1. How versions sort is an issue for a package author, not the user of the package. i.e. when i say spack install mylib@develop, it doesn't matter how develop sorts, Spack will install the develop version. Sort order matters to the package authors, who want to know that when they say depends_on('libgreat', when='@2.1:') later in the package that the pattern @2.1: will match the develop version.
  2. I think the problem with my proposal above is that sort information about the version is being made explicit in the version itself, rather than staying private inside package.py (where it really should be). Exporting these details to the user only adds headaches.

But the current way of doing things is still inflexible and hidden. It gives some package authors what they need some of the time, but it may also be seen as surprising to people reading the package.

The solution to this is to think about ways that a package can define how non-numeric versions sort, compared to the basic numbered sorting everybody understands and agrees upon. Now, the sort information would be inside the package, rather than encoded in the version number. For example:

version('develop', git='...', branch='develop', numeric_equivalent='>')

This tells Spack that in THIS package, the develop version is greater than any numeric version. The person reading the package will see that explicitly. And the person installing the package will never really care, of course.

This scheme can be used for release branches as well. For example:

version('release1.1', git='...', branch='release1.1', numeric_equivalent='1.1.>')
version('release1.2', git='...', branch='release1.2', numeric_equivalent='1.2.>')

As for whether Spack selects one of these versions if it is not explicitly specified... For example, if you ask for @1.2:, will it install @release1.2? The answer is, if a version is not verifiable (which versions based on Git branches never are), then it should only be installed if the user specifically requests it.

Thank you everyone for your comments, they've been really helpful. Any thoughts on this latest scheme to order versions?

citibeth picture citibeth on 8 Oct 2016

I think from all practical purposes in the context of HPC/package management having a single infinity-like version (@develop) is all is needed.

See julia/package.py:

    version('master',
            git='https://github.com/JuliaLang/julia.git', branch='master')
    version('release-0.5',
            git='https://github.com/JuliaLang/julia.git', branch='release-0.5')
    version('0.5.0', 'b61385671ba74767ab452363c43131fb', preferred=True)
    version('release-0.4',
            git='https://github.com/JuliaLang/julia.git', branch='release-0.4')
    version('0.4.7', '75a7a7dd882b7840829d8f165e9b9078')
    version('0.4.6', 'd88db18c579049c23ab8ef427ccedf5d')
    version('0.4.5', '69141ff5aa6cee7c0ec8c85a34aa49a6')
    version('0.4.3', '8a4a59fd335b05090dd1ebefbbe5aaac')

as for bad human design, we can argue about the best name for develop, for example you can call it infty or infinity

I think that versions should be named the same as git branches. As I mentioned above, the UI problem is fundamentally for the package developer, not the end user.

this will already work just fine with 1.0 version attached to the first git branch, 2.0 attached to the second and develop attached to the latest-greatest.

Except that 1.0 (the latest in the 1.0 release branch) will not be less than 1.1, when it should be greater. Also, 1.0 can be chosen automaticlly (i.e. if the user didn't request it specifically), which is a security problem.

citibeth picture citibeth on 8 Oct 2016

julia should just read (see test_rc_versions regresion tests)

    version('develop',
            git='https://github.com/JuliaLang/julia.git', branch='master')
    version('0.5.1.rc',
            git='https://github.com/JuliaLang/julia.git', branch='release-0.5')
    version('0.5.0', 'b61385671ba74767ab452363c43131fb', preferred=True)
    version('0.4.8.rc',
            git='https://github.com/JuliaLang/julia.git', branch='release-0.4')
    version('0.4.7', '75a7a7dd882b7840829d8f165e9b9078')
    version('0.4.6', 'd88db18c579049c23ab8ef427ccedf5d')
    version('0.4.5', '69141ff5aa6cee7c0ec8c85a34aa49a6')
    version('0.4.3', '8a4a59fd335b05090dd1ebefbbe5aaac')

to work OOB with Spack without any extra efforts.

think that versions should be named the same as git branches.

i don't agree, developers name branches whatever they want, it could be master, latest, trunk, develop and alike. What Spack package author needs to know is which branch corresponds to _infinity_ version or the latest.

Except that 1.0 (the latest in the 1.0 release branch) will not be less than 1.1, when it should be greater. Also, 1.0 can be chosen automaticlly (i.e. if the user didn't request it specifically), which is a security problem.

see above for Julia.

As for your new design (aside from the fact that I still don't agree that Spack needs those feature at all), it appears to me that would be like rewriting almost everything related to version comparison, including the Version class itself, which now should be much more complicated and each instance of this class should have some special rules to be read somehow from package.py. It might as well not fit into the current code design at all (I can't tell). It will also be far more difficult to maintain and one can expect more bug reports related to this part of Spack. And all that for something that IMHO is already possible (see Julia above).

davydden picture davydden on 8 Oct 2016

@citibeth

I think that versions should be named the same as git branches. As I mentioned above, the UI problem is fundamentally for the package developer, not the end user.

@davydden

i don't agree, developers name branches whatever they want, it could be master, latest, trunk, develop and alike. What Spack package author needs to know is which branch corresponds to infinity version or the latest.

I actually agree with @citibeth on this one. I don't think @davydden's idea is flexible enough. For example, take a package with both a develop and master branch. Are you suggesting that develop is newer than all other releases and master is older then all other releases? I think we should agree on an ordering for these common tags. develop, dev, and latest are probably newer than master and trunk. Hopefully we will never encounter a package that has all 5 lol. But regardless, All of these tags should be considered newer than a numbered release. And anything that doesn't fit within this scheme can be renamed. Why call something release-0.5 or v0.5 when you could call it 0.5?

adamjstewart picture adamjstewart on 8 Oct 2016

For example, take a package with both a develop and master branch. Are you suggesting that develop is newer than all other releases and master is older then all other releases?

presumably that is a very rare case. If we happen to have such a package, there is indeed no way to a-priori know how to compare branches.

I think we should agree on an ordering for these common tags. develop, dev, and latest are probably newer than master and trunk

whatever we agree on, this may not be the way developers actually use it. I still stand on my statement that in 99% of cases it is enough to have a single version which behaves like infinity.

Why call something release-0.5 or v0.5 when you could call it 0.5?

that's exactly my point above with Julia. We don't have to follow branch names.

davydden picture davydden on 8 Oct 2016

Here is my alternative suggestion:

  1. still use one and only one string to mark infinity (develop, infty, infinity or alike)
  2. make the following rules work: develop > x.y.z, x.develop > x.y and the same for less-than.
  3. adjust conretization rules so that any version which has develop in it (i.e. develop, 3.develop) is not chosen by default unless specified in packages.yaml or on the command line and alike.

The net result will be:

versions

This will require only minor modifications to Version class and is certainly possible and will fit into the current Spack code. It will also make julia package read better with versions like 0.5.develop.

This will not solve the issue of 5 different branches all begin master, develop, trunk or alike, but again, i don't think we will enounter this situation often.

p.s. I don't care about how we call it, it may as well be infinity. I just firmly believe that it is enough to have one special case for infinity number in Version class.

davydden picture davydden on 8 Oct 2016

I tend to think like @davydden that 99% of the cases are already covered. Anyhow I also think that what @citibeth want is not a new scheme for the numbering, but a PartitionVersion. Something like the partition set you use to define real numbers starting from rationals.

If this works it means that we may preserve @develop as our +Inf and we may write something like:

version('release-0.4',  branch='release-0.4', partition='0.5')

and release-0.4 will always compare :

  • minor to anything that is greater or equal to 0.5
  • equal to other partition=0.5 versions
  • greater to other things

If you all agree on this analysis I'll try to code up something just for the versioning class (+tests) in a branch. Then (probably after SC16) we may think how to hook this into the version directive.

alalazo picture alalazo on 8 Oct 2016

ps. i joined my last two posts for better readability.

davydden picture davydden on 8 Oct 2016

BTW : I think the title of the issue is misleading seeing where this discussion is going.

alalazo picture alalazo on 8 Oct 2016
👍1

@davydden it seems to me that 0.4.develop is the same as partition=0.5, so I think that logically we are proposing the same thing...

alalazo picture alalazo on 8 Oct 2016

@davydden it seems to me that 0.4.develop is the same as partition=0.5, so I think that logically we are proposing the same thing...

could be indeed the case. I have not looked at PartitionVersion class,
but the approach I outlined can be dirty-cheap implemented by 1.develop.3 -> 1.999999.3 inside the Version class constructor. Everything else should follow.

scratch that, it will require a bit more than that.

davydden picture davydden on 8 Oct 2016

Summary

Attempts to summarize this discussion:

  1. Everyone seems to agree that version specifiers should be multi-part, separated by dots, and compared using some sort of element-compare function for the resulting tuple.
  2. There are proposals for special elements that mean +infinity and -infinity. They've been, variously:
    a) develop = +infinity, <any other string> = -infinity
    b) > = +infinity, < = -infinity
    c) >... = +infinity, <... = -infinity

These infinities are needed to cleanly describe "anything newer than..." or "anything older than..." In real life, +infinity is probably more important than -infinity, but symmetry would demand both if we're going to have one.

  1. There's an additional proposal to allow for arbitrary relationship between non-numeric Spack versions and version specifiers as described above. Or to be more specific: a _version specifier_ is something like 1.2, 1.2a, 1.develop.3, etc. that fits the system proposed above. A _version_ is a string that maps to a version spec. If a version is already a version spec, then the mapping is the identity; if not, there is some table lookup allowing descriptive words such as develop, master, release1.2 to map to version specs. A version string that's not a version spec (such as develop), I will call a _descriptive version_.

When Spack encounters a descriptive version (packages.yaml, from the command line, etc), it will translate it to a version spec; everything else works as it does now. Where it translates to a version spec (either as soon as it's read, or at the very last moment) would be an implementation detail to work out.

The lookup table for version->version spec would be assembled by an additional kwarg to the version() declaration inside package.py files.

Evaluation

Everyone agrees on (1), and I think it's a good modification to the current system.

(3) is a key idea here in order to allow things to work naturally for the user. It also allows people to name their versions whatever they want, and to make them work however they like for their particular project. It allows us to hide version specs from the user, making things work more naturally from their point of view. (i.e. the user installs release-branch-1.2 instead of 1.2.develop.) It also allows more than one version to use this kind of stuff; for example, maybe you have TWO git branches that are both "greater than 1.2". You could map them to version specs 1.2.>.1 and 1.2.>.2, for example.

As for (2)... I think the two infinity concepts are needed. We seem to be arguing over what we will call them. I will vote for < and >. If (3) is implemented, then these symbols will only go inpackage.py files, the user will never type them directly.

citibeth picture citibeth on 9 Oct 2016
😕2 👎2

As for (2)... I think the two infinity concepts are needed.

disagree. When exactly would you need -infinity? There is no version of the software which is lesser than any other released version. It is simply not needed in this context.

I vote for keeping develop to stand for +infinity. Optionally one could add 1.develop > 1.2, but as I said above 99% of cases is already covered by simple develop > x.y.z.

davydden picture davydden on 9 Oct 2016
👍1

B.t.w., one more argument i though about but did not raise:

Version class need to work by itself without actually knowing that it is used within a Package class. The reasons is that we may re-use it elsewhere, for example to write conditionals for OS versions in packages (i.e. certain patches only apply to certain range of versions).

If we go for re-designing Version class to work close with Package, this usage case may not be possible anymore.

davydden picture davydden on 9 Oct 2016
👍2

If we go for re-designing Version class to work close with Package, this usage case may not be possible anymore.

thereby, gaining very little in release-branch-1.2 instead of 1.2.develop, we may loose much more in using Version class elsewhere in Spack and potentially even more in less clean object oriented design.

davydden picture davydden on 9 Oct 2016

There is no version of the software which is lesser than any other released version.

put in in math words, you can think of version numbers to be integers on [0, \infinity).

davydden picture davydden on 9 Oct 2016

If we go for re-designing Version class to work close with Package, this usage case may not be possible anymore.

I believe Version is stable enough to be left alone. If we ever decide to do something it should be in the direction of sub-classing it.

alalazo picture alalazo on 9 Oct 2016

Version aliases... I think that's the best way to think of things. Everything has a version number in a well-defined scheme. Non-numeric version strings actually become aliases to version numbers. When a version string is parsed from the user (with respect to a package), it is immediately converted into a tuple equal to the parsed version number, plus the raw version string as the last element of the tuple. The raw string gets carried around as part of the Version, and can be used for any printout with the user; other than that, it would have pretty much no effect.

So now the syntax would be something like:

version('infinity', git='...', alias='develop')

For the sake of clarity, I vote that we use the string "infinity" to mean infinity.

As for -infinity... Suppose I'm working on some kind of crazy "debugging" branch. I want that branch to never be selected, so it needs to be SMALLER than all its peers.

citibeth picture citibeth on 10 Oct 2016

@citibeth

it is immediately converted into a tuple equal to the parsed version number, plus the raw version string as the last element of the tuple

one should not parse it immediately as it may lead to some hard-to-find bugs because we still want the rest of Spack operate on strings. So when Spack checks which version is installed, it should output develop, not the value we use inside Version class to actually make comparison behave as if develop is +infinity. In other words, things we do to define comparison rules should not affect anyhow the rest of the Spack. This is the reason why I don't parse it immediately in https://github.com/LLNL/spack/pull/1979.

version('infinity', git='...', alias='develop')

why not just use develop and document that we treat it as +infinity for comparison? I don't think there are a lot of cases which would require extension of version function in package.py to provide alias field, right? I would just not make it more complicated that we need to.

As for -infinity... Suppose I'm working on some kind of crazy "debugging" branch. I want that branch to never be selected, so it needs to be SMALLER than all its peers.

Even in case we don't consider non-numeric versions at al (which won't be the case, see below), it will be perfectly fine to do the following

system -> -1
trunk -> -2

This would mean that inside Version class these versions will always compare less-than with anything like 0.1.

However that still won't remove the necessity to treat non-numeric versions. Somebody can add a version fancy_case or alike and we need to be able to deal with it. So I think the code you recently contributed by distinguishing numeric and non-numeric is perfectly fine and we still need to do this either way. Consequently, i still don't see any need for -infinity in versions.

Also keep in mind that there are cases like

assert_ver_gt('2.0.1a', '2.0.1')
assert_ver_lt('10a2', '10b2')

In other words, not everything in version string is a number.

davydden picture davydden on 10 Oct 2016

On Mon, Oct 10, 2016 at 10:36 AM, Denis Davydov [email protected]
wrote:

@citibeth https://github.com/citibeth

it is immediately converted into a tuple equal to the parsed version
number, plus the raw version string as the last element of the tuple

one should not parse it immediately as it may lead to some hard-to-find
bugs because we still want the rest of Spack operate on strings. So when
Spack checks which version is installed, it should output develop, not
the value we use inside Version class to actually make comparison behave
as if develop is +infinity. In other words, things we do to define
comparison rules should not affect anyhow the rest of the Spack. This is
the reason why I don't parse it immediately in #1979
https://github.com/LLNL/spack/pull/1979.

Any part of Spack that needs the original string can get it in
version[-1]. No, this is not a trivial "drop-in" change; but to me it
seems absolutely doable.

version('infinity', git='...', alias='develop')

why not just use develop and document that we treat it as +infinity for
comparison?

See @adamjstewart comments above for motivation of version aliases. They
give us a lot of flexibility and clarity.

I don't think there are a lot of cases which would require extension of
version function in package.py to provide alias field, right? I would
just not make it more complicated that we need to.

All of the release branch examples from above would benefit from aliases.

As for -infinity... Suppose I'm working on some kind of crazy "debugging"
branch. I want that branch to never be selected, so it needs to be SMALLER
than all its peers.

Even in case we don't consider non-numeric versions at al (_which won't
be the case, see below_), it will be perfectly fine to do the following

system -> -1
trunk -> -2

So you agree with me that -infinity semantics are also needed, no matter
what you call them.

Right now we have this weird scheme where "any string" = -infinity and
"develop" = infinity. And we expect the version number to do double duty
as an ordered set of tuples, plus a label users use to say which version
they want. Hence my suggested change are (a) add version aliases to avoid
the double duty, and (b) call infinity and -infinity by what they are,
rather than these arbitrary strings.

However that still won't remove the necessity to treat non-numeric
versions. Somebody can add a version fancy_case or alike and we need to
be able to deal with it.

See version aliases.

So I think the code you recently contributed by distinguishing numeric and
non-numeric is perfectly fine and we still need to do this either way.
Consequently, i still don't see any need for -infinity in versions.

What I'm proposing now would do away with arbitrary version "numbers", they
would be converted to version aliases with explicit numbers instead. Once
the core Spack is changing, it would involve a search-and-replace in the
current packages of:
version('develop', ...) ---> version('infinity', ..., alias='develop')
version('', ...) --> version('-infinity', ...,
alias='')

The user would notice little or no change.

Also keep in mind that there are cases like

assert_ver_gt('2.0.1a', '2.0.1')
assert_ver_lt('10a2', '10b2')

In other words, not everything in version string is a number.

For the purposes of this discussion, those are "numbers". They are
well-defined in an RFC referenced in the code, which we would follow. The
current codebase also treats them as "numbers", i.e. isnumeric('2.0.1a') ==
True.

citibeth picture citibeth on 10 Oct 2016

See @adamjstewart comments above for motivation of version aliases. They
give us a lot of flexibility and clarity.

it is already possible with https://github.com/LLNL/spack/pull/1979, just add a few more along with develop if really needed. No big deal.

All of the release branch examples from above would benefit from aliases.

again, already done in https://github.com/LLNL/spack/pull/1979, see regression tests.

So you agree with me that -infinity semantics are also needed, no matter
what you call them.

no, i don't. We just need to know what to do with those non-numeric versions which conceptually shall not be considered greater than releases or minor releases. Again, it is probably only system which would be used along with develop-like versions.

What I'm proposing now would do away with arbitrary version "numbers", they
would be converted to version aliases with explicit numbers instead. Once
the core Spack is changing, it would involve a search-and-replace in the
current packages of

My bottom line is that Version class is in a good shape and there is no need to re-do it and also adjust core of the Spack as you suggest. IMHO major shortcomings of Version class (w.r.t Julia package and alike) are addressed in a small PR https://github.com/LLNL/spack/pull/1979. I don't see a reason for a major rewriting of anything.

davydden picture davydden on 10 Oct 2016

why not just use develop and document that we treat it as +infinity for
comparison?

See @adamjstewart comments above for motivation of version aliases. They
give us a lot of flexibility and clarity.

Just to be clear, I'm advocating for the following scheme:

Version Comparison

A tiered system, with no aliases involved.

| Tier | Version |
| --- | --- |
| Tier 1 | develop/dev/latest |
| Tier 2 | master/head/trunk |
| Tier 3 | numeric versions |
| Tier 4 | non-numeric versions not listed above |

Things in tier 1 are newer than things in tier 2 are newer than things in tier 3, etc. Things within tier 3 are compared numerically as we always have. It should be easy to add tags to tier 1 and 2 if everyone agrees on them, but it should be done globally. There shouldn't be a single magical tag (currently develop) that means newer than everyone else, as real world projects don't all agree on a single tag. When possible, we should avoid renaming tags for the sake of Spack. They should only be renamed when simplifying numeric versions. For example, v1.0 -> 1.0, release-1.0 -> 1.0.

If necessary, there's really no reason we couldn't combine tier 1 and tier 2 into a single tier. As far as I know, the only reason they need to be treated as newer is so that packages that depend on [email protected]: can use them. But there's no reason we need a secret infinity alias. Greater than/less than comparison doesn't need to be done between numbers. You can override the __gt__ function for a reason.

Concretization

Concretization does not choose the latest version by default, it chooses the latest numeric version, as these are stable releases.

adamjstewart picture adamjstewart on 10 Oct 2016
👎2 👍1

@adamjstewart

I am sorry for constantly referring to the PR, but what you suggest can be added quickly to https://github.com/LLNL/spack/pull/1979 , more precisely around this line https://github.com/LLNL/spack/pull/1979/files#diff-aea709116ed332ec953e0764378b6420R77 .

same applies to conretization, for which Tier1/2 would be added here https://github.com/LLNL/spack/pull/1979/files#diff-aea709116ed332ec953e0764378b6420R193

davydden picture davydden on 10 Oct 2016

for a record, i just implemented what @adamjstewart suggested.

davydden picture davydden on 10 Oct 2016

A tiered system, with no aliases involved.

To be clear, outside of Version class, there are no aliases. Whatever we do inside only make comparison behave the way we need. If my interpretation of what @adamjstewart says is correct, then we both agree on this point and suggest the same thing.

davydden picture davydden on 10 Oct 2016
👍1

Without aliases we can craft version numbers that are ordered the way we
need, but we export whatever weird version string we come up with to the
user. We are trying to make the strings less weird by defining special
rules on what they mean. But we will always be left with weird strings in
some cases. Package builders will want the flexibility to name aliases
after branches without being constrained to how a particular string sorts.

Version aliases allow us to hide these version strings from the user,
giving flexibility and clarity.

Is anyone actually opposed to them from a user's perspective?
On Oct 10, 2016 11:28 AM, "Denis Davydov" [email protected] wrote:

A tiered system, with no aliases involved.

To be clear, outside of Version class, there are no aliases. Whatever we
do inside only make comparison behave the way we need. If my interpretation
of what @adamjstewart https://github.com/adamjstewart says is correct,
then we both agree on this point and suggest the same thing.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/LLNL/spack/issues/1975#issuecomment-252656299, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AB1cdwcHjJfZI4S2wzEKGSME42m8BrWsks5qyll_gaJpZM4KRtI8
.

citibeth picture citibeth on 10 Oct 2016

@davydden I still don't see any need for 999999999999 anywhere in the code. Python classes allow you to override __lt__, __gt__, and all other comparison operators. These can be overridden with if-statements. What about something like this (this is half-way pseudo-code, so it might not work perfectly):

tier1 = ('develop', 'dev', 'latest')
tier2 = ('master', 'head', 'trunk')

def __gt__(self, other):
    if self in tier1:
        if other in tier1:
            return False
        else:
            return True
    elif self in tier2:
        if other in tier1 or other in tier2:
            return False
        else:
            return True
    elif is_numeric(self):
        if other in tier1 or other in tier2:
            return False
        elif is_numeric(other):
            return numeric_comparison(self, other)
        else:
            return True
    else:
        return False

I think your code does the same thing, but tries to do it by converting everything to a number for comparison. I agree that this is fewer lines of code, but is more of an abstraction. I think the point of this Issue is that we shouldn't have as many abstractions. My pseudo-code is a bit more direct, but I agree that it is more lines, and we would have to repeat something similar in other __lt__, __eq__, etc methods, which could be incorrectly copied and pasted.

It's possible that you could convince me that your solution is superior to mine. But can you replace 999999999999 with sys.maxint so that we aren't using magic numbers?

I noticed your recent edit. There's no reason that develop, dev, and latest can't all be equal. Although maybe that would cause confusion when printing spack info with ordered versions? I doubt we'll find a package with both develop and dev, but it can't hurt to be bulletproof.

adamjstewart picture adamjstewart on 10 Oct 2016

@adamjstewart do you mind moving this comment to the PR and i improve there? We still need strict comparison, in that if a>b>c then a>c. So versions within each tier should also be ordered.

davydden picture davydden on 10 Oct 2016

@citibeth I would be fine with very limited aliases. As in, if I have a package with a tag for my-develop and I really want the version to be called my-develop instead of develop, then I would be fine with an alias for develop.

adamjstewart picture adamjstewart on 10 Oct 2016

@davydden Will do. Out of curiosity, why do you think we need strict comparison in which two versions cannot be equal?

adamjstewart picture adamjstewart on 10 Oct 2016

Out of curiosity, why do you think we need strict comparison in which two versions cannot be equal?

i don't know all the internals of Spack, but this may cause undefined behaviour for conretization, maybe different hashes or alike. I think it is a safe bet to have strict ordering.

davydden picture davydden on 10 Oct 2016

The only problem I can think of with having multiple versions that are equal (develop, dev, and latest for example) is that when printing spack info they may not always maintain the same order. Which is fine because I don't think this will ever even be the case.

i don't know all the internals of Spack, but this may cause undefined behaviour for conretization, maybe different hashes or alike. I think it is a safe bet to have strict ordering.

Can anyone who does know the internals of Spack comment on this? I am genuinely curious. I am also very much in favor of using the Version class for other things, including but not limited to OS version comparison.

adamjstewart picture adamjstewart on 10 Oct 2016
👍1

@adamjstewart Ok, explanation for my thumbs down : one special case (develop) is more than enough. Having more than one does neither achieve generality nor simplify code.

alalazo picture alalazo on 10 Oct 2016

i don't know all the internals of Spack, but this may cause undefined behaviour for conretization, maybe different hashes or alike. I think it is a safe bet to have strict ordering.

I don't think this could cause different hashes, but correct me if I'm wrong. It definitely couldn't (or at least shouldn't) cause undefined behavior for concretization, at least according to my tiered scheme. In terms of concretization, if the user does not select a version and all versions are valid, then it should sort the versions and select the first one that is numeric.

Although, hmm, what if you had a package that depended on A@master:. I hope this never needs to be the case, but then it wouldn't always choose the same version if develop and dev are both in there...

adamjstewart picture adamjstewart on 10 Oct 2016

@adamjstewart Ok, explanation for my thumbs down : one special case (develop) is more than enough. Having more than one does neither achieve generality nor simplify code.

Develop isn't a special case, it's a quite common label applied to a cutting edge development branch. And it isn't always called develop. It's usually develop in Git repos, but in other repos I've seen other common labels that mean the same thing. I don't think we should make people rename these labels if a Subversion repo calls it latest instead.

Similarly, master is a quite common label applied to a slightly more stable version that is not ready for an official release yet. And it isn't always called master. I've seen trunk and head quite commonly in Subversion/CVS.

These aren't special cases so much as they are real-world labels that we should account for and allow.

adamjstewart picture adamjstewart on 10 Oct 2016

@adamjstewart I am not arguing on common practice for repositories. I am arguing on the numbering scheme you propose, which involves a lot of conditionals in comparison functions and three different names for infinity and "almost infinity" (master, etc.)

alalazo picture alalazo on 10 Oct 2016

I've got another idea... let's get rid of 'implicit none' in FORTRAN. Because it's _REALLY CONVENIENT_ that your variables are integers if they're named i, j or k; but otherwise reals. Once you're privy to this convention, you won't have to declare data types anymore. Sure, the convention may be non-obvious to newcomers. But that's how we can distinguish between REAL FORTRAN programmers and dilettantes who can only program in Algol or (gasp) COBOL. If people want an integer variable, they should figure out how to make its name start with an 'i'.

Best of all, with implicit variable typing, you don't have to complicate the compiler implementation by carrying around data type information along with the name. Any part of the compiler that really NEEDS to know what type a variable is can already infer it directly from the name.

I am closing this thread because what I am now proposing (version aliases) is different from what I originally proposed. And it is orthogonal to minor tweaks to the Version class and implementation details on those tweaks, which seems to be what people want to talk about. I will open another thread at some point presenting version aliases in a clear and concise manner. At that time, I hope to convince us all that version aliases are a feature we want.

citibeth picture citibeth on 10 Oct 2016

@alalazo I have two counters to your complaint:

  1. I don't think we should force users to rename their versions in package.py files if they disagree with the original repo. Many repos name develop differently, and Spack should honor that. I'm not saying we should allow dozens of different tags, just the common ones.
  2. Many repos have more than one version that is newer than the latest release. For example, here are all of the packages that currently have a master version:
$ grep version.*master */package.py
boxlib/package.py:    version('master', git='https://ccse.lbl.gov/pub/Downloads/BoxLib.git')
caliper/package.py:    version('master', git='https://github.com/LLNL/Caliper.git')
cbtf-argonavis/package.py:    version('1.6', branch='master',
cbtf-krell/package.py:    version('1.6', branch='master',
cbtf-lanl/package.py:    version('1.6', branch='master',
cbtf/package.py:    version('1.6', branch='master',
cityhash/package.py:    version('master', branch='master',
cnmem/package.py:    version('git', git='https://github.com/NVIDIA/cnmem.git', branch="master")
flux/package.py:    version('master', branch='master',
hdf5-blosc/package.py:    version('master', git='https://github.com/Blosc/hdf5-blosc',
julia/package.py:    version('master',
libsplash/package.py:    version('master', branch='master',
lrzip/package.py:    version('master', git='https://github.com/ckolivas/lrzip.git')
openspeedshop/package.py:    version('2.2', branch='master',
pngwriter/package.py:    version('master', branch='master',
raja/package.py:    version('git', git='https://github.com/LLNL/RAJA.git', branch="master")
rose/package.py:    version('master', branch='master',

In particular, libsplash has both a master and a dev version. If I build something that depends on [email protected]: and choose ^libsplash@master, are you proposing that this should say that master isn't newer than 1.2?

adamjstewart picture adamjstewart on 10 Oct 2016

Reopening and changing the name to something that reflects the current discussion. This Issue is gathering a lot of attention and it would be a shame if no one noticed it anymore because it was marked as resolved/closed. Feel free to change the name if you don't agree with my choice.

adamjstewart picture adamjstewart on 10 Oct 2016
👍1

The whole develop thing has bothered me for a while. I opened this thread as a long-term way to try to think through something that would be more general and more intuitive. And before we've really figured it out, there's a rush to create a PR that effectively doubles down on the current approach. It feels like an attempted end-run around the debate process, especially since nothing was broken with the code. I think we're off on the wrong track.

This thread now has at least 5 different approaches proposed in it, making it that much harder to have a productive discussion. And then we're discussing implementation issues. Why are we discussing that before we've even agreed on what features we want to implement?

I don't have time today to open the new issue, and I think we're off on the wrong track. People who want to discuss implementation details can already do it on the PR.

citibeth picture citibeth on 10 Oct 2016

@adamjstewart Before I propose something, I would like to understand what is that we want to achieve and why :smile:

Is it re-usable classes to handle versions wherever they appear ? If so, what are the expected benefit ? Or we want to model "common" names in repository branches ?

Another thing is not clear to me. Suppose we could write something like : 

version('master', git='https://github.com/LLNL/Caliper.git')

what is that you expect from an installation of it ? Personally I would expect master to be a moving version so that if I say : 

spack install foo@master
# time passes
spack install foo@master
# previous installation gets updated

_If_ we agree on this behavior how do we want to tie it to the version ?

alalazo picture alalazo on 10 Oct 2016

@alalazo

I would write something like:

version('infinity', git='https://github.com/LLNL/Caliper.git', aliase='master')

That would address your example.

citibeth picture citibeth on 10 Oct 2016

@citibeth Not really (but aliases would be a good start). We also need to keep track of a sort of time stamp and possibly of the commit hash (or similar), and there things will get a bit more complicated and specialized.

That's why I asked what we want to achieve... versions seem to me just the tip of the iceberg.

alalazo picture alalazo on 10 Oct 2016

Can you start a new thread and expand on that?
On Oct 10, 2016 12:43 PM, "Massimiliano Culpo" [email protected]
wrote:

@citibeth https://github.com/citibeth Not really (but aliases would be
a good start). We also need to keep track of a sort of time stamp and
possibly of the commit hash (or similar), and there things will get a bit
more complicated and specialized.

That's why I asked what we want to achieve... versions seem to me just the
tip of the iceberg.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/LLNL/spack/issues/1975#issuecomment-252675210, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AB1cd2PAoVKbhXbiBCFlmMpXR5QNqFAKks5qymsjgaJpZM4KRtI8
.

citibeth picture citibeth on 10 Oct 2016

@alalazo I would like a generic Version class that can handle all versions. A possible benefit in my mind is that we could say:

if spec.architecture.platform_os.version > '10.12':

and Spack would know what to do.

In addition to that, I would like versions to be labeled master or develop. This doesn't necessarily _have_ to be a part of the Version class, but Spack should be smart enough to know that these are newer than the latest release. We could subclass Version if we really want to, but I don't think we necessarily have to.

Another thing is not clear to me. Suppose we could write something like :

version('master', git='https://github.com/LLNL/Caliper.git')

what is that you expect from an installation of it ? Personally I would expect master to be a moving version so that if I say :

spack install foo@master
# time passes
spack install foo@master
# previous installation gets updated

I would expect that the two installed versions have different hashes. I don't think this is the way things currently behave, but that's the problem with moving versions.

adamjstewart picture adamjstewart on 10 Oct 2016

But I think this is slightly off-topic. This discussion has mostly been about how to handle sorting (important when a package depends on a specific version range) and concretization (which version is chosen by default). Can you open a separate issue about what happens when you reinstall a develop or master version?

adamjstewart picture adamjstewart on 10 Oct 2016

Hi all,

Sorry I haven't been in on this discussion. It's a good one! It think I should clarify a few things.

Ok, on develop:

  1. I'm not a huge fan of the develop special case, but I think it's a decent workaround for the problem that we don't currently _know_ how to compare versions for unreleased things fetched out of repos.
  2. I actually _like_ @davydden's and @alalazo's idea to make develop function the same way as a version component as it does as a standalone version (sorry if I'm getting the credit wrong here).

On the larger problem:

  1. The Version class as initially designed was based on how RPM and, to some extent, Python's LooseVersion does version comparisons, with a couple things in mind:

    1. When people say they want "version 1.2" of something, they mean 1.2.whatever, PROBABLY 1.2.latest. That's the rationale for the way the : works and why the ranges are inclusive and not exclusive.

    2. The scheme was designed to be simple and understandable: split on alpha/numeric boundaries and any special characters, then compare lexicographically.

    3. There is no one true versioning scheme, and packaging systems should _not_ make packagers conform to any particular scheme. Uniform rules that can be applied across all packages are better than special cases that address shortcomings in particular packages' versioning schemes. If a package ships with unsortable versions, too bad. That's _their_ fault.

So, I think special cases complicate things. I _really_ don't want to add more than one special case because I think we're going to end up with something like the PHP type comparison table. I just really don't think I could live with having my name on something like that.

Stuff I do like:

  1. I would like a way to handle exclusive ranges, !=, and to compare OS version numbers. For the OS version, I think that we should probably not use things like elcapitan and sierra to identify Mac OS versions, and we could keep it simply by just using numeric designators. This is probably my bad for going with what Homebrew does. More on this later.
  2. I think there's a strong case for better syntax. I think it would actually be pretty easy to modify VersionList and VersionRange to support the setuptools syntax along with the : syntax.

Ok now about the special cases. I think there are a couple of major points missing from the discussion around develop. They are:

  1. All the rules suggested for comparing develop above are _correct_ ONLY at the time of installation, and they quickly become _wrong_ as the installed package ages.
  2. develop is a moving target. We need a way to convert it to a fixed target.

These, IMHO, are the biggest problem with develop, why I don't really like the special case, and why I don't think there IS a good, unified set of rules for comparison.

(1) causes problems for concretization because if you sort things, an installed foo@develop will be greater than [email protected] EVEN if foo@develop was installed 2 years ago. If we want Spack to be able to reason about installed packages as well as to-be-installed packages, we need to fix this.

(2) I think many of the approaches outlined above are trying to make the version system more complicated to address (1), when really we should be focusing on (2). I think you can address (2) with smarter fetchers. develop and friends really only arise when you pull something out of a VCS repo. If you can do that, you can also figure out what version it _really_ is, and install it with the _real_ version.

So, e.g., for git, if you install something @develop, you could query the commit id and the most recent tag in the history, and have develop concretize to, e.g., 1.2.1f34ced14 (the latest tag here was 1.2). That version would age reasonably well and would not screw up concretization later. The commit hash might conflict with other numeric versions so you could add a designator for things derived from commits or VCS revisions, e.g. 1.2.dev-1f34ced14 -- I haven't fully thought that part through.

@citibeth mentioned version aliases and I actually think this is a similar concept. Basically you need to separate how you refer to things (develop) from the way you store provenance about them. develop is an abstract thing that corresponds to a concrete version _right now_. Same for things like elcapitan, sierra, etc. I think we could handle a lot of the version mayhem with some simple aliases, _maybe_ a designator for VCS-derived versions, and smarter fetchers. All while keeping the versioning system relatively simple.

Thoughts?

tgamblin picture tgamblin on 10 Oct 2016
👍3

If we can extract the _actual_ version whenever we pull from a branch, that would indeed solve a lot of problems. It wouldn't even matter what branch it was. Whether I pull from develop or master or foo, if it can determine what version it _actually_ is, then that removes all need for special cases and solves the problem @alalazo brought up. 1.2.1f34ced14 or 1.2.dev-1f34ced14 sounds great to me.

adamjstewart picture adamjstewart on 10 Oct 2016

Uniform rules that can be applied across all packages are better than special cases that address shortcomings in particular packages' versioning schemes. If a package ships with unsortable versions, too bad. That's their fault.

:+1:

All the rules suggested for comparing develop above are correct ONLY at the time of installation, and they quickly become wrong as the installed package ages.

I think it is difficult to solve this without any assumptions. Let's consider the following example:

The highest version of the package was 8.4.2 at the moment of installation. develop, of course, has everything 8.4.2 has at this point in time. Now in half a year, there is 8.4.3 which contains a few commits (bugfixes) cherrypicked. Tricky part is that a few of those could be there in _old_ develop, which is installed, but others not. On another hand, _old_ develop has way more commits and new features introduced priori to 8.4.3 bugfix release. So in this case you can define comparator only on the time stamp, but not on the set of features. In other words, there is no straight-forward comparison rule for bug-fix releases.

So I think the only solution to this problem is to make installed develop invalid as soon as there are other releases in a package. That means that develop should somehow decode current maximum release in the installed package.

if you install something @develop, you could query the commit id and the most recent tag in the history, and have develop concretize to, e.g., 1.2.1f34ced14 (the latest tag here was 1.2). That version would age reasonably well and would not screw up concretization later

true, but will hold only if there are tags. If there aren't, maybe we could just take the latest stable release in a package. There will be same issue of comparing to bugfix releases.

and to compare OS version numbers

:+1:

davydden picture davydden on 10 Oct 2016

Whether I pull from develop or master or foo, if it can determine what version it actually is, then that removes all need for special cases

but the tricky part is that you decide on what develop means _a priori_, that is, before actually trying to download it. So we still need exceptions.

davydden picture davydden on 10 Oct 2016

Ok now about the special cases. I think there are a couple of major points missing from the discussion around develop. They are:

  1. All the rules suggested for comparing develop above are correct ONLY at the time of installation, and they quickly become wrong as the installed package ages.
  2. develop is a moving target. We need a way to convert it to a fixed target.

IMHO, what we can do about this depends on what assumptions we're willing to make and how much we're willing to allow Spack to dig into VCS repos.

(2) can be addressed by fishing out the git hash you used to install something, and _using_ the git hash when computing the package hash. If you install later out of the same branch, the installed package will get a different hash. Of course, this doesn't give us ordering.

The _infinity_ element can be converted to a fixed target by looking at the other versions in the Spack package. For example, suppose I have 1.infinity. I look in the Spack package to see we have 1.1, 1.2 and 2.0. Using some algorithm I haven't thought through the details yet, I "resolve" my 1.infinity as 1.2.infinity. This resolved version number will now be smaller than a future 1.3 release but larger than 1.2.

If i resolve to 1.2.infinity.<githash>, then I can preserve the git hash in my version number, which seems useful.

Suppose I install many subsequent versions of release12=1.2.infinity? I want later ones to be larger than earlier ones. I could resolve my version number to 1.2.infinity.<timestamp>.<githash>, then I will get the behavior that later installs will compare larger than earlier installs. But this breaks re-usabiliyt ofthe same install, so it needs a little more thought to get right...

Yes, packages have all sorts of crazy version schemes. But 99% of them fit the same general mold; and Spack is also flexible enough it doesn't HAVE To use package's version strings exactly.

citibeth picture citibeth on 10 Oct 2016

I actually like @davydden's and @alalazo's idea to make develop function the same way as a version component as it does as a standalone version (sorry if I'm getting the credit wrong here).

This was something I actually considered in a recent PR. I didn't do it because (1) I wanted to change as little as possible while fixing a bug at the time, and (2) I thought it needed more conceptual work. In and of itself it does not solve all the problems we need solved.

citibeth picture citibeth on 10 Oct 2016

To add a bit:

if there were no new releases prior to installing the develop version, then we can keep it without any re-installation because it still satisfies depends_on(8.4.2:) or alike.

So maybe we don't even need to decode hashes, just store somewhere available versions for a given package at the time of installation of develop. Shall they chage, the old installed develop will be re-installed next time someone runs install.

davydden picture davydden on 10 Oct 2016

In and of itself it does not solve all the problems we need solved.

it does solve the subset of problems (ie Julia package).

davydden picture davydden on 10 Oct 2016

Only if you don't care about UI
On Oct 10, 2016 2:32 PM, "Denis Davydov" [email protected] wrote:

In and of itself it does not solve all the problems we need solved.

it does solve the subset of problems (ie Julia package).

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/LLNL/spack/issues/1975#issuecomment-252702529, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AB1cd5WvW-dWVzBkzO7YfVqisjsuNFQeks5qyoStgaJpZM4KRtI8
.

citibeth picture citibeth on 10 Oct 2016

Only if you don't care about UI

i do and it's a perfectly legit solution to those problems IMHO. Not to imply that this is the only possible solution, of course.

davydden picture davydden on 10 Oct 2016

So, I don't want to kill the ideas here but honestly I think changing the way versions work is a pretty significant change and I'm probably not going to have time to integrate this stuff near-term. I am trying currently to focus on the 0.10 release, and I think we've got our hands full with that. Can we table the version rework until maybe after SC16?

tgamblin picture tgamblin on 10 Oct 2016
👍2

Question.... Where does spack use version comparison?
On Oct 10, 2016 3:36 PM, "Todd Gamblin" [email protected] wrote:

So, I don't want to kill the ideas here but honestly I think changing the
way versions work is a pretty significant change and I'm probably not going
to have time to integrate this stuff near-term. I am trying currently to
focus on the 0.10 release, and I think we've got our hands full with
that. Can we table the version rework until maybe after SC16?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/LLNL/spack/issues/1975#issuecomment-252725655, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AB1cd6HBpz6cJBtJms0UpVCmTUkDhdejks5qypOXgaJpZM4KRtI8
.

citibeth picture citibeth on 10 Oct 2016

Concretization, mainly. Also in things like spack find. Anywhere Spec objects are compared.

tgamblin picture tgamblin on 11 Oct 2016

I'm looking for more specifics... for example, is a complete ordering on
versions required for these algos? Can they work with a partial ordering?
Are versions ever compared between specs? (eg: compare [email protected] vs.
[email protected]) What exactly is comparing specs needed for in concretization?
I've seen the part in the code where it assembles a bunch of versions and
decides which one to use; is there any other place in concretization where
specs are compared?

On Mon, Oct 10, 2016 at 6:34 PM, Todd Gamblin [email protected]
wrote:

Concretization, mainly. Also in things like spack find. Anywhere Spec
objects are compared.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/LLNL/spack/issues/1975#issuecomment-252764336, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AB1cdyfKpb-Hj-psSqnYagg9Qt7adHHxks5qyr2QgaJpZM4KRtI8
.

citibeth picture citibeth on 11 Oct 2016

I think we are discussing two different but related issues:

Howe do we compare versions like @develop, @master, @1.develop etc. I think this is not an issue, really. What Version class does currently plus minor modifications (https://github.com/LLNL/spack/pull/1983) is really all we need. As for @tgamblin concern about too many infinite-like versions: if we keep proper object-oriented design of Version class and this information is encapsulated, then there is nothing to worry about. In case of the PR above for multiple infinity-like versions, outside of Version class all we need to use is isdevelop(). So apart from conceptual arguments whether or not we should keep several infinity-like versions, I see no problem with this approach. Given the last argument, i actually agree with @adamjstewart that we need to do this for a small set of widely used names. As for @alalazo comment, it does not necessarily lead to extra ifs as compared to a single infinity, see https://github.com/LLNL/spack/pull/1983.

Second issue which was correctly raised by @tgamblin is

All the rules suggested for comparing develop above are correct ONLY at the time of installation, and they quickly become wrong as the installed package ages.

this actually does not have to be develop and therefore it is a related but separate issue. Consider for example current code in Julia

version('release-0.5',  git='https://github.com/JuliaLang/julia.git', branch='release-0.5')

I fully agree with the quoted statement. Let me elaborate a bit in this regard. Consider the following example packageA:

...commit 1 [8.4.0] -> commit2 -> commit3 -> bugfix1 -> commit4 ->commit5 -> feature1 -> commit6 ...
    \
     \-> bugfix1 [8.4.1]

[] indicate released versions of software, whereas the version in Spack is:

version('8.5.rc',  git='https://github.com/example/project.git', branch='develop')

So let's assume at the time of first Spack installation of [email protected] we installed commit3 and no 8.4.1 release was available. At this point in time everything is correct and we indeed can say that 8.5.rc-commit3 > 8.4.0. Now time pass and a new bugfix release is there. Say if we delete previous installation and reinstall 8.5.rc, we get to commit4. The only thing we can say is that 8.5.rc-commit4 > 8.4.1 and 8.5.rc-commit4 > 8.5.rc-commit3, but we can not compare 8.5.rc-commit3 and 8.4.1. So if someone adds a package and writes depends_on([email protected]:), we simply have to ignore already installed moving target and reinstall as we can't guarantee that it will satisfy the requirements imposed by this condition.

The same holds even when no extra releases are there. Say there is a package which relies on a feature feature1, so what the packages will state is depends_on(.... @8.5.rc), correctly assuming that this versions should end up being the latest on the development branch. However, previously installed moving targets like @8.5.rc-commit4 will not have the required feature1 and again will shall ignore them altogether.

Bottom line is that we can't really assume anything but require that version comparison with moving targets is only valid at the time of installation.

I suggest a very simple but robust solution to this problem: every time moving target is not a root of the DAG -- reinstall. This is not elegant, but as explained above there is no general solution to this issue. The reasons is that no matter what we do and how elaborate we are in terms of keeping hashes and alike, the comparison operator is undefined.

davydden picture davydden on 11 Oct 2016

Tagging this discussion to revisit after v0.10

tgamblin picture tgamblin on 11 Oct 2016
👍1

I just stumbled over this issue when installing trilinos@master and thought this will give me [email protected] (newest version available, which works with trilinos > 12.12.1). Unfortunately this line in trilinos/package.py:

depends_on('superlu-dist@:4.3', when='@:12.6.1+superlu-dist')

prevents trilinos@master from bein compatible with [email protected], since spack thinks the master branch is below the 12.6.1 release. So also here, guessing the correct version number from e.g. git infos would solve this problem.

mathsen picture mathsen on 5 Feb 2019
Was this page helpful?
0 / 5 - 0 ratings

Related issues

adamjstewart picture adamjstewart  Â·  3Comments
TomasPuverle picture TomasPuverle  Â·  3Comments
cwsmith picture cwsmith  Â·  3Comments
JavierCVilla picture JavierCVilla  Â·  3Comments
Joseguz101 picture Joseguz101  Â·  3Comments