Skip to content

Conversation

@Eitot
Copy link
Contributor

@Eitot Eitot commented Sep 21, 2022

The Installer app assumes that the package requires Rosetta 2, unless arm64 is explicitly declared.

s. https://scriptingosx.com/2020/12/platform-support-in-macos-installer-packages-pkg/

  • Documentation is added or updated
  • New files have a LGPL 2.1 license statement
  • PKCS#11 module is tested
  • Windows minidriver is tested
  • macOS tokend is tested

Fixes #2608

@Jakuje
Copy link
Member

Jakuje commented Sep 22, 2022

Does this need any build changes so the binaries for both arches is in the release package or this change of metadata will take care of all of this?

Please, update also https://github.com/OpenSC/OpenSC/blob/master/.github/workflows/macos.yml#L14 so it rebuilds the mac ci on change of this file (or any file in the MacOSX directory).

@frankmorgner
Copy link
Member

arm64 is only available when the installer is being built on arm64. arm64 was added recently to the github runners, but since it requires some quirks, I don't believe this is enabled by default. This PR should be changed so that hostArchitectures actually reflects whether or not we are building explicitly for arm (see $BUILD_ARM in MacOSX/build-package.in. If done so, this PR will fix #2608

@Eitot
Copy link
Contributor Author

Eitot commented Sep 24, 2022

arm64 is only available when the installer is being built on arm64.

I do not follow. MacOSX/build-package.in creates universal binaries (x84_64 and arm64) when Xcode 12.2 or newer is used for building. This happens regardless of the CPU architecture of the platform on which the build is created.

This PR merely resolves a "quirk" of the Installer app whereby it requires Rosetta 2 even if the package components are all universal, like the current release version 22. With this change I can install OpenSC without installing Rosetta 2 first (which I have not installed in my machine).

This PR should be changed so that hostArchitectures actually reflects whether or not we are building explicitly for arm (see $BUILD_ARM in MacOSX/build-package.in.

Since the $BUILD_ARM environment variable is defined in the build-package script (after configure was run), the Distribution.xml script would have to be modified afterwards. Would it be sufficient to do a sed in the build-package script before productbuild is run, or is there a better solution?

@frankmorgner
Copy link
Member

Just use a second file with all architectures included. Then modify the build script to use the correct distribution file, e.g. something like this:

diff --git a/MacOSX/build-package.in b/MacOSX/build-package.in
index 509fe0df..07d55167 100755
--- a/MacOSX/build-package.in
+++ b/MacOSX/build-package.in
@@ -58,6 +58,9 @@ export CFLAGS="$CFLAGS -isysroot $SDK_PATH"
 if test -n "${BUILD_ARM}"; then
        export CFLAGS="$CFLAGS -arch x86_64 -arch arm64"
        export LDFLAGS="$LDFLAGS -arch x86_64 -arch arm64"
+       DISTRIBUTION_XML=MacOSX/Distribution_universal.xml
+else
+       DISTRIBUTION_XML=MacOSX/Distribution.xml
 fi
 export OBJCFLAGS=$CFLAGS
 
@@ -220,7 +223,7 @@ pkgbuild --root ${BUILDPATH}/target_token $COMPONENT_TOKEN --identifier org.open
 pkgbuild --root ${BUILDPATH}/target_startup --component-plist MacOSX/target_startup.plist --identifier org.opensc-project.startup --version @PACKAGE_VERSION@ --install-location / OpenSC-startup.pkg
 
 # Build product
-productbuild --distribution MacOSX/Distribution.xml --package-path . --resources MacOSX/resources "${imagedir}/OpenSC @[email protected]"
+productbuild --distribution $DISTRIBUTION_XML --package-path . --resources MacOSX/resources "${imagedir}/OpenSC @[email protected]"
 
 # Sign installer
 if test -n "${INSTALLER_SIGN_IDENTITY}"; then

@frankmorgner
Copy link
Member

Note, that I'd like to keep support for older xcode versions, because TokenD cannot be built with the newer ones.

@frankmorgner
Copy link
Member

Thank you. You also need to add the new file to MacOSX/Makefile.am.

@Eitot
Copy link
Contributor Author

Eitot commented Oct 11, 2022

@frankmorgner Can this PR be included in the 0.23 release?

@Jakuje
Copy link
Member

Jakuje commented Oct 12, 2022

I think it makes sense to include it in the releases, but I am not familiar with the code enough to merge it and before doing another RC with this, I would like to solve the issue with the osx build artifacts mentioned in #2426 (comment)

@Jakuje Jakuje merged commit 26cd7f2 into OpenSC:master Oct 14, 2022
@Jakuje
Copy link
Member

Jakuje commented Oct 14, 2022

Thank you for your contribution!

@Eitot Eitot deleted the rosetta branch October 14, 2022 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wont install on Apple silicon without Rosetta 2

3 participants