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

Pkg.Types.semver_spec intersects version specs wrong #751

Closed
Seelengrab opened this issue Sep 17, 2018 · 24 comments
Closed

Pkg.Types.semver_spec intersects version specs wrong #751

Seelengrab opened this issue Sep 17, 2018 · 24 comments

Comments

@Seelengrab
Copy link
Contributor

Seelengrab commented Sep 17, 2018

julia> Pkg.Types.semver_spec(">=0.7, ^0")
VersionSpec("0.0.0-*")

Shouldn't this give VersionSpec("0.7.0-0.*")?

EDIT: the rest was a seperate issue covered here.

Ref. this discussion on discourse and this Issue in METADATA.jl.

@fredrikekre
Copy link
Member

Looks correct to me since

VersionSpec("0.0.0-*")

is the union of

julia> Pkg.Types.semver_spec(">=0.7")
VersionSpec("0.7.0-*")

and

julia> Pkg.Types.semver_spec("^0")
VersionSpec("0.0.0-0")

?

@Seelengrab
Copy link
Contributor Author

Seelengrab commented Oct 18, 2018

I wasn't aware that this would be done via a union - I was under the impression that there was an intersection, since this

julia> Pkg.Types.semver_spec(">=0.7, ^1")
VersionSpec("0.7.0-*")

julia> Pkg.Types.semver_spec("^1")
VersionSpec("1.0.0-1")

intersects to VersionSpec("0.7.0-1"), if I understand correctly. How else would one define an upperbound starting at 0.7 and stopping at the last version of 1.x? This would very much help in preventing installations of packages on not explicitly supported newer versions.

@StefanKarpinski
Copy link
Member

I'm still not seeing where you're getting the expectation of intersection from:

julia> Pkg.Types.semver_spec(">=0.7")
VersionSpec("0.7.0-*")

julia> Pkg.Types.semver_spec("^1")
VersionSpec("1.0.0-1")

julia> Pkg.Types.semver_spec(">=0.7, ^1")
VersionSpec("0.7.0-*")

The >=0.7 spec fully contains ^1: if this was an intersection then the last version would be equal to ^1 if it was a union (it is) then it would be equal to >=0.7 (it is).

@Seelengrab
Copy link
Contributor Author

Seelengrab commented Oct 18, 2018

Yes, I understand that it's a union - all I'm saying is that I don't see a way to express VersionSpec("0.7.0-1") with the current version parsing. That the behaviour is a union instead of an intersection of version specs is what's surprising to me, not why a union gives that result.

Further, the doc actually says it's an intersection, not a union:

Version specifier format

Similar to other package managers, the Julia package manager respects semantic versioning (semver). > As an example, a version specifier is given as e.g. 1.2.3 is therefore assumed to be compatible with the versions [1.2.3 - 2.0.0) where ) is a non-inclusive upper bound. More specifically, a version specifier is either given as a caret specifier, e.g. ^1.2.3 or a tilde specifier ~1.2.3. Caret specifiers are the default and hence 1.2.3 == ^1.2.3. The difference between a caret and tilde is described in the next section. The intersection of multiple version specifiers can be formed by comma separating indiviual version specifiers.

(Emphasis mine).

@fredrikekre
Copy link
Member

Since there will not be a v0.8 release of Julia you can use

julia> Pkg.Types.semver_spec("0.7, ^1")
VersionSpec("[0.7.0-0.7, 1.0.0-1]")

Further, the doc actually says it's an intersection, not a union:

This was corrected in #773

@Seelengrab
Copy link
Contributor Author

Seelengrab commented Oct 18, 2018

Well, of course I can use that for this one special case.. What I'm saying though is that, in general, it's not possible to currently provide a version spec specifying support from version x.y.z up to but not including (x+1).0.0 because of that union - which would be useful, since that's when major breakage is expected to happen, whereas minor or patch releases are not generally expected to break code.

In terms of the package referenced in the OP, I don't expect the Logging interface to change before 2.0 in a major way - so how do I express that I do expect it to change with 2.0, since that code is only semi-reliably exported now?

@StefanKarpinski
Copy link
Member

StefanKarpinski commented Oct 18, 2018

Wouldn't Pkg.Types.VersionSpec("0.7.0-1") be the way to write it? Unless you mean something else.

This behavior of the comma does seem to be at odds with what Cargo does:

https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#multiple-requirements

This is completely unspecified except for the vague sentence:

Multiple version requirements can also be separated with a comma, e.g. >= 1.2, < 1.5.

Which, of course, doesn't tell you anything really—is it a union or an intersection? However, the example suggests that in Cargo it's an intersection since if it's a union then the example allows any version at all, whereas if it's an intersection, then it's a sensible constraint of [1.2, 1.5).

npm uses || to express a union of ranges and space separation to indicate an intersection. In any case, I think we should certainly not have a different meaning than both, so point taken.

@Seelengrab
Copy link
Contributor Author

Wouldn't Pkg.Types.VersionSpec("0.7.0-1") be the way to write it? Unless you mean something else.

