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

Packing with prerelease dependencies #1316

Closed
franknarf8 opened this Issue Dec 18, 2015 · 29 comments

Comments

Projects
None yet
2 participants
@franknarf8

franknarf8 commented Dec 18, 2015

Hello,

Firstly, I want to mention I have been using Paket for a little while now and love the software.

Here is my problem,

I have 3 projects (A, B, C) where
B depends on (A 2.0 prerelease)
C depends on (B 1.0 prerelease)

A, B are libraries and are shipped in their respective nuget packages created using Paket. We are using SemVer with prerelease tags. (using GitFlow)

Nuget packages for A are usually named something like "A 1.0-unstable0021"

Projects B resolve correctly its dependency to A on "paket update". When we use "paket pack" for package B, it creates the following nuget dependencies,
A( = 1.0.0-prerelease)

The thing is, when we try to "paket update" C, it cannot resolve A. I think he is is looking for a package versionned exactly "1.0.0-prerelease".

(The problem also applies to prerelease ranges (~>, <, >, etc.))

I don't quite know how to solve this problem. Maybe it is a comprehension issue, but if it is not, I'd be happy to contribute.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Dec 28, 2015

Member

I think we create a wrong version requirement here. Do you know what NuGet would create in package B?

Member

forki commented Dec 28, 2015

I think we create a wrong version requirement here. Do you know what NuGet would create in package B?

@franknarf8

This comment has been minimized.

Show comment
Hide comment
@franknarf8

franknarf8 Dec 28, 2015

I'm not quite sure, but from what I understood, there are no prerelease information injected in the dependency version requirement.

