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

NuGet.Packaging assemblies don't load for Feed package publishing #1965

Closed
wli3 opened this issue Feb 3, 2019 · 62 comments
Closed

NuGet.Packaging assemblies don't load for Feed package publishing #1965

wli3 opened this issue Feb 3, 2019 · 62 comments

Comments

@wli3
Copy link

wli3 commented Feb 3, 2019

The latest nuget package has a breaking change removing NuGet.Packaging.Core, and moving types to NuGet.Packaging. sdk should be the first one taking that change. As a result, arcade failed to publish.

failed publish
https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=2379512

@wli3
Copy link
Author

wli3 commented Feb 5, 2019

Maybe the title should be update to latest nuget version.

@jcagme
Copy link
Contributor

jcagme commented Feb 6, 2019

Hey @wli3, help me understand your process.

  • Are you restoring the Arcade SDK, just the Arcade version of the Microsoft.DotNet.Build.Tasks.Feed package or is this the BuildTools version of Microsoft.DotNet.Build.Tasks.Feed?

Depending on which of this you are using we'll know what's the best followy up is.

@wli3
Copy link
Author

wli3 commented Feb 6, 2019

only arcade version

@jcagme
Copy link
Contributor

jcagme commented Feb 6, 2019

What version of the package are you using?

@wli3
Copy link
Author

wli3 commented Feb 20, 2019

Change the title to use nuget 5.0.0-rtm.5821 and up. That is the root cause of the problem. NuGet has a breaking change to moved the dll nuget.packaging.core.dll

@wli3 wli3 changed the title Method not found: 'NuGet.Packaging.Core.PackageIdentity' Use nuget 5.0.0-rtm.5821 and up Feb 20, 2019
@MattGal
Copy link
Member

MattGal commented Mar 11, 2019

@JohnTortugo can you explain why you put this in the FR epic?

@JohnTortugo
Copy link
Contributor

Not sure @MattGal . Today it looks more like an Arcade issue though.

@MattGal
Copy link
Member

MattGal commented Mar 12, 2019

OK with no FR details or followup for us, I am removing the epic tag; please ping me if you feel this was in error.

@markwilkie
Copy link
Member

@jcagme - what are the next steps here?

@jcagme
Copy link
Contributor

jcagme commented Mar 13, 2019

I don't know what's the latest from @wli3. We could update the version although I don't know why would we take a dependency on a RTM package.

@markwilkie
Copy link
Member

@wli3 - thoughts on next steps?

@markwilkie
Copy link
Member

Closing based on lack of responses

@wli3
Copy link
Author

wli3 commented Mar 22, 2019

Sorry for the delay. CLI and SDK repo workaround this issue with painful product change(3 weeks). However, nuget version past 5.0.0-rtm.5821 has a breaking change of moving types. It might still cause problem like build task failure or cross gen that is similar to CLI and SDK repo. So I suggest Arcade to upgrade ASAP in case the problem appear before final release and we have to fit it in rush

@wtgodbe
Copy link
Member

wtgodbe commented Jan 15, 2020

Reopening as we just got bitten by this in AspNetCore-Tooling: dotnet/aspnetcore#18324.

@wli3 can you comment on how you worked around this in CLI/SDK?

@markwilkie @jcagme can we update to a newer version of nuget.client to unblock upstack repos? I think anyone using a new SDK with a new Arcade will be broken. Here's the call that fails:

PackageIdentity packageIdentity = packageReader.GetIdentity();
id = packageIdentity.Id;
version = packageIdentity.Version.ToString();

@nkolev92
Copy link
Contributor

@wtgodbe Is arcade depending on another component that depends on NuGet, or it depends on NuGet directly.

If it's the first one, you can add NuGet.Packaging.Core as a reference, it has type forwarding.
If it's the latter one, then simply rebuilding against a newer version of NuGet should do it.

@markwilkie
Copy link
Member

