mojo: make libmojo_public_test_support a static library.

mojo: make libmojo_public_test_support a static library.

The plan is to eventually get rid of it completely, but
for now, we want to remove ODR violations related to
MOJO_TEST_SUPPORT_EXPORT macro in non-components build.

BUG=609564, 515347
Committed: https://crrev.com/7b2368d72cd3c3fe3236ce679ed5adda380760f2
Cr-Commit-Position: refs/heads/master@{#392180}

[krasin, 2016-05-05 22:16:34]

The CQ bit was checked by krasin@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-05-05 22:17:22]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1953503004/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1953503004/1

[krasin, 2016-05-05 22:18:33]

krasin@google.com changed reviewers:
+ rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-05-05 22:21:19]

You'll want to remove the SO from isolate configs as well. For now you could
just wipe out the conditions block in mojo/mojo.isolate. The file could be
deleted but that would require a more annoying change. I can delete it later.

[commit-bot: I haz the power, 2016-05-05 22:21:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-05 22:21:50]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[krasin, 2016-05-05 22:21:51]

The CQ bit was unchecked by krasin@google.com

[Ken Rockot(use gerrit already), 2016-05-05 22:22:31]

https://codereview.chromium.org/1953503004/diff/1/mojo/mojo_public.gyp
File mojo/mojo_public.gyp (right):

https://codereview.chromium.org/1953503004/diff/1/mojo/mojo_public.gyp#newcod...
mojo/mojo_public.gyp:353: 'type': 'static_library',
move this outside of the conditions block

[krasin, 2016-05-05 22:23:56]

> move this outside of the conditions block

Done. Sorry, should have run 'gclient runhooks' locally.

[Ken Rockot(use gerrit already), 2016-05-05 22:27:01]

On 2016/05/05 at 22:23:56, krasin wrote:
> > move this outside of the conditions block
> 
> Done. Sorry, should have run 'gclient runhooks' locally.

Still need to update mojo/mojo.isolate too

[krasin, 2016-05-05 22:31:26]

> Still need to update mojo/mojo.isolate too

Thanks. Done.

[Ken Rockot(use gerrit already), 2016-05-05 22:32:43]

sorry, i should have been more specific. the isolate file specifies runtime
dependencies which need to be copied to bots in order to run test binaries. with
static linking there's no need to copy anything now. the entire conditions block
in the isolate file can be wiped out.

[Ken Rockot(use gerrit already), 2016-05-05 22:33:29]

On 2016/05/05 at 22:32:43, Ken Rockot wrote:
> sorry, i should have been more specific. the isolate file specifies runtime
dependencies which need to be copied to bots in order to run test binaries. with
static linking there's no need to copy anything now. the entire conditions block
in the isolate file can be wiped out.

(the file could just be deleted, but that would be a more annoying change - ok
to leave it mostly empty instead)

[krasin, 2016-05-06 01:02:40]

Done.
I've opened my eyes a little bit wider, and deleted not only mojo.isolate, but
also test_support_export.h.

[Ken Rockot(use gerrit already), 2016-05-06 01:06:35]

lgtm

Thanks!

[krasin, 2016-05-06 01:09:53]

The CQ bit was checked by krasin@google.com

[commit-bot: I haz the power, 2016-05-06 01:10:12]

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

[krasin, 2016-05-06 01:10:49]

Thank you for your guidance. I am not really familiar with mojo, and would be
unable to realize that we don't need this functionality anymore.

[commit-bot: I haz the power, 2016-05-06 01:18:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-06 01:18:02]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[krasin1, 2016-05-06 01:20:22]

Presubmit bot, because I created CL from @google.com, not @chromium.org, and
it's krasin@chromium.org is an OWNER for the cfi blacklist.

So, I have to say LGTM from this account.

[krasin1, 2016-05-06 01:20:27]

The CQ bit was checked by krasin@chromium.org

[commit-bot: I haz the power, 2016-05-06 01:20:56]

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

[krasin, 2016-05-06 01:45:22]

Boo. Windows bot has mojo tests broken, see
https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel...

Failures look the following:

Failed to get out-of-band test success data, dumping full stdio below:
[6684:3028:0505/181430:41059244:ERROR:icu_util.cc(183)] Invalid file descriptor
to ICU data received.

[6684:3028:0505/181430:41059244:FATAL:icu_util.cc(302)] Check failed: result. 

