Closed Bug 1564216 Opened 4 months ago Closed 4 months ago

Can't build with OSX 10.14 SDK with --enable-warnings-as-errors

Categories

(Core :: Widget: Cocoa, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- wontfix
firefox68 --- unaffected
firefox69 --- wontfix
firefox70 --- fixed

People

(Reporter: mccr8, Assigned: glandium)

References

(Blocks 2 open bugs, Regression)

Details

(Keywords: regression)

Attachments

(5 files, 1 obsolete file)

I can no longer build on OSX 10.13.6.

I get this error message (plus a number of similar ones):

0:21.03 /Users/amccreight/mc/widget/cocoa/nsCocoaUtils.mm:1174:19: error: 'AVAuthorizationStatusNotDetermined' is only available on macOS 10.14 or newer [-Werror,-Wunguarded-availability-new]
0:21.03 (int)AVAuthorizationStatusNotDetermined);
0:21.03 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I was able to build on this machine the last time I pulled, which was last week at some point. I'm not sure what has regressed. nsCocoaUtils.mm doesn't seem to have changed recently.

This particular chunk of code is guarded by
#if defined(MAC_OS_X_VERSION_10_14)
so I'm not sure why it is trying to compile it. I'm not sure where this is supposed to be defined.

You might try building with the 10.11 SDK: https://bugzilla.mozilla.org/show_bug.cgi?id=1522931#c27

This is bug 1494851.

Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1494851

If the #if defined(MAC_OS_X_VERSION_10_14) guards actually broke, there's likely going to be more than needs to be fixed than one API call. I guess we'll see.

I'll start bisecting this. My good changeset is from July 3, and my bad changeset is from July 5, so hopefully it won't take long.

I haven't gotten this quite bisected yet, but bug 1560044 is in the range and looks related. (Bug 1558924 is in the range and also mentions OSX, but I can't see how the patch would affect anything like this.)

I'm going to undupe this. As far as I can tell, this is a new regression, where one of the symptoms of the regression is bug 1494851, so I'll mark it depending on that.

Anyways, my bisection points the blame to bug 1560044. My basic approach to bisection was to do mach build, let that run for awhile, then do mach build widget/

Glandium, can you take a look at this? Thanks.

Status: RESOLVED → REOPENED
Depends on: 1494851
Flags: needinfo?(mh+mozilla)
Regressed by: 1560044
Resolution: DUPLICATE → ---

So, the problem is with the code:

 #if defined(MAC_OS_X_VERSION_10_14)

MAC_OS_X_VERSION_10_x is not meant to be used this way. The SDK for version 10.x defines it, but the SDK can be used to target older versions, which is what happens when you use that version of the SDK on an older version of the OS.

Flags: needinfo?(mh+mozilla)

At quick glance it looks like this is the only place misusing MAC_OS_X_VERSION_10_x

Note there are other uses of MAC_OS_X_VERSION_MAX_ALLOWED that should probably be replaced with MAC_OS_X_VERSION_MIN_REQUIRED, but I'll leave that to Stephen or Haik to fix.

Flags: needinfo?(spohl.mozilla.bugs)
Flags: needinfo?(haftandilian)
Assignee: nobody → mh+mozilla
Summary: Can't build on OSX 10.13 → Can't build on OSX 10.13 with --enable-warnings-as-errors
See Also: → 1564298
Blocks: buildwarning

See code review feedback in phabricator.

Flags: needinfo?(spohl.mozilla.bugs)
Flags: needinfo?(mh+mozilla)
Flags: needinfo?(haftandilian)

While there, transform the MOZ_ASSERTs into static_asserts, so that
discrepancies are caught at build time rather than runtime.

Note that I also tried to replace nsCocoaFeatures::OnMojaveOrLater with
@available, but that failed to link on older SDKs because of missing
___isOSVersionAtLeast symbol.

Depends on: 1566687
Attachment #9078345 - Attachment description: Bug 1564216 - Fix warning in nsCocoaUtils.mm with 10.14 SDK. → Bug 1564216 - Fix warnings in nsCocoaUtils.mm with 10.14 SDK.
Attachment #9076695 - Attachment is obsolete: true
Flags: needinfo?(mh+mozilla)

Thanks for taking this on and improving the asserts. It looks like your patch addresses the problem on bug 1494851 and we can now close it as a dupe of this bug.

There is less incentive to keep things building with older versions of
clang for OSX builds, and we're going to require an objective-C feature
that was added in clang 5.

Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/5f82bce8aa27
Require clang 5 for OSX builds. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/991aad183871
Fix warnings in nsCocoaUtils.mm with 10.14 SDK. r=mstange
Summary: Can't build on OSX 10.13 with --enable-warnings-as-errors → Can't build with OSX 10.14 SDK with --enable-warnings-as-errors

The priority flag is not set for this bug.
:spohl, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(spohl.mozilla.bugs)
Flags: needinfo?(spohl.mozilla.bugs)
Priority: -- → P1
Status: REOPENED → RESOLVED
Closed: 4 months ago4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70

Is this something we wanted to backport to Beta and ESR68 to make life easier for developers?

Flags: needinfo?(mh+mozilla)

People building with --enable-warnings-as-errors are asking for problems. Even more so when building esr68, which is likely to fail because of new warnings in newer versions of the rust compiler. So I'd say meh.

Flags: needinfo?(mh+mozilla)

(In reply to Mike Hommey [:glandium] from comment #22)

People building with --enable-warnings-as-errors are asking for problems. Even more so when building esr68, which is likely to fail because of new warnings in newer versions of the rust compiler. So I'd say meh.

I mean, building without it is also asking for problems because then your patches get backed out on infra because they cause warnings which are treated as errors there, soooo...

If you're really worried about warnings, you should at least do a try push with builds only, on all platforms. Because your local build with --enable-warnings-as-errors still has the possibility of making warnings on other platforms invisible to you.

(In reply to Mike Hommey [:glandium] from comment #24)

If you're really worried about warnings, you should at least do a try push with builds only, on all platforms. Because your local build with --enable-warnings-as-errors still has the possibility of making warnings on other platforms invisible to you.

Sure, but the possibility is much smaller, esp. now that we use clang everywhere, and because I effectively never write C++ code that (a) runs on multiple platforms and (b) has per-platform codepaths. So I run with --enable-warnings-as-errors locally in order to catch bustage before I push to try, as pushing to try takes much longer...

Duplicate of this bug: 1494851

(In reply to Mike Hommey [:glandium] from comment #22)

People building with --enable-warnings-as-errors are asking for problems. Even more so when building esr68, which is likely to fail because of new warnings in newer versions of the rust compiler. So I'd say meh.

FYI, when building release or beta, one should use rustup override to select the right rust version for the version of the tree being built.

Regressions: 1568573
Regressions: 1568712
You need to log in before you can comment on or make changes to this bug.