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

AssemblyLoadContext issue with NuGet in Arcade Sleet publishing #5073

Closed
rainersigwald opened this issue Jan 24, 2020 · 14 comments
Closed

AssemblyLoadContext issue with NuGet in Arcade Sleet publishing #5073

rainersigwald opened this issue Jan 24, 2020 · 14 comments
Labels
Area: Engine Issues impacting the core execution of targets and tasks. bug Partner request regression triaged
Milestone

Comments

@rainersigwald
Copy link
Member

After #4916, something is causing a break dotnet/arcade#1965.

@rainersigwald rainersigwald added bug regression Area: Engine Issues impacting the core execution of targets and tasks. labels Jan 24, 2020
@rainersigwald rainersigwald added this to the MSBuild 16.5 Preview 3 milestone Jan 24, 2020
@rainersigwald rainersigwald self-assigned this Jan 24, 2020
@rainersigwald
Copy link
Member Author

@chcosta used an instrumented MSBuild which helped isolate the problem: the Microsoft.DotNet.Build.Tasks.Feed package contains:

  • SleetLib, which references
    • NuGet at package version 4.3.0
  • NuGet at package version 5.3.0

The loader currently requires strict version matches

https://github.com/microsoft/msbuild/blob/8aa0b87c00c6f26a565cf5e10975769dad9f378b/src/Shared/MSBuildLoadContext.cs#L71-L75

So when we try to load NuGet-as-referenced-from-Sleet, it fails because it can only find NuGet 5.3.0.4 and Sleet references 4.3.0.5.

This wasn't a problem pre-ALC isolation, because we always loaded NuGet from the sdk root folder next to MSBuild. Now that's just a fallback, and the assemblies in the package match by simple name.

What I don't yet understand is what happens after we try this and it fails. Why don't we then fall back to next-to-MSBuild?

@rainersigwald
Copy link
Member Author

What I don't yet understand is what happens after we try this and it fails. Why don't we then fall back to next-to-MSBuild?

I strongly suspect that this will be fixed by #6558.

@chcosta do you remember this issue? Could we try backing dotnet/arcade@841c83d out in an experimental way?

@chcosta
Copy link
Member

chcosta commented Jun 14, 2021

I very much remember this issue and am excited to get this resolved. We can get a private build of Arcade published with that change removed. I should be able to help out tomorrow.

@rainersigwald
Copy link
Member Author

rainersigwald commented Aug 2, 2021

@chcosta This is in the latest preview release checked in now so we can probably go ahead with an Arcade fixup after the next SDK preview release.

@chcosta
Copy link
Member

chcosta commented Aug 3, 2021

I'm not sure I'll have time to look at this, this week. @dnceng, may be able to help out.

@dotnet/dnceng , this requires undoing the hack in the feed package, and validating that doesn't break the sdk /runtime repos before committing.

@greenEkatherine
Copy link

@chcosta could you please point out an issue or PR related to that hack? I would like to help but I don't have a context

@chcosta
Copy link
Member

chcosta commented Aug 3, 2021

The hack is mentioned above as dotnet/arcade@841c83d. I'm on my phone at the moment, but I think that's the right one

@chcosta
Copy link
Member

chcosta commented Aug 16, 2021

@rainersigwald , What version of the sdk contains the fix?

@chcosta
Copy link
Member

chcosta commented Aug 16, 2021

This appears to still be failing with sdk "6.0.100-rc.1.21379.2" and MSBuild "17.0.0-preview-21416-02"

@rainersigwald rainersigwald removed their assignment Aug 18, 2021
@rainersigwald rainersigwald modified the milestones: Backlog, 17.0 Aug 18, 2021
@rainersigwald
Copy link
Member Author

Hmm, that MSBuild version should have #6558. Guess that didn't fix it then (though I was pretty confident it would and don't understand how it didn't . . .).

I marked this to pull it into our current list of things to investigate.

@chcosta
Copy link
Member

chcosta commented Aug 18, 2021

I couldn't actually get a repro using a current Arcade. I think the code path for publishing has changed significantly and we no longer trip this. However, using the old Arcade with the new SDK / MSBuild, caused the same error we previously saw. I can try to get this under a debugger again if needed.

@marcpopMSFT
Copy link
Member

Is the old Arcade still in wide usage by teams and are those teams using the new SDK/MSBuild? That seems like a fairly unlikely combo. Seems like we should just close this as we unblocked the one customer we got a report from. Any concerns?

@MattGal
Copy link
Member

MattGal commented Sep 10, 2021

Is the old Arcade still in wide usage by teams and are those teams using the new SDK/MSBuild? That seems like a fairly unlikely combo. Seems like we should just close this as we unblocked the one customer we got a report from. Any concerns?

With the end-of-life of 2.1 (except perhaps certain custom builds) I think our actual usage of sleet is effectively over, so I'd be fine closing this as long as @chcosta agrees.

My only concern, and I think we can address this if it happens, is that at some point we may not have old-enough desktop msbuild on agents to not hit the issue and this could result in some fixups to the (dead) builds.

@chcosta
Copy link
Member

chcosta commented Sep 10, 2021

I'm completely ok with closing this. We have a workaround in Arcade, and as far as I can tell, this is not actually affecting Arcade anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Engine Issues impacting the core execution of targets and tasks. bug Partner request regression triaged
Projects
None yet
Development

No branches or pull requests

6 participants