Skip to content

Microsoft.AspNet.WebApi.Core 5.2.8 has System.Web.Http.dll with file version lower than the one in 5.2.7 nuget release #322

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

Closed
samarunraj opened this issue Apr 18, 2022 · 23 comments
Assignees
Milestone

Comments

@samarunraj
Copy link

The file version of System.Web.Http.dll in https://www.nuget.org/packages/Microsoft.AspNet.WebApi.Core/ nuget has actually decrement with the 5.2.8 nuget release.

The 5.2.8 version of Microsoft.AspNet.WebApi.Core nuget package has System.Web.Http.dll

File Version: 5.2.34876.0
Product Version: 5.2.8-34876 (005f94e)

The 5.2.7 version of Microsoft.AspNet.WebApi.Core nuget package has System.Web.Http.dll

File Version: 5.2.61128.0
Product Version: 5.2.7-61128 (39d3064)

This causes installer issue when performing upgrades as the version decremented instead of incrementing.

@dougbu
Copy link
Contributor

dougbu commented Apr 18, 2022

This causes installer issue when performing upgrades as the version decremented instead of incrementing.

Could you elaborate on this please❔ What errors occur exactly❔

/cc @TanayParikh @mkArtakMSFT

@samarunraj
Copy link
Author

samarunraj commented Apr 18, 2022

when you perform a major upgrade, windows installer will remove the older version with the larger file version and won't install the supposedly newer file with an older file version.

Just to simplify the ask here, the file version number needs to increment upwards to indicate this is a newer assembly.

@dougbu
Copy link
Contributor

dougbu commented Apr 18, 2022

Hmm, it looks like the build version was close to uint16.MaxValue in the previous release and has now wrapped around. That took the assembly file version backwards. I'm pretty sure this has happened before, but I don't remember how we handled it. Looking…

@dougbu
Copy link
Contributor

dougbu commented Apr 18, 2022

@joeloff I introduced the % uint16.MaxValue operation in changeset 1756613 but I don't remember why. We haven't changed the start year since 2013 which makes sense given that's when history started😸 I suspect we thought we'd change the major or minor version in less than 6 years but it's been more than 3 years since 5.2.7. 5.2.0 shipped in 2014. Any suggestions❔ (I'm assuming a build version like 100411 would cause problems somewhere.)

@mkArtakMSFT what are your thoughts on perhaps moving straight to v5.3.0 ASAP❔

@dougbu
Copy link
Contributor

dougbu commented Apr 19, 2022

Another workaround would be to change <AssemblyFileVersion>$(VersionMajor).$(VersionMinor).$(VersionBuild).$(VersionRevision)</AssemblyFileVersion> to <AssemblyFileVersion>$(VersionMajor).$(VersionMinor).$(VersionRevision).$(VersionBuild)</AssemblyFileVersion>, bump $(VersionRevision) from 0 to 61129, and reset $(VersionStartYear) to 2022. That might require changes other places we use $(VersionRevision) or $(VersionBuild) but would avoid bumping the major or minor versions. Assembly versions should be close to 5.2.61129.00419 if we built again tomorrow.

@joeloff
Copy link

joeloff commented Apr 19, 2022

@dougbu yes, file versions must be unsigned 16-bit numbers. This is true for the MSI version as well, though I don't think we're shipping those at this point, unless there was need to push out a security fix. For the MSIs, we have to 8-bits for major/minor and 16 bits for the build number. The 4th part of the version is ignored.

I wonder, given the servicing model whether we shouldn't have reset the year on patch number. For file versions 5.2.8.x is bigger than 5.2.0.y and 5.2.7.z, NuGet only cares about the major.minor.patch part since semver doesn't support build/revision except if you have prerelease labels with build metadata.

but for the file versions, I do see the issue. Recycling those would be bad when versioning rules were applied to the files and you could end up with an older version looking newer, unless you inspect other attributes of the DLLs.

@dougbu
Copy link
Contributor

dougbu commented Apr 19, 2022

