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

2.1.8 downgraded some assemblies from 2.1.7 #7546

Closed
natemcmaster opened this issue Feb 13, 2019 · 20 comments
Closed

2.1.8 downgraded some assemblies from 2.1.7 #7546

natemcmaster opened this issue Feb 13, 2019 · 20 comments
Assignees
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework Done This issue has been fixed
Milestone

Comments

@natemcmaster
Copy link
Contributor

The 2.1.8 shared framework contains some assemblies older than the 2.1.7 framework. This is a result of incorrect data in the baseline file. For instance:
https://github.com/aspnet/AspNetCore/blob/4cceccd5689ebf7712c03789df536cbb4cfc2efc/eng/Baseline.xml#L123

The actual latest stable release of Microsoft.AspNetCore is 2.1.7. This can cause runtime exceptions such as #3503 (comment) if users target 2.1.7 but rollforward to newer shared frameworks.

cc @dougbu @mikaelm12 @Eilon

@natemcmaster natemcmaster added this to the 2.1.x milestone Feb 13, 2019
@Eilon
Copy link
Member

Eilon commented Feb 13, 2019

Oops. Are there any good workarounds to this? Add explicit reference to the correct version? (Downgrading to older patch is not a "good" workaround.)

@natemcmaster
Copy link
Contributor Author

There are several workarounds.

  1. add <PackageReference Include="Microsoft.AspNetCore" Version="2.1.7" /> (NB: this is NOT the .App package)
  2. Remove version from AspNetCore.App <PackageReference Include="Microsoft.AspNetCore" />
  3. Update AspNetCore.App: <PackageReference Include="Microsoft.AspNetCore.App" Version="2.1.8" />

@natemcmaster
Copy link
Contributor Author

To help us catch this in the future, I think we should consider adding a unit test which reads this baseline and checks that it matches the latest 2.1.* on nuget.org. The SDK team did this for a similar kind of baseline file problem.

@muratg muratg added the Servicing-consider Shiproom approval is required for the issue label Feb 13, 2019
@muratg
Copy link
Contributor

muratg commented Feb 13, 2019

This looks like a patch candidate for sure.

@dougbu Would you be the right person to fix this? Could you prepare a PR and ping me on it, so that we can bring this to shiproom tomorrow? This can be a tactical PR just to fix the issue, but we definitely need to add the test @natemcmaster specifies above to prevent this from happening again.

@dougbu
Copy link
Member

dougbu commented Feb 13, 2019

I'd appreciate it if @mikaelm12 could take a look at this first. @mikaelm12 is that 🆗 with you?

But, I'll double-check the current 2.1.9 branding.

@mikaelm12
Copy link
Contributor

Yeah, I can do it.

@dougbu
Copy link
Member

dougbu commented Feb 13, 2019

Problem remains in current release/2.1 branch. I'll fix that up and leave the test to @mikaelm12

@dougbu
Copy link
Member

dougbu commented Feb 13, 2019

See #7556

@muratg muratg assigned mikaelm12 and unassigned dougbu Feb 13, 2019
@muratg muratg removed the Servicing-consider Shiproom approval is required for the issue label Feb 13, 2019
@muratg
Copy link
Contributor

muratg commented Feb 14, 2019

Is the following related to this issue?

dotnet/core#2304 (comment)

cc @joeloff

@dougbu
Copy link
Member

dougbu commented Feb 14, 2019

@muratg probably related

@natemcmaster
Copy link
Contributor Author

@muratg Yes, that's the exact same error.

@dougbu
Copy link
Member

dougbu commented Feb 14, 2019

I've manually checked the Baseline.xml file against the current NuGet versions of each package. Nothing other than Microsoft.AspNetCore was behind.

Rather than do 2.2.3 manually, I'll turn to the test…

@dougbu
Copy link
Member

dougbu commented Feb 14, 2019

One alternative: Change the Baseline.xml file to list only Major.Minor versions and have BaselineGenerator determine the latest patch version.

@natemcmaster I don't see anything but BaselineGenerator that reads Baseline.xml. Am I missing anything?

Other thoughts on this infrastructure reformat versus a separate test?

@Eilon Eilon added the feature-platform Deprecated: Cross-cutting issues related to ASP.NET Core as a platform label Feb 14, 2019
@natemcmaster
Copy link
Contributor Author

I don't think we should only list Major.Minor. This could lead to non-deterministic behavior when re-generating baselines.

@mikaelm12 was looking into adding test coverage for this. I added @dougbu to the email thread in which we're brainstorming fixes.

@danielloczi
Copy link
Contributor

FYI: App Services in Azure also updated to the latest DotNet Core SDK, so this bug could effect your apps published to there.

Today I just had to fix one of our Asp.Net applications.
In the eventlog.xml I saw the following error entry:
Application: dotnet.exe CoreCLR Version: 4.6.27317.3 Description: The process was terminated due to an unhandled exception. Exception Info: System.IO.FileNotFoundException: Could not load file or assembly 'Microsoft.AspNetCore, Version=2.1.7.0, Culture=neutral, PublicKeyToken=adb9793829ddae60'.

@muratg
Copy link
Contributor

muratg commented Feb 27, 2019

@dougbu and @mikaelm12 in addition to the automation, could you guys please review the file (for both 2.2 and 2.1) to make sure it's correct on both branches?

Two more sets of eyes would be good on this.

@mikaelm12
Copy link
Contributor

I'll sync up with @dougbu on this today

@dougbu
Copy link
Member

dougbu commented Feb 27, 2019

Though my comment above said I only did the release/2.1 branch. I checked the versions manually in both branches.

The new test should confirm what I saw.

@mikaelm12
Copy link
Contributor

I've also just manually checked the versions in both the 2.1 and 2.2 branches. The shouldn't be any version downgrade issues

@muratg
Copy link
Contributor

muratg commented Feb 27, 2019

Thanks guys!

@mikaelm12 mikaelm12 added Done This issue has been fixed and removed 2 - Working labels Mar 25, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2019
@JunTaoLuo JunTaoLuo added area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework and removed feature-platform Deprecated: Cross-cutting issues related to ASP.NET Core as a platform labels Jan 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework Done This issue has been fixed
Projects
None yet
Development

No branches or pull requests

7 participants