Adds the OCHamcrest library to the iOS build.

Adds the OCHamcrest library to the iOS build.

OCHamcrest is a dependency of the integration testing framework that will
be used by iOS code. It is only used for tests (and is marked as so in
ios/third_party/ochamcrest/BUILD.gn).

The build is a bit involved as OCHamcrest as it mix relative imports "..."
with imports of the form <OCHamcrest/...> but the source code is not found
in an OCHamcrest directory. So build such a directory structure in
<(SHARED_INTERMEDIATE_DIR)/ios/third_party/ochamcrest as part of the build.

TBR=dpranke@chromium.org
BUG=580730
TEST=None

Committed: https://crrev.com/a9045f46f887e536b148ed436851b132d8e1025c
Cr-Commit-Position: refs/heads/master@{#376985}

[rohitrao (ping after 24h), 2016-02-23 00:05:35]

rohitrao@chromium.org changed reviewers:
+ baxley@chromium.org, blundell@chromium.org, rohitrao@chromium.org

[rohitrao (ping after 24h), 2016-02-23 00:05:36]

+others for their opinion

We have two possible options for the ochamcrest build.

1) This CL.  Copies all ochamcrest files, .h and .m, into
SHARED_INTERMEDIATE_DIR and builds them from there.  This solution is nice
because the sources are listed only once, but it feels a little weird to be
building out of SHARED_INTERMEDIATE_DIR instead of the git repo.

2) The CL I sent out earlier today, which generates forwarding headers in
SHARED_INTERMEDIATE_DIR that include the real header, wherever it lives in the
git repo.  The original CL used pymod_do_main(), which isn't supported by GN. 
We can work around that by hard-coding a list of outputs in the GYP/GN file, but
that will involve duplicating the list, because every file will lead to both an
input and an output.

I'm having a hard time deciding which of these two is the least bad solution, so
I'd like to hear your opinion.  (All of the other solutions we've come up with
have definitely been worse than either of these.)

[sdefresne, 2016-02-23 09:09:00]

On 2016/02/23 at 00:05:36, rohitrao wrote:
> +others for their opinion
> 
> We have two possible options for the ochamcrest build.
> 
> 1) This CL.  Copies all ochamcrest files, .h and .m, into
SHARED_INTERMEDIATE_DIR and builds them from there.  This solution is nice
because the sources are listed only once, but it feels a little weird to be
building out of SHARED_INTERMEDIATE_DIR instead of the git repo.

For what it's worth, building files out from SHARED_INTERMEDIATE_DIR is really
not that unusual. This happens for all .cc/.m/.mm files generated from .proto or
.mojom files. It also happens for files generated as part of the build, like
version_info.cc. A quick and dirty script tells me that we already builds 139
source files from SHARED_INTERMEDIATE_DIR (and one from TARGET_OUTPUT_DIR):

$ ninja -C out/Debug-iphonesimulator -t graph|grep
'label="gen/.*\.\(cc\|mm\|m\)"'|wc -l
     139
$ ninja -C out/Debug-iphonesimulator -t graph|grep
'label="obj/.*\.\(cc\|mm\|m\)"'|wc -l
     1

> 2) The CL I sent out earlier today, which generates forwarding headers in
SHARED_INTERMEDIATE_DIR that include the real header, wherever it lives in the
git repo.  The original CL used pymod_do_main(), which isn't supported by GN. 
We can work around that by hard-coding a list of outputs in the GYP/GN file, but
that will involve duplicating the list, because every file will lead to both an
input and an output.

In my opinion, this is CL is worse as it increase the burden of maintaining the
GN build. It requires duplicating some files, using a custom script and using an
action instead of just using a standard copy target (btw, this does hardlinks
not copy if possible, so it is cheap and force rebuild if the source file
changes as expected).
 
> I'm having a hard time deciding which of these two is the least bad solution,
so I'd like to hear your opinion.  (All of the other solutions we've come up
with have definitely been worse than either of these.)

[blundell, 2016-02-23 09:09:04]

I have no problem with building out of the intermediate dir; it's what we do for
all generated files. We can make this solution cleaner by having a .gypi that
lists the ochamrest sources and then having the GN file read that into a
variable using gypi_to_gn.py. That way we'll only end up listing the files once
overall.

