Skip to content

Build installers for the servicing releases #2362

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

Merged
merged 6 commits into from
Jan 7, 2022

Conversation

RussKie
Copy link
Member

@RussKie RussKie commented Nov 30, 2021

As per this comment we don't build installers for servicing releases unless the patch version matches:

<!--
Servicing build settings for setup packages. Instructions:
* To enable a package build for the current patch release, set PatchVersion to match the current
patch version of that package. ("major.minor.patch".) This is normally the same as
PatchVersion above.
* When the PatchVersion property above is incremented at the beginning of the next servicing
release, all packages listed below automatically stop building because the property no longer
matches the metadata. (Do not delete the items!)
If the PatchVersion below is never changed from '0', the package will build in the 'master'
branch, and during a forked RTM release ("X.Y.0"). It will stop building for "X.Y.1" unless
manually enabled by updating the metadata.
-->
<ItemGroup>
<ProjectServicingConfiguration Include="Microsoft.WindowsDesktop.App.Ref" PatchVersion="0" />
</ItemGroup>

For servicing releases in which we update inbox analyzers we must generate a new version of installers.
Given that Windows Desktop SDK is an agglomeration of Windows Forms and WPF that may ship updates/fixes to their analyzers at their own cadence, manually updating <ProjectServicingConfiguration PatchVersion="0" /> may be impractical for a number of reasons, including that Windows Forms and WPF are managed by different teams. Thus the proposal is to always increment the PatchVersion property as per the servicing release version.

@mmitche
Copy link
Member

mmitche commented Nov 30, 2021

@ericstj for input

@RussKie I have some concerns with this. I don't think we want to be generating a new ref pack on every release. We generally do not update the ref packs unless there are fixes/changes to surface area (5.0, for instance, only has a single version of the ref pack). Perhaps I'm not understanding the link between the analyzers and the ref pack?

@mmitche
Copy link
Member

mmitche commented Nov 30, 2021

Other question: Do changes to the analyzers apply to all the 6.0 SDKs?

@safern
Copy link
Member

safern commented Nov 30, 2021

Perhaps I'm not understanding the link between the analyzers and the ref pack?

I don't know if this is the case for Microsoft.WindowsDesktop ref pack, but for dotnet/runtime we ship analyzers as part of the ref pack, specifically source generators and that way customers get the source generators on their build without the need to reference a package.

@ericstj
Copy link
Member

ericstj commented Nov 30, 2021

Do you need to freeze the platform manifest here? That's something we do in aspnetcore and runtime in servicing to ensure that folks don't get the platform file information as if they were targeting a serviced runtime.

Here's a change where we did that:
https://github.com/dotnet/runtime/pull/61422/files#diff-26289a7a415834587dde961fb524d0cb2e9695755276e451cbb5cc8c28c77473R7

Also, you might consider adding a safeguard that does APICompat between the serviced ref-pack and GA. That will protect you from accidentally exposing new API in servicing.

@RussKie
Copy link
Member Author

RussKie commented Nov 30, 2021

Do you need to freeze the platform manifest here? That's something we do in aspnetcore and runtime in servicing to ensure that folks don't get the platform file information as if they were targeting a serviced runtime.

I have close to no knowledge of these parts of the SDK. I'm happy to add it since we do the same in other parts of the SDK. The only question I have is that the manifest file contains the build version, e.g. 6.0.1: Microsoft.VisualBasic.Forms.resources.dll|Microsoft.WindowsDesktop.App.Ref|6.0.1.0|6.0.121.56111 - and without this file we have a discrepancy between the version written in the manifest (always 6.0.0) and actual file versions.

Also, you might consider adding a safeguard that does APICompat between the serviced ref-pack and GA. That will protect you from accidentally exposing new API in servicing.

That's a good idea, I'll add it going forward.
In Windows Forms we use Microsoft.CodeAnalysis.PublicApiAnalyzers so with a high degree of confidence I know that we haven't altered the public API surface in 6.0.1. I'm fairly sure WPF hasn't made any changes in 6.0.1 timeframe either.

@RussKie
Copy link
Member Author

RussKie commented Nov 30, 2021

Perhaps I'm not understanding the link between the analyzers and the ref pack?

I don't know if this is the case for Microsoft.WindowsDesktop ref pack, but for dotnet/runtime we ship analyzers as part of the ref pack, specifically source generators and that way customers get the source generators on their build without the need to reference a package.

In .NET 6.0 Windows Forms has added an inbox source generator and analyzers that are shipped in a ref pack. Here's the design paper for inbox source generators written by @ericstj. There was a bug in the source generator implementation that we fixed, and are now trying to ship.

@AraHaan
Copy link
Member

AraHaan commented Dec 28, 2021

If only the WindowsDesktop runtimes could be split into 2 separate runtimes to where they would get 2 separate installers.

(Since technically the .NET SDK today makes use of properties to make the framework look like they are 2 separate runtimes when someone sees WindowsDesktop.WindowsForms or WindowsDesktop.WPF referenced in their project depending on their settings)

I think doing that at the runtime side as well might help solve the issue of installers for servicing releases as then they would get 2 separate runtime packs and then if one wants to build the WindowsDesktop runtime pack locally they can clone either dotnet/winforms, or dotnet/wpf and get installers instead of needing to clone both of those and hope they can somehow package their local changes to one of those repositories into another local clone of this repository (you can get the idea on how messy it can get real fast). I just got done with spending a few days trying to figure out on building the ASP.NET Core 7.0.0-dev runtime locally as the docs for it is not accurate. For command line it does not work but in Visual Studio it builds the zip files that then the installer projects use just out of box for some reason.

@dreddy-work dreddy-work marked this pull request as ready for review January 7, 2022 06:52
@dreddy-work
Copy link
Member

We are taking in this as-is for 6.0.2. any further proposals here can be tracked with future releases.

@dreddy-work dreddy-work merged commit 3013281 into release/6.0 Jan 7, 2022
@RussKie RussKie mentioned this pull request Feb 21, 2022
@mmitche mmitche deleted the release/6.0-installers branch June 14, 2024 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants