From a38d4ebfd3850f36b26bcaa839d596a91785a15f Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Wed, 10 Jan 2024 10:32:35 +0100 Subject: [PATCH] [dotnet] Fix the supported target platform versions we support. We've used to ignore the target platform version (the "17.0" part in "net8.0-ios17.0") since our initial .NET relaese - customers could specify any valid OS version between the minimum and maximum versions, and we'd completely ignore the value [1]. The purpose of the target platform version is to specify which bindings to choose: "net8.0-ios17.0" would mean that the developer wants packages that have bindings for iOS 17.0 (and earlier iOS versions, but not later iOS versions). So saying "net8.0-ios11.0" would technically mean that the developer would want our bindings for iOS 11.0 (and earlier iOS versions, but not later iOS versions). The problem is that we don't ship any such thing... we shipped iOS 17.0 bindings in .NET 8, and that's it, you can't choose to build with something that does *not* have bindings for iOS 17.0. This will change with multi-targeting: we'll support *some* matrix of bindings. For instance, we might want to support the OS version we shipped initial support in any given .NET release + the latest OS version. For example, we might want to support both of these: * net8.0-ios17.0 * net8.0-ios17.2 This means that the target platform version (17.0/17.2) can't keep staying ignored. The proper way to do this is to change the `SdkSupportedTargetPlatformVersion` item group to only include target platform version we actually support, but the downside of this approach is that it will make existing projects fail to compile, if they happened to include an invalid target platform version. So I've added a compatibility mode for .NET 8, where we show a warning (and tell the developer what to do) instead of an error, and then in .NET 9 we'll make the warning an error instead. This is accomplished by still lying in the `SdkSupportedTargetPlatformVersion` item group in .NET 8, and detect the condition in our own logic and report the error. For .NET 9 we can remove this logic, makes ure `SdkSupportedTargetPlatformVersion` is correct, and then .NET itself will show the proper error. Side note: Android is also making an invalid target platform version an error in .NET 9: https://github.com/xamarin/xamarin-android/pull/8569. [1]: We'd ignore the value for executable projects. It did have an effect for library projects that were packed into NuGets: the target platform version would be stored in the NuGet. --- Make.versions | 43 ++++++++++++- dotnet/Makefile | 4 +- dotnet/generate-target-platforms.csharp | 33 +++++++--- dotnet/targets/Xamarin.Shared.Sdk.targets | 14 +++++ tests/dotnet/UnitTests/ProjectTest.cs | 73 +++++++++++++++++++++-- tests/dotnet/UnitTests/TestBaseClass.cs | 24 +++++--- 6 files changed, 168 insertions(+), 23 deletions(-) diff --git a/Make.versions b/Make.versions index d64cd0288cbd..d8abce58413d 100644 --- a/Make.versions +++ b/Make.versions @@ -100,5 +100,46 @@ MACCATALYST_NUGET_OS_VERSION=17.0 # So we've made the decision that the default target platform version is # always the latest target platform version. -# .NET 7 versions are bumped using maestro. +# +# Here we list all the releases we support for each platform. +# +# Format: space-separated list of TargetFramework-OSVersion +# +# Example: +# +# SUPPORTED_API_VERSIONS_IOS=net8.0-17.0 net8.0-17.2 +# +# This means the iOS workload shipped from the current branch supports projects with: +# net8.0-17.0 +# and +# net8.0-17.2 +# and even: +# net8.0-17.0;net8.0-17.2 +# +# When shipping support for a preview Xcode, we might add entries here for a preview release into a stable release. +# +# Example: +# +# SUPPORTED_API_VERSIONS_IOS=net9.0-18.0 +# +# If the current branch is stable .NET 8 using Xcode 15.0 (aka iOS 17.0), this +# would add support for trying the preview release by doing: +# +# net9.0-18.0 +# true +# +# Note that any SUPPORTED_API_VERSIONS entry below for older OS versions need a corresponding entry in +# the eng/Version.Details.xml and eng/Versions.props files. +# + +# First add the versions for the current branch. DO NOT TOUCH THIS. Add older branches below. + +SUPPORTED_API_VERSIONS_IOS=$(DOTNET_TFM)-$(IOS_NUGET_OS_VERSION) +SUPPORTED_API_VERSIONS_TVOS=$(DOTNET_TFM)-$(TVOS_NUGET_OS_VERSION) +SUPPORTED_API_VERSIONS_MACOS=$(DOTNET_TFM)-$(MACOS_NUGET_OS_VERSION) +SUPPORTED_API_VERSIONS_MACCATALYST=$(DOTNET_TFM)-$(MACCATALYST_NUGET_OS_VERSION) + +# Add older versions here! + +# (work on adding older versions is in progress) diff --git a/dotnet/Makefile b/dotnet/Makefile index 755ecbf52b1e..e15869d71f56 100644 --- a/dotnet/Makefile +++ b/dotnet/Makefile @@ -163,10 +163,10 @@ $(foreach platform,$(DOTNET_PLATFORMS),$(eval $(call ImplicitNamespaceImports,$( define SupportedTargetPlatforms Microsoft.$(1).Sdk/targets/Microsoft.$(1).Sdk.SupportedTargetPlatforms.props: $(TOP)/Versions-ios.plist.in $(TOP)/Versions-mac.plist.in Makefile ./generate-target-platforms.csharp Makefile $(Q) rm -f $$@.tmp - $(Q) ./generate-target-platforms.csharp $(1) $$@.tmp + $(Q) ./generate-target-platforms.csharp $(1) "$(DOTNET_TFM)" "$$(SUPPORTED_API_VERSIONS_$(2))" $$@.tmp $(Q) mv $$@.tmp $$@ endef -$(foreach platform,$(DOTNET_PLATFORMS),$(eval $(call SupportedTargetPlatforms,$(platform)))) +$(foreach platform,$(DOTNET_PLATFORMS),$(eval $(call SupportedTargetPlatforms,$(platform),$(shell echo $(platform) | tr a-z A-Z)))) define WorkloadTargets Workloads/Microsoft.NET.Sdk.$(1)/WorkloadManifest.json: Makefile $(TOP)/Make.config.inc $(GIT_DIRECTORY)/HEAD $(GIT_DIRECTORY)/index Makefile generate-workloadmanifest-json.csharp | Workloads/Microsoft.NET.Sdk.$(1) diff --git a/dotnet/generate-target-platforms.csharp b/dotnet/generate-target-platforms.csharp index ddd6a5a0aa1a..f28d1908a88a 100755 --- a/dotnet/generate-target-platforms.csharp +++ b/dotnet/generate-target-platforms.csharp @@ -6,7 +6,7 @@ using System.IO; using System.Xml; var args = Environment.GetCommandLineArgs (); -var expectedArgumentCount = 2; +var expectedArgumentCount = 4; if (args.Length != expectedArgumentCount + 2 /* 2 default arguments (executable + script) + 'expectedArgumentCount' arguments we're interested in */) { // first arg is "/Library/Frameworks/Mono.framework/Versions/4.8.0/lib/mono/4.5/csharp.exe" // second arg the script itself @@ -16,17 +16,22 @@ if (args.Length != expectedArgumentCount + 2 /* 2 default arguments (executable return; } -var platform = args [2]; -var outputPath = args [3]; +var argumentIndex = 2; +var platform = args [argumentIndex++]; +var dotnetTfm = args [argumentIndex++]; +var supportedTargetPlatformVersions = args [argumentIndex++].Split (' '); +var outputPath = args [argumentIndex++]; var plistPath = platform == "macOS" ? "../Versions-mac.plist.in" : "../Versions-ios.plist.in"; var doc = new XmlDocument (); doc.Load (plistPath); var nodes = doc.SelectNodes ($"/plist/dict/key[text()='KnownVersions']/following-sibling::dict[1]/key[text()='{platform}']/following-sibling::array[1]/string"); +var currentSupportedTPVs = supportedTargetPlatformVersions.Where (v => v.StartsWith (dotnetTfm + "-", StringComparison.Ordinal)).Select (v => v.Substring (dotnetTfm.Length + 1)); var minSdkVersionName = $"DOTNET_MIN_{platform.ToUpper ()}_SDK_VERSION"; var minSdkVersionString = File.ReadAllLines ("../Make.config").Single (v => v.StartsWith (minSdkVersionName + "=", StringComparison.Ordinal)).Substring (minSdkVersionName.Length + 1); var minSdkVersion = Version.Parse (minSdkVersionString); +var compatEnabled = Version.Parse (dotnetTfm.Replace ("net", "")).Major < 9; using (TextWriter writer = new StreamWriter (outputPath)) { writer.WriteLine ($""); @@ -34,16 +39,28 @@ using (TextWriter writer = new StreamWriter (outputPath)) { writer.WriteLine (""); writer.WriteLine ("\t"); - foreach (XmlNode n in nodes) { - var version = n.InnerText; - if (Version.Parse (version) < minSdkVersion) - continue; - writer.WriteLine ($"\t\t<{platform}SdkSupportedTargetPlatformVersion Include=\"{n.InnerText}\" />"); + if (compatEnabled) { + foreach (XmlNode n in nodes) { + var version = n.InnerText; + if (Version.Parse (version) < minSdkVersion) + continue; + writer.WriteLine ($"\t\t<{platform}SdkSupportedTargetPlatformVersion Include=\"{n.InnerText}\" />"); + } + + foreach (var tpv in currentSupportedTPVs) { + writer.WriteLine ($"\t\t<_Real{platform}SdkSupportedTargetPlatformVersion Include=\"{tpv}\" />"); + } + } else { + foreach (var tpv in currentSupportedTPVs) { + writer.WriteLine ($"\t\t<{platform}SdkSupportedTargetPlatformVersion Include=\"{tpv}\" />"); + } } writer.WriteLine ("\t"); writer.WriteLine ("\t"); writer.WriteLine ($"\t\t"); + if (compatEnabled) + writer.WriteLine ($"\t\t<_RealSdkSupportedTargetPlatformVersion Condition=\"'$(TargetPlatformIdentifier)' == '{platform}'\" Include=\"@(_Real{platform}SdkSupportedTargetPlatformVersion)\" />"); writer.WriteLine ("\t"); writer.WriteLine ("\t"); writer.WriteLine ($"\t\t<{platform}MinSupportedOSPlatformVersion>{minSdkVersionString}"); diff --git a/dotnet/targets/Xamarin.Shared.Sdk.targets b/dotnet/targets/Xamarin.Shared.Sdk.targets index 87082b58d069..5fe30d2202e9 100644 --- a/dotnet/targets/Xamarin.Shared.Sdk.targets +++ b/dotnet/targets/Xamarin.Shared.Sdk.targets @@ -2096,6 +2096,20 @@ + + + + <_RealTargetPlatformVersion Include="@(_RealSdkSupportedTargetPlatformVersion)" Condition="'@(_RealSdkSupportedTargetPlatformVersion)' != '' And $([MSBuild]::VersionEquals(%(Identity), $(TargetPlatformVersion)))" /> + + + <_IsCompatTargetPlatformVersion Condition="'@(_RealTargetPlatformVersion)' == ''">true + + + + diff --git a/tests/dotnet/UnitTests/ProjectTest.cs b/tests/dotnet/UnitTests/ProjectTest.cs index d1a66472c19c..d531031af13d 100644 --- a/tests/dotnet/UnitTests/ProjectTest.cs +++ b/tests/dotnet/UnitTests/ProjectTest.cs @@ -1,5 +1,6 @@ using System.Runtime.InteropServices; using System.Diagnostics; +using System.Xml; using Mono.Cecil; @@ -1541,15 +1542,13 @@ internal static IList RemovePostCurrentOnMacCatalyst (IList self [TestCase (ApplePlatform.iOS)] [TestCase (ApplePlatform.TVOS)] [TestCase (ApplePlatform.MacOSX)] - [Ignore ("Multi-targeting support has been temporarily reverted/postponed")] public void InvalidTargetPlatformVersion (ApplePlatform platform) { Configuration.IgnoreIfIgnoredPlatform (platform); // Pick a target platform version we don't support (such as iOS 6.66). var targetFrameworks = Configuration.DotNetTfm + "-" + platform.AsString ().ToLowerInvariant () + "6.66"; - var supportedApiVersion = Configuration.GetVariableArray ($"SUPPORTED_API_VERSIONS_{platform.AsString ().ToUpperInvariant ()}"); - var validTargetPlatformVersions = supportedApiVersion.Where (v => v.StartsWith (Configuration.DotNetTfm + "-", StringComparison.Ordinal)).Select (v => v.Substring (Configuration.DotNetTfm.Length + 1)); + var supportedApiVersions = GetSupportedApiVersions (platform); var project = "MultiTargetingLibrary"; var project_path = GetProjectPath (project, platform: platform); @@ -1558,8 +1557,72 @@ public void InvalidTargetPlatformVersion (ApplePlatform platform) properties ["cmdline:AllTheTargetFrameworks"] = targetFrameworks; var rv = DotNet.AssertBuildFailure (project_path, properties); var errors = BinLog.GetBuildLogErrors (rv.BinLogPath).ToArray (); - Assert.AreEqual (1, errors.Length, "Error count"); - Assert.AreEqual ($"6.66 is not a valid TargetPlatformVersion for {platform.AsString ()}. Valid versions include:\n{string.Join ('\n', validTargetPlatformVersions)}", errors [0].Message, "Error message"); + AssertErrorMessages (errors, $"6.66 is not a valid TargetPlatformVersion for {platform.AsString ()}. Valid versions include:\n{string.Join ('\n', supportedApiVersions)}"); + } + + [Test] + [TestCase (ApplePlatform.MacCatalyst)] + [TestCase (ApplePlatform.iOS)] + [TestCase (ApplePlatform.TVOS)] + [TestCase (ApplePlatform.MacOSX)] + public void UnsupportedTargetPlatformVersion (ApplePlatform platform) + { + Configuration.IgnoreIfIgnoredPlatform (platform); + + // Pick a target platform version that we don't really support, + // but don't show an error in .NET 8 because of backwards compat. + // The earliest target OS version should do.g + var minSupportedOSVersion = GetSupportedOSVersions (platform).First (); + var targetFrameworks = Configuration.DotNetTfm + "-" + platform.AsString ().ToLowerInvariant () + minSupportedOSVersion; + var supportedApiVersions = GetSupportedApiVersions (platform, isCompat: false); + + var project = "MultiTargetingLibrary"; + var project_path = GetProjectPath (project, platform: platform); + Clean (project_path); + var properties = GetDefaultProperties (); + properties ["cmdline:AllTheTargetFrameworks"] = targetFrameworks; + + if (IsTargetPlatformVersionCompatEnabled) { + var rv = DotNet.AssertBuild (project_path, properties); + var warnings = BinLog.GetBuildLogWarnings (rv.BinLogPath).ToArray (); + AssertWarningMessages (warnings, $"{minSupportedOSVersion} is not a valid TargetPlatformVersion for {platform.AsString ()}. This warning will become an error in future versions of the {platform.AsString ()} workload. Valid versions include:\n{string.Join ('\n', supportedApiVersions)}"); + } else { + var rv = DotNet.AssertBuildFailure (project_path, properties); + var errors = BinLog.GetBuildLogErrors (rv.BinLogPath).ToArray (); + AssertErrorMessages (errors, $"{minSupportedOSVersion} is not a valid TargetPlatformVersion for {platform.AsString ()}. Valid versions include:\n{string.Join ('\n', supportedApiVersions)}"); + } + } + + bool IsTargetPlatformVersionCompatEnabled { + get => Version.Parse (Configuration.DotNetTfm.Replace ("net", "")).Major < 9; + } + + string [] GetSupportedApiVersions (ApplePlatform platform, bool? isCompat = null) + { + if (isCompat is null) + isCompat = IsTargetPlatformVersionCompatEnabled; + if (isCompat.Value) + return GetSupportedOSVersions (platform); + + var supportedApiVersions = Configuration.GetVariableArray ($"SUPPORTED_API_VERSIONS_{platform.AsString ().ToUpperInvariant ()}"); + return supportedApiVersions + .Where (v => v.StartsWith (Configuration.DotNetTfm + "-", StringComparison.Ordinal)) + .Select (v => v.Substring (Configuration.DotNetTfm.Length + 1)) + .ToArray (); + } + + string [] GetSupportedOSVersions (ApplePlatform platform) + { + var plistPath = Path.Combine (Configuration.SourceRoot, platform == ApplePlatform.MacOSX ? "Versions-mac.plist.in" : "Versions-ios.plist.in"); + var doc = new XmlDocument (); + doc.Load (plistPath); + var query = $"/plist/dict/key[text()='KnownVersions']/following-sibling::dict[1]/key[text()='{platform.AsString ()}']/following-sibling::array[1]/string"; + Console.WriteLine ($"Query: {query} Nodes: {doc.SelectNodes (query)!.Cast ().Count ()}"); + return doc + .SelectNodes (query)! + .Cast () + .Select (v => v.InnerText) + .ToArray (); } [Test] diff --git a/tests/dotnet/UnitTests/TestBaseClass.cs b/tests/dotnet/UnitTests/TestBaseClass.cs index 6d1052bdb70e..2d7829fb8bd3 100644 --- a/tests/dotnet/UnitTests/TestBaseClass.cs +++ b/tests/dotnet/UnitTests/TestBaseClass.cs @@ -422,23 +422,33 @@ public static void AssertErrorCount (IList errors, int count, str Assert.Fail ($"Expected {count} errors, got {errors.Count} errors: {message}.\n\t{string.Join ("\n\t", errors.Select (v => v.Message?.TrimEnd ()))}"); } - public static void AssertErrorMessages (IList errors, params string [] errorMessages) + public static void AssertWarningMessages (IList actualWarnings, params string [] expectedWarningMessages) { - if (errors.Count != errorMessages.Length) { - Assert.Fail ($"Expected {errorMessages.Length} errors, got {errors.Count} errors:\n\t{string.Join ("\n\t", errors.Select (v => v.Message?.TrimEnd ()))}"); + AssertBuildMessages ("warning", actualWarnings, expectedWarningMessages); + } + + public static void AssertErrorMessages (IList actualErrors, params string [] expectedErrorMessages) + { + AssertBuildMessages ("error", actualErrors, expectedErrorMessages); + } + + public static void AssertBuildMessages (string type, IList actualMessages, params string [] expectedMessages) + { + if (actualMessages.Count != expectedMessages.Length) { + Assert.Fail ($"Expected {expectedMessages.Length} {type}s, got {actualMessages.Count} {type}s:\n\t{string.Join ("\n\t", actualMessages.Select (v => v.Message?.TrimEnd ()))}"); return; } var failures = new List (); - for (var i = 0; i < errorMessages.Length; i++) { - if (errors [i].Message != errorMessages [i]) { - failures.Add ($"\tUnexpected error message #{i}:\n\t\tExpected: {errorMessages [i]}\n\t\tActual: {errors [i].Message?.TrimEnd ()}"); + for (var i = 0; i < expectedMessages.Length; i++) { + if (actualMessages [i].Message != expectedMessages [i]) { + failures.Add ($"\tUnexpected {type} message #{i}:\n\t\tExpected: {expectedMessages [i]}\n\t\tActual: {actualMessages [i].Message?.TrimEnd ()}"); } } if (!failures.Any ()) return; - Assert.Fail ($"Failure when comparing error messages:\n{string.Join ("\n", failures)}\n\tAll errors:\n\t\t{string.Join ("\n\t\t", errors.Select (v => v.Message?.TrimEnd ()))}"); + Assert.Fail ($"Failure when comparing {type} messages:\n{string.Join ("\n", failures)}\n\tAll {type}s:\n\t\t{string.Join ("\n\t\t", actualMessages.Select (v => v.Message?.TrimEnd ()))}"); } } }