Make net_unittests_run work with component=shared_library.

Make net_unittests_run work with component=shared_library.

Specifically, this adds support to run net_unittests isolated via 'isolate.py
run' when built with component build.

R=vadimsh@chromium.org,brettw@chromium.org,rsleevi@chromium.org
BUG=336439
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=249150

[M-A Ruel, 2014-01-22 15:06:11]

abarth: url/url.isolate
rsleevi: crypto/ and net/
vadimsh: base/ and build/

Because of the hand picking done when building nss&openssl, I had to split
ssl.isolate in two. That's why I don't think it's a great idea, the mapping
doesn't correlate between targets built for a test and the gyp location.

[Ryan Sleevi, 2014-01-22 18:56:21]

https://codereview.chromium.org/134003004/diff/1/net/third_party/nss/ssl_base...
File net/third_party/nss/ssl_base.isolate (right):

https://codereview.chromium.org/134003004/diff/1/net/third_party/nss/ssl_base...
net/third_party/nss/ssl_base.isolate:10: '<(PRODUCT_DIR)/lib/libcrssl.so',
base's dependency is on everything *but* libcrssl.

Yeah, it's weird, but build/linux/system.gyp:ssl is
NSS:
  NSPR (system lib)
  NSS (~7 system libs that 'make up' NSS - with SSL *filtered out*)
OpenSSL:
  OpenSSL (system lib)

then net/third_party/nss/ssl.gyp is, on Linux, a dependency on crssl, which then
depends on the base system libs bits.

ssl_net.isolate should be libcrssl.so, and ssl_base.isolate should be empty. If
that doesn't work, we've got a dep bug.

[M-A Ruel, 2014-01-27 20:12:00]

On 2014/01/22 18:56:21, Ryan Sleevi wrote:
>
https://codereview.chromium.org/134003004/diff/1/net/third_party/nss/ssl_base...
> File net/third_party/nss/ssl_base.isolate (right):
> 
>
https://codereview.chromium.org/134003004/diff/1/net/third_party/nss/ssl_base...
> net/third_party/nss/ssl_base.isolate:10: '<(PRODUCT_DIR)/lib/libcrssl.so',
> base's dependency is on everything *but* libcrssl.
> 
> Yeah, it's weird, but build/linux/system.gyp:ssl is
> NSS:
>   NSPR (system lib)
>   NSS (~7 system libs that 'make up' NSS - with SSL *filtered out*)
> OpenSSL:
>   OpenSSL (system lib)
> 
> then net/third_party/nss/ssl.gyp is, on Linux, a dependency on crssl, which
then
> depends on the base system libs bits.
> 
> ssl_net.isolate should be libcrssl.so, and ssl_base.isolate should be empty.
If
> that doesn't work, we've got a dep bug.

So I had been confused with net/third_party/nss vs third_party/nss and
third_party/nss/nspr vs base/third_party/nspr. I sorted this out in the .isolate
files.

This is confirmed to work with:
rm -rf out   or   rd /q /s out
export/set GYP_DEFINES=component=shared_library
gclient runhooks
ninja -C out/Debug net_unittests_run
python tools/swarming_client/isolate.py run -s out/Debug/net_unittests.isolated

The trick here is to only build net_unittests, and nothing else, so no
dependency that would have been built by another target would still be lying
around.

[M-A Ruel, 2014-01-27 20:12:25]

https://codereview.chromium.org/134003004/diff/120001/base/base_prefs.isolate
File base/base_prefs.isolate (right):

https://codereview.chromium.org/134003004/diff/120001/base/base_prefs.isolate...
base/base_prefs.isolate:3: # found in the LICENSE file.
net_unittests doesn't depend on this library.

[Ryan Sleevi, 2014-01-27 21:27:37]

https://codereview.chromium.org/134003004/diff/120001/net/third_party/nss/ssl...
File net/third_party/nss/ssl.isolate (right):

https://codereview.chromium.org/134003004/diff/120001/net/third_party/nss/ssl...
net/third_party/nss/ssl.isolate:16: '<(PRODUCT_DIR)/crssl.ddl',
typo: ddl -> dll

https://codereview.chromium.org/134003004/diff/120001/net/third_party/nss/ssl...
File net/third_party/nss/ssl_base.isolate (right):