[rohitrao (ping after 24h), 2016-02-23 12:24:05]

LGTM i guess

Have you verified that no-op builds work properly?  Does shutil.copy change
timestamps when it copies files?

It's going to be a little weird if we ever use an ochamcrest header from
chromium code, because we'll need to use a framework-style include instead of
using the full path.

There's also the (very small) potential for us to screw up management of the
copies and have the SHARED_INTERMEDIATE_DIR version of the files be out of sync
with the real versions.  This is what scares me the most, because this could
potentially lead to an ODR violation, and that would probably cause very subtle
bugs instead of outright breakage.  (On the other hand, this is not shipping
code, which limits the scope of the damage considerably.)

Is anyone else concerned about ODR violations from file mismatches?

https://codereview.chromium.org/1723663002/diff/1/ios/third_party/ochamcrest/...
File ios/third_party/ochamcrest/ochamcrest.gyp (right):

https://codereview.chromium.org/1723663002/diff/1/ios/third_party/ochamcrest/...
ios/third_party/ochamcrest/ochamcrest.gyp:184:
'<(SHARED_INTERMEDIATE_DIR)/ochamcrest/OCHamcrest/',
Why did you go with "ochamcrest" instead of something more like a full path,
"ios/third_party/ochamcrest"?

[blundell, 2016-02-23 12:34:40]

On 2016/02/23 12:24:05, rohitrao wrote:
> LGTM i guess
> 
> Have you verified that no-op builds work properly?  Does shutil.copy change
> timestamps when it copies files?
> 
> It's going to be a little weird if we ever use an ochamcrest header from
> chromium code, because we'll need to use a framework-style include instead of
> using the full path.
> 
> There's also the (very small) potential for us to screw up management of the
> copies and have the SHARED_INTERMEDIATE_DIR version of the files be out of
sync
> with the real versions.  This is what scares me the most, because this could
> potentially lead to an ODR violation, and that would probably cause very
subtle
> bugs instead of outright breakage.  (On the other hand, this is not shipping
> code, which limits the scope of the damage considerably.)
> 
> Is anyone else concerned about ODR violations from file mismatches?

There's always a danger like that with this kind of thing, but this CL is
correct AFAICT: Whenever the source list changes the inputs to the copy target
will change, which will force it to rerun. Since the copy target is a hard
dependency this will happen before the ochamcrest target starts being built.
That target will then be rebuilt since its sources will be new. The sources that
will be built will be exactly the current set of sources that were just copied
in.
> 
>
https://codereview.chromium.org/1723663002/diff/1/ios/third_party/ochamcrest/...
> File ios/third_party/ochamcrest/ochamcrest.gyp (right):
> 
>
https://codereview.chromium.org/1723663002/diff/1/ios/third_party/ochamcrest/...
> ios/third_party/ochamcrest/ochamcrest.gyp:184:
> '<(SHARED_INTERMEDIATE_DIR)/ochamcrest/OCHamcrest/',
> Why did you go with "ochamcrest" instead of something more like a full path,
> "ios/third_party/ochamcrest"?

[sdefresne, 2016-02-23 12:58:39]

As blundell@ said, the copy will happen before the target are run, and only if
inputs are more recent than output. I can also change the python script to first
try a hard-link if you want (as is done by gn) and fall back to copy if it fails
(to reduce the creation of files).

To protect against incorrect inclusion of code from ios/third_party/ochamcrest,
I would just put a DEPS rule in ios/ that forbids everything from
ios/third_party/ochamcrest with a comment saying that it should be included as
<OCHamcrest/...>.

I'm going to rebase my patch and make the changes before landing the CL.

https://codereview.chromium.org/1723663002/diff/1/ios/third_party/ochamcrest/...
File ios/third_party/ochamcrest/ochamcrest.gyp (right):

https://codereview.chromium.org/1723663002/diff/1/ios/third_party/ochamcrest/...
ios/third_party/ochamcrest/ochamcrest.gyp:184:
'<(SHARED_INTERMEDIATE_DIR)/ochamcrest/OCHamcrest/',
On 2016/02/23 at 12:24:04, rohitrao wrote:
> Why did you go with "ochamcrest" instead of something more like a full path,
"ios/third_party/ochamcrest"?

