Skip to content

[Configuration] Use :, not ,, as ABI separator. #57

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

Merged
merged 1 commit into from
Jun 6, 2016

Conversation

jonpryor
Copy link
Contributor

@jonpryor jonpryor commented Jun 6, 2016

Update $(AndroidSupportedAbis) and
$(AndroidSupportedAbisForConditionalChecks) to use : as the ABI
separator, not ,.

; can't be used because xbuild and MSBuild don't like ; as a
property value on the command-line.

Turns out, MSBuild doesn't like , within property values on the
command-line either, because it allows multiple MSBuild properties to
be specified via one /p: use:

$ msbuild -help
...
  /property:<n>=<v>  Set or override these project-level properties. <n> is
                     the property name, and <v> is the property value. Use a
                     semicolon or a comma to separate multiple properties, or
                     specify each property separately. (Short form: /p)
                     Example:
                       /property:WarningLevel=2;OutDir=bin\Debug\

This means that it's not possible to set $(AndroidSupportedAbis) to
e.g. host-Darwin,armeabi-v8a on the command-line with MSBuild.
(It is with xbuild, but this is arguably an xbuild compatibility bug!)

Since we want to be able to easily override the
$(AndroidSupportedAbis) value on the command-line for testing,
change the ABI separator character to : which is supported on both
xbuild and MSBuild:

$ xbuild /p:AndroidSupportedAbis=host-Darwin:armeabi-v7a
# works!

@jonpryor jonpryor changed the title [Configuraiton] Use :, not ,, as ABI separator. [Configuration] Use :, not ,, as ABI separator. Jun 6, 2016
@jonpryor jonpryor force-pushed the jonp-abi-separator branch from fdc97d7 to 520b994 Compare June 6, 2016 21:33
@@ -1 +1 @@
Subproject commit 4ace37fddb3fd8a96363d8c01af2e93e04667fc7
Subproject commit a9312d2607c87a301e05db7fe2243200d2557666
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this intentional?

Update `$(AndroidSupportedAbis)` and
`$(AndroidSupportedAbisForConditionalChecks)` to use `:` as the ABI
separator, not `,`.

`;` can't be used because xbuild and MSBuild don't like `;` as a
property value on the command-line.

Turns out, MSBuild doesn't like `,` within property values on the
command-line either, because it allows multiple MSBuild properties to
be specified via one `/p:` use:

	$ msbuild -help
	...
	  /property:<n>=<v>  Set or override these project-level properties. <n> is
	                     the property name, and <v> is the property value. Use a
	                     semicolon or a comma to separate multiple properties, or
	                     specify each property separately. (Short form: /p)
	                     Example:
	                       /property:WarningLevel=2;OutDir=bin\Debug\

This means that it's not possible to set `$(AndroidSupportedAbis)` to
e.g. `host-Darwin,armeabi-v8a` on the command-line *with MSBuild*.
(It is with xbuild, but this is arguably an xbuild compatibility bug!)

Since we want to be able to easily override the
`$(AndroidSupportedAbis)` value on the command-line for testing,
change the ABI separator character to `:` which is supported on both
xbuild and MSBuild:

	$ xbuild /p:AndroidSupportedAbis=host-Darwin:armeabi-v7a
	# works!
@jonpryor jonpryor force-pushed the jonp-abi-separator branch from 520b994 to 699d607 Compare June 6, 2016 21:39
@radical radical merged commit 7010a35 into dotnet:master Jun 6, 2016
radical pushed a commit that referenced this pull request May 8, 2018
Context: https://bugzilla.xamarin.com/show_bug.cgi?id=42476
Context: commit 4df1e9f

This commit reduces allocations even further by reducing
`reference.ToString()` allocations.
grendello pushed a commit that referenced this pull request Aug 12, 2020
Fixes: dotnet/android-libzipsharp#64

Context: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1139578
Context: https://liquid.microsoft.com/Web/Object/Read/ms.security/Requirements/Microsoft.Security.SystemsADM.10039#guide

