Skip to content

Commit ec3d04b

Browse files
committed
[Xamarin.Android.Build.Tasks] Response file support for AOT
Our AOT system has issues when a user uses non ASCII characters and spaces on Windows. We use `GetShortPath` to get the old DOS 8.3 short names of paths to get around paths having spaces and unicode characters. However on the latest versions of windows `GetShortPath` seems to be supported only on the main system drive (C:). Many of our developers and CI will be building on other drives. So we really need to support paths with spaces and unicode. This commit reworks the way we provide the `--aot` pargument to the cross compilers so that it actually works in those senarios. The first thing was how the arguments were parsed. `mono` uses the build in system command line parser `GetCommandLineW` to parse arguments. Given the following `--aot` argument --aot=outfile="c:\Sandbox\repo\bin\BuildAotApplicationAndBundle AndÜmläüts_x86_True_True\obj\Release\libaot-Mono.Android.dll.so",asmwriter,mtriple=i686-linux-android,tool-prefix=C:\ProgramData\Microsoft\AndroidNDK\android-ndk-r13b\toolchains\x86-4.9\prebuilt\windows\bin\i686-linux-android-,ld-flags=C:\ProgramData\Microsoft\AndroidNDK\android-ndk-r13b\toolchains\x86-4.9\prebuilt\windows\lib\gcc\i686-linux-android\4.9.x\libgcc.a;C:\ProgramData\Microsoft\AndroidNDK\android-ndk-r13b\platforms\android-16\arch-x86\usr\lib\libc.so;C:\ProgramData\Microsoft\AndroidNDK\android-ndk-r13b\platforms\android-16\arch-x86\usr\lib\libm.so,temp-path="c:\Sandbox\repo\BuildAotApplicationAndBundle AndÜmläüts_x86_True_True\obj\Release\temp" This ends up as the following arguments --aot=outfile="c:\Sandbox\repo\bin\BuildAotApplicationAndBundle AndÜmläüts_x86_True_True\obj\Release\libaot-Mono.Android.dll.so" ,asmwriter,mtriple=i686-linux-android,tool-prefix=C:\ProgramData\Microsoft\AndroidNDK\android-ndk-r13b\toolchains\x86-4.9\prebuilt\windows\bin\i686-linux-android-,ld-flags=C:\ProgramData\Microsoft\AndroidNDK\android-ndk-r13b\toolchains\x86-4.9\prebuilt\windows\lib\gcc\i686-linux-android\4.9.x\libgcc.a;C:\ProgramData\Microsoft\AndroidNDK\android-ndk-r13b\platforms\android-16\arch-x86\usr\lib\libc.so;C:\ProgramData\Microsoft\AndroidNDK\android-ndk-r13b\platforms\android-16\arch-x86\usr\lib\libm.so,temp-path="c:\Sandbox\repo\BuildAotApplicationAndBundle AndÜmläüts_x86_True_True\obj\Release\temp" As you can see the parameters have been split where there is a space. The solution to this is to double quote the ENTIRE argument and remove any quotes within the parameter list like so "--aot=outfile=c:\Sandbox\repo\bin\BuildAotApplicationAndBundle AndÜmläüts_x86_True_True\obj\Release\libaot-Mono.Android.dll.so,asmwriter,mtriple=i686-linux-android,tool-prefix=C:\ProgramData\Microsoft\AndroidNDK\android-ndk-r13b\toolchains\x86-4.9\prebuilt\windows\bin\i686-linux-android-,ld-flags=C:\ProgramData\Microsoft\AndroidNDK\android-ndk-r13b\toolchains\x86-4.9\prebuilt\windows\lib\gcc\i686-linux-android\4.9.x\libgcc.a;C:\ProgramData\Microsoft\AndroidNDK\android-ndk-r13b\platforms\android-16\arch-x86\usr\lib\libc.so;C:\ProgramData\Microsoft\AndroidNDK\android-ndk-r13b\platforms\android-16\arch-x86\usr\lib\libm.so,temp-path=c:\Sandbox\repo\BuildAotApplicationAndBundle AndÜmläüts_x86_True_True\obj\Release\temp" This allows windows (and mac) to parse the paremters correctly as one block. There is another issue however. With the new argument line above if the `temp=` path has a space in it `mono` still has issues with that path. The good news is that we can use some domain knowledge to reduce not only the paths which need spaces but also the overall length of the argument. Because we know the cross compiler will be executed within `WorkingDirectory` we can shorten any path which is within that directory structure. `WorkingDirectory` is set to the directory of the projects csproj file. So the following E:\Some Project\My Project\obj\Release\aot\System.dll\ will become obj\Release\aot\System.dll\ This will fix the issue with the `temp=` argument. So we end up with something like this "--aot=outfile=obj\Release\libaot-Mono.Android.dll.so,asmwriter,mtriple=i686-linux-android,tool-prefix=C:\ProgramData\Microsoft\AndroidNDK\android-ndk-r13b\toolchains\x86-4.9\prebuilt\windows\bin\i686-linux-android-,ld-flags=C:\ProgramData\Microsoft\AndroidNDK\android-ndk-r13b\toolchains\x86-4.9\prebuilt\windows\lib\gcc\i686-linux-android\4.9.x\libgcc.a;C:\ProgramData\Microsoft\AndroidNDK\android-ndk-r13b\platforms\android-16\arch-x86\usr\lib\libc.so;C:\ProgramData\Microsoft\AndroidNDK\android-ndk-r13b\platforms\android-16\arch-x86\usr\lib\libm.so,temp-path=obj\Release\temp" However we might still start hitting the command line length limit on windows. This is currently 246 characters. So depending on where a user creates the project we might end up with a long command line. To work around this issue we can make use of the new `--response=` argument in `mono`. Instead of passing all the arguments to the cross compiler we can instead write them to a file and pass the path to that file as the `--response=` argument. This will reduce our command line length to be within an acceptable range unless the user creates a project in a very very deep directory structure. So the final call will be something like cross-arm --response="c:\Sandbox\repo\bin\BuildAotApplicationAndBundle AndÜmläüts_x86_True_True\obj\Release\Mono.Android.dll\response.txt" Which works perfectly. This commit also updates the `BuildAotApplication` and `BuildAotApplicationAndBundle` unit tests to use paths with both spaces and non-ascii characters to make sure we support both of those senarios.
1 parent 842dbfe commit ec3d04b

File tree

4 files changed

+51
-35
lines changed

4 files changed

+51
-35
lines changed

src/Xamarin.Android.Build.Tasks/Tasks/Aapt.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System.Diagnostics;
55
using System.IO;
66
using System.Linq;
7+
using System.Text;
78
using System.Threading;
89
using System.Xml;
910
using System.Xml.Linq;
@@ -113,6 +114,7 @@ bool RunAapt (string commandLine, IList<OutputLine> output)
113114
UseShellExecute = false,
114115
RedirectStandardOutput = true,
115116
RedirectStandardError = true,
117+
StandardOutputEncoding = Encoding.UTF8,
116118
CreateNoWindow = true,
117119
WindowStyle = ProcessWindowStyle.Hidden,
118120
WorkingDirectory = WorkingDirectory,

src/Xamarin.Android.Build.Tasks/Tasks/Aapt2.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System.Threading;
88
using System.Xml;
99
using System.Xml.Linq;
10+
using System.Text;
1011
using Microsoft.Build.Utilities;
1112
using Microsoft.Build.Framework;
1213
using System.Text.RegularExpressions;
@@ -53,6 +54,7 @@ protected bool RunAapt (string commandLine, IList<OutputLine> output)
5354
UseShellExecute = false,
5455
RedirectStandardOutput = true,
5556
RedirectStandardError = true,
57+
StandardOutputEncoding = Encoding.UTF8,
5658
CreateNoWindow = true,
5759
WindowStyle = ProcessWindowStyle.Hidden,
5860
WorkingDirectory = WorkingDirectory,

src/Xamarin.Android.Build.Tasks/Tasks/Aot.cs

