Last Comment Bug 777166 - correct cflags/libs for system libvpx
: correct cflags/libs for system libvpx
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: x86_64 FreeBSD
: -- normal (vote)
: mozilla17
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-24 16:52 PDT by bugmenot
Modified: 2012-07-27 08:57 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
actually use flags from pkg-config (2.14 KB, patch)
2012-07-24 16:52 PDT, bugmenot
no flags Details | Diff | Review
Normalize VPX pkg-config variable and restore LIBS after the check (2.43 KB, patch)
2012-07-25 13:05 PDT, bugmenot
tterribe: review+
tterribe: checkin+
Details | Diff | Review

Description bugmenot 2012-07-24 16:52:54 PDT
Created attachment 645589 [details] [diff] [review]
actually use flags from pkg-config

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
Comment 1 Kyle Huey [:khuey] (khuey@mozilla.com) (Away until 6/13) 2012-07-25 08:17:25 PDT
Comment on attachment 645589 [details] [diff] [review]
actually use flags from pkg-config

I'm going to delegate this review to derf.
Comment 2 Timothy B. Terriberry (:derf) 2012-07-25 08:31:08 PDT
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.
Comment 3 bugmenot 2012-07-25 13:05:31 PDT
Created attachment 645864 [details] [diff] [review]
Normalize VPX pkg-config variable and restore LIBS after the check

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.
Comment 4 Timothy B. Terriberry (:derf) 2012-07-26 19:02:29 PDT
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.
Comment 5 :Ehsan Akhgari (busy, don't ask for review please) 2012-07-27 08:57:36 PDT
https://hg.mozilla.org/mozilla-central/rev/c4fa4a471932

Note You need to log in before you can comment on or make changes to this bug.