Open Bug 883502 Opened 7 years ago Updated Last year

Consider killing chromium-config.mk

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(Not tracked)

People

(Reporter: Ms2ger, Unassigned)

References

(Depends on 2 open bugs, Blocks 1 open bug)

Details

(Whiteboard: [leave open])

Attachments

(1 file, 2 obsolete files)

It adds some LOCAL_INCLUDES / DEFINES and defines some OS_* variables. We can probably move everything that's necessary somewhere central.
Blocks: 883503
Depends on: 883504
Depends on: 883538
This takes every include of chromium-config.mk and moves it to the line immediately after the rules.mk include. At a minimum, this makes it possible to go through and remove the config.mk includes that are only around for chromium-config.mk.

This was mostly mechanical, with the only interesting bits in ipc/chromium/Makefile.in

Try was surprisingly happy with this: https://tbpl.mozilla.org/?tree=Try&rev=a0e214ea0b52
Assignee: nobody → bokeefe
Status: NEW → ASSIGNED
Attachment #771398 - Flags: review?(gps)
In ipc/chromium/Makefile.in, can you reference the OS_* constants you want to be testing? That'll make it easier to move to the os_* constants in moz.build.
(In reply to :Ms2ger from comment #2)
> In ipc/chromium/Makefile.in, can you reference the OS_* constants you want
> to be testing? That'll make it easier to move to the os_* constants in
> moz.build.

Sure. I added the OS_* constants to the comments after each test.
Attachment #771398 - Attachment is obsolete: true
Attachment #771398 - Flags: review?(gps)
Attachment #771412 - Flags: review?(gps)
Comment on attachment 771412 [details] [diff] [review]
Part 1: Move chromium-config.mk includes after rules.mk

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

This looks fine by me.

I do worry that by including chromium-config.mk after rules.mk that we set ourselves up for the possibility of changes in chromium-config.mk not being reflected in rules.mk, possibly due to immediate assignment (:=) in rules.mk. However, given the content in chromium-configs.mk, I think things would break loudly if this were an issue. So, I think all is well.
Attachment #771412 - Flags: review?(gps) → review+
Rebased to tip; carried forward r+

(In reply to Gregory Szorc [:gps] from comment #4)
> I do worry that by including chromium-config.mk after rules.mk that we set
> ourselves up for the possibility of changes in chromium-config.mk not being
> reflected in rules.mk, possibly due to immediate assignment (:=) in
> rules.mk. However, given the content in chromium-configs.mk, I think things
> would break loudly if this were an issue. So, I think all is well.

I took a quick look, and none of the variables in chromium-config.mk are on the right-hand side of a := in rules.mk, so we should be okay for now. If we can remove chromium-config.mk entirely, it becomes a non-issue. I'll look into doing that.
Attachment #771412 - Attachment is obsolete: true
Attachment #776391 - Flags: review+
Keywords: checkin-needed
Whiteboard: [leave open]
Depends on: 914576
Depends on: 907683
Depends on: 928709
Depends on: 969932
Assignee: bokeefe → nobody
Product: Core → Firefox Build System
No assignee, updating the status.
Status: ASSIGNED → NEW
No assignee, updating the status.
No assignee, updating the status.
You need to log in before you can comment on or make changes to this bug.