1256604 - Get rid of toolkit/content/Makefile.in

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[(not currently active) Ted Mielczarek]

Right now toolkit/content/Makefile.in exists simply to set CXXFLAGS/CPPFLAGS in DEFINES for buildconfig.html:
https://hg.mozilla.org/mozilla-central/file/5e14887312d4523ab59c3f6c6c94a679cf42b496/toolkit/content/Makefile.in

I guess this is stuck here because the full value of $CXXFLAGS isn't computable without config.mk currently.

Comment 1

[(not currently active) Ted Mielczarek]

This is not super critical, in that it is just for about:buildconfig, but chmanchester: can we make this work more sensibly now with your CFLAGS work?

Comment 2

[Chris Manchester (limited bugmail, email directly)]

Yep, after bug 1403346 we should be able to do this from moz.build.

Comment 3

[Chris Manchester (limited bugmail, email directly)]

Attachment: Bug 1256604 - Remove toolkit/content/Makefile.in and move setting CXXFLAGS for buildconfig.html to moz.build

Review commit: https://reviewboard.mozilla.org/r/197494/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/197494/

Comment 4

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8926238 [details]
Bug 1256604 - Remove toolkit/content/Makefile.in and move setting CXXFLAGS for buildconfig.html to moz.build

https://reviewboard.mozilla.org/r/197494/#review202932

No flag change just to revisit this, or get someone who knows more to look at it.  Technically this looks fine.  But I don't understand a few things:
- Why are the original `CXXFLAGS` and `CPPFLAGS` not in the new set?
- We have some weird things happening in the tree, like http://searchfox.org/mozilla-central/source/browser/app/moz.build#118.  Does this global setting even make sense in the wild?

::: toolkit/content/moz.build:14
(Diff revision 1)
>  for var in ('target', 'MOZ_CONFIGURE_OPTIONS', 'CC', 'CC_VERSION', 'CXX'):
>      DEFINES[var] = CONFIG[var]
>  
>  DEFINES['CFLAGS'] = ' '.join(CONFIG['OS_CFLAGS'])
>  
> +cxx_flags = []

How do we make sure this set doesn't bit-rot as new places to stash flags are added?

Comment 5

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8926238 [details]
Bug 1256604 - Remove toolkit/content/Makefile.in and move setting CXXFLAGS for buildconfig.html to moz.build

https://reviewboard.mozilla.org/r/197494/#review202932

The build backend hasn't actually set `CPPFLAGS` for a while, configure takes `CPPFLAGS` and exposes them as `OS_CPPFLAGS`. `CXXFLAGS` isn't used by the backend anymore (it was a little weird, it never contained all the flags, but imposed some mostly irellevant order between flags sets), the flags added in this patch reflect what used to be in `CXXFLAGS`.

> How do we make sure this set doesn't bit-rot as new places to stash flags are added?

We can either expose all the flags, potentially a larger set than was here originally, or make a case-by-case decision, and do our best not to add categories of flags, what we have already is probably more than necessary.

Comment 6

[Michael Shal [:mshal]]

Comment on attachment 8926238 [details]
Bug 1256604 - Remove toolkit/content/Makefile.in and move setting CXXFLAGS for buildconfig.html to moz.build

https://reviewboard.mozilla.org/r/197494/#review203406

::: toolkit/content/buildconfig.html:53
(Diff revision 1)
>        <td>@CFLAGS@</td>
>      </tr>
>      <tr>
>        <td>@CXX@</td>
>        <td>@CC_VERSION@</td>
>  #ifndef BUILD_FASTER

You can remove this #ifndef (and fixup this comment:  https://dxr.mozilla.org/mozilla-central/rev/2535bad09d720e71a982f3f70dd6925f66ab8ec7/config/faster/rules.mk#63) since this now builds in the faster make backend.

Comment 7

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8926238 [details]
Bug 1256604 - Remove toolkit/content/Makefile.in and move setting CXXFLAGS for buildconfig.html to moz.build

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/197494/diff/1-2/

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7908ec3fc4af
Remove toolkit/content/Makefile.in and move setting CXXFLAGS for buildconfig.html to moz.build r=mshal

Comment 9

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/7908ec3fc4af