Last Comment Bug 741289 - Sydneyaudio does not include MOZ_ALSA_CFLAGS
: Sydneyaudio does not include MOZ_ALSA_CFLAGS
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: Trunk
: ARM Linux
: -- normal (vote)
: mozilla14
Assigned To: prabindh
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-02 00:31 PDT by prabindh
Modified: 2012-04-12 10:12 PDT (History)
5 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
3 patches for config.in, autoconf.mk.in, and Makefile.in (650 bytes, application/octet-stream)
2012-04-02 00:31 PDT, prabindh
no flags Details
Unified patch for changes to enable passing of MOZ_ALSA_CFLAGS (1.00 KB, patch)
2012-04-02 10:32 PDT, prabindh
no flags Details | Diff | Splinter Review
hg patch updated per romaxa's comments (2.27 KB, patch)
2012-04-03 03:11 PDT, prabindh
no flags Details | Diff | Splinter Review
alignment to assignment line and cleanup for patch submission (1.87 KB, patch)
2012-04-03 06:24 PDT, prabindh
mh+mozilla: review+
romaxa: feedback+
Details | Diff | Splinter Review

Description prabindh 2012-04-02 00:31:11 PDT
Created attachment 611374 [details]
3 patches for config.in, autoconf.mk.in, and Makefile.in

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:10.0.1) Gecko/20100101 Firefox/10.0.1
Build ID: 20120209214325

Steps to reproduce:

Crosscompiled Mozilla embedded for ARM.


Actual results:

Error: alsa/asoundlib.h not found, coming from sydneyaudio/src compilation.


Expected results:

MOZ_ALSA_CFLAGS should be a dependency in libsydneyaudio make, as it is a dependent component. However, current autoconf.mk, as well as config.in, only "check" for ALSA, but do not pass it on. Hence if <CROSSCOMPILEROOT>/usr/include is not in the CFLAGS path, libsydneyaudio make fails. Most times we typically add <CROSSCOMPILEROOT>/usr/include globally to CFLAGS and hence this goes unnoticed. Attached patchset to fix this.
Comment 1 prabindh 2012-04-02 10:32:25 PDT
Created attachment 611512 [details] [diff] [review]
Unified patch for changes to enable passing of MOZ_ALSA_CFLAGS

Added :glandium" or ":ted" per inputs from :romaxa
Comment 2 Oleg Romashin (:romaxa) 2012-04-02 11:56:54 PDT
Comment on attachment 611512 [details] [diff] [review]
Unified patch for changes to enable passing of MOZ_ALSA_CFLAGS

One more thing,
In order to simplify patch integration, it is better to generate patch with hg export, or hg diff, also use -p -U 8
And add to header Bug number and description.
See for example https://bug738865.bugzilla.mozilla.org/attachment.cgi?id=609019
Comment 3 prabindh 2012-04-03 03:11:25 PDT
Created attachment 611759 [details] [diff] [review]
hg patch updated per romaxa's comments

In order to simplify patch integration, generated patch with hg diff, also used -p -U 8
And added to header Bug number and description.
Comment 4 Oleg Romashin (:romaxa) 2012-04-03 03:25:41 PDT
Comment on attachment 611759 [details] [diff] [review]
hg patch updated per romaxa's comments

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

> MOZ_ENABLE_LIBNOTIFY	= @MOZ_ENABLE_LIBNOTIFY@
> 
> MOZ_ALSA_LIBS           = @MOZ_ALSA_LIBS@
>+MOZ_ALSA_CFLAGS           = @MOZ_ALSA_CFLAGS@

Make this line indented with line above pls

Also I'm not able to review configure part, see https://wiki.mozilla.org/Modules/Activities#Module_Ownership_System
Ask review from glandium OR ted,  both are not needed for this patch
Comment 5 prabindh 2012-04-03 06:24:03 PDT
Created attachment 611788 [details] [diff] [review]
alignment to assignment line and cleanup for patch submission
Comment 6 Ryan VanderMeulen [:RyanVM] 2012-04-11 05:03:57 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/810e8295b080

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