https://codereview.chromium.org/134003004/diff/120001/net/third_party/nss/ssl...
net/third_party/nss/ssl_base.isolate:11: '<(PRODUCT_DIR)/lib/libcrssl.so',
Still not sure why this is in base, since base_unittests shouldn't be depending
on libssl

https://codereview.chromium.org/134003004/diff/120001/third_party/nss.isolate
File third_party/nss.isolate (right):

https://codereview.chromium.org/134003004/diff/120001/third_party/nss.isolate...
third_party/nss.isolate:11: #   linux.
Isn't this a case for where we do what libjingle/webrtc is doing?

eg:

third_party/nss = unconditionally checked out
third_party/nss/nss.isolate
third_party/nss/src = conditionally checked out bits (eg: On mac/win/iOS)

That requires a infrastructure coordination, so should be done in a separate
bug, unless you expect to see a bunch of .isolate's in this folder. If so,
should we have third_party/isolate to contain these files? IDK, sticking them at
top-level seems as weird as sticking nss.gyp here

[M-A Ruel, 2014-01-27 21:34:47]

https://codereview.chromium.org/134003004/diff/120001/net/third_party/nss/ssl...
File net/third_party/nss/ssl.isolate (right):

https://codereview.chromium.org/134003004/diff/120001/net/third_party/nss/ssl...
net/third_party/nss/ssl.isolate:16: '<(PRODUCT_DIR)/crssl.ddl',
On 2014/01/27 21:27:38, Ryan Sleevi wrote:
> typo: ddl -> dll

Argh, it means I didn't test it properly on this platform. Fixed and testing
again.

https://codereview.chromium.org/134003004/diff/120001/net/third_party/nss/ssl...
File net/third_party/nss/ssl_base.isolate (right):

https://codereview.chromium.org/134003004/diff/120001/net/third_party/nss/ssl...
net/third_party/nss/ssl_base.isolate:11: '<(PRODUCT_DIR)/lib/libcrssl.so',
On 2014/01/27 21:27:38, Ryan Sleevi wrote:
> Still not sure why this is in base, since base_unittests shouldn't be
depending
> on libssl

You can easily reproduce;
export GYP_DEFINES=component=="shared_library"
rm -rf out
build/gyp_chromium
ninja -C out/Debug base_unittests
rm out/Debug/lib/libcrssl.so
out/Debug/base_unittests

This will fail. I'm just presenting what is currently done, not why it is done.
I'm not in a position to fix this, I'm just documenting the current state.

https://codereview.chromium.org/134003004/diff/120001/third_party/nss.isolate
File third_party/nss.isolate (right):

https://codereview.chromium.org/134003004/diff/120001/third_party/nss.isolate...
third_party/nss.isolate:11: #   linux.
On 2014/01/27 21:27:38, Ryan Sleevi wrote:
> Isn't this a case for where we do what libjingle/webrtc is doing?
> 
> eg:
> 
> third_party/nss = unconditionally checked out
> third_party/nss/nss.isolate
> third_party/nss/src = conditionally checked out bits (eg: On mac/win/iOS)
> 
> That requires a infrastructure coordination, so should be done in a separate
> bug, unless you expect to see a bunch of .isolate's in this folder. If so,
> should we have third_party/isolate to contain these files? IDK, sticking them
at
> top-level seems as weird as sticking nss.gyp here

Go on your linux machine and see what is there. third_party/nss is empty. Or see
the DEPS file.

Doing this *is* bad but sadly there's no third_party/nss/src layout.

[Ryan Sleevi, 2014-01-27 21:45:29]

https://codereview.chromium.org/134003004/diff/120001/third_party/nss.isolate
File third_party/nss.isolate (right):

https://codereview.chromium.org/134003004/diff/120001/third_party/nss.isolate...
third_party/nss.isolate:11: #   linux.
On 2014/01/27 21:34:48, M-A Ruel wrote:
> On 2014/01/27 21:27:38, Ryan Sleevi wrote:
> > Isn't this a case for where we do what libjingle/webrtc is doing?
> > 
> > eg:
> > 
> > third_party/nss = unconditionally checked out
> > third_party/nss/nss.isolate
> > third_party/nss/src = conditionally checked out bits (eg: On mac/win/iOS)
> > 
> > That requires a infrastructure coordination, so should be done in a separate
> > bug, unless you expect to see a bunch of .isolate's in this folder. If so,
> > should we have third_party/isolate to contain these files? IDK, sticking
them
> at
> > top-level seems as weird as sticking nss.gyp here
> 
> Go on your linux machine and see what is there. third_party/nss is empty. Or
see
> the DEPS file.
> 
> Doing this *is* bad but sadly there's no third_party/nss/src layout.

