From 31cc3da31a0135d304a211d4ab5ea3169a876ad4 Mon Sep 17 00:00:00 2001 From: Daniel Plaisted Date: Fri, 26 May 2017 15:59:32 -0700 Subject: [PATCH 1/5] Improve error message when trying to build for a target framework that wasn't restored --- .../Microsoft.NET.Build.Tasks/LockFileExtensions.cs | 8 +++++++- .../Resources/Strings.Designer.cs | 11 ++++++++++- .../Microsoft.NET.Build.Tasks/Resources/Strings.resx | 5 ++++- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/LockFileExtensions.cs b/src/Tasks/Microsoft.NET.Build.Tasks/LockFileExtensions.cs index 6e0fe73d4629..50bc2c605bb5 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks/LockFileExtensions.cs +++ b/src/Tasks/Microsoft.NET.Build.Tasks/LockFileExtensions.cs @@ -37,7 +37,13 @@ public static ProjectContext CreateProjectContext( frameworkString : $"{frameworkString}/{runtime}"; - throw new BuildErrorException(Strings.AssetsFileMissingTarget, lockFile.Path, targetMoniker, framework.GetShortFolderName(), runtime); + string message = string.Format(Strings.AssetsFileMissingTarget, lockFile.Path, targetMoniker, framework.GetShortFolderName()); + if (!string.IsNullOrEmpty(runtime)) + { + message += " " + string.Format(Strings.AssetsFileMissingRuntimeIdentifier, runtime); + } + + throw new BuildErrorException(message); } LockFileTargetLibrary platformLibrary = lockFileTarget.GetLibrary(platformLibraryName); diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/Resources/Strings.Designer.cs b/src/Tasks/Microsoft.NET.Build.Tasks/Resources/Strings.Designer.cs index 4505fe20ea87..d982010bfe0e 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks/Resources/Strings.Designer.cs +++ b/src/Tasks/Microsoft.NET.Build.Tasks/Resources/Strings.Designer.cs @@ -80,7 +80,16 @@ internal static string AssetPreprocessorMustBeConfigured { } /// - /// Looks up a localized string similar to Assets file '{0}' doesn't have a target for '{1}'. Ensure you have restored this project for TargetFramework='{2}' and RuntimeIdentifier='{3}'.. + /// Looks up a localized string similar to You may also need to include '{0}' in your project's RuntimeIdentifiers.. + /// + internal static string AssetsFileMissingRuntimeIdentifier { + get { + return ResourceManager.GetString("AssetsFileMissingRuntimeIdentifier", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Assets file '{0}' doesn't have a target for '{1}'. Ensure you have included '{2}' in the TargetFrameworks for your project.. /// internal static string AssetsFileMissingTarget { get { diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/Resources/Strings.resx b/src/Tasks/Microsoft.NET.Build.Tasks/Resources/Strings.resx index b25f732d35c5..524dc827b3c3 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks/Resources/Strings.resx +++ b/src/Tasks/Microsoft.NET.Build.Tasks/Resources/Strings.resx @@ -130,7 +130,7 @@ Assets file '{0}' not found. Run a NuGet package restore to generate this file. - Assets file '{0}' doesn't have a target for '{1}'. Ensure you have restored this project for TargetFramework='{2}' and RuntimeIdentifier='{3}'. + Assets file '{0}' doesn't have a target for '{1}'. Ensure you have included '{2}' in the TargetFrameworks for your project. Assets file path '{0}' is not rooted. Only full paths are supported. @@ -297,4 +297,7 @@ The current .NET SDK does not support targeting {0} {1}. Either target {0} {2} or lower, or use a version of the .NET SDK that supports {0} {1}. + + You may also need to include '{0}' in your project's RuntimeIdentifiers. + \ No newline at end of file From 219db165d09d88785efbdfbda9e8f5ad788cc898 Mon Sep 17 00:00:00 2001 From: Daniel Plaisted Date: Fri, 26 May 2017 17:39:55 -0700 Subject: [PATCH 2/5] Check that assets file has been restored for the right framework and runtime before calling the compiler --- .../CheckForTargetInAssetsFile.cs | 32 ++++++++++++++++ .../LockFileExtensions.cs | 37 +++++++++++-------- ...rosoft.PackageDependencyResolution.targets | 9 +++++ 3 files changed, 63 insertions(+), 15 deletions(-) create mode 100644 src/Tasks/Microsoft.NET.Build.Tasks/CheckForTargetInAssetsFile.cs diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/CheckForTargetInAssetsFile.cs b/src/Tasks/Microsoft.NET.Build.Tasks/CheckForTargetInAssetsFile.cs new file mode 100644 index 000000000000..85d16c366b56 --- /dev/null +++ b/src/Tasks/Microsoft.NET.Build.Tasks/CheckForTargetInAssetsFile.cs @@ -0,0 +1,32 @@ +// 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. + +using Microsoft.Build.Framework; +using NuGet.ProjectModel; +using System; +using System.Collections.Generic; +using System.Text; + +namespace Microsoft.NET.Build.Tasks +{ + public class CheckForTargetInAssetsFile : TaskBase + { + [Required] + public string AssetsFilePath { get; set; } + + [Required] + public string TargetFrameworkMoniker { get; set; } + + public string RuntimeIdentifier { get; set; } + + + protected override void ExecuteCore() + { + LockFile lockFile = new LockFileCache(BuildEngine4).GetLockFile(AssetsFilePath); + + var nugetFramework = NuGetUtils.ParseFrameworkName(TargetFrameworkMoniker); + + lockFile.GetTargetAndThrowIfNotFound(nugetFramework, RuntimeIdentifier); + } + } +} diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/LockFileExtensions.cs b/src/Tasks/Microsoft.NET.Build.Tasks/LockFileExtensions.cs index 50bc2c605bb5..912d6efc78db 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks/LockFileExtensions.cs +++ b/src/Tasks/Microsoft.NET.Build.Tasks/LockFileExtensions.cs @@ -12,22 +12,8 @@ namespace Microsoft.NET.Build.Tasks { internal static class LockFileExtensions { - public static ProjectContext CreateProjectContext( - this LockFile lockFile, - NuGetFramework framework, - string runtime, - string platformLibraryName, - bool isSelfContained) + public static LockFileTarget GetTargetAndThrowIfNotFound(this LockFile lockFile, NuGetFramework framework, string runtime) { - if (lockFile == null) - { - throw new ArgumentNullException(nameof(lockFile)); - } - if (framework == null) - { - throw new ArgumentNullException(nameof(framework)); - } - LockFileTarget lockFileTarget = lockFile.GetTarget(framework, runtime); if (lockFileTarget == null) @@ -46,6 +32,27 @@ public static ProjectContext CreateProjectContext( throw new BuildErrorException(message); } + return lockFileTarget; + } + + public static ProjectContext CreateProjectContext( + this LockFile lockFile, + NuGetFramework framework, + string runtime, + string platformLibraryName, + bool isSelfContained) + { + if (lockFile == null) + { + throw new ArgumentNullException(nameof(lockFile)); + } + if (framework == null) + { + throw new ArgumentNullException(nameof(framework)); + } + + var lockFileTarget = lockFile.GetTargetAndThrowIfNotFound(framework, runtime); + LockFileTargetLibrary platformLibrary = lockFileTarget.GetLibrary(platformLibraryName); bool isFrameworkDependent = platformLibrary != null && (!isSelfContained || string.IsNullOrEmpty(lockFileTarget.RuntimeIdentifier)); diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.PackageDependencyResolution.targets b/src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.PackageDependencyResolution.targets index 8ac63aaad161..4879a87dcdc6 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.PackageDependencyResolution.targets +++ b/src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.PackageDependencyResolution.targets @@ -145,6 +145,8 @@ Copyright (c) .NET Foundation. All rights reserved. + + + + + Date: Fri, 26 May 2017 19:40:48 -0700 Subject: [PATCH 3/5] Don't check for RuntimeIdentifier in assets file before compile --- .../build/Microsoft.PackageDependencyResolution.targets | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.PackageDependencyResolution.targets b/src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.PackageDependencyResolution.targets index 4879a87dcdc6..8aab28211031 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.PackageDependencyResolution.targets +++ b/src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.PackageDependencyResolution.targets @@ -155,11 +155,16 @@ Copyright (c) .NET Foundation. All rights reserved. - + + RuntimeIdentifier="" /> Date: Tue, 30 May 2017 11:27:08 -0700 Subject: [PATCH 4/5] Avoid concatenating messages about target not found in assets file --- .../Microsoft.NET.Build.Tasks/LockFileExtensions.cs | 10 +++++++--- .../Microsoft.NET.Build.Tasks.csproj | 4 ++-- .../Resources/Strings.Designer.cs | 2 +- .../Microsoft.NET.Build.Tasks/Resources/Strings.resx | 2 +- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/LockFileExtensions.cs b/src/Tasks/Microsoft.NET.Build.Tasks/LockFileExtensions.cs index 912d6efc78db..0440b32a621b 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks/LockFileExtensions.cs +++ b/src/Tasks/Microsoft.NET.Build.Tasks/LockFileExtensions.cs @@ -23,10 +23,14 @@ public static LockFileTarget GetTargetAndThrowIfNotFound(this LockFile lockFile, frameworkString : $"{frameworkString}/{runtime}"; - string message = string.Format(Strings.AssetsFileMissingTarget, lockFile.Path, targetMoniker, framework.GetShortFolderName()); - if (!string.IsNullOrEmpty(runtime)) + string message; + if (string.IsNullOrEmpty(runtime)) { - message += " " + string.Format(Strings.AssetsFileMissingRuntimeIdentifier, runtime); + message = string.Format(Strings.AssetsFileMissingTarget, lockFile.Path, targetMoniker, framework.GetShortFolderName()); + } + else + { + message = string.Format(Strings.AssetsFileMissingRuntimeIdentifier, lockFile.Path, targetMoniker, framework.GetShortFolderName(), runtime); } throw new BuildErrorException(message); diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/Microsoft.NET.Build.Tasks.csproj b/src/Tasks/Microsoft.NET.Build.Tasks/Microsoft.NET.Build.Tasks.csproj index d98ebd7e7196..186589a72497 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks/Microsoft.NET.Build.Tasks.csproj +++ b/src/Tasks/Microsoft.NET.Build.Tasks/Microsoft.NET.Build.Tasks.csproj @@ -47,8 +47,8 @@ Strings.resx - true - true + True + True Strings.resx diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/Resources/Strings.Designer.cs b/src/Tasks/Microsoft.NET.Build.Tasks/Resources/Strings.Designer.cs index d982010bfe0e..0d6eff8eeb27 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks/Resources/Strings.Designer.cs +++ b/src/Tasks/Microsoft.NET.Build.Tasks/Resources/Strings.Designer.cs @@ -80,7 +80,7 @@ internal static string AssetPreprocessorMustBeConfigured { } /// - /// Looks up a localized string similar to You may also need to include '{0}' in your project's RuntimeIdentifiers.. + /// Looks up a localized string similar to Assets file '{0}' doesn't have a target for '{1}'. Ensure you have included '{2}' in the TargetFrameworks for your project. You may also need to include '{3}' in your project's RuntimeIdentifiers.. /// internal static string AssetsFileMissingRuntimeIdentifier { get { diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/Resources/Strings.resx b/src/Tasks/Microsoft.NET.Build.Tasks/Resources/Strings.resx index 524dc827b3c3..d411f1fc2400 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks/Resources/Strings.resx +++ b/src/Tasks/Microsoft.NET.Build.Tasks/Resources/Strings.resx @@ -298,6 +298,6 @@ The current .NET SDK does not support targeting {0} {1}. Either target {0} {2} or lower, or use a version of the .NET SDK that supports {0} {1}. - You may also need to include '{0}' in your project's RuntimeIdentifiers. + Assets file '{0}' doesn't have a target for '{1}'. Ensure you have included '{2}' in the TargetFrameworks for your project. You may also need to include '{3}' in your project's RuntimeIdentifiers. \ No newline at end of file From 635f421cb32071a51530dff71c8aea609b8413bd Mon Sep 17 00:00:00 2001 From: Daniel Plaisted Date: Tue, 30 May 2017 18:38:19 -0700 Subject: [PATCH 5/5] Check for RuntimeIdentifier in assets file up-front, and rewrite test impacted by this --- ...rosoft.PackageDependencyResolution.targets | 7 +-- .../GivenThatWeWantToBuildALibrary.cs | 50 +++++++++++++++---- 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.PackageDependencyResolution.targets b/src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.PackageDependencyResolution.targets index 8aab28211031..284f9daff61b 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.PackageDependencyResolution.targets +++ b/src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.PackageDependencyResolution.targets @@ -157,14 +157,11 @@ Copyright (c) .NET Foundation. All rights reserved. + compile errors. --> + RuntimeIdentifier="$(RuntimeIdentifier)" /> + { + // Set property to disable logic in Microsoft.NETCore.App package that will otherwise cause a failure + // when we remove everything under the rid-specific targets in the assets file + var ns = project.Root.Name.Namespace; + project.Root.Element(ns + "PropertyGroup") + .Add(new XElement(ns + "EnsureNETCoreAppRuntime", false)); + }) + .Restore(Log, testProject.Name); + + var buildCommand = new BuildCommand(Log, testAsset.TestRoot, testProject.Name); + + // Test that compilation doesn't depend on any rid-specific assets by removing them from the assets file after it's been restored + var assetsFilePath = Path.Combine(buildCommand.GetBaseIntermediateDirectory().FullName, "project.assets.json"); + + JObject assetsContents = JObject.Parse(File.ReadAllText(assetsFilePath)); + foreach (JProperty target in assetsContents["targets"]) + { + if (target.Name.Contains("/")) + { + // This is a target element with a RID specified, so remove all its contents + target.Value = new JObject(); + } + } + string newContents = assetsContents.ToString(); + File.WriteAllText(assetsFilePath, newContents); - // compile should still pass with unknown RID because references are always pulled - // from RIDLess target - var buildCommand = new MSBuildCommand(Log, "Compile", fullPathProjectFile); buildCommand - .Execute("/p:RuntimeIdentifier=unkownrid") + .Execute() .Should() .Pass(); }