VrNfcSimulator.apk and apks depending on it are not deterministic |
||||
Issue descriptionWe 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.
,
Oct 23
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?
,
Oct 23
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).
,
Oct 23
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
,
Oct 24
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.
,
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
,
Oct 25
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?
,
Oct 29
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.
,
Oct 29
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.
,
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
,
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
,
Nov 24
,
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
,
Dec 3
Is this fixed now?
,
Dec 4
Yes! |
||||
►
Sign in to add a comment |
||||
Comment 1 by bugdroid1@chromium.org
, Oct 22