Skip to content
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.

Sign up for GitHub

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

Permit loose manifest priority #41996

Merged
merged 3 commits into from
Jul 17, 2024
Merged

Permit loose manifest priority #41996

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
769b812
Permit loose manifest priority
Forgind Jul 9, 2024
a00e958
Add test
Forgind Jul 11, 2024
8accf27
Also prio workload sets if requested
Forgind Jul 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 11 additions & 14 deletions ...esolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,31 +169,28 @@ bool TryGetWorkloadSet(string workloadSetVersion, out WorkloadSet? workloadSet)
}
}

_installStateFilePath = Path.Combine(WorkloadInstallType.GetInstallStateFolder(_sdkVersionBand, _sdkRootPath), "default.json");
var installState = InstallStateContents.FromPath(_installStateFilePath);
Forgind marked this conversation as resolved.
Show resolved Hide resolved
if (_workloadSet is null)
{
var installStateFilePath = Path.Combine(WorkloadInstallType.GetInstallStateFolder(_sdkVersionBand, _sdkRootPath), "default.json");
if (File.Exists(installStateFilePath))
if (!string.IsNullOrEmpty(installState.WorkloadVersion))
{
var installState = InstallStateContents.FromPath(installStateFilePath);
if (!string.IsNullOrEmpty(installState.WorkloadVersion))
if (!TryGetWorkloadSet(installState.WorkloadVersion!, out _workloadSet))
{
if (!TryGetWorkloadSet(installState.WorkloadVersion!, out _workloadSet))
{
throw new FileNotFoundException(string.Format(Strings.WorkloadVersionFromInstallStateNotFound, installState.WorkloadVersion, installStateFilePath));
}
throw new FileNotFoundException(string.Format(Strings.WorkloadVersionFromInstallStateNotFound, installState.WorkloadVersion, _installStateFilePath));
}

// Note: It is possible here to have both a workload set and loose manifests listed in the install state. This might happen if there is a
// third-party workload manifest installed that's not part of the workload set
_manifestsFromInstallState = installState.Manifests is null ? null : WorkloadSet.FromDictionaryForJson(installState.Manifests, _sdkVersionBand);
_installStateFilePath = installStateFilePath;
}

// Note: It is possible here to have both a workload set and loose manifests listed in the install state. This might happen if there is a
// third-party workload manifest installed that's not part of the workload set
marcpopMSFT marked this conversation as resolved.
Show resolved Hide resolved
_manifestsFromInstallState = installState.Manifests is null ? null : WorkloadSet.FromDictionaryForJson(installState.Manifests, _sdkVersionBand);
}

if (_workloadSet == null && availableWorkloadSets.Any())
if (_workloadSet == null && installState.UseWorkloadSets == true && availableWorkloadSets.Any())
Forgind marked this conversation as resolved.
Show resolved Hide resolved
{
var maxWorkloadSetVersion = availableWorkloadSets.Keys.Aggregate((s1, s2) => VersionCompare(s1, s2) >= 0 ? s1 : s2);
_workloadSet = availableWorkloadSets[maxWorkloadSetVersion.ToString()];
_useManifestsFromInstallState = false;
}
}

Expand Down
50 changes: 50 additions & 0 deletions ...crosoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;
using Microsoft.DotNet.Cli;
using Microsoft.DotNet.Workloads.Workload;
using Microsoft.NET.Sdk.Localization;
using Microsoft.NET.Sdk.WorkloadManifestReader;

Expand Down Expand Up @@ -31,6 +32,40 @@ void Initialize(string featureBand = "5.0.100", [CallerMemberName] string? testN
Directory.CreateDirectory(_manifestVersionBandDirectory);
}

[Theory]
[InlineData(true)]
[InlineData(false)]
public void ItShouldPrioritizeInstallStateOverWorkloadSetUnlessSpecified(bool preferWorkloadSet)
{
Initialize();

CreateMockManifest(_manifestRoot, "8.0.400", "ios", "11.0.2", true);
CreateMockManifest(_manifestRoot, "8.0.400", "ios", "11.0.6", true);
CreateMockWorkloadSet(_manifestRoot, "8.0.400", "8.0.400", @"
{
""ios"": ""11.0.2/8.0.400""
}
");

var installStateLocation = WorkloadInstallType.GetInstallStateFolder(new SdkFeatureBand("8.0.400"), Path.GetDirectoryName(_manifestRoot)!);
var installStateFilePath = Path.Combine(installStateLocation, "default.json");
var installState = InstallStateContents.FromPath(installStateFilePath);
installState.UseWorkloadSets = preferWorkloadSet;
installState.Manifests = new Dictionary<string, string>()
{
{ "ios", "11.0.6/8.0.400" }
};
Directory.CreateDirectory(installStateLocation);
File.WriteAllText(installStateFilePath, installState.ToString());

var sdkDirectoryWorkloadManifestProvider
= new SdkDirectoryWorkloadManifestProvider(sdkRootPath: _fakeDotnetRootDirectory, sdkVersion: "8.0.400", userProfileDir: null, globalJsonPath: null);

sdkDirectoryWorkloadManifestProvider.GetManifests().Single().ManifestVersion.Should().Be(preferWorkloadSet ? "11.0.2" : "11.0.6");

Directory.Delete(Path.Combine(_manifestRoot, "8.0.400"), recursive: true);
}

[Theory]
[InlineData(true)]
[InlineData(false)]
Expand Down Expand Up @@ -418,6 +453,13 @@ public void WorkloadSetCanIncludeMultipleJsonFiles()
CreateMockManifest(_manifestRoot, "8.0.200", "android", "33.0.2-rc.1", true);
CreateMockManifest(_manifestRoot, "8.0.200", "android", "33.0.2", true);

// To prepare the resolver to work with workload sets, we need to specify 'workload sets' in the install state file.
var installStateLocation = WorkloadInstallType.GetInstallStateFolder(new SdkFeatureBand("8.0.200"), Path.GetDirectoryName(_manifestRoot)!);
var installStateFilePath = Path.Combine(installStateLocation, "default.json");
var installState = InstallStateContents.FromPath(installStateFilePath);
installState.UseWorkloadSets = true;
Directory.CreateDirectory(installStateLocation);
File.WriteAllText(installStateFilePath, installState.ToString());

var workloadSetDirectory = Path.Combine(_manifestRoot, "8.0.200", "workloadsets", "8.0.200");
Directory.CreateDirectory(workloadSetDirectory);
Expand Down Expand Up @@ -1254,6 +1296,14 @@ private void CreateMockManifest(string manifestRoot, string featureBand, string

private void CreateMockWorkloadSet(string manifestRoot, string featureBand, string workloadSetVersion, string workloadSetContents)
{
// To prepare the resolver to work with workload sets, we need to specify 'workload sets' in the install state file.
var installStateLocation = WorkloadInstallType.GetInstallStateFolder(new SdkFeatureBand(featureBand), Path.GetDirectoryName(manifestRoot)!);
var installStateFilePath = Path.Combine(installStateLocation, "default.json");
var installState = InstallStateContents.FromPath(installStateFilePath);
installState.UseWorkloadSets = true;
Directory.CreateDirectory(installStateLocation);
File.WriteAllText(installStateFilePath, installState.ToString());

var workloadSetDirectory = Path.Combine(manifestRoot, featureBand, "workloadsets", workloadSetVersion);
if (!Directory.Exists(workloadSetDirectory))
{
Expand Down