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

Prerelease versions with hyphen are incorrectly handled #263

Closed
mexx opened this issue Oct 15, 2014 · 15 comments
Closed

Prerelease versions with hyphen are incorrectly handled #263

mexx opened this issue Oct 15, 2014 · 15 comments

Comments

@mexx
Copy link
Member

mexx commented Oct 15, 2014

SemVer defines that prerelease identifiers MUST comprise only ASCII alphanumerics and hyphen [0-9A-Za-z-].
NuGet allows prerelease identifies that match following regular expression [a-z][0-9a-z-]*, see SemanticVersion.cs

paket.dependencies

source http://nuget.org/api/v2

nuget Ninject == 3.2.3-unstable-001

Output of paket update

Paket version 0.8.1.0
Parsing paket.dependencies
Resolving packages:
  - fetching versions for Ninject
    - exploring Ninject 3.2.3-unstable-003
Locked version resolutions written to ...\paket.lock
Downloading Ninject 3.2.3-unstable-003 to ...\NuGet\Cache\Ninject.3.2.3-unstable-003.nupkg
Ninject 3.2.3-unstable-003 unzipped to ..\packages\Ninject

paket's parsing of prerelease identifies do not honor the hyphens.
The problem is caused by using .Split '-' in SemVer.fs
By this the -001 part gets lost in the parsing process.

  1. We should support such prerelease identifiers correctly.
  2. If a version contains more data than we can recognize paket should fail.
@forki
Copy link
Member

forki commented Oct 15, 2014

I think we have to accept that nuget packages don't really use the full
semver spec and accept existing package versions. What do we have to fix
exactly?
On Oct 15, 2014 11:22 PM, "Max Malook" notifications@github.com wrote:

SemVer http://semver.org/ defines that prerelease identifiers MUST
comprise only ASCII alphanumerics and hyphen [0-9A-Za-z-].
NuGet http://nuget.org/ allows prerelease identifies that match
following regular expression [a-z][0-9a-z-]*, see SemanticVersion.cs
https://nuget.codeplex.com/SourceControl/latest#src/Core/SemanticVersion.cs

paket.dependencies

source http://nuget.org/api/v2