Backtrace:

	(No symbol) [0x00C54B47]

	(No symbol) [0x00C3E151]

	GetHandleVerifier [0x00CDE97C+506844]

	GetHandleVerifier [0x00CD36A5+461061]

	GetHandleVerifier [0x00CD38C9+461609]

	GetHandleVerifier [0x00CD130D+451949]

	GetHandleVerifier [0x00CD1008+451176]

	GetHandleVerifier [0x00CCF998+445432]

	GetHandleVerifier [0x00CA76CC+280876]

	BaseThreadInitThunk [0x76F4336A+18]

	RtlInitializeExceptionChain [0x77DF9882+99]

	RtlInitializeExceptionChain [0x77DF9855+54]

At this time, I have no idea what happens. Looking...

[krasin, 2016-05-06 01:51:44]

Interestingly, the failure is limited to 32-bit bot. The 64-bit bot is just fine
with the change:
https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64...

[Ken Rockot(use gerrit already), 2016-05-06 01:55:02]

That's... Odd. I don't understand the error at all. Maybe I should just try
deleting the target.
On May 5, 2016 6:51 PM, <krasin@google.com> wrote:

> Interestingly, the failure is limited to 32-bit bot. The 64-bit bot is
> just fine
> with the change:
>
>
https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64...
>
> https://codereview.chromium.org/1953503004/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[krasin, 2016-05-06 01:56:53]

Possibly.

Anyway, let's wait for the second try job for that bot to run these tests:
https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel...

Just to double check it's not a mirage.

[commit-bot: I haz the power, 2016-05-06 02:42:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-06 02:42:36]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[krasin, 2016-05-06 17:41:02]

So, these windows failures. They only happen on 32-bit Windows 7, and dbg vs rel
bots have different errors. rel bot fails while running tests, and dbg bot has a
build time error.

I've tried to reproduce them locally. Tests build and pass in both release and
debug mode. There's a chance that it's an issue with invalid incremental build.
Like, we have the old .so and also linked .lib. If we load .so, we get an ODR
violation that may cause any kind of error message. That would explain the
failing tests on the rel bot, but not the dbg bot build issue.

Why does not it fail on desktop is also not clear.

[krasin, 2016-05-06 18:39:24]

My current guess is that I accidentally removed transitive dependency on
base/base.isolate, that has to do something with icu:
https://code.google.com/p/chromium/codesearch#chromium/src/base/base.isolate&...

I've restored this dependency for one of the tests and started a try job. I
expect mojo_common_unittests to pass and other three mojo tests to fail as in
the previous attempts.

[krasin, 2016-05-06 20:50:42]

Okay, now windows bots are fixed. The guess about base.isolate was correct. Will
try to submit again.

[krasin, 2016-05-06 20:50:48]

The CQ bit was checked by krasin@google.com

[krasin, 2016-05-06 20:50:49]

The patchset sent to the CQ was uploaded after l-g-t-m from rockot@chromium.org,
krasin@chromium.org
Link to the patchset: https://codereview.chromium.org/1953503004/#ps140001
(title: "base.isolate for all")

[commit-bot: I haz the power, 2016-05-06 20:51:10]

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

[commit-bot: I haz the power, 2016-05-06 22:20:35]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-05-06 22:23:27]

Description was changed from

==========
mojo: make libmojo_public_test_support a static library.

The plan is to eventually get rid of it completely, but
for now, we want to remove ODR violations related to
MOJO_TEST_SUPPORT_EXPORT macro in non-components build.

BUG=609564,515347
==========

to

==========
mojo: make libmojo_public_test_support a static library.

The plan is to eventually get rid of it completely, but
for now, we want to remove ODR violations related to
MOJO_TEST_SUPPORT_EXPORT macro in non-components build.

BUG=609564,515347

Committed: https://crrev.com/7b2368d72cd3c3fe3236ce679ed5adda380760f2
Cr-Commit-Position: refs/heads/master@{#392180}
==========

[commit-bot: I haz the power, 2016-05-06 22:23:28]

Patchset 8 (id:??) landed as
https://crrev.com/7b2368d72cd3c3fe3236ce679ed5adda380760f2
Cr-Commit-Position: refs/heads/master@{#392180}

[krasin1, 2016-05-07 00:27:44]

A revert of this CL (patchset #8 id:140001) has been created in
https://codereview.chromium.org/1960703004/ by krasin@chromium.org.

The reason for reverting is: Broke official builds.

BUG=610020.