Closed Bug 1204919 Opened 5 years ago Closed 5 years ago

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)

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox43 fixed)

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: ehsan, Assigned: njn)

References

Details

Attachments

(1 file, 3 obsolete files)

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.
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.
Let's carve out this exception with a scalpel rather than an axe :)
Attachment #8661507 - Flags: review?(mh+mozilla)
Attachment #8661342 - Attachment is obsolete: true
Attachment #8661342 - Flags: review?(n.nethercote)
Attachment #8661467 - Attachment is obsolete: true
Attachment #8661467 - Flags: review?(n.nethercote)
Ehsan is right; -Wno-error=objc-method-access is even better.
Attachment #8661511 - Flags: review?(mh+mozilla)
Attachment #8661507 - Attachment is obsolete: true
Attachment #8661507 - Flags: review?(mh+mozilla)
> 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
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.