Skip to content

Support workload pack groups for repair, download, and show URL operations#25829

Merged
dsplaisted merged 12 commits into
dotnet:release/6.0.4xxfrom
dsplaisted:workload-pack-groups
Jun 29, 2022
Merged

Support workload pack groups for repair, download, and show URL operations#25829
dsplaisted merged 12 commits into
dotnet:release/6.0.4xxfrom
dsplaisted:workload-pack-groups

Conversation

@dsplaisted

Copy link
Copy Markdown
Member

No description provided.

@ghost

ghost commented Jun 3, 2022

Copy link
Copy Markdown

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@dsplaisted
dsplaisted force-pushed the workload-pack-groups branch from f2d30a2 to 7830222 Compare June 17, 2022 02:22
Refactors and combines a lot of the logic to download packages to cache or print the URLs to the packages for the workload install and workload update commands.
@dsplaisted
dsplaisted force-pushed the workload-pack-groups branch from 7830222 to bca2080 Compare June 17, 2022 16:45
gkulin added a commit to gkulin/sdk that referenced this pull request Jun 17, 2022
Directory.Delete(targetPath, true);
}

// Do we need to worry about signing validation here? I think not, as if the manifest is ever actually installed, it will go through signing validation

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the MSI is unsigned, the contents could be altered and we would be trusting the unsigned content. That would be the only concern here. Though the downloader should be verifying signatures

@dsplaisted
dsplaisted marked this pull request as ready for review June 22, 2022 20:55
$"TARGETDIR={msiExtractionPath}",
"/QN", // Quiet: No UI
}
};

@joeloff joeloff Jun 22, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to get rid of this and use the native APIs we have in the CLI. WindowsInstaller.InstallProduct allows you to do that. The doc comments actually mentions admin installs.

_ = WindowsInstaller.SetInternalUI(InstallUILevel.None);
WidnowsInstaller.InstallProduct(msiPath, $"TARGETDIR={msiExtractionPath} ACTION=ADMIN");

@dsplaisted

Copy link
Copy Markdown
Member Author

@joeloff @gkulin I still have to add localizable strings for some error messages, but otherwise I think this is ready for review.

var exitCode = psi.Execute();
if (exitCode != 0)

_ = WindowsInstaller.SetInternalUI(InstallUILevel.None);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did this work before without creating the directory first when you used MSIEXEC? Admin installs usually require having the TARGETDIR created before doing the install.

@@ -1,5 +1,6 @@
// Copyright (c) .NET Foundation and contributors. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
//

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I'm guessing you didn't mean to add an extra commented line here

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 That's been bothering me for a while now, especially when creating new files in VS. I think it comes from

file_header_template = Copyright (c) .NET Foundation and contributors. All rights reserved.\nLicensed under the MIT license. See LICENSE file in the project root for full license information.\n

@dsplaisted
dsplaisted enabled auto-merge June 29, 2022 18:01

@joeloff joeloff left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loc change look good

@dsplaisted
dsplaisted merged commit 0bf1504 into dotnet:release/6.0.4xx Jun 29, 2022
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.

3 participants