Lines changed: 42 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.IO;
66
using System.Linq;
77
using System.Reflection;
8+
using System.Text;
89
using System.Threading;
910
using Microsoft.Build.Framework;
1011
using Microsoft.Build.Utilities;
@@ -173,14 +174,6 @@ static string GetNdkToolchainLibraryDir (string binDir, AndroidTargetArch arch)
173174
return GetNdkToolchainLibraryDir (binDir, NdkUtil.GetArchDirName (arch));
174175
}
175176

176-
static string GetShortPath (string path)
177-
{
178-
if (Environment.OSVersion.Platform != PlatformID.Win32NT)
179-
return QuoteFileName (path);
180-
var shortPath = KernelEx.GetShortPathName (Path.GetDirectoryName (path));
181-
return Path.Combine (shortPath, Path.GetFileName (path));
182-
}
183-
184177
static string QuoteFileName(string fileName)
185178
{
186179
var builder = new CommandLineBuilder();
@@ -282,12 +275,15 @@ bool RunParallelAotCompiler (List<string> nativeLibs)
282275
return;
283276
}
284277

285-
if (!RunAotCompiler (config.AssembliesPath, config.AotCompiler, config.AotOptions, config.AssemblyPath)) {
278+
if (!RunAotCompiler (config.AssembliesPath, config.AotCompiler, config.AotOptions, config.AssemblyPath, config.ResponseFile)) {
286279
LogCodedError ("XA3001", "Could not AOT the assembly: {0}", Path.GetFileName (config.AssemblyPath));
287280
Cancel ();
288281
return;
289282
}
290283

284+
if (File.Exists (config.ResponseFile))
285+
File.Delete (config.ResponseFile);
286+
291287
lock (nativeLibs)
292288
nativeLibs.Add (config.OutputFile);
293289
}
@@ -361,6 +357,11 @@ IEnumerable<Config> GetAotConfigs ()
361357
if (!Directory.Exists (outdir))
362358
Directory.CreateDirectory (outdir);
363359

360+
// dont use a full path if the outdir is withing the WorkingDirectory.
361+
if (outdir.StartsWith (WorkingDirectory, StringComparison.InvariantCultureIgnoreCase)) {
362+
outdir = outdir.Replace (WorkingDirectory + Path.DirectorySeparatorChar, string.Empty);
363+
}
364+
364365
int level = 0;
365366
string toolPrefix = EnableLLVM
366367
? NdkUtil.GetNdkToolPrefix (AndroidNdkDirectory, arch, level = GetNdkApiLevel (AndroidNdkDirectory, AndroidApiLevel, arch))
@@ -388,19 +389,19 @@ IEnumerable<Config> GetAotConfigs ()
388389

389390
var libs = new List<string>();
390391
if (NdkUtil.UsingClangNDK) {
391-
libs.Add ($"-L{GetShortPath (toolchainLibDir)}");
392-
libs.Add ($"-L{GetShortPath (androidLibPath)}");
392+
libs.Add ($"-L{toolchainLibDir}");
393+
libs.Add ($"-L{androidLibPath}");
393394

394395
if (arch == AndroidTargetArch.Arm) {
395396
// Needed for -lunwind to work
396397
string compilerLibDir = Path.Combine (toolchainPath, "..", "sysroot", "usr", "lib", NdkUtil.GetArchDirName (arch));
397-
libs.Add ($"-L{GetShortPath (compilerLibDir)}");
398+
libs.Add ($"-L{compilerLibDir}");
398399
}
399400
}
400401

401-
libs.Add (GetShortPath (Path.Combine (toolchainLibDir, "libgcc.a")));
402-
libs.Add (GetShortPath (Path.Combine (androidLibPath, "libc.so")));
403-
libs.Add (GetShortPath (Path.Combine (androidLibPath, "libm.so")));
402+
libs.Add (Path.Combine (toolchainLibDir, "libgcc.a"));
403+
libs.Add (Path.Combine (androidLibPath, "libc.so"));
404+
libs.Add (Path.Combine (androidLibPath, "libm.so"));
404405

405406
ldFlags = string.Join(";", libs);
406407
}
@@ -422,25 +423,28 @@ IEnumerable<Config> GetAotConfigs ()
422423
aotOptions.Add ("profile-only");
423424
foreach (var p in Profiles) {
424425
var fp = Path.GetFullPath (p.ItemSpec);
425-
aotOptions.Add ($"profile={GetShortPath (fp)}");
426+
aotOptions.Add ($"profile={fp}");
426427
}
427428
}
428429
if (!string.IsNullOrEmpty (AotAdditionalArguments))
429430
aotOptions.Add (AotAdditionalArguments);
430431
if (sequencePointsMode == SequencePointsMode.Offline)
431-
aotOptions.Add ("msym-dir=" + GetShortPath (outdir));
432+
aotOptions.Add ($"msym-dir={outdir}");
432433
if (AotMode != AotMode.Normal)
433434
aotOptions.Add (AotMode.ToString ().ToLowerInvariant ());
434435

435-
aotOptions.Add ("outfile=" + GetShortPath (outputFile));
436+
aotOptions.Add ($"outfile={outputFile}");
436437
aotOptions.Add ("asmwriter");
437-
aotOptions.Add ("mtriple=" + mtriple);
438-
aotOptions.Add ("tool-prefix=" + GetShortPath (toolPrefix));
439-
aotOptions.Add ("ld-flags=" + ldFlags);
440-
aotOptions.Add ("llvm-path=" + GetShortPath (sdkBinDirectory));
441-
aotOptions.Add ("temp-path=" + GetShortPath (tempDir));
438+
aotOptions.Add ($"mtriple={mtriple}");
439+
aotOptions.Add ($"tool-prefix={toolPrefix}");
440+
aotOptions.Add ($"ld-flags={ldFlags}");
441+
aotOptions.Add ($"llvm-path={sdkBinDirectory}");
442+
aotOptions.Add ($"temp-path={tempDir}");
442443

443-
string aotOptionsStr = (EnableLLVM ? "--llvm " : "") + "--aot=" + string.Join (",", aotOptions);
444+
// we need to quote the entire --aot arguments here to make sure it is parsed
445+
// on windows as one argument. Otherwise it will be split up into multiple
446+
// values, which wont work.
447+
string aotOptionsStr = (EnableLLVM ? "--llvm " : "") + $"\"--aot={string.Join (",", aotOptions)}\"";
444448

445449
if (!string.IsNullOrEmpty (ExtraAotOptions)) {
446450
aotOptionsStr += (aotOptions.Count > 0 ? "," : "") + ExtraAotOptions;
@@ -460,23 +464,29 @@ IEnumerable<Config> GetAotConfigs ()
460464
}
461465

462466
var assembliesPath = Path.GetFullPath (Path.GetDirectoryName (resolvedPath));
463-
var assemblyPath = QuoteFileName (Path.GetFullPath (resolvedPath));
467+
var assemblyPath = Path.GetFullPath (resolvedPath);
464468

465-
yield return new Config (assembliesPath, QuoteFileName (aotCompiler), aotOptionsStr, assemblyPath, outputFile);
469+
yield return new Config (assembliesPath, aotCompiler, aotOptionsStr, assemblyPath, outputFile, Path.Combine (tempDir, "response.txt"));
466470
}
467471
}
468472
}
469473

