Use .NET SDK to generate deps.json for tools #6356

dsplaisted · 2017-04-19T14:55:46Z

This PR calls into a project in the .NET SDK to create deps.json files for .NET CLI tools instead of doing so directly. This reduces logic duplication and will fix #6087.

The corresponding SDK PR is dotnet/sdk#1128. Once both PRs have gone through review, we'll merge the SDK PR, and then update this CLI PR to also insert the updated SDK. This PR doesn't yet include tests, but when it does they will fail in CI until the SDK is updated.

dsplaisted · 2017-04-25T18:41:58Z

@eerhardt @nguerrera @livarcocc I think this is ready for review now. The corresponding SDK PR is dotnet/sdk#1128, which I still have a bit of work (tests, code review feedback) to do on.

@natemcmaster FYI

TestAssets/TestPackages/dotnet-portable-v1/dotnet-portable-v1.csproj

@@ -0,0 +1,18 @@
+<Project Sdk="Microsoft.NET.Sdk" ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">


src/Microsoft.DotNet.Cli.Utils/CommandResolution/IPackagedCommandSpecFactory.cs

-            CommandResolutionStrategy commandResolutionStrategy,
-            string depsFilePath,
-            string runtimeConfigPath);
+        //  Code review TODO: Is it OK to make breaking changes to the CLI Utils API surface?


src/Microsoft.DotNet.Cli.Utils/CommandResolution/PackagedCommandSpecFactoryWithCliRuntime.cs