In the previous example, B should only have a dependency on A with version "1.0.0", not "1.0-prerelease". Then, if we pull on a stable version of package B, it should pull a stable version of B and a stable version of A (e.g. "A 1.0.0"). Likewise, if we specify the B package to be unstable in the .dependencies file, it should pull on the latest version of B and A (including prereleases, but not excluding stable releases, e.g. "A 1.0.0-unstableXX).

The applications for "=" dependencies are not that numerous, but the applications for dependency ranges are a bit more problematic.

franknarf8 commented Dec 28, 2015

I'm not quite sure, but from what I understood, there are no prerelease information injected in the dependency version requirement.

In the previous example, B should only have a dependency on A with version "1.0.0", not "1.0-prerelease". Then, if we pull on a stable version of package B, it should pull a stable version of B and a stable version of A (e.g. "A 1.0.0"). Likewise, if we specify the B package to be unstable in the .dependencies file, it should pull on the latest version of B and A (including prereleases, but not excluding stable releases, e.g. "A 1.0.0-unstableXX).

The applications for "=" dependencies are not that numerous, but the applications for dependency ranges are a bit more problematic.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Dec 29, 2015

Member

I'm investigating the case - It's seems to be a bug, but I don't think the solution is like what you describe. Prerelease flags are not transitive across packages and their dependencies.

Member

forki commented Dec 29, 2015

I'm investigating the case - It's seems to be a bug, but I don't think the solution is like what you describe. Prerelease flags are not transitive across packages and their dependencies.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Dec 29, 2015

Member

I fixed a bug in this area in 2.39.9 but I assume it's not directly the one that impacts you.
Can please try to create a repro sample? That would help me a lot

Member

forki commented Dec 29, 2015

I fixed a bug in this area in 2.39.9 but I assume it's not directly the one that impacts you.
Can please try to create a repro sample? That would help me a lot

@franknarf8

This comment has been minimized.

Show comment
Hide comment

@forki forki closed this in b957088 Jan 7, 2016

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jan 7, 2016

Member

wow very cool repro sample. Looks like a very very strange edge case.

If you repack B with the latest Paket version then it should work.

Member

forki commented Jan 7, 2016

wow very cool repro sample. Looks like a very very strange edge case.

If you repack B with the latest Paket version then it should work.

@franknarf8

This comment has been minimized.

Show comment
Hide comment
@franknarf8

franknarf8 Jan 7, 2016

Thanks, but I think there is still an issue.
If I understood the fix correctly, the package B was repacked and the condition changed from "A (= 2.0.0-prerelease)" to "A (>= 2.0.0-prerelease)".

Thing is, now if the official A.2.0.0 package is out and A.2.1.0-unstableN also exists, the constraint "B depends on (A 2.0 prerelease)" won't be respected as the package A.2.1.0-unstableN will be selected.

I updated the repro repo.

franknarf8 commented Jan 7, 2016

Thanks, but I think there is still an issue.
If I understood the fix correctly, the package B was repacked and the condition changed from "A (= 2.0.0-prerelease)" to "A (>= 2.0.0-prerelease)".

Thing is, now if the official A.2.0.0 package is out and A.2.1.0-unstableN also exists, the constraint "B depends on (A 2.0 prerelease)" won't be respected as the package A.2.1.0-unstableN will be selected.

I updated the repro repo.

@franknarf8

This comment has been minimized.

Show comment
Hide comment
@franknarf8

franknarf8 Jan 7, 2016

Should I open a new issue? @forki

franknarf8 commented Jan 7, 2016

Should I open a new issue? @forki

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jan 7, 2016

Member

the question is: what dependency would we need then?

Member

forki commented Jan 7, 2016

the question is: what dependency would we need then?

@forki forki reopened this Jan 7, 2016

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jan 7, 2016

Member

"A (>= 2.0.0-prerelease < 2.0.1)" ?

Member

forki commented Jan 7, 2016

"A (>= 2.0.0-prerelease < 2.0.1)" ?

@franknarf8

This comment has been minimized.

Show comment
Hide comment
@franknarf8

franknarf8 Jan 7, 2016

As the package version timeline goes like so :
A.1.0.0
A.2.0.0-unstable2
A.2.0.0-unstable3
A.2.0.0
A.2.1.0-unstable1

I think the constraint "B depends on (A 2.0 prerelease)" should resolve to the closest thing to A.2.0.0. In this case, the possible answers would be [A.2.0.0-unstable2, A.2.0.0-unstable3, A.2.0.0].

I'm not quite sure how to represent this range though. Maybe something like "A (>= 2.0.0-prerelease <= 2.0.0)"

franknarf8 commented Jan 7, 2016

As the package version timeline goes like so :
A.1.0.0
A.2.0.0-unstable2
A.2.0.0-unstable3
A.2.0.0
A.2.1.0-unstable1

I think the constraint "B depends on (A 2.0 prerelease)" should resolve to the closest thing to A.2.0.0. In this case, the possible answers would be [A.2.0.0-unstable2, A.2.0.0-unstable3, A.2.0.0].

I'm not quite sure how to represent this range though. Maybe something like "A (>= 2.0.0-prerelease <= 2.0.0)"

@franknarf8

This comment has been minimized.

Show comment
Hide comment
@franknarf8

franknarf8 Jan 7, 2016

But even then, it depends on how ">= 2.0.0-prerelease" is evaluated.
If the prerelease section of the range is evaluated alphabetically, A.2.0.0-unstable2 would be a candidate, but A.2.0.0-alpha1 would not, even if it is a prerelease package...

franknarf8 commented Jan 7, 2016

But even then, it depends on how ">= 2.0.0-prerelease" is evaluated.
If the prerelease section of the range is evaluated alphabetically, A.2.0.0-unstable2 would be a candidate, but A.2.0.0-alpha1 would not, even if it is a prerelease package...

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jan 7, 2016

Member

[2.0.0-prerelease,2.0.0] works with your sample. I will create a hotfix

Member

forki commented Jan 7, 2016

[2.0.0-prerelease,2.0.0] works with your sample. I will create a hotfix

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jan 7, 2016

Member

released

Member

forki commented Jan 7, 2016

released

@franknarf8

This comment has been minimized.

Show comment
Hide comment
@franknarf8

franknarf8 Jan 7, 2016

I just tested it, it works for everything between A.2.0.0-prerelease and A.2.0.0.

Thing is now, if I only have a package A.2.0.0-alpha2, it will be ignored and resolution will crash as it is not in the range.

franknarf8 commented Jan 7, 2016

I just tested it, it works for everything between A.2.0.0-prerelease and A.2.0.0.

Thing is now, if I only have a package A.2.0.0-alpha2, it will be ignored and resolution will crash as it is not in the range.

@franknarf8

This comment has been minimized.

Show comment
Hide comment
@franknarf8

franknarf8 Jan 7, 2016

We would need the smallest valid prerelease name for the min delimiter in [2.0.0-prerelease,2.0.0]...

franknarf8 commented Jan 7, 2016

We would need the smallest valid prerelease name for the min delimiter in [2.0.0-prerelease,2.0.0]...

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jan 7, 2016

Member

[2.0.0-prerelease,2.0.0] should match 2.0.0-alpha2 - that's another bug

2016-01-07 18:58 GMT+01:00 franknarf8 notifications@github.com:

We would need the smallest valid prerelease name for the min delimiter in
[2.0.0-prerelease,2.0.0]...


Reply to this email directly or view it on GitHub
#1316 (comment).

Member

forki commented Jan 7, 2016

[2.0.0-prerelease,2.0.0] should match 2.0.0-alpha2 - that's another bug

2016-01-07 18:58 GMT+01:00 franknarf8 notifications@github.com:

We would need the smallest valid prerelease name for the min delimiter in
[2.0.0-prerelease,2.0.0]...


Reply to this email directly or view it on GitHub
#1316 (comment).

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jan 7, 2016

Member

ok this is now fixed as well.

Member

forki commented Jan 7, 2016

ok this is now fixed as well.

@franknarf8

This comment has been minimized.

Show comment
Hide comment
@franknarf8

franknarf8 Jan 7, 2016

That is awesome, thanks a lot!

Now I think there may still be an issue with the ~> operator for prereleases.
I changed the B dependency on A to be "A ~> 2.0 prerelease". I also created packages A.3.0.0-alpha1.

B resolves correctly to A.3.0.0-alpha1 and creates a package with the following dependency "A(>= 2.0.0-prerelease && < 3.0.0-prerelease)"

Unfortunately C still resolves A to A.2.1-unstable1 instead of A.3.0.0-alpha1.

See this commit :
franknarf8/paket-issue-repro@4b677d3

I think B dependency should be this following range [2.0.0-prerelease, 3.0.0).

franknarf8 commented Jan 7, 2016

That is awesome, thanks a lot!

Now I think there may still be an issue with the ~> operator for prereleases.
I changed the B dependency on A to be "A ~> 2.0 prerelease". I also created packages A.3.0.0-alpha1.

B resolves correctly to A.3.0.0-alpha1 and creates a package with the following dependency "A(>= 2.0.0-prerelease && < 3.0.0-prerelease)"

Unfortunately C still resolves A to A.2.1-unstable1 instead of A.3.0.0-alpha1.

See this commit :
franknarf8/paket-issue-repro@4b677d3

I think B dependency should be this following range [2.0.0-prerelease, 3.0.0).

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jan 7, 2016

Member

why should ~> 2.0 resolve to 3.0?

Member

forki commented Jan 7, 2016

why should ~> 2.0 resolve to 3.0?

@franknarf8

This comment has been minimized.

Show comment
Hide comment
@franknarf8

franknarf8 Jan 7, 2016

Not to 3.0, but 3.0-alpha. I thought "~> 2" meant everything in the range "[2,3)" and since A.3.0.0-alpha1 < A.3.0.0, it is in the "[2,3)" range.

Correct me if I'm wrong, but should "~> 2.0 prerelease" or "[2.0, 3) prerelease" be compatible with the following packages :
2.0.0
2.1.0-alpha
2.1.0
3.0.0-alpha

(and not with the following ones : 2.0.0-alpha, 3.0.0)

franknarf8 commented Jan 7, 2016

Not to 3.0, but 3.0-alpha. I thought "~> 2" meant everything in the range "[2,3)" and since A.3.0.0-alpha1 < A.3.0.0, it is in the "[2,3)" range.

Correct me if I'm wrong, but should "~> 2.0 prerelease" or "[2.0, 3) prerelease" be compatible with the following packages :
2.0.0
2.1.0-alpha
2.1.0
3.0.0-alpha

(and not with the following ones : 2.0.0-alpha, 3.0.0)

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jan 7, 2016

Member

no we should not accept 3.0 prereleases when using ~> 2 since it will probably already break.

Member

forki commented Jan 7, 2016

no we should not accept 3.0 prereleases when using ~> 2 since it will probably already break.

@franknarf8

This comment has been minimized.

Show comment
Hide comment
@franknarf8

franknarf8 Jan 7, 2016

Ok, that makes sense. But then, B should not resolve "~> 2 prerelease" to 3.0.0-alpha1 then, right?

franknarf8 commented Jan 7, 2016

Ok, that makes sense. But then, B should not resolve "~> 2 prerelease" to 3.0.0-alpha1 then, right?

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jan 7, 2016

Member

yes ~> 2 should not resolve 3.0.0-alpha1 - are we doing this?

Member

forki commented Jan 7, 2016

yes ~> 2 should not resolve 3.0.0-alpha1 - are we doing this?

@franknarf8

This comment has been minimized.

Show comment
Hide comment
@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jan 7, 2016

Member

ouch. will fix later tonight ;-)

Member

forki commented Jan 7, 2016

ouch. will fix later tonight ;-)

@franknarf8

This comment has been minimized.

Show comment
Hide comment
@franknarf8

franknarf8 Jan 7, 2016

Ok, if I understood properly, "~> 2.0 prerelease"
Matches :
2.0
2.1-alpha
2.1
2.2

Does not match :
2.0-alpha
3.0-alpha
3.0

Right?

franknarf8 commented Jan 7, 2016

Ok, if I understood properly, "~> 2.0 prerelease"
Matches :
2.0
2.1-alpha
2.1
2.2

Does not match :
2.0-alpha
3.0-alpha
3.0

Right?

@franknarf8

This comment has been minimized.

Show comment
Hide comment
@franknarf8

franknarf8 Jan 7, 2016

Ok cool, thanks!

franknarf8 commented Jan 7, 2016

Ok cool, thanks!

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jan 7, 2016

Member

"~> 2.0 prerelease" should match 2.0-alpha

Member

forki commented Jan 7, 2016

"~> 2.0 prerelease" should match 2.0-alpha

@forki forki closed this in 9bdf517 Jan 7, 2016

forki added a commit that referenced this issue Jan 8, 2016

forki added a commit that referenced this issue Jan 8, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment