Build error in osx_corewlan.mm with --enable-macos-target=10.7 and --enable-warnings-as-errors (--enable-warnings-as-errors is now incompatible with --enable-rust)

RESOLVED FIXED in Firefox 43

Status

defect
RESOLVED FIXED
4 years ago
a year ago

People

(Reporter: Ehsan, Assigned: njn)

Tracking

unspecified
mozilla43
Dependency tree / graph

Firefox Tracking Flags

(firefox43 fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

9:04.89 In file included from /Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/netwerk/wifi/Unified_mm_netwerk_wifi0.mm:2:
 9:04.89 /Users/ehsan/moz/src/netwerk/wifi/osx_corewlan.mm:42:44: error: instance method '-scanForNetworksWithParameters:error:' not found (return type defaults to 'id') [-Werror,-Wobjc-method-access]
 9:04.89     id scanResult = [[CWI_class interface] scanForNetworksWithParameters:nil error:nil];
 9:04.89                                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 9:04.90 /Users/ehsan/moz/src/netwerk/wifi/osx_corewlan.mm:62:34: error: instance method '-bssidData' not found (return type defaults to 'id'); did you mean '-ssidData'? [-Werror,-Wobjc-method-access]
 9:04.91         NSData* data = [anObject bssidData];
 9:04.91                                  ^~~~~~~~~
 9:04.91                                  ssidData
 9:04.91 /Users/ehsan/moz/src/netwerk/wifi/osx_corewlan.mm:88:29: error: instance method '-rssi' not found (return type defaults to 'id') [-Werror,-Wobjc-method-access]
 9:04.92         signal = [[anObject rssi] intValue];
 9:04.92                             ^~~~
This API has been removed in 10.7: <https://developer.apple.com/library/mac/releasenotes/General/APIDiffsMacOSX10_9/CoreWLAN.html> so trying to build with any OS X SDK other than 10.6 fails and --enable-warnings-as-errors fails.  This is bad because --enable-rust requires an OS X SDK > 10.6, which means that turning on rust is now incompatible with --enable-warnings-as-errors.
Summary: Build error in osx_corewlan.mm with --enable-macos-target=10.7 and --enable-warnings-as-errors → Build error in osx_corewlan.mm with --enable-macos-target=10.7 and --enable-warnings-as-errors (--enable-warnings-as-errors is now incompatible with --enable-rust)
This is done by permitting compiler warnings in netwerk/wifi when targeting
OS X SDKs that have removed the APIs used there.
Attachment #8661342 - Flags: review?(n.nethercote)
I'm getting this same error, but your patch doesn't fix it for me, presumably because I don't have the macos-target set. I do have |ac_add_options --enable-warnings-as-errors| but I don't have anything else interesting set in my mozconfig.
Getting rid of the |if| part of your patch fixes the build for me.
This is a slightly modified version of Ehsan's patch.
Attachment #8661467 - Flags: review?(n.nethercote)
Does using those APIs make any sense considering how few users of 10.6 we have? (also, how the hell does objc work? missing methods are _warnings_?)
(In reply to Mike Hommey [:glandium] from comment #6)
> Does using those APIs make any sense considering how few users of 10.6 we
> have?

I am not sure, but that's besides the point.  This bug is about fixing people's builds.  You should file a Necko bug if you want to find out the answers to that question!

> (also, how the hell does objc work? missing methods are _warnings_?)

In Objective C, you can call non-existing methods just fine.  They are implemented through message passing, and an object can choose to not respond to a message for reasons such as it not implementing the corresponding methods.
Assignee

Comment 8

4 years ago
I can reproduce this locally. I will try to find a more precise way to ignore these warnings, e.g. via -Wobjc-method-access.
Assignee: nobody → n.nethercote
-Wno-error=objc-method-access is one option, I guess.
(In reply to Ehsan Akhgari (don't ask for review please) from comment #7)
> (In reply to Mike Hommey [:glandium] from comment #6)
> > Does using those APIs make any sense considering how few users of 10.6 we
> > have?
> 
> I am not sure, but that's besides the point.  This bug is about fixing
> people's builds.

People should not be using --enable-warnings-as-errors.
Assignee

Comment 11

4 years ago
Let's carve out this exception with a scalpel rather than an axe :)
Attachment #8661507 - Flags: review?(mh+mozilla)
Assignee

Updated

4 years ago
Attachment #8661342 - Attachment is obsolete: true
Attachment #8661342 - Flags: review?(n.nethercote)
Assignee

Updated

4 years ago
Attachment #8661467 - Attachment is obsolete: true
Attachment #8661467 - Flags: review?(n.nethercote)
Assignee

Comment 12

4 years ago
Ehsan is right; -Wno-error=objc-method-access is even better.
Attachment #8661511 - Flags: review?(mh+mozilla)
Assignee

Updated

4 years ago
Attachment #8661507 - Attachment is obsolete: true
Attachment #8661507 - Flags: review?(mh+mozilla)
Assignee

Comment 13

4 years ago
> People should not be using --enable-warnings-as-errors.

I think it's a reasonable thing to do as long as you're building on tier 1 platforms with standard compilers. I've been doing it locally for the past few weeks.
(In reply to Nicholas Nethercote [:njn] from comment #13)
> > People should not be using --enable-warnings-as-errors.
> 
> I think it's a reasonable thing to do as long as you're building on tier 1
> platforms with standard compilers. I've been doing it locally for the past
> few weeks.

I have been doing it for years.  It's an entirely reasonable thing to do, if you're interested in catching new warnings that will be treated as errors locally first.  Also, this bug affects "people" and machines equally.
Attachment #8661511 - Flags: review?(mh+mozilla) → review+
(Following up comment #7)

> Does using those APIs make any sense considering how few users of 10.6 we have?

Our number of OS X 10.6 users is actually quite large -- currently about 17% of our Mac user base.
https://hg.mozilla.org/mozilla-central/rev/248f5a4b7e0e
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43

Updated

a year ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.