1740631 - macOS PKG doesn't properly support M1

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Mike Kaply [:mkaply]]

Apparently our PKG file doesn't properly report that it supports M1 even though the packaged binary is universal.

Per:

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

We need to make some changes to mark the PKG file.

Per discussion on mac Slack, we think it's a simple as adding:

 <options hostArchitectures="x86_64,arm64" />

to the Distribution XML file.

This can be verified with this tool:

https://mothersruin.com/software/SuspiciousPackage/

Comment 1

[Mike Kaply [:mkaply]]

Here's where we did the original work to create a distribution package:

https://bugzilla.mozilla.org/show_bug.cgi?id=1689376

What version of macOS do we build Firefox on? It looks like if we were on Big Sure, this would just work.

If we're on Catalina or lower, it looks like the easiest option is to create a file like this:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
        <key>arch</key>
        <array>
                <string>x86_64</string>
                <string>arm64</string>
        </array>
</dict>
</plist>

and just pass it to productbuild:

--product requirements.plist

Comment 2

[Julien Cristau [:jcristau]]

(In reply to Mike Kaply [:mkaply] from comment #1)

What version of macOS do we build Firefox on? It looks like if we were on Big Sure, this would just work.

none, we build on linux :)

Comment 3

[Aki Sasaki (not active)]

We build the pkg files on the mac signers, which are Mojave. Our productbuild call is here. We may need to support some sort of task or product flag that specifies this, since we might sign apps that don't support both arches, but we could also hardcode until that becomes a problem.

Comment 4

[Aki Sasaki (not active)]

We probably want to:

• quarantine a mac signer
• ssh in
• patch iscript, and possibly the configs
• test iscript
• push the pkg somewhere for someone (qa?) to test
• repeat the previous 3 steps until it looks good
• productionize the changes in iscript and/or ronin-puppet
• find a window to roll out: these changes will be live as soon as we roll out to the mac signers. if we want to be able to ride the trains or avoid affecting chemspills, we need to include some in-tree in-task flag; otherwise we'll be live on all branches once we roll out.
• roll out
• verify in the next nightly/beta/release/esr

Comment 5

[Andrew Halberstadt [:ahal]]

I'll take a look at this.

Comment 6

[Andrew Halberstadt [:ahal]]

I've quarantined mac-v3-signing2 to start looking into this.

Comment 7

[Andrew Halberstadt [:ahal]]

Attachment: GitHub Pull Request

This is untested (just getting something up to iterate on).

Comment 8

[Andrew Halberstadt [:ahal]]

Hey Mike, your suggested fix from comment 1 produced this package file:
https://nextcloud.ahal.ca/s/QATwiDgNMK8zQXb

I don't know anything about MacOS packaging though, would you (or someone) be able to verify that it's packaged as you expect?

Note: the PR is still not finished as we don't want to check the requirements.plist into scriptworker-scripts, but we can at least verify that the suggested fix works.

Comment 9

[Mike Kaply [:mkaply]]

Attachment: Screenshot of it working correctly.

Just had the original reporter verify and it looks good.

Comment 10

[Andrew Halberstadt [:ahal]]

Thanks! I've update the PR to something close to how we'll want to handle this in iscript. Since I'm heading out on holidays tomorrow, writing down next steps for the new year:

1. Finish iscript PR (at least need to tweak schema still, also need to check if tests need adding/updating)
2. Write Gecko counter-part patch (figure out where to check-in requirements.plist)
3. FIgure out how to test this
4. Land + delpoy iscript patch
5. Land Gecko patch

Comment 11

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1740631 - Add requirements-plist-url to mac signing payloads, r?aki!,glandium!

Comment 12

[Andrew Halberstadt [:ahal]]

Hey Mike, I'd like to check this requirements.plist file into mozilla-central. Do you know where it should live, or can you suggest someone to ask? Thanks!

Comment 13

[Andrew Halberstadt [:ahal]]

I've updated the linked pkg file to one that was generated using the attached iscript PR. Though it's just using the same requirements.plist as before and I verified that the productbuild command is constructed appropriately.. so I don't think it needs re-validation.

Comment 14

[Mike Kaply [:mkaply]]

Aki might have some ideas on where this should live?

Comment 15

[Aki Sasaki (not active)]

Lol :)
I thought maybe next to the entitlement files, but security/mac/hardenedruntime/ seems specifically not this.
https://searchfox.org/mozilla-central/source/browser/installer / mac something?
https://searchfox.org/mozilla-central/source/build/package/mac_osx ?

I think I'm just guessing a path, based on a) will it affect any other part of a build (let's not), and b) does it make any sort of intuitive sense. Not sure if a build peer or similar should chime in.

Comment 16

[Andrew Halberstadt [:ahal]]

Alright, here's a build-signing task with all the pieces finally put together:
https://firefox-ci-tc.services.mozilla.com/tasks/AWLIaxb8Qiat3affF44eQg#artifacts

The target.pkg there should be using the checked in requirements.plist. I'll find someone to verify.

Comment 17

[Andrew Halberstadt [:ahal]]

Heitor verified that the artifact produced by the above task reports that it installs natively on Apple Silicon.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e7d8b37032ca
Add requirements-plist-url to mac signing payloads, r=aki,glandium

Comment 19

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/e7d8b37032ca

Comment 20

[Mike Kaply [:mkaply]]

Would this be easy to put on the ESR?

Comment 21

[Aki Sasaki (not active)]

I think it's a matter of uplifting https://hg.mozilla.org/mozilla-central/rev/e7d8b37032ca.

Comment 22

[Mike Kaply [:mkaply]]

Attachment: ESR version of diff

Looks like gecko_taskgraph was just taskgraph on the ESR and I had to fix one conflict.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: Mainly enterprise feature (PKG)
User impact if declined: PKG can't be used on M1
Fix Landed on Version: 98
Risk to taking this patch (and alternatives if risky): Low

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Comment 23

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9265938 [details] [diff] [review]
ESR version of diff

Approved for ESR91.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9265938 [details] [diff] [review]
ESR version of diff

This breaks the Gecko decision task. Please verify that it works on Try before re-requesting uplift.

Comment 25

[Mike Kaply [:mkaply]]

I'm getting errors just running ./mach bootstrap.

I think I'm just going to punt on this one for now.

I haven't heard complaints about ESR, just a nice to have and we're approaching 102 fast.

Comment 26

[Mike Kaply [:mkaply]]

Just FYI, the error I get is:

extra keys not allowed @ data['mac-notarization']['mac-requirements']

So I Think something else is missing.

See:

https://treeherder.mozilla.org/jobs?repo=try&revision=52479d963694d57a8c2f6476aab7f067d17b3ba6

Comment 27

[Aki Sasaki (not active)]

Ah. That's in this file in the patch, taskcluster/gecko_taskgraph/config.py. We renamed taskcluster/taskgraph to taskcluster/gecko_taskgraph in central; I'm guessing we didn't in ESR. So I bet that portion of the patch needs to be applied to taskcluster/taskgraph/config.py in esr.