880254 - Move LOCAL_INCLUDES to moz.build

Status: RESOLVED INCOMPLETE

(Firefox Build System :: General, defect)

Comment 1

[(not currently active) Ted Mielczarek]

I was thinking about this at some point, and I believe we should not directly port the contents of LOCAL_INCLUDES, but have the contents of LOCAL_INCLUDES specify paths which the backend can translate into compiler include statements. So if a Makefile had something like:

LOCAL_INCLUDES += -I$(srcdir)/abc -I$(topsrcdir)/xyz

we could translate that into:
LOCAL_INCLUDES += [
    'abc',
    '/xyz',
    ]

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Patch to mozbuild-migrate

Applies on top of attachment 762127 [details] [diff] [review].

Comment 3

[Jeff Muizelaar [:jrmuizel]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #1)
> I was thinking about this at some point, and I believe we should not
> directly port the contents of LOCAL_INCLUDES, but have the contents of
> LOCAL_INCLUDES specify paths which the backend can translate into compiler
> include statements. So if a Makefile had something like:
> 
> LOCAL_INCLUDES += -I$(srcdir)/abc -I$(topsrcdir)/xyz
> 
> we could translate that into:
> LOCAL_INCLUDES += [
>     'abc',
>     '/xyz',
>     ]

One problem with this approach is that currently LOCAL_INCLUDES is used for both local includes and other system include paths that come from pkg-config for example. If we split those uses out separately then this would work.

Comment 4

[(not currently active) Ted Mielczarek]

That's a good point. Do we have any system include paths we're using like this that we couldn't simply pass globally?

Comment 5

[(no longer active)]

This is fixed now, right?

Comment 6

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

After bug 968631, BXR says there are 81 cases left.

Comment 7

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

I think ehsan owns this now :)

Comment 8

[(no longer active)]

I'm not planning to do any work inside this bug, and don't think keeping track of things in multiple places makes sense.  For the curious, follow along in bug 928196.