Merge pull request #2820 from jonsequitur/fix-2812

Improve RootCommand executable name handling in NAOT app and library scenarios

Author: baronfel

src/System.CommandLine.Tests/CompilationTests.cs

@@ -6,6 +6,8 @@
 using System.CommandLine.Suggest;
 using System.CommandLine.Tests.Utility;
 using System.IO;
+using System.Linq;
+using System.Runtime.InteropServices;
 using System.Text;
 using FluentAssertions;
 using Microsoft.DotNet.PlatformAbstractions;
@@ -42,6 +44,119 @@ public void App_referencing_system_commandline_can_be_compiled_ahead_of_time()
         }
     }
 
+    [ReleaseBuildOnlyFact]
+    public void RootCommand_name_falls_back_to_the_AppContext_value_when_hosted_as_a_native_library()
+    {
+        // When System.CommandLine is hosted inside a NativeAOT shared library there is no
+        // managed entry point (Assembly.GetEntryAssembly() returns null) and
+        // Environment.GetCommandLineArgs() reflects the host process. RootCommand must then
+        // fall back to the executable name injected into AppContext by the build targets (the
+        // assembly name). This exercises that path end-to-end through a real native library build.
+        var workingDirectory = Path.Combine(Directory.GetCurrentDirectory(), "TestApps", "NativeLibrary");
+        var publishDirectory = Path.Combine(Path.GetTempPath(), "scl-nativelib-" + Guid.NewGuid().ToString("N"));
+        var targetsPath = Path.Combine(Directory.GetCurrentDirectory(), "TestApps", "System.CommandLine.targets");
+        string rId = GetPortableRuntimeIdentifier();
+
+        Process.RunToCompletion(
+            DotnetMuxer.Path.FullName,
+            $"clean -c Release -r {rId} -p:SystemCommandLineTargetsPath=\"{targetsPath}\"",
+            workingDirectory: workingDirectory);
+
+        var stdOut = new StringBuilder();
+        var stdErr = new StringBuilder();
+
+        try
+        {
+            var exitCode = Process.RunToCompletion(
+                DotnetMuxer.Path.FullName,
+                string.Format(
+                    "publish -c Release -r {0} --self-contained -o \"{1}\" -p:SystemCommandLineDllPath=\"{2}\" -p:SystemCommandLineTargetsPath=\"{3}\" -p:TreatWarningsAsErrors=true",
+                    rId,
+                    publishDirectory,
+                    _systemCommandLineDllPath,
+                    targetsPath),
+                s =>
+                {
+                    _output.WriteLine(s);
+                    stdOut.Append(s);
+                },
+                s =>
+                {
+                    _output.WriteLine(s);
+                    stdErr.Append(s);
+                },
+                workingDirectory);
+
+            string publishOutput = $"{Environment.NewLine}STDOUT:{Environment.NewLine}{stdOut}{Environment.NewLine}STDERR:{Environment.NewLine}{stdErr}";
+
+            stdOut.ToString().Should().NotContain(": error CS", "the native library should compile cleanly. Publish output:{0}", publishOutput);
+            exitCode.Should().Be(0, "the native library should publish successfully. Publish output:{0}", publishOutput);
+
+            string nativeLibraryPath = Path.Combine(publishDirectory, NativeLibraryFileName("NativeLibrary"));
+
+            File.Exists(nativeLibraryPath).Should().BeTrue(
+                "the published native library should exist at {0}. Publish output:{1}", nativeLibraryPath, publishOutput);
+
+            string executableName = InvokeGetExecutableName(nativeLibraryPath);
+
+            // Equality to the assembly name proves the AppContext fallback was taken: had
+            // GetCommandLineArgs() been non-empty, the name would derive from the host path.
+            executableName.Should().Be("NativeLibrary");
+        }
+        finally
+        {
+            try
+            {
+                if (Directory.Exists(publishDirectory))
+                {
+                    Directory.Delete(publishDirectory, recursive: true);
+                }
+            }
+            catch (UnauthorizedAccessException)
+            {
+                // On Windows the native library may still be locked after NativeLibrary.Free
+            }
+        }
+    }
+
+    private static string NativeLibraryFileName(string assemblyName) =>
+        OperatingSystem.IsWindows() ? $"{assemblyName}.dll"
+            : OperatingSystem.IsMacOS() ? $"{assemblyName}.dylib"
+            : $"{assemblyName}.so";
+
+    [UnmanagedFunctionPointer(CallingConvention.Cdecl)]
+    private delegate int GetExecutableNameDelegate(IntPtr buffer, int bufferLength);
+
+    private static string InvokeGetExecutableName(string nativeLibraryPath)
+    {
+        IntPtr handle = NativeLibrary.Load(nativeLibraryPath);
+
+        try
+        {
+            IntPtr export = NativeLibrary.GetExport(handle, "get_executable_name");
+            var getExecutableName = Marshal.GetDelegateForFunctionPointer<GetExecutableNameDelegate>(export);
+
+            int length = getExecutableName(IntPtr.Zero, 0);
+            byte[] buffer = new byte[length];
+
+            GCHandle pinned = GCHandle.Alloc(buffer, GCHandleType.Pinned);
+            try
+            {
+                getExecutableName(pinned.AddrOfPinnedObject(), length);
+            }
+            finally
+            {
+                pinned.Free();
+            }
+
+            return Encoding.UTF8.GetString(buffer);
+        }
+        finally
+        {
+            NativeLibrary.Free(handle);
+        }
+    }
+
     private void PublishAndValidate(string appName, string warningText, string additionalArgs = null)
     {
         var stdOut = new StringBuilder();
@@ -86,7 +201,7 @@ private void PublishAndValidate(string appName, string warningText, string addit
     private static string GetPortableRuntimeIdentifier()
     {
         string osPart = OperatingSystem.IsWindows() ? "win" : (OperatingSystem.IsMacOS() ? "osx" : "linux");
-        return $"{osPart}-{RuntimeEnvironment.RuntimeArchitecture}";
+        return $"{osPart}-{Microsoft.DotNet.PlatformAbstractions.RuntimeEnvironment.RuntimeArchitecture}";
     }
 }
 

src/System.CommandLine.Tests/ParserTests.RootCommandAndArg0.cs

@@ -2,8 +2,11 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using System.CommandLine.Parsing;
+using System.IO;
 using System.Linq;
+using System.Runtime.InteropServices;
 using FluentAssertions;
+using FluentAssertions.Execution;
 using Xunit;
 
 namespace System.CommandLine.Tests;
@@ -15,18 +18,29 @@ public partial class RootCommandAndArg0
         [Fact]
         public void When_parsing_a_string_array_a_root_command_can_be_omitted_from_the_parsed_args()
         {
+            var option = new Option<string>("-x");
             var command = new Command("outer")
             {
                 new Command("inner")
                 {
-                    new Option<string>("-x")
+                    option
                 }
             };
 
             var result1 = command.Parse(Split("inner -x hello"));
             var result2 = command.Parse(Split("outer inner -x hello"));
 
+            using var _ = new AssertionScope();
+
             result1.Diagram().Should().Be(result2.Diagram());
+
+            foreach (var result in new[] { result1, result2 })
+            {
+                result.Errors.Should().BeEmpty();
+                result.RootCommandResult.Command.Name.Should().Be("outer");
+                result.CommandResult.Command.Name.Should().Be("inner");
+                result.GetValue(option).Should().Be("hello");
+            }
         }
 
         [Fact]
@@ -40,8 +54,6 @@ public void When_parsing_a_string_array_input_then_a_full_path_to_an_executable_
                 }
             };
 