Assuming Arcade, the .net sdk needs to be updated to get a new nuget.exe. I seem to recall that this happened already (but I don't know for sure). So perhaps this build is getting nuget from elsewhere somehow? Or, perhaps the sdk doesn't have the latest one?

cc/ @mmitche

@wtgodbe
Copy link
Member

wtgodbe commented Jan 15, 2020

Toolset & SDK use a 5.5.0-preview.2 version of NuGet.Build.Tasks, from 3 weeks ago:
https://github.com/dotnet/toolset/blob/master/eng/Version.Details.xml#L72-L74
https://github.com/dotnet/sdk/blob/master/eng/Version.Details.xml#L22-L24

Whereas Arcade uses Nuget.Versioning version 4.4.0, Nuget.Client version 5.3.0:

<NuGetVersioningVersion>4.4.0</NuGetVersioningVersion>
<NuGetVersion>5.3.0</NuGetVersion>
. So I believe we'd need to update Nuget in this repo as @nkolev92 suggested

@wtgodbe
Copy link
Member

wtgodbe commented Jan 15, 2020

Nuget.Versioning 4.4.0 is from 2017 - it appears we use that version because anything newer would conflict with the MSBuild version used by the SDK: #2189. The SDK now uses MSBuild version 16.6.0-preview-20064-05: https://github.com/dotnet/toolset/blob/30eddcbf073098f8958a0a0321aad4a6e3ad3b79/eng/Version.Details.xml#L40. MSbuild uses a 5.4.0 version of Nuget: https://github.com/microsoft/msbuild/blob/ab9b2f36a5ff7a85f842b205d5529e77fdc9d7ab/eng/Packages.props#L4. So I believe we can bump the Nuget.Versioning version up to at least 5.3.0, and see if that resolves the issue. I'll try this all out with an internal branch.

@wtgodbe
Copy link
Member

wtgodbe commented Jan 15, 2020

Here's the internal Arcade build with my branch: https://dev.azure.com/dnceng/internal/_build/results?buildId=484408

@wli3
Copy link
Author

wli3 commented Jan 15, 2020

Last time nuget added type forwarding, so it passed. I don't know the cause of this time.

@markwilkie
Copy link
Member

I see - yes, we can (and should) update these versions. @mmitche, any thoughts on not having more than one place where we're defining the nuget version?

@mmitche
Copy link
Member

mmitche commented Jan 15, 2020

I see - yes, we can (and should) update these versions. @mmitche, any thoughts on not having more than one place where we're defining the nuget version?

I'm not sure how we would get the SDK's nuget version...but primarily I was concerned about two copies of nuget.exe itself. Maybe @tmat has some ideas here?

@jaredpar
Copy link
Member

@wtgodbe where is a link to the actual fix that you built that off of? Looking through the thread I just see a build, not a PR.

@wtgodbe
Copy link
Member

wtgodbe commented Jan 15, 2020

Here it is, I just bumped NuGetVersioningVersion to 5.3.0

@wtgodbe
Copy link
Member

wtgodbe commented Jan 15, 2020

Updating the NugetVersioningVersion in Arcade didn't work in my internal build: d2bd58c62f9104d66f415141a1a27b665c78690c. Still getting the same error:

D:\a\1\s.packages\microsoft.dotnet.arcade.sdk\5.0.0-beta.20065.1\tools\SdkTasks\PublishArtifactsInManifest.proj(65,5): error : Method not found: 'NuGet.Versioning.NuGetVersion NuGet.Packaging.Core.PackageIdentity.get_Version()'.

I'll try adding a direct reference to Nuget.Packaging.Core

@chcosta
Copy link
Member

chcosta commented Jan 21, 2020

I have a local repro. Sleet might be a red herring.

It appears that the conflict is between Arcade's feed package using NuGet v 5.3.0.4 and dotnet loading v5.5.0.1. I don't see any older versions of NuGet being loaded. I'm continuing to investigate.

@wtgodbe
Copy link
Member

wtgodbe commented Jan 21, 2020

The SDK first updated it's nuget dependency to 5.5 in december, which lines up with the timeline of when the bug started occuring (extensions uses an SDK from november and doesn't have this issue): dotnet/toolset#3735

@chcosta
Copy link
Member

chcosta commented Jan 22, 2020

Yes, I don't have a solution yet. The failure does correspond with that change. There is a conflict between the NuGet which comes with the Arcade Feed package and the one in the dotnet sdk. Updating Arcade to the same version of NuGet doesn't fix the issue (I'd presume some types moved and an additional package reference is needed to make the Arcade Feed package work). Removing NuGet assemblies from the Feed package seems to allow .net core to load from its sdk and things work.... I'm still investigating, and don't have an optimal solution at the moment. @nguerrera , does this sound reasonable?

I wonder if we can build the feed package and exclude NuGet assemblies from the package on core.

@nguerrera
Copy link
Contributor

cc @dsplaisted.

@chcosta I didn't follow what exactly that entails. Do you have a change we can review?

@nguerrera
Copy link
Contributor

Are you saying you can remove your dependency on NuGet altogether?

@chcosta
Copy link
Member

chcosta commented Jan 23, 2020

Update, just saw this info on my GitHub page and realized I hadn't hit the "Comment" button like I intended to yesterday. Posting this, and then I'll add an additional update with the latest.

Our version of Sleet was compiled against NuGet API v4.4.0. NuGet made a change where “PackageIdentity” moved from the “NuGet.Packaging.Core” assembly to “NuGet.Packaging”.

Next steps:

  1. @emgarten believes that we could, fairly “easily”, merge in sleet master to roll us forward to NuGet version 5.2 and from there make the update to version 5.4. This should, theoretically” fix the problem because we will start looking for PackageIdentity in the correct assembly.
  1. work with @jcagme to get a version of sleetlib published with the update
  1. @rainersigwald believes that Load plugins in an AssemblyLoadContext msbuild#4916, will “fix” the issue going forward such that we don’t hit this because we shouldn’t be mixing assemblies. This will require dotnet/toolset to update to a version of MSBuild that contains the fix.

@nguerrera , sorry for the slow response, it was a busy day. I'll reach out to you tomorrow because not all the details are particularly clear yet. I just send out mail with slightly more info

@chcosta
Copy link
Member

chcosta commented Jan 23, 2020

@nguerrera noticed that the sdk repo is already building with dotnet/msbuild#4916 and that corresponds with the symptoms being seen here. Assembly isolation is likely causing this problem since not all NuGet packages are being loaded.

The quickest fix would be to update global.json to use a version of the sdk that does not contain dotnet/msbuild#4916. The most recent version I could find is 5.0.100-alpha1-016143. I have a mail out asking if that is a sane path forward or if there are alternative paths to unblock. Given the symptoms, I don't think that an update to Sleet will fix the issue we are seeing.

@markwilkie
Copy link
Member

For folks following this issue, the conversation somehow got onto email. (hence the lack of apparent activity on this issue). Currently @rainersigwald and @chcosta are working through both the closest path to unblock as well as the longer term solution.

This is a very high priority item.

@chcosta
Copy link
Member

chcosta commented Jan 24, 2020

I'm in the process of trying to validate an interim solution using an updated Arcade SDK in a dotnet-sdk build. Unfortunately, I don't think there's an easy way to validate the publishing step itself apart from an update to master, so the current validation build is more of a sanity check. I'll prepare a PR with the interim solution.

@chcosta
Copy link
Member

chcosta commented Jan 24, 2020

Working with @wtgodbe to validate the change in an aspnetcore build.

@chcosta
Copy link
Member

chcosta commented Jan 24, 2020

Workaround available: Arcade SDK 5.0.0-beta.20074.3 from https://dotnetfeed.blob.core.windows.net/dotnet-core/index.json

@wtgodbe ran a build of aspnetcore-tooling that used the one-off Arcade SDK I produced and it appears to be a viable workaround.

I actually think it's probably fine for us to just make this change in Arcade (even though it's a hack). I'll submit a PR to Arcade but anyone that needs to unblock themselves can use the Arcade version listed above.

