Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] Special Char Support (#6273)
Browse files Browse the repository at this point in the history
What would we like?  The ability for Xamarin.Android projects to
reside in directories containing non-ASCII characters.

The problem(s)?  For starters, many Android SDK utilities don't
support non-ASCII characters when running on Windows, e.g.:

  * https://issuetracker.google.com/issues/188679588

Android Studio (kinda/sorta) "enforces" this, showing a warning
message when **Save location** contains non-ASCII characters:

![Android Studio New Project dialog with non-ASCII characters](https://user-images.githubusercontent.com/184788/134550831-b5c5335a-11ca-4296-b30c-cb54e3302917.png)

> Save location: /tmp/テスト
> …
> ⚠️ Your project location contains non-ASCII characters.

These are not particularly encouraging foundations.

Improve support for non-ASCII characters by updating various parts
of our build system to:

 1. Prefer *relatives* paths when possible, as we can ensure
    ASCII-only paths this way, and

 2. "Otherwise" change *how* we invoke Android tools to avoid their
    use of full paths and directory traversal.

Additionally, we've long used GNU Binutils to build `libxamarin-app.so`
(commit decfbcc, fc3f028, others), and we discovered that
Binutils 2.36 introduced [code][0] to deal extra long path names on
Windows which makes compilation (in our case using `as`) and linking
to break if the source/object files reside in a directory which has
non-ASCII characters (in our case they were `テスト`):

	Xamarin.Android.Common.targets(1887,3): error XA3006: Could not compile native assembly file: typemaps.armeabi-v7a.s [SmokeTestBuildWithSpecialCharactersFalse\テスト\テスト.csproj]
	  [aarch64-linux-android-as.EXE stderr] Assembler messages: (TaskId:187)
	  [aarch64-linux-android-as.EXE stderr] Fatal error: can't create typemaps.arm64-v8a.o: Invalid argument (TaskId:187)

Downgrade to Binutils 2.35.2, which doesn't contain the offending
code block and will work for us, so long as we use relative paths.
It will **still** break with full paths if the path contains special
characters, but since our use case involves only relative paths,
this is fine.

There is also a bug in [`aapt2`][1] which stops it processing a
directory which has certain special characters on Windows.  This will
only appear on projects which have library resources, such as
Xamarin.Forms.  The problem only appears when processing a directory
into a `.flata` archive.  Oddly it works fine when processing a file
directly.  In order to work around this we now generate a resource
only `res.zip` file when we extract the library resources.
`res.zip` is then used during the generation of the `.flata` archive.
Weirdly, this works where directory transversal does not.

Additionally it seems to be slightly quicker during incremental builds:

  * Before

        581 ms  Aapt2Compile                               1 calls

  * After

        366 ms  Aapt2Compile                               1 calls

So the time taken to generate the `res.zip` is offset by the quicker
execution of `aapt2 compile`.

Finally, Java tooling such as `zipalign` and `apksigner` also failed
with the special characters.  This was because we were using full
paths to the files we were signing/aligning. Switching over to use
relative paths fixes this particular problem.

Add a set of Unit tests to make sure we can handle the special characters.

[0]: https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=bfd/bfdio.c;h=463b3879c52ba6beac47190f8eb0810b0c330e65;hb=HEAD#l124
[1]: https://issuetracker.google.com/issues/188679588
  • Loading branch information
dellis1972 committed Oct 18, 2021
1 parent e244cbf commit fd5f31c
Show file tree
Hide file tree
Showing 11 changed files with 113 additions and 21 deletions.
Expand Up @@ -15,7 +15,7 @@ namespace Xamarin.Android.Prepare
//
partial class Configurables
{
const string BinutilsVersion = "2.37-XA.1";
const string BinutilsVersion = "2.35.2-XA.1";

const string MicrosoftOpenJDK11Version = "11.0.10";
const string MicrosoftOpenJDK11Release = "9.1";
Expand Down
25 changes: 18 additions & 7 deletions src/Xamarin.Android.Build.Tasks/Tasks/Aapt2Compile.cs
Expand Up @@ -15,7 +15,7 @@
using Microsoft.Android.Build.Tasks;

namespace Xamarin.Android.Tasks {

public class Aapt2Compile : Aapt2 {
public override string TaskPrefix => "A2C";

Expand All @@ -37,8 +37,8 @@ public class Aapt2Compile : Aapt2 {

protected override int GetRequiredDaemonInstances ()
{
return Math.Min ((ResourcesToCompile ?? ResourceDirectories).Length, DaemonMaxInstanceCount);
}
return Math.Min ((ResourcesToCompile ?? ResourceDirectories)?.Length ?? 1, DaemonMaxInstanceCount);
}

public async override System.Threading.Tasks.Task RunTaskAsync ()
{
Expand All @@ -56,6 +56,7 @@ public async override System.Threading.Tasks.Task RunTaskAsync ()
void ProcessDirectory (ITaskItem item, object lockObject)
{
var flatFile = item.GetMetadata ("_FlatFile");
bool isArchive = false;
bool isDirectory = flatFile.EndsWith (".flata", StringComparison.OrdinalIgnoreCase);
if (string.IsNullOrEmpty (flatFile)) {
FileAttributes fa = File.GetAttributes (item.ItemSpec);
Expand All @@ -65,9 +66,13 @@ void ProcessDirectory (ITaskItem item, object lockObject)
string fileOrDirectory = item.GetMetadata ("ResourceDirectory");
if (string.IsNullOrEmpty (fileOrDirectory) || !isDirectory)
fileOrDirectory = item.ItemSpec;
if (isDirectory && !Directory.Exists (fileOrDirectory)) {
LogWarning ($"Ignoring directory '{fileOrDirectory}' as it does not exist!");
return;
}
if (isDirectory && !Directory.EnumerateDirectories (fileOrDirectory).Any ())
return;

string outputArchive = isDirectory ? GetFullPath (FlatArchivesDirectory) : GetFullPath (FlatFilesDirectory);
string targetDir = item.GetMetadata ("_ArchiveDirectory");
if (!string.IsNullOrEmpty (targetDir)) {
Expand All @@ -83,14 +88,20 @@ void ProcessDirectory (ITaskItem item, object lockObject)
filename = $"{filename}.flata";
outputArchive = Path.Combine (outputArchive, filename);
expectedOutputFile = outputArchive;
string archive = item.GetMetadata (ResolveLibraryProjectImports.ResourceDirectoryArchive);
if (!string.IsNullOrEmpty (archive) && File.Exists (archive)) {
LogDebugMessage ($"Found Compressed Resource Archive '{archive}'.");
fileOrDirectory = archive;
isArchive = true;
}
} else {
if (IsInvalidFilename (fileOrDirectory)) {
LogDebugMessage ($"Invalid filename, ignoring: {fileOrDirectory}");
return;
}
expectedOutputFile = Path.Combine (outputArchive, flatFile);
}
RunAapt (GenerateCommandLineCommands (fileOrDirectory, isDirectory, outputArchive), expectedOutputFile);
RunAapt (GenerateCommandLineCommands (fileOrDirectory, isDirectory, isArchive, outputArchive), expectedOutputFile);
if (isDirectory) {
lock (lockObject)
archives.Add (new TaskItem (expectedOutputFile));
Expand All @@ -100,7 +111,7 @@ void ProcessDirectory (ITaskItem item, object lockObject)
}
}

protected string[] GenerateCommandLineCommands (string fileOrDirectory, bool isDirectory, string outputArchive)
protected string[] GenerateCommandLineCommands (string fileOrDirectory, bool isDirectory, bool isArchive, string outputArchive)
{
List<string> cmd = new List<string> ();
cmd.Add ("compile");
Expand All @@ -111,7 +122,7 @@ protected string[] GenerateCommandLineCommands (string fileOrDirectory, bool isD
cmd.Add (GetFullPath (ResourceSymbolsTextFile));
}
if (isDirectory) {
cmd.Add ("--dir");
cmd.Add (isArchive ? "--zip" : "--dir");
cmd.Add (GetFullPath (fileOrDirectory).TrimEnd ('\\'));
} else
cmd.Add (GetFullPath (fileOrDirectory));
Expand Down
2 changes: 1 addition & 1 deletion src/Xamarin.Android.Build.Tasks/Tasks/AndroidApkSigner.cs
Expand Up @@ -102,7 +102,7 @@ protected override string GenerateCommandLineCommands ()
if (!string.IsNullOrEmpty (AdditionalArguments))
cmd.AppendSwitch (AdditionalArguments);

cmd.AppendSwitchIfNotNull (" ", Path.GetFullPath (ApkToSign));
cmd.AppendSwitchIfNotNull (" ", ApkToSign);

return cmd.ToString ();
}
Expand Down
Expand Up @@ -107,6 +107,7 @@ public override bool RunTask ()
}
var fileTaskItem = new TaskItem (file, new Dictionary<string, string> () {
{ "ResourceDirectory", directory.ItemSpec },
{ ResolveLibraryProjectImports.ResourceDirectoryArchive, directory.GetMetadata (ResolveLibraryProjectImports.ResourceDirectoryArchive) },
{ "StampFile", generateArchive ? stampFile : file },
{ "FilesCache", filesCache},
{ "Hash", stampFile },
Expand Down
21 changes: 15 additions & 6 deletions src/Xamarin.Android.Build.Tasks/Tasks/JavaToolTask.cs
Expand Up @@ -14,10 +14,10 @@ namespace Xamarin.Android.Tasks
public abstract class JavaToolTask : AndroidToolTask
{
/*
Example Javac output for errors. Regex Matches on the first line, we then need to
Example Javac output for errors. Regex Matches on the first line, we then need to
process the second line to get the column number so the IDE can correctly
mark where the error is.
mark where the error is.
TestMe.java:1: error: class, interface, or enum expected
public classo TestMe { }
^
Expand All @@ -28,7 +28,7 @@ 2 errors
*/
const string CodeErrorRegExString = @"(?<file>.+\.java):(?<line>\d+):(?<error>.+)";
/*
Sample OutOfMemoryError raised by java. RegEx matches the java.lang.* line of the error
and splits it into an exception and an error
Expand Down Expand Up @@ -80,6 +80,8 @@ at com.android.dx.command.Main.main(Main.java:106)

public virtual string DefaultErrorCode => null;

public string WorkingDirectory { get; set; }

protected override string ToolName {
get { return OS.IsWindows ? "java.exe" : "java"; }
}
Expand All @@ -100,6 +102,13 @@ protected override bool HandleTaskExecutionErrors ()
return base.HandleTaskExecutionErrors ();
}

protected override string GetWorkingDirectory ()
{
if (!string.IsNullOrEmpty (WorkingDirectory))
return WorkingDirectory;
return base.GetWorkingDirectory ();
}

protected override string GenerateFullPathToTool ()
{
return Path.Combine (ToolPath, ToolExe);
Expand Down Expand Up @@ -155,7 +164,7 @@ bool ProcessOutput (string singleLine)
return true;
}
errorText.AppendLine (singleLine);
}
}
return true;
}

Expand All @@ -173,7 +182,7 @@ protected override void LogEventsFromTextOutput (string singleLine, MessageImpor
Log.LogMessage (MessageImportance.High, singleLine);
errorLines.Add (singleLine);
return;
}
}
base.LogEventsFromTextOutput (singleLine, messageImportance);
}
}
Expand Down
Expand Up @@ -61,9 +61,12 @@ public class ResolveLibraryProjectImports : AndroidTask

internal const string OriginalFile = "OriginalFile";
internal const string AndroidSkipResourceProcessing = "AndroidSkipResourceProcessing";

internal const string ResourceDirectoryArchive = "ResourceDirectoryArchive";
static readonly string [] knownMetadata = new [] {
OriginalFile,
AndroidSkipResourceProcessing
AndroidSkipResourceProcessing,
ResourceDirectoryArchive
};

AssemblyIdentityMap assemblyMap = new AssemblyIdentityMap();
Expand Down Expand Up @@ -177,6 +180,7 @@ public override bool RunTask ()
string importsDir = Path.Combine (outDirForDll, ImportsDirectory);
string nativeimportsDir = Path.Combine (outDirForDll, NativeImportsDirectory);
string resDir = Path.Combine (importsDir, "res");
string resDirArchive = Path.Combine (resDir, "..", "res.zip");
string assetsDir = Path.Combine (importsDir, "assets");

// Skip already-extracted resources.
Expand All @@ -194,6 +198,7 @@ public override bool RunTask ()
if (Directory.Exists (resDir)) {
var taskItem = new TaskItem (Path.GetFullPath (resDir), new Dictionary<string, string> {
{ OriginalFile, assemblyPath },
{ ResourceDirectoryArchive, Path.GetFullPath (resDirArchive)},
});
if (bool.TryParse (assemblyItem.GetMetadata (AndroidSkipResourceProcessing), out skip) && skip)
taskItem.SetMetadata (AndroidSkipResourceProcessing, "True");
Expand Down Expand Up @@ -290,8 +295,11 @@ public override bool RunTask ()
// which resulted in missing resource issue.
// Here we replaced copy with use of '-S' option and made it to work.
if (Directory.Exists (resDir)) {
CreateResourceArchive (resDir, resDirArchive);
var taskItem = new TaskItem (Path.GetFullPath (resDir), new Dictionary<string, string> {
{ OriginalFile, assemblyPath }
{ OriginalFile, assemblyPath },
{ ResourceDirectoryArchive, Path.GetFullPath (resDirArchive)},

});
if (bool.TryParse (assemblyItem.GetMetadata (AndroidSkipResourceProcessing), out skip) && skip)
taskItem.SetMetadata (AndroidSkipResourceProcessing, "True");
Expand Down Expand Up @@ -335,6 +343,7 @@ public override bool RunTask ()
string outDirForDll = Path.Combine (OutputImportDirectory, aarIdentityName);
string importsDir = Path.Combine (outDirForDll, ImportsDirectory);
string resDir = Path.Combine (importsDir, "res");
string resDirArchive = Path.Combine (resDir, "..", "res.zip");
string assetsDir = Path.Combine (importsDir, "assets");

bool updated = false;
Expand All @@ -357,6 +366,7 @@ public override bool RunTask ()
resolvedResourceDirectories.Add (new TaskItem (Path.GetFullPath (resDir), new Dictionary<string, string> {
{ OriginalFile, Path.GetFullPath (aarFile.ItemSpec) },
{ AndroidSkipResourceProcessing, skipProcessing },
{ ResourceDirectoryArchive, Path.GetFullPath (resDirArchive)},
}));
}
if (Directory.Exists (assetsDir))
Expand Down Expand Up @@ -406,13 +416,15 @@ public override bool RunTask ()
}
}
if (Directory.Exists (resDir)) {
CreateResourceArchive (resDir, resDirArchive);
var skipProcessing = aarFile.GetMetadata (AndroidSkipResourceProcessing);
if (string.IsNullOrEmpty (skipProcessing)) {
skipProcessing = "True";
}
resolvedResourceDirectories.Add (new TaskItem (Path.GetFullPath (resDir), new Dictionary<string, string> {
{ OriginalFile, aarFullPath },
{ AndroidSkipResourceProcessing, skipProcessing },
{ ResourceDirectoryArchive, Path.GetFullPath (resDirArchive)},
}));
}
if (Directory.Exists (assetsDir))
Expand All @@ -422,6 +434,16 @@ public override bool RunTask ()
}
}

void CreateResourceArchive (string resDir, string outputFile)
{
var fileMode = File.Exists (outputFile) ? FileMode.Open : FileMode.CreateNew;
Files.ArchiveZipUpdate (outputFile, f => {
using (var zip = new ZipArchiveEx (f, fileMode)) {
zip.AddDirectory (resDir, "res");
}
});
}

static void AddJar (IDictionary<string, ITaskItem> jars, string destination, string path, string originalFile = null)
{
var fullPath = Path.GetFullPath (Path.Combine (destination, path));
Expand Down
Expand Up @@ -830,6 +830,7 @@ public class Foo {
rawToDelete,
},
};
libProj.SetProperty ("Deterministic", "true");
var appProj = new XamarinAndroidApplicationProject () {
IsRelease = true,
ProjectName = "App1",
Expand Down
Expand Up @@ -23,6 +23,27 @@ namespace Xamarin.Android.Build.Tests
[Parallelizable (ParallelScope.Children)]
public partial class BuildTest : BaseTest
{
[Test]
[Category ("SmokeTests")]
public void SmokeTestBuildWithSpecialCharacters ([Values (false, true)] bool forms)
{
var testName = "テスト";

var rootPath = Path.Combine (Root, "temp", TestName);
var proj = forms ?
new XamarinFormsAndroidApplicationProject () :
new XamarinAndroidApplicationProject ();
proj.ProjectName = testName;
proj.IsRelease = true;
if (forms) {
proj.PackageReferences.Clear ();
proj.PackageReferences.Add (KnownPackages.XamarinForms_4_7_0_1142);
}
using (var builder = CreateApkBuilder (Path.Combine (rootPath, proj.ProjectName))){
Assert.IsTrue (builder.Build (proj), "Build should have succeeded.");
}
}

public static string GetLinkedPath (ProjectBuilder builder, bool isRelease, string filename)
{
return Builder.UseDotNet && isRelease ?
Expand Down
Expand Up @@ -2237,7 +2237,7 @@ because xbuild doesn't support framework reference assemblies.
<_JarSignerSuffix Condition=" '$(AndroidPackageFormat)' == 'aab' ">-Signed</_JarSignerSuffix>
</PropertyGroup>
<AndroidSignPackage Condition=" '$(AndroidUseApkSigner)' != 'true' "
UnsignedApk="%(ApkAbiFilesIntermediate.FullPath)"
UnsignedApk="%(ApkAbiFilesIntermediate.Identity)"
SignedApkDirectory="$(OutDir)"
FileSuffix="$(_JarSignerSuffix)"
KeyStore="$(_ApkKeyStore)"
Expand All @@ -2257,7 +2257,7 @@ because xbuild doesn't support framework reference assemblies.
</ItemGroup>
<Delete Files="%(ApkAbiFilesSigned.FullPath)" Condition=" '$(AndroidUseApkSigner)' == 'true' "/>
<AndroidZipAlign Condition=" '$(AndroidUseApkSigner)' == 'true' "
Source="%(ApkAbiFilesIntermediate.FullPath)"
Source="%(ApkAbiFilesIntermediate.Identity)"
DestinationDirectory="$(OutDir)"
ToolPath="$(ZipAlignToolPath)"
ToolExe="$(ZipalignToolExe)"
Expand All @@ -2268,7 +2268,7 @@ because xbuild doesn't support framework reference assemblies.
</ItemGroup>
<AndroidApkSigner Condition=" '$(AndroidUseApkSigner)' == 'true' "
ApkSignerJar="$(ApkSignerJar)"
ApkToSign="%(ApkAbiFilesAligned.FullPath)"
ApkToSign="%(ApkAbiFilesAligned.Identity)"
KeyStore="$(_ApkKeyStore)"
KeyAlias="$(_ApkKeyAlias)"
KeyPass="$(_ApkKeyPass)"
Expand All @@ -2292,7 +2292,7 @@ because xbuild doesn't support framework reference assemblies.
</ItemGroup>
<Message Text="Unaligned android package '%(ApkAbiFilesUnaligned.FullPath)'" Condition=" '$(AndroidUseApkSigner)' != 'True' And '$(AndroidPackageFormat)' != 'aab' "/>
<AndroidZipAlign Condition=" '$(AndroidUseApkSigner)' != 'True' And '$(AndroidPackageFormat)' != 'aab' "
Source="%(ApkAbiFilesUnaligned.FullPath)"
Source="%(ApkAbiFilesUnaligned.Identity)"
DestinationDirectory="$(OutDir)"
ToolPath="$(ZipAlignToolPath)"
ToolExe="$(ZipalignToolExe)"
Expand Down
Expand Up @@ -56,6 +56,10 @@ This file is used by all project types, including binding projects.
<Output TaskParameter="ResolvedEnvironmentFiles" ItemName="LibraryEnvironments" />
<Output TaskParameter="ResolvedResourceDirectoryStamps" ItemName="_LibraryResourceDirectoryStamps" />
</ReadLibraryProjectImportsCache>
<ItemGroup>
<FileWrites Include="@(ResolvedResourceDirectories->'%(ResourceDirectoryArchive)')"
Condition=" '%(ResolvedResourceDirectories.ResourceDirectoryArchive)' != '' And Exists ('%(ResolvedResourceDirectories.ResourceDirectoryArchive)')" />
</ItemGroup>
</Target>

<Target Name="_BuildLibraryImportsCache"
Expand Down
23 changes: 23 additions & 0 deletions tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs
Expand Up @@ -178,6 +178,29 @@ public void MonoSymbolicateAndroidStackTrace ()
}) ;
}

[Test]
[Category ("UsesDevice"), Category ("SmokeTests")]
public void SmokeTestBuildAndRunWithSpecialCharacters ()
{
AssertHasDevices ();
var testName = "テスト";

var rootPath = Path.Combine (Root, "temp", TestName);
var proj = new XamarinFormsAndroidApplicationProject () {
ProjectName = testName,
IsRelease = true,
};
proj.SetAndroidSupportedAbis ("armeabi-v7a", "x86");
proj.SetDefaultTargetDevice ();
using (var builder = CreateApkBuilder (Path.Combine (rootPath, proj.ProjectName))){
Assert.IsTrue (builder.Install (proj), "Install should have succeeded.");
Assert.IsTrue (builder.RunTarget (proj, "_Run", doNotCleanupOnUpdate: true), "Project should have run.");
var timeoutInSeconds = 120;
Assert.IsTrue (WaitForActivityToStart (proj.PackageName, "MainActivity",
Path.Combine (Root, builder.ProjectDirectory, "startup-logcat.log"), timeoutInSeconds));
}
}

[Test, Category ("MonoSymbolicate")]
public void MonoSymbolicateNetStandardStackTrace ()
{
Expand Down

0 comments on commit fd5f31c

Please sign in to comment.