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

Allow .NET SDK versions to be provided with --platform-version argument #2236

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Allow .NET SDK versions to be provided with --platform-version argument #2236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
201d30b
Allow .NET SDK versions to be provided with --platform-version argument
cormacpayne Nov 1, 2023
b5de087
Merge branch 'main' into corm/fix-dotnet-sdk-detect
william-msft Nov 2, 2023
5f9314f
Merge branch 'main' of https://github.com/microsoft/oryx into corm/fi…
cormacpayne Nov 7, 2023
2fff167
Update failing tests
cormacpayne Nov 7, 2023
20a746d
Merge branch 'corm/fix-dotnet-sdk-detect' of https://github.com/micro…
cormacpayne Nov 7, 2023
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
97 changes: 51 additions & 46 deletions src/BuildScriptGenerator/DotNetCore/DotnetCorePlatform.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,29 @@ public void ResolveVersions(RepositoryContext context, PlatformDetectorResult de
dotNetCorePlatformDetectorResult.PlatformVersion = resolvedRuntimeVersion;

var versionMap = this.versionProvider.GetSupportedVersions();
var sdkVersion = this.GetSdkVersion(context, dotNetCorePlatformDetectorResult.PlatformVersion, versionMap);
var sdkVersion = versionMap[dotNetCorePlatformDetectorResult.PlatformVersion];

// Check for a global.json file and update the SDK and runtime versions based on the result
if (this.commonOptions.EnableDynamicInstall
&& context.SourceRepo.FileExists(DotNetCoreConstants.GlobalJsonFileName))
{
var availableSdks = versionMap.Values;
sdkVersion = this.globalJsonSdkResolver.GetSatisfyingSdkVersion(
context.SourceRepo,
dotNetCorePlatformDetectorResult.PlatformVersion,
availableSdks);

// Update the runtime version based on the SDK version from the global.json
if (versionMap.Any(v => v.Value.Equals(sdkVersion, StringComparison.OrdinalIgnoreCase)))
{
dotNetCorePlatformDetectorResult.PlatformVersion =
versionMap
.Where(v => v.Value.Equals(sdkVersion, StringComparison.OrdinalIgnoreCase))
.Select(v => v.Key)
.FirstOrDefault();
}
}

dotNetCorePlatformDetectorResult.SdkVersion = sdkVersion;
}

Expand Down Expand Up @@ -336,25 +358,6 @@ private static void SetStartupFileNameInfoInManifestFile(
buildProperties[DotNetCoreManifestFilePropertyKeys.StartupDllFileName] = startupDllFileName;
}

private string GetSdkVersion(
RepositoryContext context,
string runtimeVersion,
Dictionary<string, string> versionMap)
{
if (this.commonOptions.EnableDynamicInstall
&& context.SourceRepo.FileExists(DotNetCoreConstants.GlobalJsonFileName))
{
var availableSdks = versionMap.Values;
var globalJsonSdkVersion = this.globalJsonSdkResolver.GetSatisfyingSdkVersion(
context.SourceRepo,
runtimeVersion,
availableSdks);
return globalJsonSdkVersion;
}

return versionMap[runtimeVersion];
}

private string GetBuildConfiguration()
{
var configuration = this.dotNetCoreScriptGeneratorOptions.MSBuildConfiguration;
Expand All @@ -366,10 +369,31 @@ private string GetBuildConfiguration()
return configuration;
}