Meanwhile, the current theory is that MSBuild is not loading some assemblies because of its strict version matching. @rainersigwald is continuing to investigate.

@wtgodbe
Copy link
Member

wtgodbe commented Jan 24, 2020

Woohoo! Once the workaround is in Arcade I'll make sure the new version gets pushed to aspnetcore-tooling/sdk

@chcosta
Copy link
Member

chcosta commented Jan 25, 2020

Sounds good, also, there's no reason you can't use the workaround version as I produced that from an official build (not my local dev box). whichever is necessary to unblock you.

@wtgodbe
Copy link
Member

wtgodbe commented Jan 25, 2020

@chcosta doesn't appear to have worked in master: https://dev.azure.com/dnceng/internal/_build/results?buildId=496713

@chcosta
Copy link
Member

chcosta commented Jan 25, 2020

@wtgodbe, can you use the version I listed above? That's the version I validated. The other version merged into master *should * be the same, but I haven't validated it yet. 😕

@chcosta
Copy link
Member

chcosta commented Jan 25, 2020

Additionally, it looks like my change is in the 5.0.0-beta.20074.6 arcade sdk but you got the build before. Do now that I still haven't validated the package contents yet though

@chcosta
Copy link
Member

chcosta commented Jan 25, 2020

Looks like the build is passing with the latest arcade sdk https://dev.azure.com/dnceng/internal/_build/results?buildId=496755&view=results

@chcosta chcosta changed the title Arcade publishing uses old version of NuGet NuGet.Packaging assemblies don't load for Feed package publishing Jan 26, 2020
@chcosta chcosta closed this as completed Jan 27, 2020
mmitche added a commit to mmitche/arcade that referenced this issue Jan 26, 2023
After the sleet removal, we should be able to remove a long standing workound that required that the arcade SDK load a very old version on NuGet.Versioning because of a conflicting dependency with sleet.  I have unified the references under NuGet.Packaging's version, so that source-build can properly override the NuGet version.

The relevant issues around the old workaround are dotnet#6014 and dotnet#1965
mmitche added a commit to mmitche/arcade that referenced this issue Jan 26, 2023
After the sleet removal, we should be able to remove a long standing workound that required that the arcade SDK load a very old version on NuGet.Versioning because of a conflicting dependency with sleet.  I have unified the references under NuGet.Packaging's version, so that source-build can properly override the NuGet version.

The relevant issues around the old workaround are dotnet#6014 and dotnet#1965
mmitche added a commit that referenced this issue Jan 26, 2023
After the sleet removal, we should be able to remove a long standing workound that required that the arcade SDK load a very old version on NuGet.Versioning because of a conflicting dependency with sleet.  I have unified the references under NuGet.Packaging's version, so that source-build can properly override the NuGet version.

The relevant issues around the old workaround are #6014 and #1965
dotnet-bot pushed a commit to dotnet/dotnet that referenced this issue Jan 27, 2023
After the sleet removal, we should be able to remove a long standing workound that required that the arcade SDK load a very old version on NuGet.Versioning because of a conflicting dependency with sleet.  I have unified the references under NuGet.Packaging's version, so that source-build can properly override the NuGet version.

The relevant issues around the old workaround are dotnet/arcade#6014 and dotnet/arcade#1965

Original commit: dotnet/arcade@3900335

[[ commit created by automation ]]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests