Skip to content

Installer/finalizer in c# #44611

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 35 commits into from
Mar 14, 2025
Merged

Installer/finalizer in c# #44611

merged 35 commits into from
Mar 14, 2025

Conversation

kasperk81
Copy link
Contributor

@kasperk81 kasperk81 commented Nov 3, 2024

TODOs:
- [ ] get WixToolset.Dtf.WindowsInstaller (https://www.nuget.org/packages/WixToolset.Dtf.WindowsInstaller/4.0.6) uploaded to one of dotnet nuget feed and undo NuGet.config change

  • adjust <FinalizerExe>$(ArtifactsDir)bin/finalizer/$(Architecture)/$(Configuration)/bin/finalizer.exe</FinalizerExe> in GenerateMSIs.targets
  • manual testing

fix #43876

@kasperk81 kasperk81 force-pushed the finalizer-csharp branch 3 times, most recently from 7cabc19 to 8abb521 Compare November 4, 2024 12:05
@joeloff
Copy link
Member

joeloff commented Nov 4, 2024

What is the size of the EXE? We need to ensure that the static SDK does not exceed 250MB - that's historically been a tripwire when we see a noticeable increase in download failures, especially outside North America.

It would be good to ensure that the logging mechanism preserves the structure logging that dutil provides - we chose the C/dutil implementation specifically for size and the logging. The other obstacle is the WiX version. There are very hard requirements about the copy we're using currently and until we wrap up the v5 migration work, we'll probably have to sit on this PR.

As for the pending reboot change - we don't care about what the registry says too much. What the finalizer is doing is passing the exit code from the MSI packages back to the bundle engine if it removed any of them.

Do appreciate your initiative - we've talked about moving this to managed single file executable before

@kasperk81
Copy link
Contributor Author

The AOT-published finalizer.exe binary for win-x64 comes in at 2MB. This setup skips the need for C++ and CMake in dotnet/sdk, making it way easier to debug and maintain the C# code, just like the rest of the repo. Plus, you can use dotnet run for quick local testing, which keeps the workflow nice and simple.

The version limitations are a bit of a drawback, but this is just an initial idea. We can fine-tune it down the line once those version issues are sorted out.

@joeloff
Copy link
Member

joeloff commented Nov 4, 2024

The AOT-published finalizer.exe binary for win-x64 comes in at 2MB. This setup skips the need for C++ and CMake in dotnet/sdk, making it way easier to debug and maintain the C# code, just like the rest of the repo. Plus, you can use dotnet run for quick local testing, which keeps the workflow nice and simple.

The version limitations are a bit of a drawback, but this is just an initial idea. We can fine-tune it down the line once those version issues are sorted out.

This sounds very promising, thank you

@kasperk81
Copy link
Contributor Author

@joeloff if you like to test this branch, download https://www.nuget.org/api/v2/package/WixToolset.Dtf.WindowsInstaller/4.0.6 at c:\temp\packages\ and run .\build.cmd -c Release. it'll be generated at $(ArtifactsDir)bin\finalizer\$(Configuration)\$(SdkTargetFramework)-win-$(Architecture)\publish\finalizer.exe

@marcpopMSFT marcpopMSFT added this to the 10.0.1xx milestone Nov 5, 2024
@marcpopMSFT marcpopMSFT removed the untriaged Request triage from a team member label Nov 5, 2024
@kasperk81 kasperk81 force-pushed the finalizer-csharp branch 3 times, most recently from cbc70e9 to d178e49 Compare November 7, 2024 22:24
@kasperk81 kasperk81 force-pushed the finalizer-csharp branch 2 times, most recently from b25c785 to 7f5d417 Compare November 8, 2024 07:33
@joeloff
Copy link
Member

joeloff commented Nov 12, 2024

@kasperk81 thank you for all the updates. I was out yesterday. Probably one or two more things to address. I'll try and do another pass later tonight

@kasperk81
Copy link
Contributor Author

I've put out a PR at #45973

@jkoritzinsky that fixed windows build. resolved merge conflict

@joeloff can you run the test?

@kasperk81
Copy link
Contributor Author

ping

@ViktorHofer
Copy link
Member

just FYI, @joeloff is currently unavailable. cc @marcpopMSFT

@nagilson
Copy link
Member

@/joeloff is available now but I think he has a large backlog of stuff

@ViktorHofer
Copy link
Member

ViktorHofer commented Mar 14, 2025

@joeloff please review this PR and perform the manual testing. I just resolved the latest merge conflicts.

@joeloff
Copy link
Member

joeloff commented Mar 14, 2025

This still needs to be tested manually and all the logs need to be reviewed.

@kasperk81
Copy link
Contributor Author

This still needs to be tested manually and all the logs need to be reviewed.

we can add tests for those logs, if it's possible to automate: run installer, then run uninstaller, capture logs and assert

@joeloff
Copy link
Member

joeloff commented Mar 14, 2025

For some of the basic single installs, possibly. For the more complicated SxS installs with multiple SDKs and Visual Studio it's a bit more complicated where you want the same standalone installer SDK version as what shipped in Visual Studio. Then, install the workloads from Visual Studio and remove the SDK where the finalizer should skip. The other other cases involve installing the SDK, installing workloads, then upgrading the SDK to a newer version in which case the uninstall of the previous SDK should not let the finalizer remove the workloads.

We have to verify the machine state in this case. I can simulate these manually. I should be able to get this done today and report back and then hopefully we can merge this PR.

Having the AOT built copies of the finalizer would be a great win IMO and make it much simpler to maintain going forward

@joeloff
Copy link
Member

joeloff commented Mar 14, 2025

Here's the latest log from installing workloads from the CLI and running the new finalizer - everything looks good to me
Microsoft_.NET_SDK_10.0.100-dev_(x64)_20250314162837_031_Finalizer.log

@joeloff joeloff merged commit 37cae14 into dotnet:main Mar 14, 2025
39 checks passed
@joeloff
Copy link
Member

joeloff commented Mar 14, 2025

Thank you @kasperk81 for your contribution

@kasperk81
Copy link
Contributor Author

thanks @joeloff and everyone.

@kasperk81 kasperk81 deleted the finalizer-csharp branch March 15, 2025 02:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

.NET SDK Finalizer Feature Band calculation doesn't match SDK's calculation
9 participants