Comment on attachment 8388118 [details] [diff] [review] Alternative approach Although this definitely improves the moz.build readability for this case, there is still the more general problem of needing -I flags on external source directories, correct? For example, we might also need a DIRECTX_INCLUDES for flags like this: ./gfx/angle/src/libEGL/Makefile.in:CXXFLAGS += -I'$(MOZ_DIRECTX_SDK_PATH)/include' Does it make sense to introduce *_INCLUDES moz.build variables for each type of external include directory we'd need? (I haven't done an exhaustive search, so maybe those are the only two...)

(In reply to comment #3) > Comment on attachment 8388118 [details] [diff] [review] > --> https://bugzilla.mozilla.org/attachment.cgi?id=8388118 > Alternative approach > > Although this definitely improves the moz.build readability for this case, > there is still the more general problem of needing -I flags on external source > directories, correct? For example, we might also need a DIRECTX_INCLUDES for > flags like this: > > ./gfx/angle/src/libEGL/Makefile.in:CXXFLAGS += > -I'$(MOZ_DIRECTX_SDK_PATH)/include' > > Does it make sense to introduce *_INCLUDES moz.build variables for each type of > external include directory we'd need? (I haven't done an exhaustive search, so > maybe those are the only two...) Why is this stuff needed at all? Is it just that we treat anything with a leading slash as a path name relative to the topsrcdir? Can't we just stat() to see if those directories exist in topsrcdir, and if not, fall back to not assuming that?

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #4) > Why is this stuff needed at all? Is it just that we treat anything with a > leading slash as a path name relative to the topsrcdir? Can't we just > stat() to see if those directories exist in topsrcdir, and if not, fall back > to not assuming that? Good point. I thought we did more with LOCAL_INCLUDES in mozbuild, but it's really just checking for a leading slash to make it relative to either topsrcdir or srcdir. Adding a stat() here doesn't add any discernible overhead, either. I guess the only downside is the word "LOCAL_" would lose its meaning :)

There is a graceful way to handle this: https://code.google.com/p/chromium/wiki/GNLanguage#Naming_things

(In reply to comment #8) > There is a graceful way to handle this: > https://code.google.com/p/chromium/wiki/GNLanguage#Naming_things WFM! It's actually a good idea I think. File a bug please?