Changes: dotnet/android-libzipsharp@1.0.10...1.0.20

  * dotnet/android-libzipsharp@1752f95: Statically Linux libzip.so (#70)
  * dotnet/android-libzipsharp@2b1762f: Fix Dll SearchPath to include Assembly Directory. (#69)
  * dotnet/android-libzipsharp@28b4639: Merge pull request #68 from dellis1972/fixwindowssearch
  * dotnet/android-libzipsharp@fdabcda: Fix Windows to look in Assembly Directory for 32bit dll.
  * dotnet/android-libzipsharp@b332af0: Fix Native Crash on Windows. (#67)
  * dotnet/android-libzipsharp@96eb5e3: Use DefaultDllImportSearchPathsAttribute (#66)
  * dotnet/android-libzipsharp@755a42a: Bump libzip to 1.7.3 and go back to mingw (#65)
  * dotnet/android-libzipsharp@5ae5e70: Merge pull request #60 from xamarin/msvc-static-link
  * dotnet/android-libzipsharp@30ff680: Build libzip with static CRT and VS2019
  * dotnet/android-libzipsharp@bad320e: Merge pull request #59 from dellis1972/theswitcharoo
  * dotnet/android-libzipsharp@d7bc2c5: Merge pull request #58 from xamarin/optimize-winbuild
  * dotnet/android-libzipsharp@34dc213: Make 64 bit Linux native lib the default.
  * dotnet/android-libzipsharp@d3aad35: fixup! Optimize libzip build
  * dotnet/android-libzipsharp@0970a01: Optimize libzip build
  * dotnet/android-libzipsharp@d321af1: Merge pull request #57 from xamarin/fix-win32-packaging
  * dotnet/android-libzipsharp@80b739d: Fix a typo which caused 64-bit dll to be packaged for 32-bit Windows
  * dotnet/android-libzipsharp@1665db0: Bump the version (to 1.0.12), to prepare to release (#56)
  * dotnet/android-libzipsharp@dd5e939: Throw exception instead of silently failing if zip save/close fails (#54)
  * dotnet/android-libzipsharp@2df5b16: Fix enumerating zip with deleted entries (#53)
  * dotnet/android-libzipsharp@a042554: Add .editorconfig, copied from xamarin-android (#55)
  * dotnet/android-libzipsharp@a0973d4: Bump libzip to 1.6.1 (#49)

Changes: nih-at/libzip@rel-1-5-1...v1.7.3

  * Context: https://libzip.org/news/release-1.7.3.html
  * Context: https://libzip.org/news/release-1.7.2.html
  * Context: https://libzip.org/news/release-1.7.1.html
  * Context: https://libzip.org/news/release-1.7.0.html

Two primary changes of note in this xamarin/LibZipSharp bump:

 1. Bump to `libzip` 1.7.3, which contains numerious fixes and
    improvements over the previously used 1.5.1 release.

 2. Use of `DefaultDllImportSearchPathsAttribute` so that native
    library dependencies are loaded securely, i.e. w/o allowing
    "other" libraries to be loaded from unsafe directories.

Unfortunately, the `libzip` bump itself caused issues, PR #4751 and
PR #4937 each had integration tests "randomly" SIGSEGV.  This was
eventually tracked down to a bug within `libzip` itself, fixed at:

  * nih-at/libzip#202
jonpryor pushed a commit that referenced this pull request Aug 20, 2020
Fixes: dotnet/android-libzipsharp#64

Context: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1139578
Context: https://liquid.microsoft.com/Web/Object/Read/ms.security/Requirements/Microsoft.Security.SystemsADM.10039#guide

Changes: dotnet/android-libzipsharp@1.0.10...1.0.20

  * dotnet/android-libzipsharp@1752f95: Statically Linux libzip.so (#70)
  * dotnet/android-libzipsharp@2b1762f: Fix Dll SearchPath to include Assembly Directory. (#69)
  * dotnet/android-libzipsharp@28b4639: Merge pull request #68 from dellis1972/fixwindowssearch
  * dotnet/android-libzipsharp@fdabcda: Fix Windows to look in Assembly Directory for 32bit dll.
  * dotnet/android-libzipsharp@b332af0: Fix Native Crash on Windows. (#67)
  * dotnet/android-libzipsharp@96eb5e3: Use DefaultDllImportSearchPathsAttribute (#66)
  * dotnet/android-libzipsharp@755a42a: Bump libzip to 1.7.3 and go back to mingw (#65)
  * dotnet/android-libzipsharp@5ae5e70: Merge pull request #60 from xamarin/msvc-static-link
  * dotnet/android-libzipsharp@30ff680: Build libzip with static CRT and VS2019
  * dotnet/android-libzipsharp@bad320e: Merge pull request #59 from dellis1972/theswitcharoo
  * dotnet/android-libzipsharp@d7bc2c5: Merge pull request #58 from xamarin/optimize-winbuild
  * dotnet/android-libzipsharp@34dc213: Make 64 bit Linux native lib the default.
  * dotnet/android-libzipsharp@d3aad35: fixup! Optimize libzip build
  * dotnet/android-libzipsharp@0970a01: Optimize libzip build
  * dotnet/android-libzipsharp@d321af1: Merge pull request #57 from xamarin/fix-win32-packaging
  * dotnet/android-libzipsharp@80b739d: Fix a typo which caused 64-bit dll to be packaged for 32-bit Windows
  * dotnet/android-libzipsharp@1665db0: Bump the version (to 1.0.12), to prepare to release (#56)
  * dotnet/android-libzipsharp@dd5e939: Throw exception instead of silently failing if zip save/close fails (#54)
  * dotnet/android-libzipsharp@2df5b16: Fix enumerating zip with deleted entries (#53)
  * dotnet/android-libzipsharp@a042554: Add .editorconfig, copied from xamarin-android (#55)
  * dotnet/android-libzipsharp@a0973d4: Bump libzip to 1.6.1 (#49)

Changes: nih-at/libzip@rel-1-5-1...v1.7.3

  * Context: https://libzip.org/news/release-1.7.3.html
  * Context: https://libzip.org/news/release-1.7.2.html
  * Context: https://libzip.org/news/release-1.7.1.html
  * Context: https://libzip.org/news/release-1.7.0.html

Two primary changes of note in this xamarin/LibZipSharp bump:

 1. Bump to `libzip` 1.7.3, which contains numerious fixes and
    improvements over the previously used 1.5.1 release.

 2. Use of `DefaultDllImportSearchPathsAttribute` so that native
    library dependencies are loaded securely, i.e. w/o allowing
    "other" libraries to be loaded from unsafe directories.

Unfortunately, the `libzip` bump itself caused issues, PR #4751 and
PR #4937 each had integration tests "randomly" SIGSEGV.  This was
eventually tracked down to a bug within `libzip` itself, fixed at:

  * nih-at/libzip#202
@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