MSVC version detection is broken CC is set to a wrapper (like sccache)

RESOLVED FIXED in 4.10.9

Status

NSPR
NSPR
P2
normal
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: Jacek Caban, Assigned: Jacek Caban)

Tracking

other
4.10.9
x86
Windows
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

3 years ago
Created attachment 8599843 [details] [diff] [review]
fix

I discovered it when working on bug 856404. MSVC version detection is broken when CC is set to a wrapper script, which means that current m-c builds get it wrong, because official builds use sccache. CC is set like this:

CC=python2.7 c:/builds/moz2_slave/try-w32-d-00000000000000000000/build/src/sccache/sccache.py cl

And version detection is based on such invocation:

CC_VERSION=`"${CC}" -v 2>&1 | sed -ne "$_MSVC_VER_FILTER"`

Note the quoting. The whole value of CC will be interpreted as a single file name. "${CC}" -v returns something like:

c:/builds/moz2_slave/try-w32-d-00000000000000000000/build/src/nsprpub/configure: line 7097: python2.7 c:/builds/moz2_slave/try-w32-d-00000000000000000000/build/src/sccache/sccache.py cl: No such file or directory

The whole build works only by accident, because following errors are not critical.

The simple and least invasive fix is dropping quotes around ${CC}. I think that's how it's interpreted in every other place, so it should be safe. The attached patch also adds a simple check to ensure that version is detected to avoid such problems in the future.
Attachment #8599843 - Flags: review?(ted)
(Assignee)

Comment 1

3 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cfe9f0523c7e
Attachment #8599843 - Flags: review?(ted) → review+
(Assignee)

Comment 2

3 years ago
Created attachment 8604070 [details] [diff] [review]
patch for checkin

Thanks for the review.
Attachment #8604070 - Flags: checked-in?
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
Whiteboard: [NSPR checkin]
wtc, can you please help get this NSPR patch landed?
Flags: needinfo?(wtc)

Comment 4

2 years ago
Comment on attachment 8604070 [details] [diff] [review]
patch for checkin

Review of attachment 8604070 [details] [diff] [review]:
-----------------------------------------------------------------

Patch checked in: https://hg.mozilla.org/projects/nspr/rev/c4194dc04aa5
Attachment #8604070 - Flags: review+
Attachment #8604070 - Flags: checked-in?
Attachment #8604070 - Flags: checked-in+

Updated

2 years ago
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Flags: needinfo?(wtc)
Keywords: checkin-needed
Priority: -- → P2
Resolution: --- → FIXED
Whiteboard: [NSPR checkin]
Target Milestone: --- → 4.10.9
Blocks: 1190607
You need to log in before you can comment on or make changes to this bug.