Lazyness when I made the change yesterday. I agree that it should have been
"ios/third_party/ochamcrest" (in fact I told this to blundell offline when he
was looking at the CL).

[sdefresne, 2016-02-23 13:25:00]

Description was changed from

==========
Adds the OCHamcrest library to the iOS build.

Adapted from a patch by baxley@chromium.org.

BUG=580730
TEST=None

patch from issue 1711953003 at patchset 80001
(http://crrev.com/1711953003#ps80001)
==========

to

==========
Adds the OCHamcrest library to the iOS build.

OCHamcrest is a dependency of the integration testing framework that will
be used by iOS code. It is only used for tests (and is marked as so in
ios/third_party/ochamcrest/BUILD.gn).

The build is a bit involved as OCHamcrest as it mix relative imports "..."
with imports of the form <OCHamcrest/...> but the source code is not found
in an OCHamcrest directory. So build such a directory structure in
<(SHARED_INTERMEDIATE_DIR)/ios/third_party/ochamcrest as part of the build.

BUG=580730
TEST=None
==========

[sdefresne, 2016-02-23 13:42:45]

PTAL

[sdefresne, 2016-02-23 13:43:18]

sdefresne@chromium.org changed reviewers:
+ dpranke@chromium.org

[sdefresne, 2016-02-23 13:43:39]

Description was changed from

==========
Adds the OCHamcrest library to the iOS build.

OCHamcrest is a dependency of the integration testing framework that will
be used by iOS code. It is only used for tests (and is marked as so in
ios/third_party/ochamcrest/BUILD.gn).

The build is a bit involved as OCHamcrest as it mix relative imports "..."
with imports of the form <OCHamcrest/...> but the source code is not found
in an OCHamcrest directory. So build such a directory structure in
<(SHARED_INTERMEDIATE_DIR)/ios/third_party/ochamcrest as part of the build.

BUG=580730
TEST=None
==========

to

==========
Adds the OCHamcrest library to the iOS build.

OCHamcrest is a dependency of the integration testing framework that will
be used by iOS code. It is only used for tests (and is marked as so in
ios/third_party/ochamcrest/BUILD.gn).

The build is a bit involved as OCHamcrest as it mix relative imports "..."
with imports of the form <OCHamcrest/...> but the source code is not found
in an OCHamcrest directory. So build such a directory structure in
<(SHARED_INTERMEDIATE_DIR)/ios/third_party/ochamcrest as part of the build.

TBR=dpranke@chromium.org
BUG=580730
TEST=None
==========

[rohitrao (ping after 24h), 2016-02-23 13:55:34]

LGTM although I don't totally understand the hard link code.

[sdefresne, 2016-02-23 13:58:56]

The CQ bit was checked by sdefresne@chromium.org

[commit-bot: I haz the power, 2016-02-23 13:59:03]

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

[commit-bot: I haz the power, 2016-02-23 15:22:53]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-02-23 15:24:03]

Description was changed from

==========
Adds the OCHamcrest library to the iOS build.

OCHamcrest is a dependency of the integration testing framework that will
be used by iOS code. It is only used for tests (and is marked as so in
ios/third_party/ochamcrest/BUILD.gn).

The build is a bit involved as OCHamcrest as it mix relative imports "..."
with imports of the form <OCHamcrest/...> but the source code is not found
in an OCHamcrest directory. So build such a directory structure in
<(SHARED_INTERMEDIATE_DIR)/ios/third_party/ochamcrest as part of the build.

TBR=dpranke@chromium.org
BUG=580730
TEST=None
==========

to

==========
Adds the OCHamcrest library to the iOS build.

OCHamcrest is a dependency of the integration testing framework that will
be used by iOS code. It is only used for tests (and is marked as so in
ios/third_party/ochamcrest/BUILD.gn).

The build is a bit involved as OCHamcrest as it mix relative imports "..."
with imports of the form <OCHamcrest/...> but the source code is not found
in an OCHamcrest directory. So build such a directory structure in
<(SHARED_INTERMEDIATE_DIR)/ios/third_party/ochamcrest as part of the build.

TBR=dpranke@chromium.org
BUG=580730
TEST=None

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

[commit-bot: I haz the power, 2016-02-23 15:24:04]

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