Closed Bug 599567 Opened 14 years ago Closed 14 years ago

configure.in: fix YASM detection on Win32 and COMPILE_ENVIRONMENT check on all platforms

Categories

(Firefox Build System :: General, defect)

Product:
Firefox Build System
Component:
General
Type:
defect
Priority:
Not set
Severity:
major

Tracking

(blocking2.0 beta7+)

Status:
RESOLVED FIXED
Milestone:
mozilla2.0b7
Tracking Flags:
Tracking Status
blocking2.0 --- beta7+

People

(Reporter: sgautherie, Assigned: sgautherie)

References

(
URL
)

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

(Av1) Add explicit "no yasm" check, Reverse "min. version" check, Unregress 'COMPILE_ENVIRONMENT' spelling [Checked in: Comment 5]
2.58 KB, patch
khuey
: review+
Details | Diff | Splinter Review
(Cv2) Fix unrelated WINDRES_VERSION checks, Fix previous ("B") patch, Add missing 'COMPILE_ENVIRONMENT' check
4.21 KB, patch
ted
: review-
Details | Diff | Splinter Review
No description provided.
Flags: in-testsuite-
Wow, I messed this up pretty bad, didn't I? Thanks for the fix, Serge.
Ugh, this should block because it breaks localizers.
blocking2.0: --- → ?
Yes, localization is important.
blocking2.0: ? → beta7+
Attachment #478492 - Flags: review?(ted.mielczarek) → review+
http://hg.mozilla.org/mozilla-central/rev/60611631d69c Thanks Serge!
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Attachment #478492 - Attachment description: (Av1) Add explicit "no yasm" check, Reverse "min. version" check, Unregress 'COMPILE_ENVIRONMENT' spelling → (Av1) Add explicit "no yasm" check, Reverse "min. version" check, Unregress 'COMPILE_ENVIRONMENT' spelling [Checked in: Comment 5]
Severity: normal → major
Summary: configure.in: fix yasm detection on WINNT:x86 → configure.in: fix YASM detection on Win32 and COMPILE_ENVIRONMENT check on all platforms
Also pushed http://hg.mozilla.org/mozilla-central/rev/71e8b5aee972 because the Win32 specific checks need to be conditioned on COMPILE_ENVIRONMENT too.
(In reply to comment #6) > Also pushed http://hg.mozilla.org/mozilla-central/rev/71e8b5aee972 because the > Win32 specific checks need to be conditioned on COMPILE_ENVIRONMENT too. That changeset looks odd to me: *The version check misses added parenthesis. *Shouldn't the COMPILE_ENVIRONMENT check just be on the GNU_CC check line? Or even around the whole "case ... esac" block?
I didn't check everything and I'm not very familiar with '-z COMPILE_ENVIRONMENT' case, but this looks fine in the '-n COMPILE_ENVIRONMENT' case: please, double-check.
Attachment #478524 - Flags: review?(khuey)
Comment on attachment 478524 [details] [diff] [review] (Cv1) Revert previous ("B") patch, Move 'COMPILE_ENVIRONMENT' check to protect whole MOZ_WEBM block I don't like this because it changes the value of variables like MOZ_MEDIA, MOZ_WEBM, etc based on COMPILE_ENVIRONMENT, but I'll pass the call along to ted.
Attachment #478524 - Flags: review?(khuey) → review?(ted.mielczarek)
Cv1, with comment 9 suggestion(s), plus an (un)related fix.
Attachment #478524 - Attachment is obsolete: true
Attachment #479619 - Flags: review?(khuey)
Attachment #478524 - Flags: review?(ted.mielczarek)
Attachment #479619 - Flags: review?(khuey) → review?(ted.mielczarek)
Comment on attachment 479619 [details] [diff] [review] (Cv2) Fix unrelated WINDRES_VERSION checks, Fix previous ("B") patch, Add missing 'COMPILE_ENVIRONMENT' check Why don't you just wrap this entire block in test -n COMPILE_ENVIRONMENT? None of it is necessary in that situation.
Comment on attachment 479619 [details] [diff] [review] (Cv2) Fix unrelated WINDRES_VERSION checks, Fix previous ("B") patch, Add missing 'COMPILE_ENVIRONMENT' check r- per my previous comment.
Attachment #479619 - Flags: review?(ted.mielczarek) → review-
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: