From 0bce0db01854537ed420c3dfa8b0b0e0efef4854 Mon Sep 17 00:00:00 2001 From: Forgind <12969783+Forgind@users.noreply.github.com> Date: Thu, 18 Jan 2024 14:17:32 -0800 Subject: [PATCH 1/3] Avoid elevation if we will noop --- .../dotnet-workload/install/MsiInstallerBase.cs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/Cli/dotnet/commands/dotnet-workload/install/MsiInstallerBase.cs b/src/Cli/dotnet/commands/dotnet-workload/install/MsiInstallerBase.cs index 60d086bb4d1a..dfd113a4dfdf 100644 --- a/src/Cli/dotnet/commands/dotnet-workload/install/MsiInstallerBase.cs +++ b/src/Cli/dotnet/commands/dotnet-workload/install/MsiInstallerBase.cs @@ -537,6 +537,11 @@ protected void UpdateDependent(InstallRequestType requestType, string providerKe public void RemoveManifestsFromInstallState(SdkFeatureBand sdkFeatureBand) { string path = Path.Combine(WorkloadInstallType.GetInstallStateFolder(sdkFeatureBand, DotNetHome), "default.json"); + var installStateContents = File.Exists(path) ? InstallStateContents.FromString(File.ReadAllText(path)) : new InstallStateContents(); + if (installStateContents.Manifests == null) + { + return; + } if (!File.Exists(path)) { @@ -550,7 +555,6 @@ public void RemoveManifestsFromInstallState(SdkFeatureBand sdkFeatureBand) { if (File.Exists(path)) { - var installStateContents = InstallStateContents.FromString(File.ReadAllText(path)); installStateContents.Manifests = null; File.WriteAllText(path, installStateContents.ToString()); } @@ -570,6 +574,14 @@ public void RemoveManifestsFromInstallState(SdkFeatureBand sdkFeatureBand) public void SaveInstallStateManifestVersions(SdkFeatureBand sdkFeatureBand, Dictionary manifestContents) { string path = Path.Combine(WorkloadInstallType.GetInstallStateFolder(sdkFeatureBand, DotNetHome), "default.json"); + var installStateContents = File.Exists(path) ? InstallStateContents.FromString(File.ReadAllText(path)) : new InstallStateContents(); + if (installStateContents.Manifests != null && // manifestContents should not be null here + installStateContents.Manifests.Count == manifestContents.Count && + installStateContents.Manifests.All(m => manifestContents.TryGetValue(m.Key, out var val) && val.Equals(m.Value))) + { + return; + } + Elevate(); if (IsElevated) @@ -577,7 +589,7 @@ public void SaveInstallStateManifestVersions(SdkFeatureBand sdkFeatureBand, Dict // Create the parent folder for the state file and set up all required ACLs SecurityUtils.CreateSecureDirectory(Path.GetDirectoryName(path)); - var installStateContents = File.Exists(path) ? InstallStateContents.FromString(File.ReadAllText(path)) : new InstallStateContents(); + installStateContents.Manifests = manifestContents; File.WriteAllText(path, installStateContents.ToString()); From 4d4cd59eb664f8666724edbdb5c4deb0aec5de2d Mon Sep 17 00:00:00 2001 From: Forgind <12969783+Forgind@users.noreply.github.com> Date: Thu, 18 Jan 2024 14:19:46 -0800 Subject: [PATCH 2/3] Optimize mode setting --- .../dotnet-workload/install/MsiInstallerBase.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Cli/dotnet/commands/dotnet-workload/install/MsiInstallerBase.cs b/src/Cli/dotnet/commands/dotnet-workload/install/MsiInstallerBase.cs index dfd113a4dfdf..ce9328326b03 100644 --- a/src/Cli/dotnet/commands/dotnet-workload/install/MsiInstallerBase.cs +++ b/src/Cli/dotnet/commands/dotnet-workload/install/MsiInstallerBase.cs @@ -285,17 +285,21 @@ protected uint RepairMsi(string productCode, string logFile) /// protected void UpdateInstallMode(SdkFeatureBand sdkFeatureBand, bool newMode) { + string path = Path.Combine(WorkloadInstallType.GetInstallStateFolder(sdkFeatureBand, DotNetHome), "default.json"); + var installStateContents = File.Exists(path) ? InstallStateContents.FromString(File.ReadAllText(path)) : new InstallStateContents(); + if (installStateContents.UseWorkloadSets == newMode) + { + return; + } + Elevate(); if (IsElevated) { - string path = Path.Combine(WorkloadInstallType.GetInstallStateFolder(sdkFeatureBand, DotNetHome), "default.json"); // Create the parent folder for the state file and set up all required ACLs SecurityUtils.CreateSecureDirectory(Path.GetDirectoryName(path)); - var installStateContents = File.Exists(path) ? InstallStateContents.FromString(File.ReadAllText(path)) : new InstallStateContents(); installStateContents.UseWorkloadSets = newMode; File.WriteAllText(path, installStateContents.ToString()); - SecurityUtils.SecureFile(path); } else if (IsClient) From c587d53ca39ff029e8a2e2879df9f2bcc1de7c80 Mon Sep 17 00:00:00 2001 From: Forgind <12969783+Forgind@users.noreply.github.com> Date: Fri, 19 Jan 2024 13:02:43 -0800 Subject: [PATCH 3/3] Simplify reading InstallStateContents --- .../dotnet/commands/dotnet-workload/InstallStateContents.cs | 5 +++++ .../commands/dotnet-workload/install/FileBasedInstaller.cs | 6 +++--- .../commands/dotnet-workload/install/MsiInstallerBase.cs | 6 +++--- .../MockPackWorkloadInstaller.cs | 4 ++-- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/Cli/dotnet/commands/dotnet-workload/InstallStateContents.cs b/src/Cli/dotnet/commands/dotnet-workload/InstallStateContents.cs index d6fcc23bf654..3c30ce66ab73 100644 --- a/src/Cli/dotnet/commands/dotnet-workload/InstallStateContents.cs +++ b/src/Cli/dotnet/commands/dotnet-workload/InstallStateContents.cs @@ -25,6 +25,11 @@ public static InstallStateContents FromString(string contents) return JsonSerializer.Deserialize(contents, s_options); } + public static InstallStateContents FromPath(string path) + { + return File.Exists(path) ? FromString(File.ReadAllText(path)) : new InstallStateContents(); + } + public override string ToString() { return JsonSerializer.Serialize(this, s_options); diff --git a/src/Cli/dotnet/commands/dotnet-workload/install/FileBasedInstaller.cs b/src/Cli/dotnet/commands/dotnet-workload/install/FileBasedInstaller.cs index 08cd93199320..526842ea5e49 100644 --- a/src/Cli/dotnet/commands/dotnet-workload/install/FileBasedInstaller.cs +++ b/src/Cli/dotnet/commands/dotnet-workload/install/FileBasedInstaller.cs @@ -458,7 +458,7 @@ public void RemoveManifestsFromInstallState(SdkFeatureBand sdkFeatureBand) if (File.Exists(path)) { - var installStateContents = File.Exists(path) ? InstallStateContents.FromString(File.ReadAllText(path)) : new InstallStateContents(); + var installStateContents = InstallStateContents.FromString(File.ReadAllText(path)); installStateContents.Manifests = null; File.WriteAllText(path, installStateContents.ToString()); } @@ -468,7 +468,7 @@ public void SaveInstallStateManifestVersions(SdkFeatureBand sdkFeatureBand, Dict { string path = Path.Combine(WorkloadInstallType.GetInstallStateFolder(_sdkFeatureBand, _dotnetDir), "default.json"); Directory.CreateDirectory(Path.GetDirectoryName(path)); - var installStateContents = File.Exists(path) ? InstallStateContents.FromString(File.ReadAllText(path)) : new InstallStateContents(); + var installStateContents = InstallStateContents.FromPath(path); installStateContents.Manifests = manifestContents; File.WriteAllText(path, installStateContents.ToString()); } @@ -477,7 +477,7 @@ public void UpdateInstallMode(SdkFeatureBand sdkFeatureBand, bool newMode) { string path = Path.Combine(WorkloadInstallType.GetInstallStateFolder(sdkFeatureBand, _dotnetDir), "default.json"); Directory.CreateDirectory(Path.GetDirectoryName(path)); - var installStateContents = File.Exists(path) ? InstallStateContents.FromString(File.ReadAllText(path)) : new InstallStateContents(); + var installStateContents = InstallStateContents.FromPath(path); installStateContents.UseWorkloadSets = newMode; File.WriteAllText(path, installStateContents.ToString()); } diff --git a/src/Cli/dotnet/commands/dotnet-workload/install/MsiInstallerBase.cs b/src/Cli/dotnet/commands/dotnet-workload/install/MsiInstallerBase.cs index ce9328326b03..62bb3a9bcd8d 100644 --- a/src/Cli/dotnet/commands/dotnet-workload/install/MsiInstallerBase.cs +++ b/src/Cli/dotnet/commands/dotnet-workload/install/MsiInstallerBase.cs @@ -286,7 +286,7 @@ protected uint RepairMsi(string productCode, string logFile) protected void UpdateInstallMode(SdkFeatureBand sdkFeatureBand, bool newMode) { string path = Path.Combine(WorkloadInstallType.GetInstallStateFolder(sdkFeatureBand, DotNetHome), "default.json"); - var installStateContents = File.Exists(path) ? InstallStateContents.FromString(File.ReadAllText(path)) : new InstallStateContents(); + var installStateContents = InstallStateContents.FromPath(path); if (installStateContents.UseWorkloadSets == newMode) { return; @@ -541,7 +541,7 @@ protected void UpdateDependent(InstallRequestType requestType, string providerKe public void RemoveManifestsFromInstallState(SdkFeatureBand sdkFeatureBand) { string path = Path.Combine(WorkloadInstallType.GetInstallStateFolder(sdkFeatureBand, DotNetHome), "default.json"); - var installStateContents = File.Exists(path) ? InstallStateContents.FromString(File.ReadAllText(path)) : new InstallStateContents(); + var installStateContents = InstallStateContents.FromPath(path); if (installStateContents.Manifests == null) { return; @@ -578,7 +578,7 @@ public void RemoveManifestsFromInstallState(SdkFeatureBand sdkFeatureBand) public void SaveInstallStateManifestVersions(SdkFeatureBand sdkFeatureBand, Dictionary manifestContents) { string path = Path.Combine(WorkloadInstallType.GetInstallStateFolder(sdkFeatureBand, DotNetHome), "default.json"); - var installStateContents = File.Exists(path) ? InstallStateContents.FromString(File.ReadAllText(path)) : new InstallStateContents(); + var installStateContents = InstallStateContents.FromPath(path); if (installStateContents.Manifests != null && // manifestContents should not be null here installStateContents.Manifests.Count == manifestContents.Count && installStateContents.Manifests.All(m => manifestContents.TryGetValue(m.Key, out var val) && val.Equals(m.Value))) diff --git a/src/Tests/dotnet-workload-install.Tests/MockPackWorkloadInstaller.cs b/src/Tests/dotnet-workload-install.Tests/MockPackWorkloadInstaller.cs index 61d6dc6f24d8..37a204706607 100644 --- a/src/Tests/dotnet-workload-install.Tests/MockPackWorkloadInstaller.cs +++ b/src/Tests/dotnet-workload-install.Tests/MockPackWorkloadInstaller.cs @@ -170,7 +170,7 @@ public void RemoveManifestsFromInstallState(SdkFeatureBand sdkFeatureBand) string path = Path.Combine(WorkloadInstallType.GetInstallStateFolder(sdkFeatureBand, _dotnetDir), "default.json"); if (File.Exists(path)) { - var installStateContents = File.Exists(path) ? InstallStateContents.FromString(File.ReadAllText(path)) : new InstallStateContents(); + var installStateContents = InstallStateContents.FromPath(path); installStateContents.Manifests = null; File.WriteAllText(path, installStateContents.ToString()); } @@ -180,7 +180,7 @@ public void SaveInstallStateManifestVersions(SdkFeatureBand sdkFeatureBand, Dict { string path = Path.Combine(WorkloadInstallType.GetInstallStateFolder(sdkFeatureBand, _dotnetDir), "default.json"); Directory.CreateDirectory(Path.GetDirectoryName(path)); - var installStateContents = File.Exists(path) ? InstallStateContents.FromString(File.ReadAllText(path)) : new InstallStateContents(); + var installStateContents = InstallStateContents.FromPath(path); installStateContents.Manifests = manifestContents; File.WriteAllText(path, installStateContents.ToString()); }