Can you add -d explain to the ninja command line and add that here? Might make it obvious what's going on.

D:\src\chrome\src>ninja -C out\Release mini_installer -d explain
ninja: Entering directory `out\Release'
ninja explain: output C:/Windows/System32/msvcrt20.dll of phony edge with no inputs doesn't exist
ninja explain: C:/Windows/System32/msvcrt20.dll is dirty
ninja explain: output C:/Windows/System32/msvcrt40.dll of phony edge with no inputs doesn't exist
ninja explain: C:/Windows/System32/msvcrt40.dll is dirty
ninja explain: chrome.7z is dirty
ninja explain: setup.ex_ is dirty
ninja explain: gen/chrome/installer/mini_installer/packed_files.rc is dirty
ninja explain: obj/chrome/installer/mini_installer/archive.stamp is dirty
ninja explain: gen/chrome/installer/mini_installer/packed_files.rc is dirty
ninja explain: obj/chrome/installer/mini_installer/mini_installer/packed_files.res is dirty
ninja explain: mini_installer.exe is dirty
[0/5] ACTION //chrome/installer/mini_installer:archive(//build/toolchain/win:x64)

So mini_installer:archive cares that chrome.7z is dirty?! But that's the step supposed to generate it? (i.e. it would always be dirty in the next round...)

As with compiler errors the first one is usually the most important, and the others mere cascading failures.

Probably the error is the dependency on the non-existent msvcrt40.dll. I'll walk over and we can discuss.

Just tried a GYP build with the same config (not x64 though but not sure that matters..?) and it doesn't repro.

So looks like this is a GN specific issue.

Actually, interesting, just tried x64 GYP build and it does repro. So it's x64 specific, not GN (though GN makes x64 default so I'll remove the Build-Tools-GN label but keep Proj-GN-Migration).

To be precise, that is: "Release + x64 + component" repros in both GYP/GN.

GYP's explain output is similar to GN's:

inja: Entering directory `out_gyp\Release_x64'
ninja explain: output C:/Windows/System32/msvcrt20.dll of phony edge with no inputs doesn't exist
ninja explain: C:/Windows/System32/msvcrt20.dll is dirty
ninja explain: output C:/Windows/System32/msvcrt40.dll of phony edge with no inputs doesn't exist
ninja explain: C:/Windows/System32/msvcrt40.dll is dirty
ninja explain: chrome.7z is dirty
ninja explain: setup.ex_ is dirty
ninja explain: obj/chrome/installer/mini_installer.gen/packed_files.rc is dirty
ninja explain: obj/chrome/installer/mini_installer.gen/packed_files.rc is dirty
ninja explain: obj/chrome/installer/obj/chrome/installer/mini_installer.gen/mini_installer.packed_files.res is dirty
ninja explain: mini_installer.exe is dirty
[0/4] ACTION Create installer archive

This probably means that installer tests are triggered on any change (regardless of whether it truly affects that binary per the unnecessary rebuild) -- IIRC being able to exclude unaffected tests was the reason the no-op build was made to work by Scott and Nico last year.

I just tried with this args.gn and I cannot repro:

is_component_build = true
is_debug = false
is_win_fastlink = true

enable_nacl = false
target_cpu = "x64"
is_syzyasan = false
is_chrome_branded = false

That is, when I go:

    ninja -C out\Release_gn_component mini_installer

it says "no work to do."

Can you share the args.gn and/or GYP_DEFINES that you are using?

However I think it is probably something about your machine that is triggering this behavior rather than a difference in build settings.

It is odd that your build has references to msvcrt20.dll and msvcrt40.dll. I don't think Microsoft has ever shipped such binaries and it seems clear that the references to these non-existent binaries is what is causing your problem. When I search my .ninja files for references to msvcrt*.dll I find nothing.

So, you may need to investigate on your machine - find out where those msvcrt20.dll and msvcrt40.dll references are coming from.

chrome\tools\build\win\create_installer_archive.py does have some references to msvc*.dll and C:/Windows/System32/msvc*0.dll but those shouldn't lead to ninja dependencies so I think that is unrelated.

My args.gn:

   is_component_build = true
   is_debug = false
   is_chrome_branded = true
   dcheck_always_on = true
   is_win_fastlink = true
   enable_nacl = false

out\Release>findstr /m /si msvcrt20.dll *.*

chrome.7z
gen\chrome\installer\mini_installer\archive.d
gen\chrome\installer\mini_installer\packed_files.rc
gen\temp_installer_archive\Chrome-bin\52.0.2727.0\52.0.2727.0.manifest
gen\temp_installer_archive\Chrome-bin\52.0.2727.0\Installer\msvcrt20.dll
gen\temp_installer_archive\Chrome-bin\52.0.2727.0\msvcrt20.dll
mini_installer.exe
msvcrt20.dll
obj\chrome\installer\mini_installer\mini_installer\packed_files.res

Find attached my archive.d and packed_files.rc: they do clearly mention those inputs, is ninja using those somehow?

Deleted: archive.d 12.9 KB

archive.d 12.9 KB View Download

Deleted: packed_files.rc 8.3 KB

packed_files.rc 8.3 KB View Download

So those DLLs are in my out\Release (which is a recent out-dir, nuked on Apr 20 when migrating to GN) and they are being picked up by a script's glob (probably create_installer_archive.py).