private string GetMaxSatisfyingRuntimeVersionAndVerify(string runtimeVersion)
private string GetMaxSatisfyingRuntimeVersionAndVerify(string platformVersion)
{
// versionMap is a mapping of runtime versions -> SDK versions
var versionMap = this.versionProvider.GetSupportedVersions();

// First try to exactly match the given version with an existing SDK version
var exactSdkVersion = versionMap.Values.FirstOrDefault(v => v.Equals(platformVersion, StringComparison.OrdinalIgnoreCase));
if (exactSdkVersion != null)
{
// Return the corresponding runtime version for the exact-matched SDK version
return versionMap
.Where(v => v.Value.Equals(exactSdkVersion, StringComparison.OrdinalIgnoreCase))
.Select(v => v.Key)
.FirstOrDefault();
}

// Next try to exactly match the given version with an existing runtime version
var exactRuntimeVersion = versionMap.Keys.FirstOrDefault(v => v.Equals(platformVersion, StringComparison.OrdinalIgnoreCase));
if (exactRuntimeVersion != null)
{
return versionMap.Keys
.Where(v => v.Equals(exactRuntimeVersion, StringComparison.OrdinalIgnoreCase))
.FirstOrDefault();
}

// Since our semantic versioning library does not work with .NET Core preview version format, here
// we do some trivial way of finding the latest version which matches a given runtime version
// Runtime versions are usually like: 1.0, 2.1, 3.1, 5.0 etc.
Expand All @@ -378,7 +402,7 @@ private string GetMaxSatisfyingRuntimeVersionAndVerify(string runtimeVersion)
// have '-' in their names.
var nonPreviewRuntimeVersions = versionMap.Keys.Where(version => version.IndexOf("-") < 0);
var maxSatisfyingVersion = SemanticVersionResolver.GetMaxSatisfyingVersion(
runtimeVersion,
platformVersion,
nonPreviewRuntimeVersions);

// Check if a preview version is available
Expand All @@ -388,7 +412,7 @@ private string GetMaxSatisfyingRuntimeVersionAndVerify(string runtimeVersion)
// Preview versions: 5.0.0-preview.3.20214.6, 5.0.0-preview.2.20160.6, 5.0.0-preview.1.20120.5
var previewRuntimeVersions = versionMap.Keys
.Where(version => version.Contains("-"))
.Where(version => version.StartsWith(runtimeVersion))
.Where(version => version.StartsWith(platformVersion))
.OrderByDescending(version => version);
if (previewRuntimeVersions.Any())
{
Expand All @@ -400,11 +424,11 @@ private string GetMaxSatisfyingRuntimeVersionAndVerify(string runtimeVersion)
{
var exception = new UnsupportedVersionException(
DotNetCoreConstants.PlatformName,
runtimeVersion,
platformVersion,
versionMap.Keys);
this.logger.LogError(
exception,
$"Exception caught, the version '{runtimeVersion}' is not supported for the .NET Core platform.");
$"Exception caught, the version '{platformVersion}' is not supported for the .NET Core platform.");
throw exception;
}

Expand All @@ -414,9 +438,9 @@ private string GetMaxSatisfyingRuntimeVersionAndVerify(string runtimeVersion)
private string GetRuntimeVersionUsingHierarchicalRules(string detectedVersion)
{
// Explicitly specified version by user wins over detected version
if (!string.IsNullOrEmpty(this.dotNetCoreScriptGeneratorOptions.DotNetCoreRuntimeVersion))
if (!string.IsNullOrEmpty(this.dotNetCoreScriptGeneratorOptions.DotNetCorePlatformVersion))
{
return this.dotNetCoreScriptGeneratorOptions.DotNetCoreRuntimeVersion;
return this.dotNetCoreScriptGeneratorOptions.DotNetCorePlatformVersion;
}

// If a version was detected, then use it.
Expand All @@ -435,24 +459,5 @@ private string GetRuntimeVersionUsingHierarchicalRules(string detectedVersion)
var defaultVersion = this.versionProvider.GetDefaultRuntimeVersion();
return defaultVersion;
}

private bool TryGetExplicitVersion(out string explicitVersion)
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this one just never used?

{
explicitVersion = null;

var platformName = this.commonOptions.PlatformName;
if (platformName.EqualsIgnoreCase(DotNetCoreConstants.PlatformName))
{
if (string.IsNullOrWhiteSpace(this.dotNetCoreScriptGeneratorOptions.DotNetCoreRuntimeVersion))
{
return false;
}

explicitVersion = this.dotNetCoreScriptGeneratorOptions.DotNetCoreRuntimeVersion;
return true;
}

return false;
}
}
}
2 changes: 1 addition & 1 deletion src/BuildScriptGenerator/DotNetCore/DotnetCoreScriptGeneratorOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public class DotNetCoreScriptGeneratorOptions
/// </summary>
public string MSBuildConfiguration { get; set; }

