1354785 - Changing manifest files do not update embedded manifest in binary files

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Masatoshi Kimura [:emk]]

Steps to reproduce:
1. Edit browser/app/firefox.exe.64.manifest.
2. mach build binaries
3. Open the built firefox.exe with a binary editor and inspect the embedded manifest.

Actual result:
The embedded manifest does not reflect the change.

Expected result:
The embedded manifest should be updated.

Bug 1348069 regressed this. I found this when I was testing bug 1354020.

Comment 1

[Masatoshi Kimura [:emk]]

Attachment: Bug 1354785 - Add dependency on manifest files to res files.

Before bug 1348069, MS manifest tool was used to embed manifest files.[1]

The Makefile used to use EXTRA_DEPS to invoke the manifest tool when a manifest files is changed. But it is no longer effective because the manifest file namepattern is no longer $@.exe.manifest.

Now manifest files will be embedded via .res files. So we have to rebuild .res files to update embedded manifests.

[1] https://dxr.mozilla.org/mozilla-central/rev/35c7be9c2db288d1d449e3cc586c4164d642c5fd/config/rules.mk#642-655

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

Comment 2

[Masatoshi Kimura [:emk]]

Ping?

This bug may burn the tree every time someone changes manifest files.

Comment 3

[Gregory Szorc [:gps]]

I'll review this now.

Comment 4

[Gregory Szorc [:gps]]

Comment on attachment 8856092 [details]
Bug 1354785 - Add dependency on manifest files to res files.

https://reviewboard.mozilla.org/r/128038/#review133044

We would ideally have a variable that was directly added to $(RESFILE)s dependencies. But it isn't worth scope bloating here. We can deal with this as part of eventually porting this logic to moz.build.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c39e811e3a09
Add dependency on manifest files to res files. r=gps

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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