I'm aware of what we have, I was trying to say whether this should be a TODO()
[or pre-requisite]  to make NSS support this.

The answer to this is somewhat contingent on how you plan to handle other
third-party libraries - particularly those that may be conditionally checked
out, since I don't think this will be your only case, and sticking them all in
src/third_party seems... gross :)

[M-A Ruel, 2014-01-28 21:41:26]

https://codereview.chromium.org/134003004/diff/120001/third_party/nss.isolate
File third_party/nss.isolate (right):

https://codereview.chromium.org/134003004/diff/120001/third_party/nss.isolate...
third_party/nss.isolate:11: #   linux.
On 2014/01/27 21:45:30, Ryan Sleevi wrote:
> On 2014/01/27 21:34:48, M-A Ruel wrote:
> > Doing this *is* bad but sadly there's no third_party/nss/src layout.
> 
> I'm aware of what we have, I was trying to say whether this should be a TODO()
> [or pre-requisite]  to make NSS support this.
> 
> The answer to this is somewhat contingent on how you plan to handle other
> third-party libraries - particularly those that may be conditionally checked
> out, since I don't think this will be your only case, and sticking them all in
> src/third_party seems... gross :)

I think it's going to be gross for a while. It's a trade off here, I'm focusing
on getting this working at all in a way that doesn't interfere too much with
others. I built unit_tests in component builds and the next CL is going to be
quite fun. That said, not polluting third_part is something I care about so I'll
like about alternatives. One option I am playing with is "optional includes" or
"includes inside condition". I'm still undecided about the route I'm going to
take, it'll depend how bad things are going to be. Once I made enough progress,
I'll implement the correct fix and will go back moving the .isolate files
accordingly.

[Ryan Sleevi, 2014-01-28 21:55:44]

LGTM, given that I think the burden of "grossness" is outweighed by the
significant benefit to developer productivity of isolates and the potential
workflow harm of moving/changing how NSS is checked out.

To be clear, I'm *not* opposed to changing DEPS to checkout NSS into
third_party/nss/src, adding third_party/nss into SVN, and moving the .isolate
there, I just realize that will involve multiple steps that will likely take
several weeks to coordinate the infrastructure changes.

[Vadim Sh., 2014-01-29 19:26:45]

lgtm with question

https://codereview.chromium.org/134003004/diff/140001/build/linux/system.isolate
File build/linux/system.isolate (right):

https://codereview.chromium.org/134003004/diff/140001/build/linux/system.isol...
build/linux/system.isolate:5: # This file is only used with the condition
"desktop_linux==1 or chromeos==1".
I don't see where this is enforced/defined... base.isolate unconditionally
includes system.isolate.

[M-A Ruel, 2014-01-29 21:48:56]

https://codereview.chromium.org/134003004/diff/140001/build/linux/system.isolate
File build/linux/system.isolate (right):

https://codereview.chromium.org/134003004/diff/140001/build/linux/system.isol...
build/linux/system.isolate:5: # This file is only used with the condition
"desktop_linux==1 or chromeos==1".
On 2014/01/29 19:26:46, Vadim Sh. wrote:
> I don't see where this is enforced/defined... base.isolate unconditionally
> includes system.isolate.

It is really just informative to help correlate with how base.gyp imports
system.gyp.

[commit-bot: I haz the power, 2014-01-29 21:51:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/maruel@chromium.org/134003004/140001

[commit-bot: I haz the power, 2014-01-29 22:35:04]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[M-A Ruel, 2014-01-30 02:04:06]

Brett happens to be the only owner in both third_party/ and url/, both are
needed so here it is.

[brettw, 2014-02-04 04:27:21]

lgtm

[M-A Ruel, 2014-02-05 17:46:55]

The CQ bit was checked by maruel@chromium.org

[commit-bot: I haz the power, 2014-02-05 17:51:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/maruel@chromium.org/134003004/160001

[commit-bot: I haz the power, 2014-02-05 23:33:55]

Change committed as 249150