470-
bool RunAotCompiler (string assembliesPath, string aotCompiler, string aotOptions, string assembly)
474+
bool RunAotCompiler (string assembliesPath, string aotCompiler, string aotOptions, string assembly, string responseFile)
471475
{
472476
var stdout_completed = new ManualResetEvent (false);
473477
var stderr_completed = new ManualResetEvent (false);
478+
479+
using (var sw = new StreamWriter (responseFile, append: false, encoding: new UTF8Encoding (encoderShouldEmitUTF8Identifier: false))) {
480+
sw.WriteLine (aotOptions + " " + QuoteFileName (assembly));
481+
}
482+
474483
var psi = new ProcessStartInfo () {
475-
FileName = aotCompiler,
476-
Arguments = aotOptions + " " + assembly,
484+
FileName = QuoteFileName (aotCompiler),
485+
Arguments = $"--response={QuoteFileName (responseFile)}",
477486
UseShellExecute = false,
478487
RedirectStandardOutput = true,
479488
RedirectStandardError = true,
489+
StandardOutputEncoding = Encoding.UTF8,
480490
CreateNoWindow=true,
481491
WindowStyle=ProcessWindowStyle.Hidden,
482492
WorkingDirectory = WorkingDirectory,
@@ -536,16 +546,18 @@ struct Config {
536546
public string AotOptions { get; }
537547
public string AssemblyPath { get; }
538548
public string OutputFile { get; }
549+
public string ResponseFile { get; }
539550

540551
public bool Valid { get; private set; }
541552

542-
public Config (string assembliesPath, string aotCompiler, string aotOptions, string assemblyPath, string outputFile)
553+
public Config (string assembliesPath, string aotCompiler, string aotOptions, string assemblyPath, string outputFile, string responseFile)
543554
{
544555
AssembliesPath = assembliesPath;
545556
AotCompiler = aotCompiler;
546557
AotOptions = aotOptions;
547558
AssemblyPath = assemblyPath;
548559
OutputFile = outputFile;
560+
ResponseFile = responseFile;
549561
Valid = true;
550562
}
551563

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -732,9 +732,9 @@ public void BuildMkBundleApplicationReleaseAllAbi ()
732732
[Test]
733733
[TestCaseSource (nameof (AotChecks))]
734734
[Category ("Minor")]
735-
public void BuildAotApplication (string supportedAbis, bool enableLLVM, bool expectedResult)
735+
public void BuildAotApplicationAndÜmläüts (string supportedAbis, bool enableLLVM, bool expectedResult)
736736
{
737-
var path = Path.Combine ("temp", string.Format ("BuildAotApplication_{0}_{1}_{2}", supportedAbis, enableLLVM, expectedResult));
737+
var path = Path.Combine ("temp", string.Format ("BuildAotApplication AndÜmläüts_{0}_{1}_{2}", supportedAbis, enableLLVM, expectedResult));
738738
var proj = new XamarinAndroidApplicationProject () {
739739
IsRelease = true,
740740
BundleAssemblies = false,
@@ -768,7 +768,7 @@ public void BuildAotApplication (string supportedAbis, bool enableLLVM, bool exp
768768
// LLVM passes a direct path to libc.so, and we need to use the libc.so
769769
// which corresponds to the *minimum* SDK version specified in AndroidManifest.xml
770770
// Since we overrode minSdkVersion=16, that means we should use libc.so from android-16.
771-
var rightLibc = new Regex (@"^\s*\[AOT\].*cross-.*--llvm.*,ld-flags=.*android-16.arch-.*.usr.lib.libc\.so", RegexOptions.Multiline);
771+
var rightLibc = new Regex (@"\s*\[aot-compiler stdout].*android-16.arch-.*.usr.lib.libc\.so", RegexOptions.Multiline);
772772
var m = rightLibc.Match (string.Join ("\n",b.LastBuildOutput));
773773
Assert.IsTrue (m.Success, "AOT+LLVM should use libc.so from minSdkVersion!");
774774
}
@@ -803,9 +803,9 @@ public void BuildAotApplication (string supportedAbis, bool enableLLVM, bool exp
803803
[Test]
804804
[TestCaseSource (nameof (AotChecks))]
805805
[Category ("Minor")]
806-
public void BuildAotApplicationAndBundle (string supportedAbis, bool enableLLVM, bool expectedResult)
806+
public void BuildAotApplicationAndBundleAndÜmläüts (string supportedAbis, bool enableLLVM, bool expectedResult)
807807
{
808-
var path = Path.Combine ("temp", string.Format ("BuildAotApplicationAndBundle_{0}_{1}_{2}", supportedAbis, enableLLVM, expectedResult));
808+
var path = Path.Combine ("temp", string.Format ("BuildAotApplicationAndBundle AndÜmläüts_{0}_{1}_{2}", supportedAbis, enableLLVM, expectedResult));
809809
var proj = new XamarinAndroidApplicationProject () {
810810
IsRelease = true,
811811
BundleAssemblies = true,

0 commit comments

Comments
 (0)