No, that's precisely what I mean - but how can I specify that in e.g. Project.toml? I have to use the comma seperated list there, and the union prevents me from solving this easily without having to write all possibly existing future minor versions before the next major version.

@StefanKarpinski
Copy link
Member

Why can't you can write 0.7.0-1 in Project.toml?

@fredrikekre
Copy link
Member

and the union prevents me from solving this easily without having to write all possibly existing future minor versions before the next major version.

julia> Pkg.Types.semver_spec("1.2.3")
VersionSpec("1.2.3-1")

@Seelengrab
Copy link
Contributor Author

@StefanKarpinski Because that's an invalid version specifier.

A modified Project.toml for a newly generated Package with that specifier:

authors = <snip>
name = "testPkg"
uuid = "f191ba14-d2dc-11e8-1583-790a11c3e9da"
version = "0.1.0"

[compat]
julia = "0.7.0-1"

[deps]

which gives the following Error when trying do dev:

(v1.0) pkg> dev .
 Resolving package versions...
ERROR: invalid version specifier: 0.7.0-1
Stacktrace:
 [1] error(::String) at ./error.jl:33
<snip>

@fredrikekre yes, that would work for only having one major version jump - but this quickly becomes longer with multiple jumps, or in the case of 0.7.0-1 not expressable at all.

@StefanKarpinski
Copy link
Member

StefanKarpinski commented Oct 18, 2018

There's two issues here:

  1. Whether the syntax matches that of other systems like Cargo and npm.
  2. Whether you need the intersection to express anything.

I'm much more concerned about 1 than 2—largely because if syntax matches Cargo and is expressive enough for Cargo, then I think it's unlikely that it's insufficiently expressive for us.

Regarding 2, however, @Seelengrab can you please describe the set of versions you want to express as an open/closed interval with fully specified point version endpoints?

@Seelengrab
Copy link
Contributor Author

Seelengrab commented Oct 18, 2018

I want to specify in the particular case of IOLogging.jl the compatible version range as [0.7.0-2.0.0).

How Cargo and npm handle this, I do not know. I suspect both intersection and union are needed for the general case though.

@StefanKarpinski
Copy link
Member

Because that's an invalid version specifier.

Ah, well. It shouldn't be. It should be a valid version specifier for the version range [0.7.0-2.0.0).

@StefanKarpinski
Copy link
Member

I want to specify in the particular case of IOLogging.jl the compatible version range as [0.7.0-2).

I asked for fully specified patch version endpoints. This is ambiguous. Do you mean [0.7.0-2.0.0) or something else?

@Seelengrab
Copy link
Contributor Author

I edited my response - but I'm not sure which one is right. I basically want to support any version greater than or equal to 0.7.0, but not support any version greater than or equal to 2.0.0. I guess that's equivalent to [0.7.0-2.0.0).

@StefanKarpinski
Copy link
Member

Ok, I understand what you're saying now. Thanks for clarifying.

@fredrikekre
Copy link
Member

So, as the current implementation you basically need one specifier per "breaking version" which is a minor bump pre v1 and a major bump post v1. I think that makes sense. So for the current state of Julia releases, if you want to support Julia v0.7.X and 1.X you need (and should) use something like "0.7, 1". Technically, Julia v0.8 could be breaking, and you should not claim to support that without testing it. If Julia 0.8 is release, you can test the package and add 0.8 to the list, and have something like "0.7, 0.8, 1".

@StefanKarpinski
Copy link
Member

Does no one care that our syntax is completely different from Cargo? @KristofferC, I assume that was not the intention and that this was a mistake due to the poor specification of the Cargo docs?

@StefanKarpinski
Copy link
Member

StefanKarpinski commented Oct 18, 2018

I'm reopening this because I think it was unintentional to diverge from Cargo's behavior.

@KristofferC
Copy link
Member

KristofferC commented Oct 18, 2018

As Fredrik says, the way it is right now, you just list what potentially breaking versions you are compatible with. With no more 0 major versions being released for Julia this will in practice you would have to list for example 1, 2, 3 instead of >= 1, < 4 to be compatble with [1.0.0 - 4.0.0).

@StefanKarpinski
Copy link
Member

So it seems the divergence from Cargo is intended. I have to say, it makes sense to me to just list the major and/or minor versions that you're known to be compatible with. It would be nice to support hyphen range syntax in these too so that 0.7-1 would just work.

@KristofferC
Copy link
Member

Adding things to

Pkg.jl/src/versions.jl

Lines 340 to 345 in dfb341a

const version = "v?([0-9]+?)(?:\\.([0-9]+?))?(?:\\.([0-9]+?))?"
const ver_regs =
[
Regex("^([~^]?)?$version\$") => semver_interval, # 0.5 ^0.4 ~0.3.2
Regex("^((?:≥)|(?:>=)|(?:=)|(?:<)|(?:=))v?$version\$") => inequality_interval,# < 0.2 >= 0.5,2
]
should be fairly simple.

@StefanKarpinski
Copy link
Member

Ok, I'll close this and make that a feature request issue instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants