1415742 - Move channel-prefs.js installation to moz.build and un-set DIST_SUBDIR for browser/app

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Chris Manchester (limited bugmail, email directly)]

Moving channel-prefs.js to moz.build was awkward because it didn't end up under dist/bin/browser. Bug 1255485 was awkward for the same reason, so we'll unset DIST_SUBDIR in browser/app and move stuff that needs it elsewhere.

Comment 1

[Chris Manchester (limited bugmail, email directly)]

Attachment: Bug 1415742 - Move channel-prefs.js installation to moz.build.

This was awkward because it doesn't want to end up under dist/bin/borwser,
but DIST_SUBDIR is exported here. firefox.exe doesn't want to end up under
dist/bin/browser either, so we unset DIST_SUBDIR for this directory and
move the files that need it to a different directory.

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

Comment 2

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8926661 [details]
Bug 1415742 - Move channel-prefs.js installation to moz.build.

https://reviewboard.mozilla.org/r/197906/#review203130

OK, this looks fine.  Consider commenting on why `browser/` reaches into `browser/app/`; that's not going to make sense to anybody reading this in the future (if and when `DIST_SUBDIR` dies).

::: commit-message-e2f87:3
(Diff revision 1)
> +Bug 1415742 - Move channel-prefs.js installation to moz.build.
> +
> +This was awkward because it doesn't want to end up under dist/bin/borwser,

s/borwser/browser/

::: browser/app/profile/channel-prefs.js:5
(Diff revision 1)
>  /* This Source Code Form is subject to the terms of the Mozilla Public
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
> +#filter substitution

I wonder if there are any preprocessed files that _don't_ want substitution.  Maybe we can get rid of this noise some day?

Comment 3

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8926661 [details]
Bug 1415742 - Move channel-prefs.js installation to moz.build.

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

Comment 4

[Chris Manchester (limited bugmail, email directly)]

I had to move some DEFINES as well to fix some test failures on try. Re-requesting review...

Comment 5

[(not currently active) Ted Mielczarek]

Comment on attachment 8926661 [details]
Bug 1415742 - Move channel-prefs.js installation to moz.build.

https://reviewboard.mozilla.org/r/197906/#review203458

::: browser/app/moz.build:44
(Diff revision 2)
>  SOURCES += [
>      'nsBrowserApp.cpp',
>  ]
>  
> -FINAL_TARGET_FILES += ['blocklist.xml']
> -FINAL_TARGET_FILES.defaults += ['permissions']
> +# Neither channel-prefs.js nor firefox.exe want to end up in dist/bin/browser.
> +DIST_SUBDIR = ""

super-nit: we generally use single quotes (although I see double quotes in the context in a `Files` subcontext above)

Comment 6

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8926661 [details]
Bug 1415742 - Move channel-prefs.js installation to moz.build.

https://reviewboard.mozilla.org/r/197906/#review203530

I'm fine with this.

::: browser/moz.build:44
(Diff revision 2)
>      EXTRA_COMPONENTS += [
>          '../build/prebuilt-interfaces.manifest',
>      ]
>  
> +
> +# These defines are read in firefox.js

nit: full sentence.

Comment 7

[(not currently active) Ted Mielczarek]

Comment on attachment 8926661 [details]
Bug 1415742 - Move channel-prefs.js installation to moz.build.

https://reviewboard.mozilla.org/r/197906/#review203532

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/44ad5267ace9
Move channel-prefs.js installation to moz.build. r=nalexander,ted

Comment 9

[Ryan VanderMeulen [:RyanVM][PTO June 24-28]]

https://hg.mozilla.org/mozilla-central/rev/44ad5267ace9