-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Installer/finalizer in c# #44611
Conversation
91258b6
to
42e2009
Compare
7cabc19
to
8abb521
Compare
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 |
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 |
@joeloff if you like to test this branch, download https://www.nuget.org/api/v2/package/WixToolset.Dtf.WindowsInstaller/4.0.6 at |
cbc70e9
to
d178e49
Compare
b25c785
to
7f5d417
Compare
@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 |
@jkoritzinsky that fixed windows build. resolved merge conflict @joeloff can you run the test? |
ping |
just FYI, @joeloff is currently unavailable. cc @marcpopMSFT |
@/joeloff is available now but I think he has a large backlog of stuff |
Co-authored-by: Jeremy Koritzinsky <[email protected]>
@joeloff please review this PR and perform the manual testing. I just resolved the latest merge conflicts. |
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 |
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 |
Here's the latest log from installing workloads from the CLI and running the new finalizer - everything looks good to me |
Thank you @kasperk81 for your contribution |
thanks @joeloff and everyone. |
TODOs:
- [ ] getWixToolset.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<FinalizerExe>$(ArtifactsDir)bin/finalizer/$(Architecture)/$(Configuration)/bin/finalizer.exe</FinalizerExe>
inGenerateMSIs.targets
fix #43876