public string DotNetCoreRuntimeVersion { get; set; }
public string DotNetCorePlatformVersion { get; set; }

public string DefaultRuntimeVersion { get; set; }
}
Expand Down
2 changes: 1 addition & 1 deletion src/BuildScriptGeneratorCli/Options/DotNetCoreScriptGeneratorOptionsSetup.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public DotNetCoreScriptGeneratorOptionsSetup(IConfiguration configuration)
public void Configure(DotNetCoreScriptGeneratorOptions options)
{
options.MSBuildConfiguration = this.GetStringValue(SettingsKeys.MSBuildConfiguration);
options.DotNetCoreRuntimeVersion = this.GetStringValue(SettingsKeys.DotNetVersion);
options.DotNetCorePlatformVersion = this.GetStringValue(SettingsKeys.DotNetVersion);
options.DefaultRuntimeVersion = this.GetStringValue(SettingsKeys.DotNetDefaultVersion);
}
}
Expand Down
62 changes: 51 additions & 11 deletions tests/Oryx.BuildImage.Tests/DotNetCore/DotNetCoreDynamicInstallTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,6 @@ public void DynamicInstall_ReInstallsSdk_IfSentinelFileIsNotPresent()
[Fact, Trait("category", "githubactions")]
public void BuildsApplication_IgnoresExplicitRuntimeVersionBasedSdkVersion_AndUsesSdkVersionSpecifiedInGlobalJson()
{
// Here we are testing building a 2.1 runtime version app with a 3.1 sdk version

// Arrange
var expectedSdkVersion = "3.1.201";
var globalJsonTemplate = @"
Expand All @@ -222,7 +220,7 @@ public void BuildsApplication_IgnoresExplicitRuntimeVersionBasedSdkVersion_AndUs
}";
var globalJsonContent = globalJsonTemplate.Replace("#version#", expectedSdkVersion);
var appName = NetCoreApp21WebApp;
var runtimeVersion = "2.1";
var runtimeVersion = "3.1";
var volume = CreateSampleAppVolume(appName);
var appDir = volume.ContainerDir;
var appOutputDir = "/tmp/output";
Expand Down Expand Up @@ -272,8 +270,6 @@ public void BuildsApplication_IgnoresExplicitRuntimeVersionBasedSdkVersion_AndUs
[Fact, Trait("category", "githubactions")]
public void BuildsApplication_IgnoresRuntimeVersionBasedSdkVersion_AndUsesSdkVersionSpecifiedInGlobalJson()
{
// Here we are testing building a 2.1 runtime version app with a 3.1 sdk version

// Arrange
var expectedSdkVersion = "3.1.201";
var globalJsonTemplate = @"
Expand All @@ -285,7 +281,7 @@ public void BuildsApplication_IgnoresRuntimeVersionBasedSdkVersion_AndUsesSdkVer
}";
var globalJsonContent = globalJsonTemplate.Replace("#version#", expectedSdkVersion);
var appName = NetCoreApp21WebApp;
var runtimeVersion = "2.1";
var runtimeVersion = "3.1";
var volume = CreateSampleAppVolume(appName);
var appDir = volume.ContainerDir;
var appOutputDir = "/tmp/output";
Expand Down Expand Up @@ -344,7 +340,7 @@ public void BuildsApplication_UsingPreviewVersionOfSdk()
}";
var globalJsonContent = globalJsonTemplate.Replace("#version#", expectedSdkVersion);
var appName = NetCoreApp50MvcApp;
var runtimeVersion = "5.0";
var runtimeVersion = "7.0";
var volume = CreateSampleAppVolume(appName);
var appDir = volume.ContainerDir;
var appOutputDir = "/tmp/output";
Expand Down Expand Up @@ -430,8 +426,6 @@ public void BuildsAppAfterInstallingAllRequiredPlatforms()
[Fact, Trait("category", "githubactions")]
public void BuildsApplication_ByDynamicallyInstallingSDKs_IntoCustomDynamicInstallationDir()
{
// Here we are testing building a 2.1 runtime version app with a 3.1 sdk version

// Arrange
var expectedSdkVersion = "3.1.201";
var globalJsonTemplate = @"
Expand All @@ -443,7 +437,7 @@ public void BuildsApplication_ByDynamicallyInstallingSDKs_IntoCustomDynamicInsta
}";
var globalJsonContent = globalJsonTemplate.Replace("#version#", expectedSdkVersion);
var appName = NetCoreApp21WebApp;
var runtimeVersion = "2.1";
var runtimeVersion = "3.1";
var volume = CreateSampleAppVolume(appName);
var appDir = volume.ContainerDir;
var appOutputDir = "/tmp/output";
Expand Down Expand Up @@ -611,7 +605,7 @@ public static TheoryData<string, string, string, string> SupportedVersionAndImag

[Theory, Trait("category", "githubactions")]
[MemberData(nameof(SupportedVersionAndImageNameData))]
public void BuildsApplication_AfterInstallingSupportedSdk(
public void BuildsApplication_SpecifyingRuntimeVersion_AfterInstallingSupportedSdk(
string runtimeVersion,
string sdkVersion,
string appName,
Expand Down Expand Up @@ -655,6 +649,52 @@ public void BuildsApplication_AfterInstallingSupportedSdk(
result.GetDebugInfo());
}

[Theory, Trait("category", "githubactions")]
[MemberData(nameof(SupportedVersionAndImageNameData))]
public void BuildsApplication_SpecifyingSdkVersion_AfterInstallingSupportedSdk(
string runtimeVersion,
string sdkVersion,
string appName,
string imageName)
{
var volume = CreateSampleAppVolume(appName);
var appDir = volume.ContainerDir;
var appOutputDir = "/tmp/output";
var manifestFile = $"{appOutputDir}/{FilePaths.BuildManifestFileName}";
var script = new ShellScriptBuilder()
.AddBuildCommand(
$"{appDir} -i /tmp/int -o {appOutputDir} " +
$"--platform {DotNetCoreConstants.PlatformName} --platform-version {sdkVersion}")
.AddFileExistsCheck(manifestFile)
.AddCommand($"cat {manifestFile}")
.ToString();

// Act
var result = _dockerCli.Run(new DockerRunArguments
{
ImageId = imageName,
EnvironmentVariables = new List<EnvironmentVariable> { CreateAppNameEnvVar(appName) },
Volumes = new List<DockerVolume> { volume },
CommandToExecuteOnRun = "/bin/bash",
CommandArguments = new[] { "-c", script }
});

// Assert
RunAsserts(
() =>
{
Assert.True(result.IsSuccess);
Assert.Contains(string.Format(SdkVersionMessageFormat, sdkVersion), result.StdOut);
Assert.Contains(
$"{ManifestFilePropertyKeys.DotNetCoreRuntimeVersion}=\"{runtimeVersion}",
result.StdOut);
Assert.Contains(
$"{ManifestFilePropertyKeys.DotNetCoreSdkVersion}=\"{sdkVersion}",
result.StdOut);
},
result.GetDebugInfo());
}

public static TheoryData<string, string> UnsupportedVersionAndImageNameData
{
get
Expand Down