I have a copy of both of these DLLs in C:\Windows\SysWOW64 (which create_installer_archive.py copies from: https://code.google.com/p/chromium/codesearch#chromium/src/chrome/tools/build/win/create_installer_archive.py&l=503) so that explains how they got there.

They are not in C:\Windows\System32 though which is where ninja appears to be looking for them. So there is some lookup that's wrong between the x64 and x86 builds (explains why this is new to me in GN because it toggled me to x64?).

TL;DR - I think that the bug is that those DLLs are ending up in your output directory at all. The correct fix is to prevent that.

Can you try running "gn gen out\gn_test" and then paste the output from this command:

    dir out\gn_test\*.dll | find /i /v "api-ms"

These are the files that I see:

msvcp140.dll
msvcp140d.dll
pgort140.dll
ucrtbase.dll
ucrtbased.dll
vccorlib140.dll
vccorlib140d.dll
vcruntime140.dll
vcruntime140d.dll

If I run "gn args out\gn_test" and add target_cpu="x86" to the gn args then the contents of those files changes, but the set of files remains the same, and msvcrt20.dll and msvcrt40.dll never appear.

My guess is that CopyVisualStudioRuntimeDLLs is wrong. It should only apply when DEPOT_TOOLS_WIN_TOOLCHAIN=0 and even then (if needed at all!) it should be copying exactly the DLLs that we need, not overly broad lists like msvc*0.dll. Code like this is also very sensitive to whether you are running a 32-bit or 64-bit version of Python.

Then again, I'm still not clear as to why/where it is trying to copy a non-existent DLL. I don't see any code that does that.

Be aware of  crbug.com/611934  which point out that CopyVisualStudioRuntimeDLLs actually doesn't work correctly for component builds when DEPOT_TOOLS_WIN_TOOLCHAIN=0. That is, that function fails to handle one of the few cases where it is relevant.

D:\src\chrome\src>dir out\gn_test\*.dll | find /i /v "api-ms"
 Volume in drive D is Source2
 Volume Serial Number is 768A-F5DC

 Directory of D:\src\chrome\src\out\gn_test

2016-04-05  01:07 PM           639,808 msvcp140.dll
2016-04-05  01:07 PM         1,004,864 msvcp140d.dll
2016-04-05  01:07 PM            56,480 pgort140.dll
2016-04-05  01:08 PM           992,960 ucrtbase.dll
2016-04-05  01:08 PM         1,810,112 ucrtbased.dll
2016-04-05  01:07 PM           394,568 vccorlib140.dll
2016-04-05  01:07 PM         1,022,280 vccorlib140d.dll
2016-04-05  01:07 PM            89,416 vcruntime140.dll
2016-04-05  01:07 PM           135,504 vcruntime140d.dll
              49 File(s)      7,006,152 bytes
               0 Dir(s)  53,335,420,928 bytes free

Wasn't aware that depot_tools now was managing the MSVC DLLs. CopyVisualStudioRuntimeDLLs() was written to make installer+component build work back in the Win8 days (to make for an installer that could be installed on another machine with all the redistributables included) , i.e. 2012, it changed since but maybe it's no longer required at all? is supporting non-depot tools builds with no hassle even a goal?

That seems to confirm that create_installer_archive.py is what is copying those DLLs, and therefore what needs fixing.

My understanding is that the CRT and api-ms* DLLs have to be in the same directory as the executables or else the executables won't run (assuming a machine that doesn't have VS 2015 installed). So, depot_tools *has* to copy those DLLs in when using the depot_tools toolchain.

Therefore create_installer_archive.py should either only copy the DLLs in if they are not present, or should only copy them in if DEPOT_TOOLS_WIN_TOOLCHAIN=0, or something like that.

But mostly create_installer_archive.py should be more careful about what DLLs it copies in. It seems to be trying the 'more is better' strategy which is failing. It should probably copy the logic from depot_tools and copy exactly that set.

Most/all non-Google employees use the system toolchain so having it work correctly is a priority.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e7bd0348e692e635c039328cd06aac1ce23b7d4f

commit e7bd0348e692e635c039328cd06aac1ce23b7d4f
Author: brucedawson <brucedawson@chromium.org>
Date: Wed Jun 01 18:37:18 2016

Copy local CRT DLLs to gn output directories

When DEPOT_TOOLS_WIN_TOOLCHAIN=0 it is assumed that the developer has a
local install of Visual Studio, and therefore the CRT DLLs don't need to
be copied to the output directory in order to run binaries. However,
when creating packages for running elsewhere (isolates or the
mini_installer) those DLLs are needed. In order to avoid special cases
elsewhere this change copies them to the output directories when 'gn gen'
or 'gn args' runs.

This change also makes the newly copied files writable so that they can
easily be deleted. For backwards compatibility it also makes the files
writable before deleting them. This avoids a problem with pgort140.dll
which can end up read-only and thus impossible to copy over or delete
without first removing the read-only flag.

This change also removes the need for create_installer_archive.py to
copy CRT DLLs, which should avoid bugs with over-copying of these DLLs.
This change also updates that script to say that ucrtbase*.dll is
required.

BUG= 611934 , 585556 ,  610336 

Review-Url: https://codereview.chromium.org/2021353003
Cr-Commit-Position: refs/heads/master@{#397189}

[modify] https://crrev.com/e7bd0348e692e635c039328cd06aac1ce23b7d4f/build/vs_toolchain.py
[modify] https://crrev.com/e7bd0348e692e635c039328cd06aac1ce23b7d4f/chrome/tools/build/win/create_installer_archive.py

If the fix seems to consistently resolve the problem then please mark this as verified.