Closed
Bug 777166
Opened 12 years ago
Closed 12 years ago
correct cflags/libs for system libvpx
Categories
(Firefox Build System :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla17
People
(Reporter: bugmenot, Unassigned)
Details
Attachments
(1 file, 1 obsolete file)
2.43 KB,
patch
|
derf
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:17.0) Gecko/17.0 Firefox/17.0 Build ID: 20120725002704 Steps to reproduce: pkg-config-0.26 ./configure --with-system-libvpx Actual results: 1. MOZ_LIBVPX_INCLUDES and MOZ_LIBVPX_LIBS are never populated 2. -lvpx is propagated to OS_LIBS and so not only libxul.so but firefox (app binary) ends up being linked against -lvpx (which it doesn't use) Expected results: 1. MOZ_LIBVPX_(INCLUDES|LIBS) should get CFLAGS from pkg-config 2. CFLAGS/LIBS should be restored after PKG_CHECK_MODULES
Attachment #645589 -
Attachment is patch: true
Attachment #645589 -
Flags: review?(khuey)
Attachment #645589 -
Flags: feedback?(stransky)
Comment on attachment 645589 [details] [diff] [review] actually use flags from pkg-config I'm going to delegate this review to derf.
Attachment #645589 -
Flags: review?(khuey) → review?(tterribe)
Comment 2•12 years ago
|
||
Comment on attachment 645589 [details] [diff] [review] actually use flags from pkg-config >+_SAVE_CFLAGS=$CFLAGS >+_SAVE_LDFLAGS=$LDFLAGS >+_SAVE_LIBS=$LIBS ... >+CFLAGS=$_SAVE_CFLAGS >+LDFLAGS=$_SAVE_LDFLAGS >+LIBS=$_SAVE_LIBS Can you explain to me what this is needed for? There are dozens of places we use the macros contained in this construct without these guards.
After adding a few printf's I've tracked spurious -lvpx to AC_CHECK_LIB. So, you're right, those guards are unnecessary, except LIBS.
Attachment #645589 -
Attachment is obsolete: true
Attachment #645589 -
Flags: review?(tterribe)
Attachment #645589 -
Flags: feedback?(stransky)
Attachment #645864 -
Flags: review?(tterribe)
Updated•12 years ago
|
Attachment #645864 -
Flags: review?(tterribe) → review+
Attachment #645864 -
Flags: checkin?(tterribe)
Comment 4•12 years ago
|
||
Comment on attachment 645864 [details] [diff] [review] Normalize VPX pkg-config variable and restore LIBS after the check Greenish on try: https://tbpl.mozilla.org/?tree=Try&rev=3663e5f77560 https://hg.mozilla.org/integration/mozilla-inbound/rev/c4fa4a471932 bugmenot: Just a note for next time, if you could follow the guidelines in http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed for the commit message, that would be super-helpful. Less thinking for me.
Attachment #645864 -
Flags: checkin?(tterribe) → checkin+
Comment 5•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c4fa4a471932
Status: UNCONFIRMED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•