883502 - Consider killing chromium-config.mk

Status: NEW

(Firefox Build System :: General, defect)

Description

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

It adds some LOCAL_INCLUDES / DEFINES and defines some OS_* variables. We can probably move everything that's necessary somewhere central.

Comment 1

[Brian O'Keefe [:bokeefe]]

Attachment: Part 1: Move chromium-config.mk includes after rules.mk

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

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+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.

Comment 3

[Brian O'Keefe [:bokeefe]]

Attachment: Part 1: Move chromium-config.mk includes after rules.mk

(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.

Comment 4

[Gregory Szorc [: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.

Comment 5

[Brian O'Keefe [:bokeefe]]

Attachment: Part 1: Move chromium-config.mk includes after rules.mk (r=gps), for checkin

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.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d5d03b2c7fab

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d5d03b2c7fab

Comment 8

[Sylvestre Ledru [:Sylvestre]]

No assignee, updating the status.

Comment 9

[Sylvestre Ledru [:Sylvestre]]

No assignee, updating the status.

Comment 10

[Sylvestre Ledru [:Sylvestre]]

No assignee, updating the status.