New issue
Advanced search Search tips

Issue 897969 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 4
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug-Regression
Proj-VR
Proj-XR

Blocking:
issue 383340



Sign in to add a comment

VrNfcSimulator.apk and apks depending on it are not deterministic

Project Member Reported by thakis@chromium.org, Oct 22

Issue description

We have a bot that checks that our build is deterministic. It builds all tests, then copies the build dir elsewhere, builds everything again, and then compares outputs. Outputs should be the same both times. The android build used to be 100% deterministic; now we have 3 small regressions. For this bug:

  apks/VrNfcSimulator.apk

fails this test. Need to figure out why and fix.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 22

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

commit 8d774d9ef69d1284a601a122b75f147377b475a5
Author: Nico Weber <thakis@chromium.org>
Date: Mon Oct 22 23:52:46 2018

Update deterministic build whitelist for android and mac a bit.

Both bots are red; this improves things a bit (but likely not completely,
I only looked at a handful recent builds each).

Bug:  897969 , 897970 ,330262
Change-Id: I2c399774175e57285eafc1c0de99a6f893e62fea
Reviewed-on: https://chromium-review.googlesource.com/c/1295270
Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601785}
[modify] https://crrev.com/8d774d9ef69d1284a601a122b75f147377b475a5/tools/determinism/deterministic_build_whitelist.pyl

Status: Assigned (was: Untriaged)
Traveling this week, so probably won't be able to make much progress on this til next week. Do you happen to have the first failed build?
Oh sorry, link to bot: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Deterministic%20Android

I think this has been failing for a long time (at least a month, possibly much longer).
ChromePublicTestVr.apk also seems to be non-deterministic, which makes sense since they have pretty much the same deps. Could the workaround for a desugar issue be potentially causing this? https://cs.chromium.org/chromium/src/chrome/android/BUILD.gn?q=vrnfcsimulator&sq=package:chromium&dr=C&l=757
Summary: VrNfcSimulator.apk and apks depending on it are not deterministic (was: VrNfcSimulator.apk is not deterministic)
I don't know. But you're right, looks like a few more apks have this problem:

    'apks/ChromePublicTest.apk',
    'apks/VrNfcSimulator.apk',
    'browser_tests_apk/browser_tests-debug.apk',
    'unit_tests_apk/unit_tests-debug.apk',

is my current list. Let's assume for now that this is all due to this issue, I'll update the whitelist.
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 25

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

commit d80798db579edc48b03fe101122965e4572cafe5
Author: Nico Weber <thakis@chromium.org>
Date: Thu Oct 25 01:46:42 2018

Update Android deterministic build whitelist.

apks/ChromePublicTest.apk was different in both of the two builds below,
the test apks were different once each.

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Deterministic%20Android/2948
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Deterministic%20Android/2950

Bug:  897969 , 383340 
Change-Id: I6b55470c5c1507856cbc416bc484b5cc1b5427f8
Reviewed-on: https://chromium-review.googlesource.com/c/1297264
Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602567}
[modify] https://crrev.com/d80798db579edc48b03fe101122965e4572cafe5/tools/determinism/deterministic_build_whitelist.pyl

If the APKs you listed are indeed non-deterministic for the same reason, then this isn't VR-related. ChromePublicTestVr.apk and VrNfcSimulator.apk depend on stuff from ChromePublicTest.apk, not the other way around.

I imagine there isn't a huge amount of overlap in test-only dependencies between the unit_tests and browser_tests targets and the instrumentation test targets - does GN have a good way of listing out the dependency tree for a target for a given set of GN args so we can compare between the various targets and see what overlaps?
Using:

gn desc <output directory> $TARGET deps --tree > $TARGET_deps.txt

to list all of the deps for each target, then use a simple Python script to take each list of deps, turn it into a set, and get the intersection of all deps, we get 3013 shared dependencies between all the affected APKs. So that's not particularly helpful.

If there's another target that should share a lot of dependencies, but is still deterministic, we might be able to narrow that down to a manageable number.
I excluded all the dependencies from ChromePublic.apk, which seems to be deterministic. This brought the number down to only 100 shared dependencies, almost all of which are related to instrumentation tests.

List is attached, although I'm not sure what can be done to narrow it further or test for which one is the culprit.
culprit_deps.txt
6.1 KB View Download
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 23

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

commit c934990c2e8e83850b1886f8bc9013c3d5d1a364
Author: Andrew Grieve <agrieve@chromium.org>
Date: Fri Nov 23 18:46:58 2018

compare_build_artifacts.py: Give more info about diffs in zip files

Bug:  897969 
Change-Id: Ifef819c0308ffd5065e54915065cd8a586ba217f
Reviewed-on: https://chromium-review.googlesource.com/c/1349749
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610652}
[modify] https://crrev.com/c934990c2e8e83850b1886f8bc9013c3d5d1a364/tools/determinism/compare_build_artifacts.py

Project Member

Comment 11 by bugdroid1@chromium.org, Nov 23

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

commit c9e7df88cd0db957326dd4ed9dd988799afae6dd
Author: Andrew Grieve <agrieve@chromium.org>
Date: Fri Nov 23 22:19:16 2018

Android: Fix multidex zip files having non-deterministic file ordering

Works around d8 bug:
https://issuetracker.google.com/issues/119945929

Bug:  897969 
Change-Id: I30c18836040d025ed9dbba3ebde6c33d8459b5be
Reviewed-on: https://chromium-review.googlesource.com/c/1349854
Reviewed-by: Eric Stevenson <estevenson@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610678}
[modify] https://crrev.com/c9e7df88cd0db957326dd4ed9dd988799afae6dd/build/android/gyp/dex.py

Cc: jbudorick@chromium.org
 Issue 897970  has been merged into this issue.
Project Member

Comment 13 by bugdroid1@chromium.org, Nov 27

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

commit 30327733e35d9d76f94161a010b70ab39fa59e53
Author: Nico Weber <thakis@chromium.org>
Date: Tue Nov 27 15:05:33 2018

android deterministic: Remove remaining whitelist entries.

Looks like https://chromium-review.googlesource.com/c/chromium/src/+/1349854
fixed these too.

Bug:  897970 , 897969 
Change-Id: I09dac10a533511fbab3d49e9c0055ebc7b507f2d
Reviewed-on: https://chromium-review.googlesource.com/c/1350190
Commit-Queue: agrieve <agrieve@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611109}
[modify] https://crrev.com/30327733e35d9d76f94161a010b70ab39fa59e53/tools/determinism/deterministic_build_whitelist.pyl

Is this fixed now?
Owner: agrieve@chromium.org
Status: Fixed (was: Assigned)
Yes!

Sign in to add a comment