Comment on attachment 8376895 [details] [diff] [review] Move the LOCAL_INCLUDES in media to moz.build Review of attachment 8376895 [details] [diff] [review]: ----------------------------------------------------------------- ::: media/webrtc/signaling/test/moz.build @@ +20,5 @@ > 'NR_SOCKET_IS_VOID_PTR', 'HAVE_STRDUP'): > DEFINES[var] = True > + > +GENERATED_INCLUDES += [ > + '.', I don't think this is necessary

(In reply to :Ms2ger from comment #3) > Comment on attachment 8376895 [details] [diff] [review] > Move the LOCAL_INCLUDES in media to moz.build > > Review of attachment 8376895 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: media/webrtc/signaling/test/moz.build > @@ +20,5 @@ > > 'NR_SOCKET_IS_VOID_PTR', 'HAVE_STRDUP'): > > DEFINES[var] = True > > + > > +GENERATED_INCLUDES += [ > > + '.', > > I don't think this is necessary This is not trivial to figure out, and I have been bitten by this once already (see bug 972459.) Please file a follow-up if you want, but I'm not volunteering to figure out which one of these includes are actually necessary. :-)

Comment on attachment 8377194 [details] [diff] [review] Move the LOCAL_INCLUDES in media to moz.build Review of attachment 8377194 [details] [diff] [review]: ----------------------------------------------------------------- r+ with the following fixed. ::: media/mtransport/test/moz.build @@ +41,5 @@ > + ] > + > +if CONFIG['OS_TARGET'] == 'Darwin': > + LOCAL_INCLUDES += [ > + '/media/mtransport/third_party/nrappkit/src/port/darwin/include', This path used to be added for the BSDs too. ::: media/webrtc/signaling/test/moz.build @@ +20,5 @@ > 'NR_SOCKET_IS_VOID_PTR', 'HAVE_STRDUP'): > DEFINES[var] = True > + > +GENERATED_INCLUDES += [ > + '.', Adding '.' is useless. @@ +66,5 @@ > + ] > + > +if CONFIG['OS_TARGET'] == 'Darwin': > + LOCAL_INCLUDES += [ > + '/media/mtransport/third_party/nrappkit/src/port/darwin/include', Same as media/mtransport/test/moz.build

(In reply to :Ehsan Akhgari (needinfo? me!) (slow responsiveness, emailapocalypse) from comment #5) > This is not trivial to figure out, and I have been bitten by this once > already (see bug 972459.) > > Please file a follow-up if you want, but I'm not volunteering to figure out > which one of these includes are actually necessary. :-) Here, it's easy: -I. is already in INCLUDES, before LOCAL_INCLUDES. All adding it to LOCAL_INCLUDES was doing was to put it twice on the command line.

(In reply to Mike Hommey [:glandium] from comment #7) > (In reply to :Ehsan Akhgari (needinfo? me!) (slow responsiveness, > emailapocalypse) from comment #5) > > This is not trivial to figure out, and I have been bitten by this once > > already (see bug 972459.) > > > > Please file a follow-up if you want, but I'm not volunteering to figure out > > which one of these includes are actually necessary. :-) > > Here, it's easy: -I. is already in INCLUDES, before LOCAL_INCLUDES. All > adding it to LOCAL_INCLUDES was doing was to put it twice on the command > line. Ah I was confused because I did not note that this is GENERATED_INCLUDES, not LOCAL_INCLUDES :/