mingw build does not set GCC_VERSION.

RESOLVED FIXED in mozilla1.9.3a5

Status

Firefox Build System
General
RESOLVED FIXED
8 years ago
4 months ago

People

(Reporter: Jacek Caban, Assigned: Jacek Caban)

Tracking

Trunk
mozilla1.9.3a5
x86
Linux
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

8 years ago
Created attachment 436862 [details] [diff] [review]
fix

I noticed that after mingw build breakage from bug 551254. The obvious fix is attached. There are, however, two things to consider:

- Setting GCC_VERSION in configure script seems more like a side effect of version-depended configurations and there are other platforms that don't set it. Should it be cleaned up?

= The patch from bug 551254 is the only code that uses GCC_VERSION in such way. Shouldn't it use things that seem more standard like GNU_CC?
(Assignee)

Updated

8 years ago
Attachment #436862 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

8 years ago
Attachment #436862 - Attachment is patch: true
(Assignee)

Updated

8 years ago
Blocks: 551254
Attachment #436862 - Flags: review?(ted.mielczarek) → review+
Comment on attachment 436862 [details] [diff] [review]
fix

Yeah, that code probably should have used GNU_CXX. This is good for consistency anyway.
Assignee: nobody → jacek
(Assignee)

Comment 2

8 years ago
Created attachment 437811 [details] [diff] [review]
Don't use GCC_VERSION to detect GCC in makefiles.

Thanks for the review. I've attached a patch that removes use of GCC_VERSION. It does a bit more as it reverses the logic to 'not msvc' instead. I think that's a better solution.
Attachment #437811 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

8 years ago
Keywords: checkin-needed

Updated

8 years ago
Duplicate of this bug: 558576
(Assignee)

Comment 4

8 years ago
Created attachment 441020 [details] [diff] [review]
Don't use GCC_VERSION to detect GCC in makefiles. (rebased)

The second patch rebased against current m-c.
Attachment #437811 - Attachment is obsolete: true
Attachment #441020 - Flags: review?(ted.mielczarek)
Attachment #437811 - Flags: review?(ted.mielczarek)
Comment on attachment 441020 [details] [diff] [review]
Don't use GCC_VERSION to detect GCC in makefiles. (rebased)

I'd rather have the block in config/Makefile.in explicitly check for ifdef GNU_CXX instead of assuming gcc if we're not using MSVC. I know there are people actively using stuff like SunCC and ICC, we should make sure we don't break them.
Attachment #441020 - Flags: review?(ted.mielczarek) → review-
(Assignee)

Comment 6

8 years ago
Created attachment 442904 [details] [diff] [review]
Don't use GCC_VERSION to detect GCC in makefiles.

Thanks for the review, I've attached a fixed patch.
Attachment #441020 - Attachment is obsolete: true
Attachment #442904 - Flags: review?(ted.mielczarek)
Attachment #442904 - Flags: review?(ted.mielczarek) → review+
http://hg.mozilla.org/mozilla-central/rev/ca141b029e79
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a5
(Assignee)

Comment 8

8 years ago
Thanks for landing, but the bug is not yet fixed. Attachment 436862 [details] [diff] also requires landing.
Status: RESOLVED → REOPENED
Keywords: checkin-needed
Resolution: FIXED → ---
oops, please next time specify in the whiteboard what needs to land
the missing push
http://hg.mozilla.org/mozilla-central/rev/5e68d39a2b4a
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED

Updated

8 years ago
Keywords: checkin-needed

Updated

6 years ago
Blocks: 802343

Updated

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