nuget Ninject == 3.2.3-unstable-001`

Output of paket update

Paket version 0.8.1.0
Parsing paket.dependencies
Resolving packages:

  • fetching versions for Ninject
    • exploring Ninject 3.2.3-unstable-003
      Locked version resolutions written to ...\paket.lock
      Downloading Ninject 3.2.3-unstable-003 to ...\NuGet\Cache\Ninject.3.2.3-unstable-003.nupkg
      Ninject 3.2.3-unstable-003 unzipped to ..\packages\Ninject

paket's parsing of prerelease identifies do not honor the hyphens.
The problem is caused by using .Split '-' in SemVer.fs

let patch, preRelease =
match l with
| 0 -> 0, ""
| 1 ->
let splitted' = splitted.[0].Split '-'
0,
if splitted'.Length > 1 then splitted'.[1]
else ""
| 2 ->
let splitted' = splitted.[1].Split '-'
0,
if splitted'.Length > 1 then splitted'.[1]
else ""
| _ ->
let splitted' = splitted.[2].Split '-'
Int32.Parse splitted'.[0],
if splitted'.Length > 1 then splitted'.[1]
else ""

By this the -001 part gets lost in the parsing process.

  1. We should support such prerelease identifiers correctly.
  2. If a version contains more data than we can recognize paket should
    fail.


Reply to this email directly or view it on GitHub
#263.

@mexx
Copy link
Member Author

mexx commented Oct 15, 2014

Yes, I'm with you, NuGet do not use the full SemVer spec, but its a full subset of it, so if we implement the full one we get NuGet for free. The question is only why should we bother as we can't get any of this extended versions from NuGet.

What do we have to fix exactly?

At least we have to support the scenario with hyphen in prerelease identifiers.

And as nice to have fail when someone defined a version in dependencies file we can not fully understand.
For the given example I would expect the current version of paket to fail, as prerelease identifiers with hyphen are not supported right now.

@forki
Copy link
Member

forki commented Oct 16, 2014

Ok. I think that sounds like a good idea. Do you want to look into this?
On Oct 15, 2014 11:37 PM, "Max Malook" notifications@github.com wrote:

Yes, I'm with you, NuGet do not use the full SemVer spec, but its a full
subset of it, so if we implement the full one we get NuGet for free. The
question is only why should we bother as we can't get any of this extended
versions from NuGet.

What do we have to fix exactly?

At least we have to support the scenario with hyphen in prerelease
identifiers.

And as nice to have fail when someone defined a version in dependencies
file we can not fully understand.
For the given example I would expect the current version of paket to
fail, as prerelease identifiers with hyphen are not supported right now.


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

@mexx
Copy link
Member Author

mexx commented Oct 16, 2014

Maybe, after this weekend I think. You know what I mean ;)

@mavnn
Copy link
Contributor

mavnn commented Oct 17, 2014

@mexx be a bit careful: nuget v2.x not only doesn't implement the full pre-release spec, it actually implements bits of it wrong (lexical sorts when it should sort on numeric values).

So if you implement it "right" you'll be incompatible with NuGet.

@forki
Copy link
Member

forki commented Oct 17, 2014

Yes that's what I meant. We need to accept all the wrong versions.
But for new packages we should be careful.
On Oct 17, 2014 12:58 PM, "Michael Newton" notifications@github.com wrote:

@mexx https://github.com/mexx be a bit careful: nuget v2.x not only
doesn't implement the full pre-release spec, it actually implements bits of
it wrong (lexical sorts when it should sort of numeric values).

So if you implement it "right" you'll be incompatible with NuGet.


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

mexx added a commit to mexx/Paket that referenced this issue Oct 22, 2014
@mexx
Copy link
Member Author

mexx commented Oct 22, 2014

I've provided the fix for the current problem, but there should be a general decision about how to handle versions.

@Andrea
Copy link

Andrea commented Nov 22, 2014

Should this work when using update-from-nuget? I seem to have an issue with this

(note, the repo does contain those packages)

  • fetching versions for NuGet.CommandLine
    • exploring NuGet.CommandLine 2.8.3
      Locked version resolutions written to c:\honourbound\trunk\Game\Source\Code\paket.lock
      Something went wrong with the download of FileHelpers-Stable 2.9.9 - automatic retry with --force.
      Something went wrong with the download of FSharp.Compiler.Service 0.0.58 - automatic retry with --force
      .
      Something went wrong with the download of FSharp.Core.3 0.0.2 - automatic retry with --force.
      Downloading SimpleJson 0.30.0 to C:\Users\Andrea\AppData\Local\NuGet\Cache\SimpleJson.0.30.0.nupkg
      Downloading FileHelpers-Stable 2.9.9 to C:\Users\Andrea\AppData\Local\NuGet\Cache\FileHelpers-Stable.2.
      9.9.nupkg
      Downloading FSharp.Compiler.Service 0.0.58 to C:\Users\Andrea\AppData\Local\NuGet\Cache\FSharp.Compiler
      .Service.0.0.58.nupkg
      Downloading FSharp.Core.3 0.0.2 to C:\Users\Andrea\AppData\Local\NuGet\Cache\FSharp.Core.3.0.0.2.nupkg
      Something went wrong with the download of Discipline Oak 0.1.30 - automatic retry with --force.
      Something went wrong with the download of Duality Editor 0.1.189 - automatic retry with --force.
      Something went wrong with the download of Community Express (Steamworks) 0.1.0.0-CI00002 - automatic re
      try with --force.
      Something went wrong with the download of Duality Scripting Plugin 0.1.113-beta - automatic retry with
      --force.
      Something went wrong with the download of Spine runtime c# 0.1.16 - automatic retry with --force.
      Paket failed with:
      Could not retrieve data from https://www.myget.org/F/6416d9912a7c4d46bc983870fb440d25/Packages(Id='D
      iscipline Oak',Version='0.1.30')
      Message: The remote server returned an error: (404) Not Found.

Is this a well know issue? debugging...

@mexx
Copy link
Member Author

mexx commented Nov 23, 2014

@Andrea #288 was reverted, since it caused problems, so dashes in prerelease identifiers are still not handled correctly.
We anyway have to adjust the version scheme when NuGet v3 is there, that's what #289 is for.

Your problem seams to be unrelated to this issue, as far I can see.

@forki this issue should be reopened, since it's not fixed yet.

@Andrea
Copy link

Andrea commented Nov 24, 2014

@mexx ok good to know. Any pointers as to what the problem could be welcome, a debug session sent me in very a different direction

@mexx
Copy link
Member Author

mexx commented Nov 27, 2014

@Andrea sorry, but I have no idea. @forki Wait, could it be the problem with NuGet v3 support and myget.org?

@forki
Copy link
Member

forki commented Nov 28, 2014

Can I see the paket.dependencies file?

forki added a commit that referenced this issue Oct 30, 2015
@bigbearzhu
Copy link

This issue still exists. Should we reopen this?

@baronfel
Copy link
Contributor

I'm slacking on this terribly, but there was a related fix in FAKE for the SemVer parser that corrected our handling of prerelease versions. That fix is here: fsprojects/FAKE#1325, specifically the SemVerHelper. I've been meaning to port those fixes over to Paket, in both the semver structures in the bootstrapper and Paket proper.

@jam40jeff
Copy link
Contributor

Thanks, @baronfel. I just came here to report this issue and was pleased to see it's already being worked on.

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

7 participants