Always embed manifests, even in component builds!

Always embed manifests, even in component builds!

This is finally possible following the conjunction of scottmg's work on embedding manifests as resources post-link and mine/caitkp's work on the version assembly manifest and its compatibility with the component build installer :)!

BUG=276953, 346843, 127233
TEST=
A) Installer still works in component builds
B) Self-destruct now works in component builds
C) Incremental linking still works for chrome_dll/unit_tests/browser_tests.

R=robertshield@chromium.org, scottmg@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=253534

[gab, 2014-02-25 23:01:14]

Scott/Robert PTAL; we are finally done with external manifests, bringing the
component build installer closer to the real installer (the main difference now
being that we still have to augment the version assembly manifest to refer to
the component DLLs -- and copy the component DLLs besides setup.exe in
Installer/, but I also have a crazy plan for that).

Greg/Cait: FYI.

W00000T! :)
Gab

[scottmg, 2014-02-25 23:04:23]

https://codereview.chromium.org/179903004/diff/20001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/179903004/diff/20001/build/common.gypi#newcod...
build/common.gypi:4713: 'EmbedManifest': 'false',
This should still be true, gyp still respects this setting and doesn't embed if
it's false.

[gab, 2014-02-25 23:08:34]

https://codereview.chromium.org/179903004/diff/20001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/179903004/diff/20001/build/common.gypi#newcod...
build/common.gypi:4713: 'EmbedManifest': 'false',
On 2014/02/25 23:04:23, scottmg wrote:
> This should still be true, gyp still respects this setting and doesn't embed
if
> it's false.

Indeed, whoops, also removed redundancy in remoting*.gypi now.

[scottmg, 2014-02-25 23:12:16]

lgtm, please confirm that the manifests do indeed get embedded, and that
touching a cc in unit_tests get rebuilding results in an incremental link.

[gab, 2014-02-25 23:15:05]

On 2014/02/25 23:12:16, scottmg wrote:
> lgtm, please confirm that the manifests do indeed get embedded, and that
> touching a cc in unit_tests get rebuilding results in an incremental link.

Yep already verified everything I put in TEST= with patch set 1; I'll re-verify
before landing tomorrow.

Note: I re-changed common.gypi for the setting to only be for executables as it
originally was.

[gab, 2014-02-25 23:19:16]

On 2014/02/25 23:15:05, gab wrote:
> On 2014/02/25 23:12:16, scottmg wrote:
> > lgtm, please confirm that the manifests do indeed get embedded, and that
> > touching a cc in unit_tests get rebuilding results in an incremental link.
> 
> Yep already verified everything I put in TEST= with patch set 1; I'll
re-verify
> before landing tomorrow.
> 
> Note: I re-changed common.gypi for the setting to only be for executables as
it
> originally was.

And of course this also means I have to re-add the explicit EmbedManifests rules
for the remoting DLLs.

[grt (UTC plus 2), 2014-02-25 23:48:13]

this is so awesome!

[robertshield, 2014-02-26 01:20:18]

LGTM, nice cleanup :-)

[gab, 2014-02-26 14:02:40]

The CQ bit was checked by gab@chromium.org

[commit-bot: I haz the power, 2014-02-26 14:03:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gab@chromium.org/179903004/90001

[commit-bot: I haz the power, 2014-02-26 14:03:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-26 14:03:40]

Failed to apply patch for tools/win/copy-installer.bat:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file tools/win/copy-installer.bat
  Hunk #1 FAILED at 51.
  1 out of 1 hunk FAILED -- saving rejects to file
tools/win/copy-installer.bat.rej

Patch:       tools/win/copy-installer.bat
Index: tools/win/copy-installer.bat
diff --git a/tools/win/copy-installer.bat b/tools/win/copy-installer.bat
index
506a4805bea572ddfef535d8aab25142c6eea4b6..d86876076b5068a3a09f544a48749c520dacaab5
100755
--- a/tools/win/copy-installer.bat
+++ b/tools/win/copy-installer.bat
@@ -51,7 +51,7 @@ SET TO=\%OUTPUT%\%BUILDTYPE%%ARCH%
 
 REM Figure out what files to copy based on the component type (shared/static).
 IF EXIST "%FROM%\base.dll" (
-SET TOCOPY=setup.exe setup.exe.manifest chrome.7z *.dll
+SET TOCOPY=setup.exe chrome.7z *.dll
 SET ARCHIVETODELETE=chrome.packed.7z
 SET INSTALLER=setup.exe
 ) ELSE (

[gab, 2014-02-26 14:07:45]

The CQ bit was checked by gab@chromium.org

[commit-bot: I haz the power, 2014-02-26 14:08:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gab@chromium.org/179903004/110001

[gab, 2014-02-26 19:09:53]

Committed patchset #7 manually as r253534 (presubmit successful).