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

VS doesn't restore on build/rebuild due to migration of versionprefix in netfx;netstandard project #4487

Closed
natidea opened this issue Feb 3, 2017 · 16 comments
Assignees
Labels
Functionality:Restore Resolution:External This issue appears to be External to nuget Type:Bug
Milestone

Comments

@natidea
Copy link

natidea commented Feb 3, 2017

From @krwq on February 2, 2017 17:13

Repro:

  • Clone https://github.com/aspnet/antiforgery and sync (i.e. git reset --hard) to 5cb5178619d9211eee0722d8737b425e2eec1375 (this is currently dev~2 or pre-migration)
  • Open sln found in the root
  • Migrate project when VS asks you to do so
  • Build/Rebuild
  • Notice that restore doesn't happen and errors that the project is not restored

VS build: 15.0.0-RC.4+26130.1.d15rel
I have previously tested this exact scenario on one of the previous builds (26020) and it has worked before (I was unable to repro it working - possibly some of the nuget packages disappeared - I did errors during restore though which means that at least VS tried to restore).

cc: @srivatsn @rrelyea

Copied from original issue: dotnet/project-system#1447

Update from @natidea:
A simplified repro is:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFrameworks>netcoreapp1.0;net451</TargetFrameworks>
    <VersionPrefix>1.2.0</VersionPrefix>
    <RuntimeIdentifier>win7-x86</RuntimeIdentifier>
  </PropertyGroup>

</Project>
@natidea
Copy link
Author

natidea commented Feb 3, 2017

From @333fred on February 2, 2017 18:55

@srivatsn I've labelled as blocking release, as this seems like a rather bad one.

@natidea
Copy link
Author

natidea commented Feb 3, 2017

From @333fred on February 2, 2017 18:59

Another project that demonstrates this issue: exceptionless/Exceptionless.Net@31ef359.

@natidea
Copy link
Author

natidea commented Feb 3, 2017

From @srivatsn on February 2, 2017 19:44

@natidea can you take a look to see if we're not nominating or if restore is not happening?

@natidea
Copy link
Author

natidea commented Feb 3, 2017

NuGet is being nominated correctly for all the projects, but I do not see any project.assets.json generated. Let me get some logs to determine if there is a NuGet error.
/cc @alpaix @rrelyea

@natidea
Copy link
Author

natidea commented Feb 3, 2017

From @alpaix on February 3, 2017 0:30

Duplicate to #4419. The fix will be inserted into VS soon.
As a temporary workaround add $(PackageVersion) property to every .csproj file in the solution

<PackageVersion>1.0.0</PackageVersion>

@natidea
Copy link
Author

natidea commented Feb 3, 2017

Here is the NuGet Exception:

System.InvalidOperationException: Sequence contains more than one element at
System.Linq.Enumerable.SingleOrDefault[TSource](IEnumerable`1 source) at
NuGet.SolutionRestoreManager.VsSolutionRestoreService.GetNonEvaluatedPropertyOrNull(IVsTargetFrameworks tfms, String propertyName) at
NuGet.SolutionRestoreManager.VsSolutionRestoreService.GetPackageVersion(IVsTargetFrameworks tfms) at
NuGet.SolutionRestoreManager.VsSolutionRestoreService.ToPackageSpec(ProjectNames projectNames, IVsProjectRestoreInfo projectRestoreInfo) at
NuGet.SolutionRestoreManager.VsSolutionRestoreService.ToDependencyGraphSpec(ProjectNames projectNames, IVsProjectRestoreInfo projectRestoreInfo) at
NuGet.SolutionRestoreManager.VsSolutionRestoreService.NominateProjectAsync(String projectUniqueName, IVsProjectRestoreInfo projectRestoreInfo, CancellationToken token)

This was introduced by this PR: NuGet/NuGet.Client#1146

Inspecting the project after migration, I see:

TFM PackageVersion
net451 1.0.0.0
netcoreapp1.1 or netstandard1.3 1.2.0 or 1.0.0

This is what is causing this exception. I was able to resolve this by removing the <VersionPrefix>1.2.0</VersionPrefix> in all projects where it occurred.

NuGet explicitly does not handle cases where different PackageVersions are returned for different TFMs, though perhaps it shouldn't crash restore, especially if this is a common migrate scenario.

/cc @alpaix @rrelyea @emgarten

@natidea
Copy link
Author

natidea commented Feb 3, 2017

From @alpaix on February 3, 2017 0:50

We need better error handling for parsing errors. It was assumed initially all exceptions from the Nominate API would be caught without crashing VS. This story is also covered in #3870.

@natidea
Copy link
Author

natidea commented Feb 3, 2017

@alpaix To clarify, this does not crash VS, it just breaks restore. But I think that's what you meant.

I believe the fix to #4419 only covers the case where the conflict is between 1.0.0.0 and 1.0.0. In the first sample, the entry <VersionPrefix>1.2.0</VersionPrefix> is in some projects post migration and this appears to only set PackageVersion for the netcoreapp TFMs. A simple repro is the following project which fails to restore:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFrameworks>netcoreapp1.0;net451</TargetFrameworks>
    <VersionPrefix>1.2.0</VersionPrefix>
    <RuntimeIdentifier>win7-x86</RuntimeIdentifier>
  </PropertyGroup>

</Project>

@natidea
Copy link
Author

natidea commented Feb 3, 2017

From @davkean on February 3, 2017 1:19

Should we move this over to NuGet (you can use: https://github-issue-mover.appspot.com/)?

@rrelyea
Copy link
Contributor

rrelyea commented Feb 3, 2017

believe that restore is having a hard time because Version is different for each tfm.
why does migrate put the version into versionprefix? (i think that is ignored in net451 targets?)
/cc @alpaix

@rrelyea
Copy link
Contributor

rrelyea commented Feb 3, 2017

Potential Fix:
have migration use version instead of versionprefix when migrating 1.67.0.* from project.json.
or similar...

Workaround:
Change x.y.z in migrated multitargetting project to x.y.z

@rrelyea rrelyea added the Resolution:External This issue appears to be External to nuget label Feb 3, 2017
@rrelyea rrelyea changed the title VS doesn't restore on build/rebuild VS doesn't restore on build/rebuild due to migration of versionprefix in netfx;netstandard project Feb 3, 2017
@livarcocc
Copy link

livarcocc commented Feb 3, 2017

What are the semantic differences between versionprefix and version? if I have version 1.0.0-* and pass --version-suffix to dotnet pack, will the right version be constructed.

We have versionprefix in migration because we must maintain the same semantics as the project.json version property.

Also, while everyone who migrates will hit this, fixing this in migration does not address the root cause of the problem?

People who have multiTFM with netfx and netcore and add a versionprefix themselves will hit this. I don't believe this is a migration issue and this should be fixed somewhere else.

@blackdwarf
Copy link

@livarcocc is right. Migration's job is to create a semantically same csproj to project.json. Also, if we migrate it in such a way --version-suffix could stop working which would be a breaking change for .NET Core.

@alpaix
Copy link

alpaix commented Feb 3, 2017

The problem is $(VersionPrefix) property is ignored by msbuild on desktop frameworks. As a result NuGet receives two different package versions without any indication which one to prefer.

@rrelyea
Copy link
Contributor

rrelyea commented Feb 4, 2017

SDK plans a clever fix: dotnet/sdk#814

@nkolev92
Copy link
Member

nkolev92 commented Feb 8, 2017

This is fixed now:

Verified with VS build: 26206.0.d15rel

I could repro the with the same project on 26204.0.d15rel. The restore was correct in 26206.0.d15rel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Functionality:Restore Resolution:External This issue appears to be External to nuget Type:Bug
Projects
None yet
Development

No branches or pull requests

6 participants