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

Fix to handle weird malformed portable-only libraries. #2215

Merged
merged 1 commit into from Apr 3, 2017

Conversation

Projects
None yet
3 participants
@OniBait
Contributor

OniBait commented Mar 31, 2017

Fix for issue #1645

The nuget parser handles strings in the form of:
{Identifier}{Version}-{Profile}

(see NuGet.VersionUtility.ParseFrameworkName)

So the string: portable-net45+win8+wp8 gets parsed to:
.NETPortable,Version=v0.0,Profile=net45+win8+wp8

The string: portable45-net45+win8+wp8 gets parsed to:
.NETPortable,Version=v4.5,Profile=net45+win8+wp8

Both end up being "valid", but for portable frameworks, the version doesn't matter (the profile does).

The fix is to use a regex to strip out "portable-" and "portable{version}-" when doing platform matching.

(note: NuGet pack seems to be using the portable45 for whatever reason now)

Fix to handle weird malformed portable-only libraries.
The nuget parser handles folders in the form of:
{Identifier}{Version}-{Profile}

So the string: `portable-net45+win8+wp8` gets parsed to:
`.NETPortable,Version=v0.0,Profile=net45+win8+wp8`

The string: `portable45-net45+win8+wp8` gets parsed to:
`.NETPortable,Version=v4.5,Profile=net45+win8+wp8`

Both end up being "valid", but for portable frameworks, the version doesn't matter (the profile does).

The fix is to use a regex to strip out "portable-" and "portable{version}-" when doing platform matching.
@inosik

This comment has been minimized.

Show comment
Hide comment
@inosik

inosik Apr 3, 2017

Contributor

(note: NuGet pack seems to be using the portable45 for whatever reason now)

Yes, I have seen this on a couple of other packages now. BTW, this behavior is still not documented.

Contributor

inosik commented Apr 3, 2017

(note: NuGet pack seems to be using the portable45 for whatever reason now)

Yes, I have seen this on a couple of other packages now. BTW, this behavior is still not documented.

@OniBait

This comment has been minimized.

Show comment
Hide comment
@OniBait

OniBait Apr 3, 2017

Contributor

Yeah. Closest I've found to any sort of documentation on it is in the old v2 code here:
https://github.com/NuGet/NuGet2/blob/2.13/src/Core/Utility/VersionUtility.cs#L241

Although, I guess one could argue that technically it is documented here:
https://docs.microsoft.com/en-us/nuget/create-packages/supporting-multiple-target-frameworks#framework-version-folder-structure

with the caveat that for portable frameworks, it should have the profile after:
{framework name}[{version}]-{profile}

In any case, the following code in the NuGet library works:
NuGetFramework.Parse("portable45-net45+win8+wp8")
returns:
[.NETPortable,Version=v4.5,Profile=Profile78]

Contributor

OniBait commented Apr 3, 2017

Yeah. Closest I've found to any sort of documentation on it is in the old v2 code here:
https://github.com/NuGet/NuGet2/blob/2.13/src/Core/Utility/VersionUtility.cs#L241

Although, I guess one could argue that technically it is documented here:
https://docs.microsoft.com/en-us/nuget/create-packages/supporting-multiple-target-frameworks#framework-version-folder-structure

with the caveat that for portable frameworks, it should have the profile after:
{framework name}[{version}]-{profile}

In any case, the following code in the NuGet library works:
NuGetFramework.Parse("portable45-net45+win8+wp8")
returns:
[.NETPortable,Version=v4.5,Profile=Profile78]

@OniBait

This comment has been minimized.

Show comment
Hide comment
@OniBait

OniBait Apr 3, 2017

Contributor

Alternatively, does it make sense to reference the NuGet DLLs and use their methods for parsing frameworks? I'm not well versed enough in the project to know if there was a conscious decision to not have NuGet as a dependency...

Contributor

OniBait commented Apr 3, 2017

Alternatively, does it make sense to reference the NuGet DLLs and use their methods for parsing frameworks? I'm not well versed enough in the project to know if there was a conscious decision to not have NuGet as a dependency...

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Apr 3, 2017

Member
Member

forki commented Apr 3, 2017

@forki forki merged commit e240de6 into fsprojects:master Apr 3, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment