Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Xamarin.Android.Build.Tasks] Properly quote -libraryjars (#666) #670

Merged
merged 1 commit into from
Jun 23, 2017

Conversation

jonpryor
Copy link
Member

Context: https://bugzilla.xamarin.com/show_bug.cgi?id=32861

Scenario: Build a project in Release configuration with
$(AndroidEnableProguard)=True on Windows.

Expected result: it works!

Actual result: It doesn't:

C:\Program Files\Java\jdk1.8.0_131\\bin\java.exe -jar ...
PROGUARD : error : C:\Program Files (Access is denied)

Commit 8467ad6 attempted to fix this by "double-quoting" the
-libraryjars option. Unfortunately, we inadvertantly triple-quoted
the value (!)

-libraryjars "\"'C:\Program Files (x86)\Android\android-sdk\platforms\android-25\android.jar'\""

The \" is unexpected, and problematic. It should be:

-libraryjars "'C:\Program Files (x86)\Android\android-sdk\platforms\android-25\android.jar'"

The "actual" cause -- other than inadequate testing, and a need to run
unit tests more frequently on Windows -- is that we used
CommandLineBuilder.AppendSwitchIfNotNull() with -libraryjars.
This is problematic because AppendSwitchIfNotNull() will "escape"
embedded double-quotes, which is what is introducing the \" quotes
and the 3rd level of quoting.

Use CommandLineBuilder.AppendSwitchUnquotedIfNotNull() instead, to
avoid the extra and erroneous quoting.

Additionally, -include needs double-quoting as well on Windows.
Fixup -include generation in the <Proguard/> and
<CreateMultiDexMainDexClassList/> tasks.

Context: https://bugzilla.xamarin.com/show_bug.cgi?id=32861

Scenario: Build a project in Release configuration with
`$(AndroidEnableProguard)`=True on Windows.

Expected result: it works!

Actual result: It doesn't:

	C:\Program Files\Java\jdk1.8.0_131\\bin\java.exe -jar ...
	PROGUARD : error : C:\Program Files (Access is denied)

Commit 8467ad6 attempted to fix this by "double-quoting" the
`-libraryjars` option. Unfortunately, we inadvertantly *triple*-quoted
the value (!)

	-libraryjars "\"'C:\Program Files (x86)\Android\android-sdk\platforms\android-25\android.jar'\""

The `\"` is unexpected, and problematic. It *should* be:

	-libraryjars "'C:\Program Files (x86)\Android\android-sdk\platforms\android-25\android.jar'"

The "actual" cause -- other than inadequate testing, and a need to run
unit tests more frequently on Windows -- is that we used
`CommandLineBuilder.AppendSwitchIfNotNull()` with `-libraryjars`.
This is problematic because `AppendSwitchIfNotNull()` will "escape"
embedded double-quotes, which is what is introducing the `\"` quotes
and the 3rd level of quoting.

Use `CommandLineBuilder.AppendSwitchUnquotedIfNotNull()` instead, to
avoid the extra and erroneous quoting.

Additionally, `-include` needs double-quoting as well on Windows.
Fixup `-include` generation in the `<Proguard/>` and
`<CreateMultiDexMainDexClassList/>` tasks.
@Aguilex Aguilex merged commit 9359872 into dotnet:d15-3 Jun 23, 2017
jonpryor pushed a commit that referenced this pull request Jun 26, 2020
Changes: dotnet/java-interop@1de5501...60efc99

Fixes: dotnet/java-interop#639
Fixes: dotnet/java-interop#664

  * dotnet/java-interop@60efc99: Update to NUnit 3.12/etc. (#667)
  * dotnet/java-interop@de35d29: [jnimarshalmethod-gen] support methods with 14+ arguments (#670)
  * dotnet/java-interop@fd2c94e: [generator] Mark protected types nested in sealed classes as private (#672)
  * dotnet/java-interop@11b7f85: [Java.Interop+MonoAndroid] Remove NullableAttributes.cs, avoid CS0436 (#671)
  * dotnet/java-interop@ef1d37b: [tools] Add response file support (#665)
  * dotnet/java-interop@1383981: [logcat-parse] Support the Android 10 `logcat` format (#663)
@github-actions github-actions bot locked and limited conversation to collaborators Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants