557000 - mingw build does not set GCC_VERSION.

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Jacek Caban]

Attachment: 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?

Comment 1

[(not currently active) Ted Mielczarek]

Comment on attachment 436862 [details] [diff] [review]
fix

Yeah, that code probably should have used GNU_CXX. This is good for consistency anyway.

Comment 2

[Jacek Caban]

Attachment: 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.

Comment 4

[Jacek Caban]

Attachment: Don't use GCC_VERSION to detect GCC in makefiles. (rebased)

The second patch rebased against current m-c.

Comment 5

[(not currently active) 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.

Comment 6

[Jacek Caban]

Attachment: Don't use GCC_VERSION to detect GCC in makefiles.

Thanks for the review, I've attached a fixed patch.

Comment 7

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/ca141b029e79

Comment 8

[Jacek Caban]

Thanks for landing, but the bug is not yet fixed. Attachment 436862 [details] [diff] also requires landing.

Comment 9

[Marco Bonardo [:mak]]

oops, please next time specify in the whiteboard what needs to land

Comment 10

[Marco Bonardo [:mak]]

the missing push
http://hg.mozilla.org/mozilla-central/rev/5e68d39a2b4a