Make setup.exe compatible with the component build.

Make setup.exe compatible with the component build.

The problem with the current component build is that chrome.exe and setup.exe only know to look for DLLs like base.dll in the current directory (except chrome.dll for which they're hardcoded to know where to look). On an install those DLLs are in the version directory so chrome.exe and setup.exe fail to run not finding required DLLs...

To fix this we generate config files (to point in the version directory) and manifests (to list all the DLL dependencies explicitly) to be installed  beside the exes.

Each DLL also has a manifest in the version directory to give it an "assemblyIdentity" (i.e. a unique name which is referred to as a dependency by each exe's manifest).

In order for external manifests to work, embedded manifests had to be disabled for component builds (otherwise Windows ignores the external one).

Since embedded manifests are disabled and the generated manifests are only generated in the archive to be extracted at install-time. This CL also adds a copy step for the usually embedded manifests to be dropped in the build directory so that chrome.exe and setup.exe are usable from there without requiring an install.

This doesn't make mini_installer.exe compatible with the component build yet (as mini_installer runs setup.exe right after extracting it and without any other DLLs/manifests beside it).

However it is now possible to use setup.exe (which takes the exact same parameters as mini_installer.exe) from the build output directory with a component build :)!!!

BUG=127233
TEST=Turn on component build with gyp config "component=shared_libary" and make sure we can install chrome with setup.exe.
Make sure we can run the installed Chrome.
Make sure we can uninstall the installed Chrome (i.e. that setup.exe in <version_dir>/Installer is able to find its DLLs).

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=137118

[gab, 2012-03-14 23:53:36]

This makes sure that base.dll and all other DLLs in a component build end up in
the installer archive.

This doesn't quite work yet though... as:

I noticed that the DLLs are correctly copied to:
chrome/installer/Debug/obj/mini_installer/temp_installer_archive/Chrome-bin/
when building the mini-installer solution.

But they are not copied to:
build/Debug/obj/mini_installer/temp_installer_archive/Chrome-bin/
where the regular non-component files still live (and actually even the
timestamp is from yesterday)

Also, it seems wrong that we have the build in two places... is there actually a
bug with the build's file placement/dependencies or is there something I don't
fully understand? The sole fact that the timestamps on those files weren't being
updated when I clearly changed things is clearly wrong.
maruel@ was also suggesting earlier today that it seems wrong that the installer
has its own build output directory (which from a quick `ls` seems to contain a
duplicate of everything in the global output directory (build\Debug)


I will try a full clean build overnight (after rm -rf build\Debug) to see if the
DLLs do end up in the installer archive (as it is now they don't seem to be as
the installer still complains about a missing base.dll).

In the mean time, please take a look at the changes themselves.

Cheers,
Gab

[robertshield, 2012-03-15 01:09:45]

Taking a look at the change in just a minute, some comments inline:

On Wed, Mar 14, 2012 at 7:53 PM, <gab@chromium.org> wrote:

> Reviewers: robertshield,
>
> Message:
> This makes sure that base.dll and all other DLLs in a component build end
> up in
> the installer archive.
>
> This doesn't quite work yet though... as:
>
> I noticed that the DLLs are correctly copied to:
> chrome/installer/Debug/obj/**mini_installer/temp_installer_**
> archive/Chrome-bin/
> when building the mini-installer solution.


> But they are not copied to:
> build/Debug/obj/mini_**installer/temp_installer_**archive/Chrome-bin/
> where the regular non-component files still live (and actually even the
> timestamp is from yesterday)
>
> Also, it seems wrong that we have the build in two places... is there
> actually a
> bug with the build's file placement/dependencies or is there something I
> don't
> fully understand?


My guess is that this is a side-effect of building from mini_installer.sln.

The correct location for the files is build/Debug, not
chrome/installer/Debug.


> The sole fact that the timestamps on those files weren't being
> updated when I clearly changed things is clearly wrong.

maruel@ was also suggesting earlier today that it seems wrong that the
> installer
> has its own build output directory (which from a quick `ls` seems to
> contain a
> duplicate of everything in the global output directory (build\Debug)
>

All individual .sln files have their own output directory, though many of
them may inherit a property that points it to build\Debug. My guess is that
the path is not being correctly propagated to mini_installer.gyp for some
reason.


>
>
> I will try a full clean build overnight (after rm -rf build\Debug) to see
> if the
> DLLs do end up in the installer archive (as it is now they don't seem to
> be as
> the installer still complains about a missing base.dll).
>
>
If you build mini_installer.exe from all.sln, things should to the right
place.


> In the mean time, please take a look at the changes themselves.
>
> Cheers,
> Gab
>
> Description:
> Make the mini_installer compatible with component build.
>
> BUG=
> TEST=Turn on component build with gyp config "component=shared_libary" and
> make
> sure the install is scuessful.
>
>
> Please review this at
https://chromiumcodereview.**appspot.com/9701050/<https://chromiumcodereview....
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
>
> Affected files:
>  M chrome/installer/mini_**installer.gyp
>  M chrome/installer/mini_**installer/chrome.release
>  M chrome/tools/build/win/create_**installer_archive.py
>
>
> Index: chrome/installer/mini_**installer.gyp
> diff --git a/chrome/installer/mini_**installer.gyp
> b/chrome/installer/mini_**installer.gyp
> index 8ce8228951a41abc0c7c01a913a157**c141566bb8..**
> d0aea29e7f7827cd9bdf27f630df20**f0a4659c22 100644
> --- a/chrome/installer/mini_**installer.gyp
> +++ b/chrome/installer/mini_**installer.gyp
> @@ -203,6 +203,7 @@
>                 '<(PRODUCT_DIR)/nacl_irt_x86_**64.nexe',
>                 '<(PRODUCT_DIR)/locales/en-US.**pak',
>                 '<(PRODUCT_DIR)/icudt.dll',
> +                '<(is_component_build)',
>               ],
>               'outputs': [
>                 'xxx.out',
> @@ -225,12 +226,24 @@
>                 #'--last_chrome_installer=C:/**Temp/base',
>                 #'--setup_exe_format=DIFF',
>                 #'--diff_algorithm=COURGETTE',
> +                '--component_build', '<(is_component_build)',
>               ],
>               'message': 'Create installer archive'
>             },
>           ],
>         },
>       ],
> +      'conditions': [
> +        [ 'component == "shared_library"', {
> +          'variables': {
> +            'is_component_build': 'True',
> +          },
> +        }, { # else normal build with a single chrome.dll
> +          'variables': {
> +            'is_component_build': 'False',
> +          },
> +        }],
> +      ],
>     }],
>     [ 'branding == "Chrome"', {
>       'variables': {
> Index: chrome/installer/mini_**installer/chrome.release
> diff --git a/chrome/installer/mini_**installer/chrome.release
> b/chrome/installer/mini_**installer/chrome.release
> index 9aede424adb95a48c38d0ac9a9a368**c5a6aefb3d..**
> 1334114e0d9b41fc043149e03ddaf2**155b3319d7 100644
> --- a/chrome/installer/mini_**installer/chrome.release
> +++ b/chrome/installer/mini_**installer/chrome.release
> @@ -59,3 +59,6 @@ default_apps\external_**extensions.json:
> %(VersionDir)s\default_apps\
>  gcswf32.dll: %(VersionDir)s\
>  plugin.vch: %(VersionDir)s\
>  FlashPlayerCPLApp.cpl: %(VersionDir)s\
> +
> +[COMPONENT]
> +*.dll: %(VersionDir)s\
> \ No newline at end of file
> Index: chrome/tools/build/win/create_**installer_archive.py
> diff --git a/chrome/tools/build/win/**create_installer_archive.py
> b/chrome/tools/build/win/**create_installer_archive.py
> index a7adbba5058e3748d21be3e57bf03b**ec8899e6ea..**
> 8a2139de33342666ec8fe6158ea0ce**9b7a24f900 100755
> --- a/chrome/tools/build/win/**create_installer_archive.py
> +++ b/chrome/tools/build/win/**create_installer_archive.py
> @@ -89,7 +89,8 @@ def CompressUsingLZMA(build_dir, compressed_file,
> input_file):
>   RunSystemCommand(cmd)
>
>
> -def CopyAllFilesToStagingDir(**config, distribution, staging_dir,
> build_dir):
> +def CopyAllFilesToStagingDir(
> +    config, distribution, component_build, staging_dir, build_dir):
>   """Copies the files required for installer archive.
>   Copies all common files required for various distributions of Chromium
> and
>   also files for the specific Chromium build specified by distribution.
> @@ -101,6 +102,9 @@ def CopyAllFilesToStagingDir(**config, distribution,
> staging_dir, build_dir):
>     CopySectionFilesToStagingDir(**config, distribution.upper(),
>                                  staging_dir, build_dir)
>
> +  if component_build:
> +    CopySectionFilesToStagingDir(**config, "COMPONENT", staging_dir,
> build_dir)
> +
>
>  def CopySectionFilesToStagingDir(**config, section, staging_dir,
> build_dir):
>   """Copies installer archive files specified in section to staging dir.
> @@ -324,11 +328,13 @@ def main(options):
>   # building the optimized mini_installer.
>   if options.build_dir != options.output_dir:
>     CopyAllFilesToStagingDir(**config, options.distribution,
> -                             staging_dir, options.output_dir)
> +                             options.component_build, staging_dir,
> +                             options.output_dir)
>
>   # Now copy the remainder of the files from the build dir.
>   CopyAllFilesToStagingDir(**config, options.distribution,
> -                           staging_dir, options.build_dir)
> +                           options.component_build, staging_dir,
> +                           options.output_dir)
>
>   version_numbers = current_version.split('.')
>   current_build_number = version_numbers[2] + '.' + version_numbers[3]
> @@ -380,6 +386,8 @@ def _ParseOptions():
>            '{BSDIFF|COURGETTE}.')
>   parser.add_option('-n', '--output_name', default='chrome',
>       help='Name used to prefix names of generated archives.')
> +  parser.add_option('-c', '--component_build', default='False',
> +      help='Install a component build.')
>
>   options, args = parser.parse_args()
>   if not options.build_dir:
>
>
>

[gab, 2012-03-15 16:54:59]

So building mini_installer.sln from a clean build\Debug doesn't put obj\* back
in build\Debug which shows that your intuition that mini_installer.sln doesn't
point to the correct output dir was correct (I'll look into that).

This patch works however in that I unzipped the archives and all the DLLs are
there. I ran mini_installer.exe and correctly had all the DLLs land in
%USERPROFILE%\AppData\Local\Chromium\Application\19.0.1070.0

The problem now is that chrome.exe doesn't know to look up in .\19.0.1070.0 and
crashes with "base.dll not found". I suspect this is because in the debug build,
the DLLs are typically found in the same directory (build\Debug) as chrome.exe.

I see two solutions:
1) Make the --component_build=True flag I just created copy the DLLs directly in
%(ChromeDir)s\ instead of %(VersionDir)s\
2) Make chrome.exe also look in %(VersionDir)s\ for the DLLs in a component
build.

(1) is the easiest I think, but is less clean, let me know what you prefer.

[gab, 2012-03-15 17:01:13]

On 2012/03/15 16:54:59, gab wrote:
> So building mini_installer.sln from a clean build\Debug doesn't put obj\* back
> in build\Debug which shows that your intuition that mini_installer.sln doesn't
> point to the correct output dir was correct (I'll look into that).
> 
> This patch works however in that I unzipped the archives and all the DLLs are
> there. I ran mini_installer.exe and correctly had all the DLLs land in
> %USERPROFILE%\AppData\Local\Chromium\Application\19.0.1070.0
> 
> The problem now is that chrome.exe doesn't know to look up in .\19.0.1070.0
and
> crashes with "base.dll not found". I suspect this is because in the debug
build,
> the DLLs are typically found in the same directory (build\Debug) as
chrome.exe.
> 
> I see two solutions:
> 1) Make the --component_build=True flag I just created copy the DLLs directly
in
> %(ChromeDir)s\ instead of %(VersionDir)s\
> 2) Make chrome.exe also look in %(VersionDir)s\ for the DLLs in a component
> build.
> 
> (1) is the easiest I think, but is less clean, let me know what you prefer.

(1) is also not going to work with the auto-update mechanism as we wouldn't have
all the files in the version dir (we probably want the auto-update mechanism to
work in component build mode as having the component build to test the
auto-updater would be nice too?)

[grt (UTC plus 2), 2012-03-15 17:48:41]

Look out: you'll hit the same problem with setup.exe, which lives in
Application\W.X.Y.Z\Installer.  It needs base.dll among others.

[gab, 2012-03-16 14:59:15]

On 2012/03/15 17:48:41, grt wrote:
> Look out: you'll hit the same problem with setup.exe, which lives in
> Application\W.X.Y.Z\Installer.  It needs base.dll among others.

So setup.exe and chrome.exe are the only two right? And they both work right now
because chrome.dll's position is "hardcoded" (or algorithmically found?).

Seems like adding the version dir to the DLL path would work, no?

[grt (UTC plus 2), 2012-03-16 15:58:13]

setup.exe doesn't load chrome.dll.  it works right now because it links
everything it needs statically in shipping builds.  for
component=shared_library builds, it needs to be able to find base.dll and
friends at load time.

[gab, 2012-05-09 01:34:20]

(please ignore patchset 2... diffed against wrong branch)

Wooohoo!! So it looks like this is going to work :)!

If I extract the archive from mini_installer I can now run chrome.exe it without
getting the error about not finding base.dll! There are other errors, but I'll
look into those tomorrow (if anyone has a quick idea, the error showing up in
debug.log by chrome.exe is:
[0508/213006:ERROR:client_util.cc(384)] Could not get Chrome DLL version.
[0508/213006:ERROR:client_util.cc(442)] Could not find exported function
RelaunchChromeBrowserWithNewCommandLineIfNeeded
)

The basic idea is there though so I'm looking for a generic review on the
design. I put most of the logic in create_installer_archive.py so that the
manifest generation logic doesn't inflate setup.exe.

The remaining problem now is that setup.exe also needs access to the DLLs. I can
easily make a manifest for setup.exe as well, but it's not clear to me where it
is when it's ran by mini_installer.exe (I know it ultimately lands in
<version_dir>\Installer\setup.exe, but I don't think that's where it's first ran
from). Also, I'll need a way to get the manifests beside it before running it
(or temporarily drop all the DLLs beside setup.exe on install if it's in Temp\?)

I figured you guys probably had insights into how that works ;)

Cheers,
Gab

http://codereview.chromium.org/9701050/diff/8002/chrome/installer/setup/insta...
File chrome/installer/setup/install_worker.cc (right):

http://codereview.chromium.org/9701050/diff/8002/chrome/installer/setup/insta...
chrome/installer/setup/install_worker.cc:797: // For the component build to work
with the installer, we need to drop a
It would be nice to #ifdef to code below to make sure it doesn't clutter the
release setup binary, but I wasn't sure what to ifdef on...

[grt (UTC plus 2), 2012-05-09 15:49:40]

Looks cool.  This will be a big win for WinDevs.  Color me uncomfortably
excited!

The errors at Chrome startup are caused by Chrome not knowing what version it
is.  Ordinarily, it finds it by looking at values placed in the registry by the
installer (after first looking for chrome.dll next to chrome.exe).  Since you're
running Chrome without running the installer, it can't figure out its version. 
You can tell it via --chrome-version=W.X.Y.Z on the command line when you run
chrome.exe.

As for setup.exe, it's extracted from a resource in mini_installer.exe into a
temp dir and run directly.  I think getting all of its needed DLLs in there will
be tricky.  For this first pass, I suggest you get the simpler case of having
setup.exe in the build directory do the right thing (i.e., install a usable
component build of Chrome).  This will mean that the mini_installer itself still
can't be used in the component build, but developers can use setup.exe itself to
install component Chrome.  This is enough to greatly improve the usefulness of
the component build for installer-related work.

https://chromiumcodereview.appspot.com/9701050/diff/8002/chrome/installer/set...
File chrome/installer/setup/install_worker.cc (right):

https://chromiumcodereview.appspot.com/9701050/diff/8002/chrome/installer/set...
chrome/installer/setup/install_worker.cc:797: // For the component build to work
with the installer, we need to drop a
On 2012/05/09 01:34:20, gab wrote:
> It would be nice to #ifdef to code below to make sure it doesn't clutter the
> release setup binary, but I wasn't sure what to ifdef on...

#if defined(COMPONENT_BUILD)

https://chromiumcodereview.appspot.com/9701050/diff/8002/chrome/installer/set...
chrome/installer/setup/install_worker.cc:800: static const wchar_t
kChromeExeConfig[] = L"chrome.exe.config";
this is the portable way to do literals that go into FilePaths:
static const FilePath::CharType kFooBar[] = FILE_PATH_LITERAL("FooBar");

https://chromiumcodereview.appspot.com/9701050/diff/8002/chrome/installer/set...
chrome/installer/setup/install_worker.cc:804:
src_path.Append(kChromeExeConfig).value(),
nit: indentation

https://chromiumcodereview.appspot.com/9701050/diff/8002/chrome/installer/set...
chrome/installer/setup/install_worker.cc:809:
src_path.Append(kChromeExeManifest).value(),
nit: indentation

https://chromiumcodereview.appspot.com/9701050/diff/8002/chrome/tools/build/w...
File chrome/tools/build/win/create_installer_archive.py (right):

https://chromiumcodereview.appspot.com/9701050/diff/8002/chrome/tools/build/w...
chrome/tools/build/win/create_installer_archive.py:348: "  <assemblyIdentity \n"
nit: remove the space before the newline

https://chromiumcodereview.appspot.com/9701050/diff/8002/chrome/tools/build/w...
chrome/tools/build/win/create_installer_archive.py:349: "     
name='chrome.chromeexe' version='0.0.0.0' type='win32' />\n"]
i hate with the heat of a thousand suns this trend of putting a space before />
for empty elements.  for my sanity, please don't do it unless you can convince
me that my hatred is better spent on something else.

https://chromiumcodereview.appspot.com/9701050/diff/8002/chrome/tools/build/w...
chrome/tools/build/win/create_installer_archive.py:357: "          language='*'
/>\n"
(cue the sound of greg running through the halls screaming)

https://chromiumcodereview.appspot.com/9701050/diff/8002/chrome/tools/build/w...
chrome/tools/build/win/create_installer_archive.py:375: "      type='win32'
processorArchitecture='x86' />\n"
(oh, god, no)

[gab, 2012-05-09 21:13:24]

Addressed comments (see below).

As for setup.exe I'm not sure I understand what you mean... how do you want
people to package a component build with a non-component setup.exe? In a
component build there is no full chrome.dll which setup.exe can statically link
in itself (this is how it works in non-component build from what I understand
and the fact that this is not the case for component builds is the problem no?).

https://chromiumcodereview.appspot.com/9701050/diff/8002/chrome/installer/set...
File chrome/installer/setup/install_worker.cc (right):

https://chromiumcodereview.appspot.com/9701050/diff/8002/chrome/installer/set...
chrome/installer/setup/install_worker.cc:797: // For the component build to work
with the installer, we need to drop a
On 2012/05/09 15:49:40, grt wrote:
> On 2012/05/09 01:34:20, gab wrote:
> > It would be nice to #ifdef to code below to make sure it doesn't clutter the
> > release setup binary, but I wasn't sure what to ifdef on...
> 
> #if defined(COMPONENT_BUILD)

Done.

https://chromiumcodereview.appspot.com/9701050/diff/8002/chrome/installer/set...
chrome/installer/setup/install_worker.cc:800: static const wchar_t
kChromeExeConfig[] = L"chrome.exe.config";
On 2012/05/09 15:49:40, grt wrote:
> this is the portable way to do literals that go into FilePaths:
> static const FilePath::CharType kFooBar[] = FILE_PATH_LITERAL("FooBar");

Done.

https://chromiumcodereview.appspot.com/9701050/diff/8002/chrome/installer/set...
chrome/installer/setup/install_worker.cc:804:
src_path.Append(kChromeExeConfig).value(),
On 2012/05/09 15:49:40, grt wrote:
> nit: indentation

Done.

https://chromiumcodereview.appspot.com/9701050/diff/8002/chrome/installer/set...
chrome/installer/setup/install_worker.cc:809:
src_path.Append(kChromeExeManifest).value(),
On 2012/05/09 15:49:40, grt wrote:
> nit: indentation

Done.

https://chromiumcodereview.appspot.com/9701050/diff/8002/chrome/tools/build/w...
File chrome/tools/build/win/create_installer_archive.py (right):

https://chromiumcodereview.appspot.com/9701050/diff/8002/chrome/tools/build/w...
chrome/tools/build/win/create_installer_archive.py:348: "  <assemblyIdentity \n"
On 2012/05/09 15:49:40, grt wrote:
> nit: remove the space before the newline

Done.

https://chromiumcodereview.appspot.com/9701050/diff/8002/chrome/tools/build/w...
chrome/tools/build/win/create_installer_archive.py:349: "     
name='chrome.chromeexe' version='0.0.0.0' type='win32' />\n"]
On 2012/05/09 15:49:40, grt wrote:
> i hate with the heat of a thousand suns this trend of putting a space before
/>
> for empty elements.  for my sanity, please don't do it unless you can convince
> me that my hatred is better spent on something else.

Me make Greg happy :)

https://chromiumcodereview.appspot.com/9701050/diff/8002/chrome/tools/build/w...
chrome/tools/build/win/create_installer_archive.py:357: "          language='*'
/>\n"
On 2012/05/09 15:49:40, grt wrote:
> (cue the sound of greg running through the halls screaming)

Can I get a recording of this to soothe my soul if I ever get another full
rebuild?!

https://chromiumcodereview.appspot.com/9701050/diff/8002/chrome/tools/build/w...
chrome/tools/build/win/create_installer_archive.py:375: "      type='win32'
processorArchitecture='x86' />\n"
On 2012/05/09 15:49:40, grt wrote:
> (oh, god, no)

Asked like that, I cannot refuse and shall fulfill your will.

https://chromiumcodereview.appspot.com/9701050/diff/8002/chrome/tools/build/w...
chrome/tools/build/win/create_installer_archive.py:480: if not
options.input_file:
FYI.
This is required and it is a mistake that it wasn't in previous versions.

https://chromiumcodereview.appspot.com/9701050/diff/8002/chrome/tools/build/w...
chrome/tools/build/win/create_installer_archive.py:487:
options.resource_file_path = os.path.join(options.build_dir,
FYI.
This was a typo and would crash if this path was ever hit (which I did hit while
I was trying things out).

[robertshield, 2012-05-09 21:42:41]

Looks good, I think that what Greg meant re. setup.exe was that you leave it as
a component-build type thing, and not worry (yet) about making it work from the
temp dir that mini_installer extracts it to.

Unless you have an easy way of making it work from that dir. 

Also, good work on finding an easy way to make Greg run screaming through the
halls.

https://chromiumcodereview.appspot.com/9701050/diff/15002/chrome/tools/build/...
File chrome/tools/build/win/create_installer_archive.py (right):

https://chromiumcodereview.appspot.com/9701050/diff/15002/chrome/tools/build/...
chrome/tools/build/win/create_installer_archive.py:331:
"</configuration>".format(version = current_version))
no spaces around '=', consider using %

https://chromiumcodereview.appspot.com/9701050/diff/15002/chrome/tools/build/...
chrome/tools/build/win/create_installer_archive.py:359: " 
</dependency>\n".format(dll_basename = name))
python style states that there are no spaces around = when naming parameters, so
dll_basename = name -> dll_basename=name

also, slightly more pythonic would be to use the '%' operator after the string

https://chromiumcodereview.appspot.com/9701050/diff/15002/chrome/tools/build/...
chrome/tools/build/win/create_installer_archive.py:377:
"</assembly>".format(dll_basename = name))
no spaces around '=', consider using %

https://chromiumcodereview.appspot.com/9701050/diff/15002/chrome/tools/build/...
chrome/tools/build/win/create_installer_archive.py:381:
"chrome.{dll_basename}.manifest".format(dll_basename = name)), 'w')
no spaces around '=', consider using %, like so:

"chrome.%s.manifest" % name

[gab, 2012-05-10 00:12:55]

Addressed Robert's comments (see below).

I made the script also drop setup.exe.config and setup.exe.manifest in
<version_dir>/Installer so that uninstall (and other uses of setup.exe
post-install) work.

I had trouble when testing this, but it might have been due to something cached
in Windows... I'm rebooting now and will take a look tomorrow... either way PTAL
at the code :).

Cheers,
Gab

https://chromiumcodereview.appspot.com/9701050/diff/15002/chrome/tools/build/...
File chrome/tools/build/win/create_installer_archive.py (right):

https://chromiumcodereview.appspot.com/9701050/diff/15002/chrome/tools/build/...
chrome/tools/build/win/create_installer_archive.py:331:
"</configuration>".format(version = current_version))
On 2012/05/09 21:42:41, robertshield wrote:
> no spaces around '=', consider using %

Done.

https://chromiumcodereview.appspot.com/9701050/diff/15002/chrome/tools/build/...
chrome/tools/build/win/create_installer_archive.py:359: " 
</dependency>\n".format(dll_basename = name))
On 2012/05/09 21:42:41, robertshield wrote:
> python style states that there are no spaces around = when naming parameters,
so
> dll_basename = name -> dll_basename=name

Done.

> 
> also, slightly more pythonic would be to use the '%' operator after the string

Actually according to:
http://docs.python.org/library/string.html#format-examples
.format() is the "new" way to go (unless you need to be compatible with python
<= 2.5).

An interesting post on StackOverflow as to why format() is better:
http://stackoverflow.com/questions/5082452/python-string-formatting-vs-format

https://chromiumcodereview.appspot.com/9701050/diff/15002/chrome/tools/build/...
chrome/tools/build/win/create_installer_archive.py:377:
"</assembly>".format(dll_basename = name))
On 2012/05/09 21:42:41, robertshield wrote:
> no spaces around '=', consider using %

Done.

https://chromiumcodereview.appspot.com/9701050/diff/15002/chrome/tools/build/...
chrome/tools/build/win/create_installer_archive.py:381:
"chrome.{dll_basename}.manifest".format(dll_basename = name)), 'w')
On 2012/05/09 21:42:41, robertshield wrote:
> no spaces around '=', 

Done.

[gab, 2012-05-11 14:13:50]

make it work

[gab, 2012-05-11 14:21:22]

Adding last couple components to make it work:

1) Need trustInfo tag in exe manifests.

2) Don't need assemblyIdentity for exes (as they are never referred to by
another manifest... although msdn states it's "required": VS2010 doesn't even
put it in its generated manifests...)

3) Copy current manifests beside chrome.exe and setup.exe in build output so
that they work from there as intended (as opposed to only from an install where
the special manifests have been deployed...).

I omitted copying chrome/app/chrome.dll.manifest for now as it seems irrelevant
to do it for one DLL, but not the others... I added a TODO for this and also for
the fact that the exe manifests with the dependencies should probably be built
on top of the existing ones to avoid discrepancies between the component and
non-component builds.

[grt (UTC plus 2), 2012-05-11 15:10:50]

very cool.  would you update the CL description with a bit more detail about how
this works?

https://chromiumcodereview.appspot.com/9701050/diff/6007/chrome/chrome_exe.gypi
File chrome/chrome_exe.gypi (right):

https://chromiumcodereview.appspot.com/9701050/diff/6007/chrome/chrome_exe.gy...
chrome/chrome_exe.gypi:53: ]
the prevailing style for the gyp files seems to call for trailing commas
everywhere (presumably so subsequent additions touch fewer lines, but who
knows).  please put one on this line and the next.

https://chromiumcodereview.appspot.com/9701050/diff/6007/chrome/chrome_instal...
File chrome/chrome_installer.gypi (right):

https://chromiumcodereview.appspot.com/9701050/diff/6007/chrome/chrome_instal...
chrome/chrome_installer.gypi:333: ]
commas here, too

https://chromiumcodereview.appspot.com/9701050/diff/6007/chrome/installer/set...
File chrome/installer/setup/install_worker.cc (right):

https://chromiumcodereview.appspot.com/9701050/diff/6007/chrome/installer/set...
chrome/installer/setup/install_worker.cc:805: install_list->AddMoveTreeWorkItem(
consider using Copy rather than Move here so that setup.exe can be run more than
once by a developer.  otherwise, mini_installer has to be rebuilt to regenerate
the config file, no?

https://chromiumcodereview.appspot.com/9701050/diff/6007/chrome/tools/build/w...
File chrome/tools/build/win/create_installer_archive.py (right):

https://chromiumcodereview.appspot.com/9701050/diff/6007/chrome/tools/build/w...
chrome/tools/build/win/create_installer_archive.py:315: # Adds component build
DLLs and required config files and manifests in order
?? Adds -> Generates ??

[gab, 2012-05-11 15:56:07]

fixed nits

[gab, 2012-05-11 15:56:51]

Done + see new description/title!

https://chromiumcodereview.appspot.com/9701050/diff/6007/chrome/chrome_exe.gypi
File chrome/chrome_exe.gypi (right):

https://chromiumcodereview.appspot.com/9701050/diff/6007/chrome/chrome_exe.gy...
chrome/chrome_exe.gypi:53: ]
On 2012/05/11 15:10:50, grt wrote:
> the prevailing style for the gyp files seems to call for trailing commas
> everywhere (presumably so subsequent additions touch fewer lines, but who
> knows).  please put one on this line and the next.

Done.

https://chromiumcodereview.appspot.com/9701050/diff/6007/chrome/chrome_instal...
File chrome/chrome_installer.gypi (right):

https://chromiumcodereview.appspot.com/9701050/diff/6007/chrome/chrome_instal...
chrome/chrome_installer.gypi:333: ]
On 2012/05/11 15:10:50, grt wrote:
> commas here, too

Done.

https://chromiumcodereview.appspot.com/9701050/diff/6007/chrome/installer/set...
File chrome/installer/setup/install_worker.cc (right):

https://chromiumcodereview.appspot.com/9701050/diff/6007/chrome/installer/set...
chrome/installer/setup/install_worker.cc:805: install_list->AddMoveTreeWorkItem(
On 2012/05/11 15:10:50, grt wrote:
> consider using Copy rather than Move here so that setup.exe can be run more
than
> once by a developer.  otherwise, mini_installer has to be rebuilt to
regenerate
> the config file, no?

hmmm?! that's moved from the temp directory of the extracted archive, the
archive itself is not modified... so that on the next run it's re-extracted and
we start from scratch... no?!

https://chromiumcodereview.appspot.com/9701050/diff/6007/chrome/tools/build/w...
File chrome/tools/build/win/create_installer_archive.py (right):

https://chromiumcodereview.appspot.com/9701050/diff/6007/chrome/tools/build/w...
chrome/tools/build/win/create_installer_archive.py:315: # Adds component build
DLLs and required config files and manifests in order
On 2012/05/11 15:10:50, grt wrote:
> ?? Adds -> Generates ??

Yes, but "copies" for the DLLs, let me know if the new comment works for you.

[grt (UTC plus 2), 2012-05-11 17:01:22]

lgtm once you add the missing commas!111!!!!

https://chromiumcodereview.appspot.com/9701050/diff/6007/chrome/tools/build/w...
File chrome/tools/build/win/create_installer_archive.py (right):

https://chromiumcodereview.appspot.com/9701050/diff/6007/chrome/tools/build/w...
chrome/tools/build/win/create_installer_archive.py:315: # Adds component build
DLLs and required config files and manifests in order
On 2012/05/11 15:56:51, gab wrote:
> On 2012/05/11 15:10:50, grt wrote:
> > ?? Adds -> Generates ??
> 
> Yes, but "copies" for the DLLs, let me know if the new comment works for you.

Right on.

https://chromiumcodereview.appspot.com/9701050/diff/17009/chrome/chrome_exe.gypi
File chrome/chrome_exe.gypi (right):

https://chromiumcodereview.appspot.com/9701050/diff/17009/chrome/chrome_exe.g...
chrome/chrome_exe.gypi:54: }
comma here, too

https://chromiumcodereview.appspot.com/9701050/diff/17009/chrome/chrome_insta...
File chrome/chrome_installer.gypi (right):

https://chromiumcodereview.appspot.com/9701050/diff/17009/chrome/chrome_insta...
chrome/chrome_installer.gypi:334: }
comma

[gab, 2012-05-11 17:28:49]

commas like there is no tomorrow

[gab, 2012-05-11 17:30:24]

Oops, didn't realize I was missing them on both lines!

Cheers,
Gab

https://chromiumcodereview.appspot.com/9701050/diff/17009/chrome/chrome_exe.gypi
File chrome/chrome_exe.gypi (right):

https://chromiumcodereview.appspot.com/9701050/diff/17009/chrome/chrome_exe.g...
chrome/chrome_exe.gypi:54: }
On 2012/05/11 17:01:22, grt wrote:
> comma here, too

Whoopsie!

https://chromiumcodereview.appspot.com/9701050/diff/17009/chrome/chrome_insta...
File chrome/chrome_installer.gypi (right):

https://chromiumcodereview.appspot.com/9701050/diff/17009/chrome/chrome_insta...
chrome/chrome_installer.gypi:334: }
On 2012/05/11 17:01:22, grt wrote:
> comma

Oops :)!

[commit-bot: I haz the power, 2012-05-11 17:30:43]

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

[commit-bot: I haz the power, 2012-05-11 17:48:37]

Try job failure for 9701050-14005 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-05-11 18:00:36]

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

[commit-bot: I haz the power, 2012-05-11 18:18:02]

Try job failure for 9701050-14005 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-05-11 18:34:11]

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

[commit-bot: I haz the power, 2012-05-11 18:52:06]

Try job failure for 9701050-14005 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[gab, 2012-05-11 19:39:50]

trying to rebase to fix %&#@&# CQ

[commit-bot: I haz the power, 2012-05-11 19:41:18]

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

[commit-bot: I haz the power, 2012-05-11 19:57:26]

Try job failure for 9701050-18022 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[M-A Ruel, 2012-05-11 20:04:12]

On 2012/05/11 19:57:26, I haz the power (commit-bot) wrote:
> Try job failure for 9701050-18022 (retry) on win for step "compile" (clobber
> build).
> It's a second try, previously, step "compile" failed.
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

I suspect your CL really has a problem. :)

[commit-bot: I haz the power, 2012-05-11 20:05:28]

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

[gab, 2012-05-11 20:09:29]

On Fri, May 11, 2012 at 4:04 PM, <maruel@chromium.org> wrote:

> On 2012/05/11 19:57:26, I haz the power (commit-bot) wrote:
>
>> Try job failure for 9701050-18022 (retry) on win for step "compile"
>> (clobber
>> build).
>> It's a second try, previously, step "compile" failed.
>>
>
> http://build.chromium.org/p/**tryserver.chromium/**
>
buildstatus?builder=win&**number=11348<http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number=11348>
>
> I suspect your CL really has a problem. :)
>

Starting to think that too seeing other people committing with successful
CQs as of 15 minutes ago... can't see what though.. I just completed a full
clean build of the mini_installer.sln, trying to locally build specifically
the v8_snapshot target that's failing consistently now...


>
>
https://chromiumcodereview.**appspot.com/9701050/<https://chromiumcodereview....
>

[commit-bot: I haz the power, 2012-05-11 20:22:22]

Try job failure for 9701050-18022 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-05-11 21:16:27]

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

[commit-bot: I haz the power, 2012-05-11 21:36:32]

Try job failure for 9701050-18022 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-05-12 01:05:31]

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

[commit-bot: I haz the power, 2012-05-12 01:09:30]

Try job failure for 9701050-18022 on win for step "update".
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

Step "update" is always a major failure.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-05-12 01:31:14]

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

[commit-bot: I haz the power, 2012-05-12 01:48:23]

Try job failure for 9701050-18022 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[gab, 2012-05-12 02:09:10]

Ok this is ridiculous... im pretty sure there is nothing wrong with this
patch that would create this error.

I just did a full clobber build Debug+Release component and
non-component... and they all passed (the Debug build had some errors
linking (not the one reported by the try bot), but a second call to build
in a row completes it... seems like some dependency issues, but that's the
case on master as well (cause I tried a full build on master too... so not
this patch's fault)

On Fri, May 11, 2012 at 9:48 PM, <commit-bot@chromium.org> wrote:

> Try job failure for 9701050-18022 (retry) on win for step "compile"
> (clobber
> build).
> It's a second try, previously, step "compile" failed.
> http://build.chromium.org/p/**tryserver.chromium/**
>
buildstatus?builder=win&**number=11427<http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number=11427>
>
>
>
https://chromiumcodereview.**appspot.com/9701050/<https://chromiumcodereview....
>

[commit-bot: I haz the power, 2012-05-12 13:14:00]

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

[commit-bot: I haz the power, 2012-05-12 13:32:36]

Try job failure for 9701050-18022 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[gab, 2012-05-13 20:22:48]

My tries are succeeding when I specify -r with my working revision... I'll try
to rebase again...

----------------------------------------------------------
try success for master#9a1c78 on win @ r136649
http://build.chromium.org/p/tryserver.chromium/

You are awesome! Try succeeded!

http://build.chromium.org/p/tryserver.chromium/builders/win/builds/11458
slave: vm345-m4
----------------------------------------------------------

[commit-bot: I haz the power, 2012-05-13 20:25:42]

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

[commit-bot: I haz the power, 2012-05-13 20:44:02]

Try job failure for 9701050-18022 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[gab, 2012-05-14 02:08:46]

rebase to ToT r136815

[commit-bot: I haz the power, 2012-05-14 02:09:18]

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

[commit-bot: I haz the power, 2012-05-14 02:31:03]

Try job failure for 9701050-21003 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-05-14 15:23:27]

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

[commit-bot: I haz the power, 2012-05-14 15:40:27]

Try job failure for 9701050-21003 (retry) on win for step "runhooks" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-05-14 15:43:22]

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

[commit-bot: I haz the power, 2012-05-14 16:05:13]

Try job failure for 9701050-21003 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-05-14 16:19:16]

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

[commit-bot: I haz the power, 2012-05-14 16:39:33]

Try job failure for 9701050-21003 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-05-14 16:59:44]

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

[commit-bot: I haz the power, 2012-05-14 17:20:28]

Try job failure for 9701050-21003 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-05-14 17:24:52]

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

[commit-bot: I haz the power, 2012-05-14 17:52:19]

Try job failure for 9701050-21003 (retry) (retry) on win for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-05-14 18:00:21]

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

[commit-bot: I haz the power, 2012-05-14 18:24:08]

Try job failure for 9701050-21003 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[gab, 2012-05-14 19:53:19]

Oops, the patch actually did have a problem (thanks Greg for bringing me back to
earth and have me stop clicking that button :)).

It's a good thing the CQ did catch this as the resulting fix makes for a better
patch in which now VS manifest generation is only disabled for DLLs, chrome.exe,
and setup.exe (under the component build only obviously).

PTAL.

Cheers,
Gab

[commit-bot: I haz the power, 2012-05-14 20:25:16]

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

[gab, 2012-05-14 20:39:10]

Patch set 14 addresses the too broad settings I had originally put in common.gyp
and correctly puts them in their respective projects.

PTAL.

Thanks,
Gab

[grt (UTC plus 2), 2012-05-15 02:30:00]

lgtm.

robert: do you have time for a quick look?  the approach now removes manifests
globally for dll targets, but is opt-in for exe targets.

[robertshield, 2012-05-15 02:34:23]

Sure, taking a peek now.

On Mon, May 14, 2012 at 10:30 PM, <grt@chromium.org> wrote:

> lgtm.
>
> robert: do you have time for a quick look?  the approach now removes
> manifests
> globally for dll targets, but is opt-in for exe targets.
>
>
https://chromiumcodereview.**appspot.com/9701050/<https://chromiumcodereview....
>

[robertshield, 2012-05-15 02:42:50]

lgtm

https://chromiumcodereview.appspot.com/9701050/diff/17035/chrome/tools/build/...
File chrome/tools/build/win/create_installer_archive.py (right):

https://chromiumcodereview.appspot.com/9701050/diff/17035/chrome/tools/build/...
chrome/tools/build/win/create_installer_archive.py:350: # Write setup.exe.config
to point to the version directory (which is under it
"under it" -> "one level up from setup.exe"?

https://chromiumcodereview.appspot.com/9701050/diff/17035/chrome/tools/build/...
chrome/tools/build/win/create_installer_archive.py:403: # Write
chrome.{dllname}.manifest in the version directory for each DLL list
list -> listed

[gab, 2012-05-15 03:01:39]

Thanks Greg and Robert!

Time to make friends back with the CQ (and apologies for getting mad at it
earlier today... the resulting modified patch is better and less likely to bring
manifest corner cases).

Cheers,
Gab

https://chromiumcodereview.appspot.com/9701050/diff/17035/chrome/tools/build/...
File chrome/tools/build/win/create_installer_archive.py (right):

https://chromiumcodereview.appspot.com/9701050/diff/17035/chrome/tools/build/...
chrome/tools/build/win/create_installer_archive.py:350: # Write setup.exe.config
to point to the version directory (which is under it
On 2012/05/15 02:42:50, robertshield wrote:
> "under it" -> "one level up from setup.exe"?

Done.

https://chromiumcodereview.appspot.com/9701050/diff/17035/chrome/tools/build/...
chrome/tools/build/win/create_installer_archive.py:403: # Write
chrome.{dllname}.manifest in the version directory for each DLL list
On 2012/05/15 02:42:50, robertshield wrote:
> list -> listed

Done.

[commit-bot: I haz the power, 2012-05-15 03:03:26]

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

[commit-bot: I haz the power, 2012-05-15 04:05:54]

The commit queue went berserk retrying too often for a
seemingly flaky test. Builder is linux_rel, revision is 137067, job name
was 9701050-15037 (previous was lost) (previous was lost) (previous was lost)
(previous was lost).

[commit-bot: I haz the power, 2012-05-15 04:28:12]

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

[commit-bot: I haz the power, 2012-05-15 05:30:58]

The commit queue went berserk retrying too often for a
seemingly flaky test. Builder is android, revision is 137076, job name
was 9701050-15037 (previous was lost) (previous was lost) (previous was lost)
(previous was lost).

[commit-bot: I haz the power, 2012-05-15 10:34:01]

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

[commit-bot: I haz the power, 2012-05-15 11:40:19]

The commit queue went berserk retrying too often for a
seemingly flaky test. Builder is android, revision is 137103, job name
was 9701050-15037 (previous was lost) (previous was lost) (previous was lost)
(previous was lost).

[commit-bot: I haz the power, 2012-05-15 12:41:25]

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

[commit-bot: I haz the power, 2012-05-15 14:07:00]

Change committed as 137118

[grt (UTC plus 2), 2012-05-15 14:11:58]

On 2012/05/15 14:07:00, I haz the power (commit-bot) wrote:
> Change committed as 137118

WOOHOO!

[M-A Ruel, 2012-05-15 14:14:42]

It's concerning. :D

2012/5/15 <grt@chromium.org>

> On 2012/05/15 14:07:00, I haz the power (commit-bot) wrote:
>
>> Change committed as 137118
>>
>
> WOOHOO!
>
>
https://chromiumcodereview.**appspot.com/9701050/<https://chromiumcodereview....
>

[gab, 2012-05-15 14:55:42]

Woo woo :)!

On Tue, May 15, 2012 at 10:14 AM, Marc-Antoine Ruel <maruel@chromium.org>wrote:

> It's concerning. :D
>
> 2012/5/15 <grt@chromium.org>
>
> On 2012/05/15 14:07:00, I haz the power (commit-bot) wrote:
>>
>>> Change committed as 137118
>>>
>>
>> WOOHOO!
>>
>>
https://chromiumcodereview.**appspot.com/9701050/<https://chromiumcodereview....
>>
>
>

[gab, 2012-05-16 00:15:25]

Had to revert this as it was causing CRT DLLs not found error for some people:
http://codereview.chromium.org/10391153/

Continuing this patch at: https://chromiumcodereview.appspot.com/10399041

Cheers,
Gab

[rvargas (doing something else), 2012-05-16 01:09:58]

So I guess this CL was the one responsible for all modules failing to build (but
ending up with all modules built, just without manifest) like so:

mt.exe : general error c101008a: Failed to save the updated manifest to the file
"..\build\Debug\obj\base\base.dll.embed.manifest". The parameter is incorrect.

(VS '10)

http://codereview.chromium.org/9701050/diff/15037/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/9701050/diff/15037/build/common.gypi#newcode2836
build/common.gypi:2836: # same directory as chrome.exe and setup.exe):
What does this exactly mean? I don't think this is going to work, does it?

[gab, 2012-05-16 01:15:54]

I'm not sure why this would this mt.exe error, but it's possible.

http://codereview.chromium.org/9701050/diff/15037/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/9701050/diff/15037/build/common.gypi#newcode2836
build/common.gypi:2836: # same directory as chrome.exe and setup.exe):
On 2012/05/16 01:09:58, rvargas wrote:
> What does this exactly mean? I don't think this is going to work, does it?

It will, but not this way. Follow
https://chromiumcodereview.appspot.com/10399041/ if you're intrigued.