Fix mini_installer in gn component builds

Fix mini_installer in gn component builds

Due to its use of source sets the gn component mini_installer build
of setup.exe creates references to six DLLs that would not be needed if
linking with real .lib files (which avoid pulling in unneeded .obj
files). Fixing this to not pull in those DLLs would require significant
effort and would probably end up being fragile.

Therefore the pragmatic fix is to just extract the six extra DLLs. This
represents 22.4 MB of disk space (87% of that is from net.dll) which is
not enough to matter in the context of mini_installer component builds.

BUG=596885
Committed: https://crrev.com/fb41afdaca7c25eeb8e416da3f3b2d5d8f067556
Cr-Commit-Position: refs/heads/master@{#383542}

[brucedawson, 2016-03-28 18:23:13]

Description was changed from

==========
Fix mini_installer in gn component builds

Due to its use of source sets the gn component mini_installer build
of setup.exe creates references to six DLLs that would not be needed if
linking with real .lib files (which avoid pulling in unneeded .obj
files). Fixing this to not pull in those DLLs would require significant
effort and would probably end up being fragile.

Therefore the pragmatic fix is to just extract the six extra DLLs. This
represents 22.4 MB of disk space (87% of that is from net.dll) which is
not enough to matter in the context of mini_installer component builds.

BUG=596885
==========

to

==========
Fix mini_installer in gn component builds

Due to its use of source sets the gn component mini_installer build
of setup.exe creates references to six DLLs that would not be needed if
linking with real .lib files (which avoid pulling in unneeded .obj
files). Fixing this to not pull in those DLLs would require significant
effort and would probably end up being fragile.

Therefore the pragmatic fix is to just extract the six extra DLLs. This
represents 22.4 MB of disk space (87% of that is from net.dll) which is
not enough to matter in the context of mini_installer component builds.

BUG=596885
==========

[brucedawson, 2016-03-28 18:23:13]

brucedawson@chromium.org changed reviewers:
+ brettw@chromium.org

[brucedawson, 2016-03-28 18:26:12]

Brett/Dirk, what do you think of this solution to mini_installer in gn component
builds? It seems like the best solution to me.

I remain worried about the use of source sets - similar issues could creep in to
code that we actually ship to customers - but that can be a separate discussion.

[brettw, 2016-03-28 18:29:16]

Since this is just for archiving component builds for debug purposes, I think
this solution is fine.

https://codereview.chromium.org/1835993002/diff/1/chrome/tools/build/win/crea...
File chrome/tools/build/win/create_installer_archive.py (right):

https://codereview.chromium.org/1835993002/diff/1/chrome/tools/build/win/crea...
chrome/tools/build/win/create_installer_archive.py:508: if
options.component_build == '1':
My brief reading of this function implies that this whole function will only get
called for component builds. Do we need this extra check?

[brucedawson, 2016-03-28 18:37:01]

> My brief reading of this function implies that this whole function will only
get
> called for component builds. Do we need this extra check?

Good catch. I removed the check. The one call to the function already contains
the check.

I also noticed that the original list was mostly alphabetized so I fixed the one
out-of-order DLL. I put the source-set-required DLLs separately at the end
(alphabetized separately) since it felt like there was some value in
distinguishing between the logically and physically required DLLs.

[brettw, 2016-03-28 18:58:07]

lgtm

[brucedawson, 2016-03-28 19:03:25]

On 2016/03/28 18:58:07, brettw wrote:
> lgtm

Just added a missing comma (oops), now landing.

[brucedawson, 2016-03-28 19:03:31]

The CQ bit was checked by brucedawson@chromium.org

[brucedawson, 2016-03-28 19:03:31]

The patchset sent to the CQ was uploaded after l-g-t-m from brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/1835993002/#ps40001
(title: "Fixed missing comma")

[commit-bot: I haz the power, 2016-03-28 19:03:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1835993002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1835993002/40001

[commit-bot: I haz the power, 2016-03-28 19:42:49]

Description was changed from

==========
Fix mini_installer in gn component builds

Due to its use of source sets the gn component mini_installer build
of setup.exe creates references to six DLLs that would not be needed if
linking with real .lib files (which avoid pulling in unneeded .obj
files). Fixing this to not pull in those DLLs would require significant
effort and would probably end up being fragile.

Therefore the pragmatic fix is to just extract the six extra DLLs. This
represents 22.4 MB of disk space (87% of that is from net.dll) which is
not enough to matter in the context of mini_installer component builds.

BUG=596885
==========

to

==========
Fix mini_installer in gn component builds

Due to its use of source sets the gn component mini_installer build
of setup.exe creates references to six DLLs that would not be needed if
linking with real .lib files (which avoid pulling in unneeded .obj
files). Fixing this to not pull in those DLLs would require significant
effort and would probably end up being fragile.

Therefore the pragmatic fix is to just extract the six extra DLLs. This
represents 22.4 MB of disk space (87% of that is from net.dll) which is
not enough to matter in the context of mini_installer component builds.

BUG=596885
==========

[commit-bot: I haz the power, 2016-03-28 19:42:50]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-03-28 19:45:12]

Description was changed from

==========
Fix mini_installer in gn component builds

Due to its use of source sets the gn component mini_installer build
of setup.exe creates references to six DLLs that would not be needed if
linking with real .lib files (which avoid pulling in unneeded .obj
files). Fixing this to not pull in those DLLs would require significant
effort and would probably end up being fragile.

Therefore the pragmatic fix is to just extract the six extra DLLs. This
represents 22.4 MB of disk space (87% of that is from net.dll) which is
not enough to matter in the context of mini_installer component builds.

BUG=596885
==========

to

==========
Fix mini_installer in gn component builds

Due to its use of source sets the gn component mini_installer build
of setup.exe creates references to six DLLs that would not be needed if
linking with real .lib files (which avoid pulling in unneeded .obj
files). Fixing this to not pull in those DLLs would require significant
effort and would probably end up being fragile.

Therefore the pragmatic fix is to just extract the six extra DLLs. This
represents 22.4 MB of disk space (87% of that is from net.dll) which is
not enough to matter in the context of mini_installer component builds.

BUG=596885

Committed: https://crrev.com/fb41afdaca7c25eeb8e416da3f3b2d5d8f067556
Cr-Commit-Position: refs/heads/master@{#383542}
==========

[commit-bot: I haz the power, 2016-03-28 19:45:14]

Patchset 3 (id:??) landed as
https://crrev.com/fb41afdaca7c25eeb8e416da3f3b2d5d8f067556
Cr-Commit-Position: refs/heads/master@{#383542}

[Dirk Pranke, 2016-03-28 20:03:21]

lgtm. If this wasn't for something that most people don't care about (a
testing-only
build of the mini_installer), I'd be more inclined to push for the right fixes,
which
is to either split up the targets or turn them into libraries if we don't
actually
want these dependencies.

On 2016/03/28 18:26:12, brucedawson wrote:
> I remain worried about the use of source sets - similar issues could creep in
to
> code that we actually ship to customers - but that can be a separate
discussion.

There's kinda two different issues here.

The first is controlling the list of files that get shipped to customers. You're
right
to be concerned about this, however, the file lists are checked-in and need
reviews,
so I don't think we'll see changes occurring here too randomly.

The second is the general source_set vs. static_library issue, and there are
real
issues here as well, but the issues occur in both directions. If you bundle
stuff
as static_libraries, then it becomes fairly easy to create structures where you
rely on the code-stripping of static libraries in order to link; this can cause 
problems in other environments (like component builds). And, as you see,
using source_sets instead of static_libraries cause the same problem in the
opposite direction.

As a general rule of thumb, I think using source_sets instead of
static_libraries
forces you to think about the logical structure of what is getting grouped more
effectively. However, certainly some things should be libraries instead of
source_sets,
and retrofitting source_sets on top of libraries can be tricky as well.