-            command.Parse(Split("inner -x hello")).Errors.Should().BeEmpty();
-
             var parserResult = command.Parse(Split($"\"{RootCommand.ExecutablePath}\" inner -x hello"));
             parserResult
                .Errors
@@ -52,18 +64,29 @@ public void When_parsing_a_string_array_input_then_a_full_path_to_an_executable_
         [Fact]
         public void When_parsing_an_unsplit_string_a_root_command_can_be_omitted_from_the_parsed_args()
         {
+            var option = new Option<string>("-x");
             var command = new Command("outer")
             {
                 new Command("inner")
                 {
-                    new Option<string>("-x")
+                    option
                 }
             };
 
             var result1 = command.Parse("inner -x hello");
             var result2 = command.Parse("outer inner -x hello");
 
+            using var _ = new AssertionScope();
+
             result1.Diagram().Should().Be(result2.Diagram());
+
+            foreach (var result in new[] { result1, result2 })
+            {
+                result.Errors.Should().BeEmpty();
+                result.RootCommandResult.Command.Name.Should().Be("outer");
+                result.CommandResult.Command.Name.Should().Be("inner");
+                result.GetValue(option).Should().Be("hello");
+            }
         }
 
         [Fact]
@@ -82,25 +105,6 @@ public void When_parsing_an_unsplit_string_then_input_a_full_path_to_an_executab
             result2.RootCommandResult.IdentifierToken.Value.Should().Be(RootCommand.ExecutablePath);
         }
 
-        [Fact]
-        public void When_parsing_an_unsplit_string_then_a_renamed_RootCommand_can_be_omitted_from_the_parsed_args()
-        {
-            var rootCommand = new Command("outer")
-            {
-                new Command("inner")
-                {
-                    new Option<string>("-x")
-                }
-            };
-
-            var result1 = rootCommand.Parse("inner -x hello");
-            var result2 = rootCommand.Parse("outer inner -x hello");
-            var result3 = rootCommand.Parse($"{RootCommand.ExecutableName} inner -x hello");
-
-            result2.RootCommandResult.Command.Should().BeSameAs(result1.RootCommandResult.Command);
-            result3.RootCommandResult.Command.Should().BeSameAs(result1.RootCommandResult.Command);
-        }
-
         [Fact]
         public void When_parsing_a_string_array_option_values_containing_command_name_after_slash_are_not_treated_as_root_command()
         {
@@ -112,6 +116,8 @@ public void When_parsing_a_string_array_option_values_containing_command_name_af
 
             var result = command.Parse(Split("/p:Key=something/myapp"));
 
+            using var _ = new AssertionScope();
+
             result.Errors.Should().BeEmpty();
             result.GetResult(option).Should().NotBeNull();
         }