@samarunraj I remain unsure exactly what scenarios are broken due to the lower assembly file version in this release. You mentioned "windows installer" but the assembly file version is less important than other metadata such as the assembly version. Only one value has wrapped.

[assembly: AssemblyFileVersion("5.2.34876.0")]
[assembly: AssemblyInformationalVersion("5.2.8-34876 (005f94e123afdf3111f23f0b982880f8db9b10a7)")]
[assembly: SatelliteContractVersion("5.2.8.0")]
[assembly: AssemblyVersion("5.2.8.0")]

That said, we should do something different in our next release.

@dougbu
Copy link
Contributor

dougbu commented Apr 19, 2022

@joeloff you're correct that we didn't ship any installers and don't have plans for new ones anytime soon. But wouldn't resetting the patch number also have wrapped the [AssemblyFileVersion] value❔ I'm not sure why that would be any better or worse for @samarunraj's scenario.

we should do something different in our next release.

Should have said "probably" in there because I thought we had options like using the fourth portion of the [AssemblyFileVersion] for uniqueness.

@samarunraj
Copy link
Author

The file version is the one MSI is concerned about and that is the one that has wrapped, the other versions don't matter from the installer perspective.

For better understand my use case, I have System.Web.Http.dll installed using our own installer MSI and when I update the nuget to 5.2.8 and do a major product upgrade using the new MSI, System.Web.Http.dll is missing after the upgrade since the version requirements to do the upgrade was not met (The version needs to be newer for this)

@dougbu
Copy link
Contributor

dougbu commented Apr 19, 2022

I'm missing something still @samarunraj. An MSI shouldn't ignore the [AssemblyVersion] value. Could you please describe your MSI and the error(s) you see❔

@samarunraj
Copy link
Author

It doesn't and there are no installer error, just missing files after the install. The only way to get around it by setting https://docs.microsoft.com/en-us/windows/win32/msi/reinstallmode property to force reinstall of all files regardless of version.

The default behavior is it won't install a file if you do (MajorUpgrade - wix snippet below) and the file version of the file in MSI being used to upgrade is lower than what was already installed on the machine.

<MajorUpgrade AllowDowngrades="no" AllowSameVersionUpgrades="no" DowngradeErrorMessage="[NEWER_VERSION_FOUND_MESSAGE]" Schedule="afterInstallValidate"/>

@joeloff
Copy link

joeloff commented Apr 19, 2022

@samarunraj you can also change the scheduling of RemoveExistingProducts (schedule it for after InstallExecute). If the new MSI is installed first, the reference count for the component will be bumped and not removed when the older copy of the MSI is removed. If you remove the old MSI first (after InstallValidate) you can end up with files being removed. Of course changing the scheduling is not always possible as that determines what you can/cannot do during upgrades.

@dougbu
Copy link
Contributor

dougbu commented Apr 20, 2022

@joeloff could you clarify what you said about the fourth part of the [AssemblyFileVersion] value being ignored❔ I liked my idea above until you said that.

Separately, what do you feel is the urgency of a fix here❔ Other installers may be making similar assumptions…

@joeloff
Copy link

joeloff commented Apr 20, 2022

Oh, I was talking about MSI versions - they have restrictions around versioning.

@dougbu
Copy link
Contributor

dougbu commented Apr 20, 2022

I was talking about MSI versions

So, the assembly attribute has no similar restrictions❔ If that's the case, we could use my earlier idea and be done with it for 6.5 years or so.

This issue is specific to going from

[assembly: AssemblyFileVersion("5.2.61128.0")]

to

[assembly: AssemblyFileVersion("5.2.34876.0")]

because our $(VersionBuild) wrapped.

@joeloff
Copy link

joeloff commented Apr 20, 2022

Right, so that makes file version lower, which is used by the MSI to determine whether a specific file is older/newer when performing an upgrade. So, let's say you have two MSIs. MSI v1.0.0 has a file Foo.dll that is versioned as 5.2.66128.0 and MSI v1.0.1 has a newer copy of Foo.dll, but the file version is lower, 5.2.34876.0

When you install v1.0.0 and then the v1.0.1 MSI it can decide to not patch the file, because a newer copy is on disk, then remove the old MSI (which removes the DLL), then installs the new MSI, but skips the file. This is what's happening in @samarunraj case. The reason for this is that the action to remove the old MSI (RemoveExistingProducts) happens early in the install process (typically after the InstallValidateAction). So the user has to perform a repair of the MSI to get the missing file back on disk.

If you allow the new MSI to be installed first, the the reference count of the file will be 2 until the old MSI is removed. However, because the new DLL has an older version, you end up with the file on disk, but you will have the older copy of the file.

@dougbu
Copy link
Contributor

dougbu commented Apr 21, 2022

@mkArtakMSFT @TanayParikh @MackinnonBuck @joeloff let's have a quick meeting about this issue soon -- like a special edition "Monthly AspNetWebStack Triage"

@dougbu dougbu added this to the 3.2.9 milestone Apr 26, 2022
@dougbu dougbu self-assigned this Apr 26, 2022
@dougbu
Copy link
Contributor

dougbu commented Apr 26, 2022

@samarunraj thanks for reporting this issue. We agree this is a bug and have marked it as such. We're planning a near-term 3.2.9 release to address the problem.

@dougbu
Copy link
Contributor

dougbu commented May 9, 2022

/fyi I've updated our build system to address this issue. We're not ready for another official build or release. Assembly attributes now look like

[assembly: CompilationRelaxations(8)]
[assembly: RuntimeCompatibility(WrapNonExceptionThrows = true)]
[assembly: Debuggable(/*Could not decode attribute arguments.*/)]
[assembly: AssemblyConfiguration("")]
[assembly: AssemblyTrademark("")]
[assembly: ComVisible(false)]
[assembly: CLSCompliant(true)]
[assembly: NeutralResourcesLanguage("en-US")]
[assembly: AssemblyMetadata("Serviceable", "True")]
[assembly: AssemblyProduct("Microsoft ASP.NET MVC")]
[assembly: AssemblyTitle("System.Web.Cors")]
[assembly: AssemblyDescription("")]
[assembly: AssemblyCompany("Microsoft Corporation.")]
[assembly: AssemblyCopyright("© Microsoft Corporation. All rights reserved.")]
[assembly: AssemblyFileVersion("5.2.61129.7")]
[assembly: AssemblyInformationalVersion("5.2.9-preview1-61136 (2fa23f210cef96eed816b0f8c1361c745bd9450c)")]
[assembly: SatelliteContractVersion("5.2.9.0")]
[assembly: TargetFramework(".NETFramework,Version=v4.5", FrameworkDisplayName = ".NET Framework 4.5")]
[assembly: AssemblyVersion("5.2.9.0")]

The important parts are the higher [AssemblyFileVersion] value and the fact it (and [assemblyInformationalVersion]) increments with each build, not daily.

@avivanoff
Copy link

avivanoff commented May 9, 2022

@dougbu, when are you planning to release the fix? It would be nice to have it soon so we do not have to rollback a branch of our code to work around this bug.

@henk-van-der-vaart
Copy link

Note that ALSO "Microsoft.AspNet.Cors" Version="5.2.8" has System.Web.Cors DLL with file version "5.2.34876.0".
Which is lower than "Microsoft.AspNet.Cors" Version="5.2.7" that has System.Web.Cors DLL with file version "5.2.61128.0"

@dougbu
Copy link
Contributor

dougbu commented May 13, 2022

@henk-van-der-vaart the problem here is consistent across assemblies released in 5.2.8 and I'm fixing them all.

@dougbu
Copy link
Contributor

dougbu commented May 27, 2022

Fixed in TFS changeset 1770739. Should be able to publish 3.2.9 / 5.2.9 containing this fix next week (Memorial Day may delay things).

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

5 participants