1780056 - Enforce a deployment target of 10.12 (Intel) for local builds

Status: RESOLVED FIXED

(Firefox Build System :: General, task, P3)

Description

[Stephen A Pohl [:spohl] (OOO until 1/26)]

While working to remove workarounds for old SDKs, Markus and I noticed that we are not currently enforcing a deployment target for local builds. We should do so in order to help developers catch issues related to unavailable APIs before patches land on try or CI.

I'm currently enforcing a deployment target by setting the following in my .mozconfig file:

export CFLAGS="-mmacosx-version-min=10.12 -Werror=unguarded-availability-new"
export CXXFLAGS="-mmacosx-version-min=10.12 -Werror=unguarded-availability-new"

Also, note that even though we target 11.0 and above for aarch64 builds in CI, we should still enforce a deployment target of 10.12 for developer builds regardless of the developer's underlying platform since patches will need to be able to target 10.12 for Intel once they land on try and/or CI.

Glandium, would you happen to know how we could enforce a deployment target of 10.12 for local builds?

Comment 1

[Markus Stange [:mstange]]

(In reply to Stephen A Pohl [:spohl] from comment #0)

Also, note that even though we target 11.0 and above for aarch64 builds in CI, we should still enforce a deployment target of 10.12 for developer builds regardless of the developer's underlying platform since patches will need to be able to target 10.12 for Intel once they land on try and/or CI.

Oh, right. Does that work? I think the SDK might be enforcing a minimum deployment target of macOS 11 for arm64 builds. But you're right that this means we wouldn't get the availability warnings for local arm64 developer builds.

Comment 2

[Stephen A Pohl [:spohl] (OOO until 1/26)]

(In reply to Markus Stange [:mstange] from comment #1)

(In reply to Stephen A Pohl [:spohl] from comment #0)

Also, note that even though we target 11.0 and above for aarch64 builds in CI, we should still enforce a deployment target of 10.12 for developer builds regardless of the developer's underlying platform since patches will need to be able to target 10.12 for Intel once they land on try and/or CI.

Oh, right. Does that work? I think the SDK might be enforcing a minimum deployment target of macOS 11 for arm64 builds. But you're right that this means we wouldn't get the availability warnings for local arm64 developer builds.

Yes, it works for me locally and has been correctly emitting availability warnings.

Comment 3

[Mike Hommey [:glandium]]

Glandium, would you happen to know how we could enforce a deployment target of 10.12 for local builds?

We already do. https://searchfox.org/mozilla-central/rev/2bbb0c0a90df20702df8c8011a8996536a83cb75/build/moz.configure/toolchain.configure#46-61

Comment 4

[Mike Hommey [:glandium]]

Ah, for local builds only, you can tweak the conditions and add developer_options in there.

Comment 5

[Stephen A Pohl [:spohl] (OOO until 1/26)]

Attachment: Bug 1780056: Set the deployment target for local macOS builds to 10.12 for all platforms and enable -Werror=unguarded-availability-new. r=glandium

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4babb7807e00
Set the deployment target for local macOS builds to 10.12 for all platforms and enable -Werror=unguarded-availability-new. r=glandium

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f9a30192ab33
Fix python black lint failure. CLOSED TREE

Comment 8

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/4babb7807e00
https://hg.mozilla.org/mozilla-central/rev/f9a30192ab33

Comment 9

[Chris Peterson [:cpeterson]]

Stephen, what's the difference between -Wunguarded-availability-new and -Wunguarded-availability? I see 158 -Wunguarded-availability warnings in a local build. Are they a concern?

I'm not overriding --with-macos-sdk in my mozconfig; I'm building for Apple Silicon using the default MacOSX12.3.sdk with Xcode 13.4.1 on macOS 12.5.1.

Some example warnings:

widget/cocoa/nsTouchBarUpdater.mm:55:30 [-Wunguarded-availability] 'touchBar' is only available on macOS 10.12.2 or newer
widget/cocoa/nsTouchBarUpdater.mm:79:33 [-Wunguarded-availability] 'touchBar' is only available on macOS 10.12.2 or newer
widget/cocoa/nsTouchBarUpdater.mm:81:28 [-Wunguarded-availability] 'touchBar' is only available on macOS 10.12.2 or newer
widget/cocoa/nsTouchBarUpdater.mm:88:10 [-Wunguarded-availability] 'toggleTouchBarCustomizationPalette:' is only available on macOS 10.12.2 or newer

Comment 10

[Stephen A Pohl [:spohl] (OOO until 1/26)]

(In reply to Chris Peterson [:cpeterson] from comment #9)

Stephen, what's the difference between -Wunguarded-availability-new and -Wunguarded-availability? I see 158 -Wunguarded-availability warnings in a local build. Are they a concern?

From what I was able to gather, it was in Xcode 9 that both of these two new flags were introduced, i.e. -Wunguarded-availability and -Wunguarded-availability-new. Also, when building with Xcode, -Wunguarded-availability is off by default and -Wunguarded-availability-new is on by default. -Wunguarded-availability-new only warns about APIs introduced in or after macOS 10.13. This would explain why we have warnings related to TouchBar, since those APIs were introduced prior to macOS 10.13. I do not believe that the warnings are of any particular concern at the moment, and even less for TouchBar because this was a feature that was developed with full awareness that not only would these APIs only work on specific versions of the OS, but it was even restricted to only new hardware as well. I recall that all the code was always reviewed with this in mind. We could file bugs to clean things up, and possibly add @available checks where possible.