+                else
+                {
+                    arguments.Add("--fx-version");
+                    arguments.Add(new Muxer().SharedFxVersion);


src/Microsoft.DotNet.Cli.Utils/CommandResolution/ProjectToolsCommandResolver.cs

+            var toolTargetFramework = toolLockFile.Targets.First().TargetFramework.GetShortFolderName();
+            args.Add($"/p:TargetFramework={toolTargetFramework}");
+
+


src/Microsoft.DotNet.Cli.Utils/CommandResolution/ProjectToolsCommandResolver.cs

+
+            if (result != 0)
+            {
+                //  TODO: Can / should we show the MSBuild output if there is a failure?


src/Microsoft.DotNet.Cli.Utils/ForwardingAppImplementation.cs

+    /// </summary>
+    internal class ForwardingAppImplementation
+    {
+        private const string s_hostExe = "dotnet";


src/Microsoft.DotNet.Cli.Utils/ForwardingAppImplementation.cs

+        private readonly IEnumerable<string> _argsToForward;
+        private readonly string _depsFile;
+        private readonly string _runtimeConfig;
+        private readonly string _additionalProbingPath;


src/dotnet/ForwardingApp.cs

-    /// <summary>
-    /// A class which encapsulates logic needed to forward arguments from the current process to another process
-    /// invoked with the dotnet.exe host.
-    /// </summary>
    public class ForwardingApp


src/dotnet/commands/dotnet-msbuild/MSBuildForwardingApp.cs

@@ -10,39 +10,25 @@
 using Microsoft.DotNet.Cli;
 using Microsoft.DotNet.Cli.CommandLine;
 using System.Diagnostics;
+using Microsoft.DotNet.Cli.Utils;

 namespace Microsoft.DotNet.Tools.MSBuild
 {
    public class MSBuildForwardingApp


TestAssets/TestPackages/dotnet-portable/Program.cs

+
+            var coreAssembly = typeof(object).GetTypeInfo().Assembly;
+            string coreFolder = Path.GetDirectoryName(coreAssembly.Location);
+            string frameworkVersion = Path.GetFileName(coreFolder);


eerhardt · 2017-04-26T16:56:05Z

using Microsoft.DotNet.Tools.Common;

Copyright. on all new files.

Refers to: src/Microsoft.DotNet.Cli.Utils/Extensions/LockFileExtensions.cs:1 in 6139036. [](commit_id = 6139036, deletion_comment = False)

src/Microsoft.DotNet.Cli.Utils/Extensions/LockFileExtensions.cs

+
+namespace Microsoft.DotNet.Cli.Utils
+{
+    static class LockFileExtensions


test/Microsoft.DotNet.Cli.Utils.Tests/GivenAProjectToolsCommandResolver.cs

+        {
+            //  When using the product, the ToolDepsJsonGeneratorProject property is used to get this path, but for testing
+            //  we'll hard code the path inside the SDK since we don't have a project to evaluate here
+            return Path.Combine(new RepoDirectoriesProvider().Stage2Sdk, @"Sdks\Microsoft.NET.Sdk\build\GenerateDeps\GenerateDeps.proj");


test/dotnet.Tests/PackagedCommandTests.cs

+
+            new GenericCommand(toolPrefersCLIRuntime ? "portable-v1-prefercli" : "portable-v1")
+                .WithWorkingDirectory(testInstance.Root)
+                .WithEnvironmentVariable("DOTNET_MULTILEVEL_LOOKUP", "0")


test/dotnet.Tests/PackagedCommandTests.cs

+                .Execute(toolPrefersCLIRuntime ? "portable-v1-prefercli" : "portable-v1");
+
+            result.Should().Pass()
+                .And.HaveStdOutContaining("I'm running on shared framework version 1.1.1!");    


src/Microsoft.DotNet.Cli.Utils/Extensions/LockFileExtensions.cs

+            var packageFolders = lockFile.GetNormalizedPackageFolders();
+
+            var packageFoldersCount = packageFolders.Count();
+            var userPackageFolder = packageFoldersCount == 1 ? string.Empty : packageFolders.First();


src/Microsoft.DotNet.Cli.Utils/CommandResolution/PackagedCommandSpecFactoryWithCliRuntime.cs

@@ -21,9 +21,41 @@ private static void AddAditionalParameters(string commandPath, IList<string> arg
        {
            if(PrefersCliRuntime(commandPath))
            {
-                arguments.Add("--fx-version");
-                arguments.Add(new Muxer().SharedFxVersion);
+                var runtimeConfigFile = Path.ChangeExtension(commandPath, FileNameSuffixes.RuntimeConfigJson);


TestAssets/TestPackages/dotnet-portable/Program.cs

+
+        public static string GetDataFromAppDomain(string propertyName)
+        {
+            var appDomainType = typeof(object).GetTypeInfo().Assembly?.GetType("System.AppDomain");


…tead of doing so directly

…mework from test assets

…ET Core

… shared framework

For test assets, don't explicitly include the runtimeconfig file, as the pack command does it automatically

livarcocc · 2017-04-27T17:52:59Z

@dotnet-bot Test this please

…ools-deps

…ore 1.0 shared framework

dsplaisted · 2017-04-27T23:30:13Z

@dotnet-bot test Linux x64 Release Build

dsplaisted · 2017-04-27T23:30:41Z

@dotnet-bot test Linux x64 Release

dsplaisted · 2017-04-27T23:31:18Z

@dotnet-bot test Ubuntu x64 Release Build

livarcocc · 2017-04-27T23:42:31Z

@dotnet-bot test Linux x64 Release Build

livarcocc · 2017-04-27T23:43:01Z

@dotnet-bot Test Linux x64 Release Build

dsplaisted requested review from nguerrera, livarcocc and eerhardt April 19, 2017 14:55

dnfclas added the cla-already-signed label Apr 19, 2017

dsplaisted mentioned this pull request Apr 19, 2017

Add SDK support for generating deps.json files for .NET CLI Tools dotnet/sdk#1128

Merged

dsplaisted force-pushed the generate-tools-deps branch 2 times, most recently from 2608da3 to 878037a Compare April 22, 2017 01:58

dsplaisted changed the base branch from master to release/2.0.0 April 25, 2017 18:38

dsplaisted force-pushed the generate-tools-deps branch from b9c3f93 to 6139036 Compare April 25, 2017 18:44

natemcmaster mentioned this pull request Apr 25, 2017

Re-enable DotNetCliToolReference in samples for netcoreapp2.0 aspnet/Universe#506

Closed

livarcocc reviewed Apr 25, 2017

View reviewed changes

TestAssets/TestPackages/dotnet-portable-v1/dotnet-portable-v1.csproj

@@ -0,0 +1,18 @@

<Project Sdk="Microsoft.NET.Sdk" ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">

This comment was marked as spam.

Sign in to view

livarcocc reviewed Apr 25, 2017

View reviewed changes