@@ -130,10 +136,101 @@ public void When_parsing_a_string_array_option_values_ending_with_slash_command_
 
             var result = command.Parse(Split(arg));
 
+            using var _ = new AssertionScope();
+
             result.Errors.Should().BeEmpty();
             result.GetResult(option).Should().NotBeNull();
         }
 
+        [Theory]
+        [MemberData(nameof(PathLikeFirstArgsMatchingTheRootCommand))]
+        public void When_parsing_a_string_array_a_path_like_first_arg_matching_the_root_command_is_treated_as_the_root_command(string pathLikeRootArg)
+        {
+            var option = new Option<string>("-x");
+            var command = new Command("outer")
+            {
+                new Command("inner")
+                {
+                    option
+                }
+            };
+
+            var result = command.Parse([pathLikeRootArg, "inner", "-x", "hello"]);
+
+            using var _ = new AssertionScope();
+
+            result.Errors.Should().BeEmpty();
+            result.RootCommandResult.IdentifierToken.Value.Should().Be(pathLikeRootArg);
+            result.CommandResult.Command.Name.Should().Be("inner");
+            result.GetValue(option).Should().Be("hello");
+        }
+
+        public static TheoryData<string> PathLikeFirstArgsMatchingTheRootCommand
+        {
+            get
+            {
+                var data = new TheoryData<string>
+                {
+                    "./outer",
+                    "tools/outer"
+                };
+
+                if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+                {
+                    data.Add(@".\outer");
+                    data.Add(@"tools\outer");
+                }
+
+                return data;
+            }
+        }
+
+        [Fact]
+        public void RootCommand_matches_its_executable_name_as_the_first_arg_the_same_way_a_named_Command_matches_its_name()
+        {
+            // Guards the assumption that a named Command can stand in for a RootCommand
+            // in arg0-matching tests: RootCommand.Name is forced to the executable name and
+            // cannot be changed, so the equivalent scenario uses the executable name as arg0.
+            var option = new Option<string>("-x");
+            var rootCommand = new RootCommand
+            {
+                new Command("inner")
+                {
+                    option
+                }
+            };
+
+            var result = rootCommand.Parse([RootCommand.ExecutableName, "inner", "-x", "hello"]);
+
+            using var _ = new AssertionScope();
+
+            result.Errors.Should().BeEmpty();
+            result.RootCommandResult.IdentifierToken.Value.Should().Be(RootCommand.ExecutableName);
+            result.CommandResult.Command.Name.Should().Be("inner");
+            result.GetValue(option).Should().Be("hello");
+        }
+
+#if NETFRAMEWORK
+        [Fact]
+        public void When_parsing_a_string_array_arguments_containing_invalid_path_characters_the_netframework_PathGetFileName_exception_is_ignored()
+        {
+            const string invalidPath = "my\0app";
+
+            // On .NET Framework, this throws ArgumentException for illegal characters in path.
+            Action getFileName = () => Path.GetFileName(invalidPath);
+
+            var argument = new Argument<string>("value");
+            var command = new Command("myapp") { argument };
+            var result = command.Parse(invalidPath);
+
+            using var _ = new AssertionScope();
+
+            getFileName.Should().Throw<ArgumentException>();
+            result.Errors.Should().BeEmpty();
+            result.GetValue(argument).Should().Be(invalidPath);
+        }
+#endif
+
         string[] Split(string value) => CommandLineParser.SplitCommandLine(value).ToArray();
     }
 }

src/System.CommandLine.Tests/RootCommandTests.cs

@@ -48,5 +48,17 @@ public void Setting_HelpName_does_not_change_Name()
 
             rootCommand.Name.Should().Be(RootCommand.ExecutableName);
         }
+
+        [Fact]
+        public void ExecutablePath_is_not_null()
+        {
+            RootCommand.ExecutablePath.Should().NotBeNull();
+        }
+
+        [Fact]
+        public void ExecutableName_is_not_null_or_empty()
+        {
+            RootCommand.ExecutableName.Should().NotBeNullOrEmpty();
+        }
     }
 }

src/System.CommandLine.Tests/System.CommandLine.Tests.csproj

@@ -10,6 +10,7 @@
   <ItemGroup>
     <Content Include="TestApps/**/*.csproj" CopyToOutputDirectory="PreserveNewest" />
     <Content Include="TestApps/**/*.cs" CopyToOutputDirectory="PreserveNewest" />
+    <Content Include="..\System.CommandLine\build\System.CommandLine.targets" CopyToOutputDirectory="PreserveNewest" Link="TestApps\System.CommandLine.targets" />
   </ItemGroup>
 
   <ItemGroup>