[Xamarin.Android.Build.Tasks] The option for multi-dex fails when the path to the Android SDK contains a space #575
Conversation
| @@ -66,8 +66,8 @@ at com.android.dx.command.dexer.Main.main(Main.java:215) | |||
| at com.android.dx.command.Main.main(Main.java:106) | |||
| */ | |||
| const string ExceptionRegExString = @"(?<exception>java.lang.+):(?<error>.+)"; | |||
| Regex codeErrorRegEx = new Regex (CodeErrorRegExString, RegexOptions.Compiled); | |||
| Regex exceptionRegEx = new Regex (ExceptionRegExString, RegexOptions.Compiled); | |||
| protected Regex codeErrorRegEx = new Regex (CodeErrorRegExString, RegexOptions.Compiled); | |||
There was a problem hiding this comment.
This should probably be protected static readonly. This isn't something that should be changed (readonly), and I can't think of any reason we'd ever need to change this on a per-instance basis (static).
There was a problem hiding this comment.
This should also be PascalCased as CodeErrorRegex, as it's not a "publicly visible" member.
|
|
||
| return cmd.ToString (); | ||
| cmd.AppendSwitchIfNotNull ("-jar ", ProguardJarPath); | ||
| cmd.AppendSwitchUnquotedIfNotNull ("-injars ", $"{enclosingChar}'" + string.Join ($"{enclosingChar}{Path.PathSeparator}{enclosingChar}", jars) + $"'{enclosingChar}"); |
There was a problem hiding this comment.
I thought we were going to try to do an -injar per file. Did that not work?
foreach (var j in jars) {
cmd.AppendSwitchIfNotNull ("-injars ", $"\"{j}\"");
}
There was a problem hiding this comment.
I will give it a go. I forgot, was focused on getting it working lol
| cmd.AppendSwitch ("-dontwarn"); | ||
| cmd.AppendSwitch ("-forceprocessing"); | ||
| cmd.AppendSwitchIfNotNull ("-outjars ", tempJar); | ||
| cmd.AppendSwitchIfNotNull ("-libraryjars ", $"'{Path.Combine (AndroidSdkBuildToolsPath, "lib", "shrinkedAndroid.jar")}'"); |
There was a problem hiding this comment.
Why does the value here need quotes? The CommandLineBuilder.AppendSwitchIfNotNull() documentation states:
This method encapsulates individual file names with quotation marks as necessary.
which suggests that this value will be double quoted. Is that necessary?
There was a problem hiding this comment.
I thought the same thing. But when there are spaces in the path its Java that throws a wobbly even if its double quoted. I get an access denied error when we access the shrinkedAndroid.jar from Program Files (x86) even with double quotes, add the single quotes and the error goes away. Same for the include later.
Its totally weird, but I did spend some time testing this out. This works :)
There was a problem hiding this comment.
Indeed, totally weird. This would make sense if we were going through some "intermediary," such as a batch file/shell script/"wrapper program" which "forwarded" command-line parameters w/o further quoting, but that shouldn't be the case here; we're invoking java.exe, aren't we?
:-/
| cmd.AppendSwitch ("-dontoptimize"); | ||
| cmd.AppendSwitch ("-dontobfuscate"); | ||
| cmd.AppendSwitch ("-dontpreverify"); | ||
| cmd.AppendSwitchIfNotNull ("-include ", $"'{Path.Combine (AndroidSdkBuildToolsPath, "mainDexClasses.rules")}'"); |
|
|
||
| void GenerateProguardCommands (CommandLineBuilder cmd) | ||
| { | ||
| var enclosingChar = OS.IsWindows ? "\"" : string.Empty; |
There was a problem hiding this comment.
This doesn't appear to be used anywhere. I think it's vestigial.
… path to the Android SDK contains a space Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=33052 Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=38693 The google tooling around multidex is pretty broken on windows. It does not handle paths with spaces and has over quoting path issues. This is one reason why we are shipping our own Proguard. This commit completely removes the user of mainDexClasses.bat and introduces code which will create the ` multidex.keep` by making the required calls manually.
|
ok I just retested this on windows and I have to back out the foreach on the jar file change. Proguard does NOT like it
Also we do need the single quotes on windows otherwise we get
for the shrinkedAndroid.jar and
for the mainDexClasses.rules |
Changes: dotnet/java-interop@b81cfbb...4f47ec8 Fixes: dotnet/java-interop#572 Context: dotnet/project-system#1821 Context: dotnet/java-interop#509 * dotnet/java-interop@4f47ec8: [generator] Make DIM invoking plumbing private (#581) * dotnet/java-interop@2a0f863: [Java.Interop.sln] Let VS update the project type guids. (#579) * dotnet/java-interop@9a01c9b: [Java.Interop.Tools.Cecil] remove System.Linq in IsSubclassOf (#577) * dotnet/java-interop@cd33da6: [generator] Bind protected nested types (#578) * dotnet/java-interop@fa10e98 :[generator] Do bind package-private nested types (#575)
Changes: dotnet/java-interop@e92f341...178e849 Fixes: dotnet/java-interop#572 Context: dotnet/project-system#1821 Context: dotnet/java-interop#509 * dotnet/java-interop@178e849: [generator] Make DIM invoking plumbing private (#581) * dotnet/java-interop@b7b10bc: [Java.Interop.sln] Let VS update the project type guids. (#579) * dotnet/java-interop@3c590a4: [Java.Interop.Tools.Cecil] remove System.Linq in IsSubclassOf (#577) * dotnet/java-interop@d1bbed7: [generator] Bind protected nested types (#578) * dotnet/java-interop@5db30a8: [generator] Do bind package-private nested types (#575)
Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=33052
Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=38693
The google tooling around multidex is pretty broken on
windows. It does not handle paths with spaces and has
over quoting path issues. This is one reason why we are
shipping our own Proguard.
This commit completely removes the user of mainDexClasses.bat
and introduces code which will create the
multidex.keepbymaking the required calls manually.