compare_build_artifacts failing on chromium.win/Windows deterministic due to updates to xtb files |
||||||||
Issue descriptionFiled by sheriff-o-matic@appspot.gserviceaccount.com on behalf of ortuno@chromium.org compare_build_artifacts failing on chromium.win/Windows deterministic Builders failed on: - Windows deterministic: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Windows%20deterministic
,
Nov 30
Could you take a look, Nico? A lot of CLs made it into the first failing build (https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Windows%20deterministic/10903). Nothing jumped out at me, but I don't know exactly what to look for. Here's one example: unit_tests.exe : DIFFERENT (unexpected): 53839 out of 118800896 bytes are different (0.05%) unit_tests.exe.pdb : DIFFERENT (unexpected): 28 out of 319504384 bytes are different (0.00%) 0xa51f160 : 00000000400000402e72737263000000806c04000040e906006e0400007ed106 '....@..@.rsrc....l...@...n...~..' 00000000400000402e72737263000000886c04000040e906006e0400007ed106 '....@..@.rsrc....l...@...n...~..' ^ 0x111f7040: 1a0036110a000c000040e906806c0400400000402e727372630000001a003611 '..6......@...l..@..@.rsrc.....6.' 1a0036110a000c000040e906886c0400400000402e727372630000001a003611 '..6......@...l..@..@.rsrc.....6.' ^ 0x1170bfa0: 80200000400000401d68000027a372be0000000000002a680400000000000000 '. ..@..@.h..'.r.......*h........' 80200000400000401d680000752c0ea00000000000002a680400000000000000 '. ..@..@.h..u,........*h........' ++ ^^^ ^ ^^ 0x12e2c2e0: 80200000400000401d68000027a372be000000000a00000080200000004c0400 '. ..@..@.h..'.r.......... ...L..' 80200000400000401d680000752c0ea0000000000a00000080200000084c0400 '. ..@..@.h..u,........... ...L..' ++ ^^^ ^ ^ ^^ 0x12e2c300: 400000401d680000e17aa063000000000b000000000000006800000040000042 '@..@.h...z.c............h...@..B' 400000401d6800004d0a89ce000000000b000000000000006800000040000042 '@..@.h..M...............h...@..B' +++++++ + ^ 0x12edc960: 00000000806c04000901000000000b00ffffffff0000000000d63e0008020000 '.....l....................>.....' 00000000886c04000901000000000b00ffffffff0000000000d63e0008020000 '.....l....................>.....' ^ 0x13046000: 942e31018121b837010000008121b83784b2769c4c4c44205044422e96000000 '..1..!.7.....!.7..v.LLD PDB.....' 942e310122b1d8a30100000022b1d8a33348d20b4c4c44205044422e96000000 '..1."......."...3H..LLD PDB.....' 942e310122b1d8a30100000022b1d8a33348d20b4c4c44205044422e96000000 '..1."......."...3H..LLD PDB.....
,
Nov 30
,
Dec 1
I tried doing a component build of installer_util_unittests.exe locally and couldn't repro. The bot is doing a static library build, but I'd be a bit surprised if that's the cause (nevertheless, I'll try that next). Here are the 5 exes (+ their pdbs) that show differences: installer_util_unittests notification_helper_unittests setup setup_unittests unit_tests setup.exe is the only non-test binary.
,
Dec 1
No local diffs in a release build either. So I'm guessing there's some incrementality issue. I did full builds in two build dirs. The bot does incremental builds in one and full builds in the other. Intriguing!
,
Dec 3
,
Dec 3
I'll try to bisect. I'm starting by getting ce48cf76141dac6cd5de72034096cd00cd57f6fa / r612334 (last known good) and doing two full builds. Then I'll sync halfway into the changes in https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Windows%20deterministic/10903 and do an incremental build and one full build and compare installer_util_unittests, then repeat.
,
Dec 3
Bad (from https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Windows%20deterministic/10903): 612400 First half-way point (testing now): 612367 / 96eb4c261239c633a485817b39e3583e7f3ab9b2
,
Dec 3
I can repro with the approach in comment 7 at 612367. Now doing two full builds at r612334 again and will then try again one incremental and one full build at 612350. Also attaching the two different exes, diff looks pretty small actually. The compare script should probably print the start of the diff even if the size doesn't match.
,
Dec 3
Actually, the exe diffs looks like this is .rsrc / grit-related, so maybe it's triggered by 612348. Trying 612347 locally first and if there's no diff there, then 612348.
,
Dec 3
612347 is 8adb2d699d485be7080541f9b37fffea60ef1983
,
Dec 3
No diff at 612347 . Trying 612348 / 3a393aebc4b0bbfbfeb7a2fca4b99cb8285ac75e
,
Dec 3
It's due to 612348 . This being Windows-only and only affecting a small number of binaries suggests that there's something we need to do on xtb updates that almost everything gets right but some rule somewhere doesn't...
,
Dec 3
I hacked up compare_build_artifacts.py to compare all files and ran it with --recursive, here's the output:
Unexpected files with diffs:
gen\chrome\installer\util\installer_util_strings.rc
installer_util_unittests.exe
installer_util_unittests.exe.pdb
obj\chrome\installer\util\strings\installer_util_strings.res
obj\chrome\installer\util\strings\installer_util_strings.res_ms_rc
toolchain.ninja
diff --git a/tools/determinism/compare_build_artifacts.py b/tools/determinism/compare_build_artifacts.py
index 3546f763980d..ba690736f121 100755
--- a/tools/determinism/compare_build_artifacts.py
+++ b/tools/determinism/compare_build_artifacts.py
@@ -24,17 +24,19 @@ BASE_DIR = os.path.dirname(os.path.abspath(__file__))
def get_files_to_compare(build_dir, recursive=False):
"""Get the list of files to compare."""
+ #return set(['installer_util_unittests.exe'])
# TODO(maruel): Add '.pdb'.
allowed = frozenset(
- ('', '.apk', '.app', '.dll', '.dylib', '.exe', '.nexe', '.so'))
+ ('.pak',))
# .bin is for the V8 snapshot files natives_blob.bin, snapshot_blob.bin
- non_x_ok_exts = frozenset(('.apk', '.bin', '.isolated', '.zip'))
+ non_x_ok_exts = frozenset()
def check(f):
if not os.path.isfile(f):
return False
if os.path.basename(f).startswith('.'):
return False
+ return True
ext = os.path.splitext(f)[1]
if ext in non_x_ok_exts:
return True
@@ -325,7 +327,8 @@ def compare_build_artifacts(first_dir, second_dir, ninja_path, target_platform,
unexpected_diffs.append(f)
tag = 'unexpected'
result = 'DIFFERENT (%s): %s' % (tag, result)
- print('%-*s: %s' % (max_filepath_len, f, result))
+ if tag != 'equal':
+ print('%-*s: %s' % (max_filepath_len, f, result))
unexpected_diffs.sort()
print('Equals: %d' % len(equals))
,
Dec 3
,
Dec 3
Filed bug 911183 for the toolchain.ninja diff, that's unrelated.
,
Dec 3
Diffing gen\chrome\installer\util\installer_util_strings.rc (opened as `:e ++enc=utf16le` after doing `set encoding=utf-8`) this line is different: IDS_INSTALL_OS_ERROR_TE "ఇన్స్టాలేషన్ సమయంలో ఆపరేటింగ్ సిస్టమ్ లోపం సంభవించింది. దయచేసి vs IDS_INSTALL_OS_ERROR_TE "ఇన్స్టాలేషన్ సమయంలో ఆపరేటింగ్ సిస్టమ్ ఎర్రర్ ఏర్పడింది. దయచేసి Maybe grit's rc writer doesn't use depfiles or something?
,
Dec 3
Actually IDS_INSTALL_OS_ERROR_TE "ఇన్స్టాలేషన్ సమయంలో ఆపరేటింగ్ సిస్టమ్ లోపం సంభవించింది. దయచేసి Chromiumని మళ్లీ డౌన్లోడ్ చేయండి." vs IDS_INSTALL_OS_ERROR_TE "ఇన్స్టాలేషన్ సమయంలో ఆపరేటింగ్ సిస్టమ్ ఎర్రర్ ఏర్పడింది. దయచేసి Chromiumని మళ్లీ డౌన్లోడ్ చేయండి."
,
Dec 3
Oh, looks like the h and rc here are created by chrome/installer/util/prebuild/create_string_rc.py which just doesn't use a depfile at all, which is probably the bug, and which explains why this is Windows only. This explains installer_util_unittests for sure, probably setup, setup_unittests, and unit_tests. notification_helper_unittests might be something else, but let's fix this first and then see what the bot does. create_string_rc has been around since initial.commit, but grt has touched it some over the years. (grt, just cc'ing you so you're not surprised when I send you a patch in a bit, nothing else to do for you here.)
,
Dec 3
,
Dec 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1c59d48c61bfd44779d00e7e381cb0a11e0f0d5e commit 1c59d48c61bfd44779d00e7e381cb0a11e0f0d5e Author: Nico Weber <thakis@chromium.org> Date: Mon Dec 03 20:33:21 2018 win: Make chrome/installer/util/prebuild/create_string_rc write a depfile. create_string_rc.py globs a bunch of xtb files off disk behind gn's and ninja's back, so that they didn't know that create_string_rc.py needs to rerun when the xtb files are updated. Make the script write a depfile so that ninja knows to re-run the script if an xtb file is udpated. (This is incomplete: if an xtb file is added or removed and nothing else changes, the step still won't re-run. The Right Fix is to list all the xtb files in the gn file and pass them to the script, then make the script assert that the glob matches the passed-in line. That way, if an xtb is added or removed, the command-line tracking will make sure that the command re-runs. But xtb files being added or removed is super rare, so let's punt on this for now.) Bug: 910465 Change-Id: I7050dee3c62d997b750d8b150504a04d086518b8 Reviewed-on: https://chromium-review.googlesource.com/c/1359087 Reviewed-by: Robert Shield <robertshield@chromium.org> Commit-Queue: Nico Weber <thakis@chromium.org> Cr-Commit-Position: refs/heads/master@{#613222} [modify] https://crrev.com/1c59d48c61bfd44779d00e7e381cb0a11e0f0d5e/chrome/installer/util/BUILD.gn [modify] https://crrev.com/1c59d48c61bfd44779d00e7e381cb0a11e0f0d5e/chrome/installer/util/prebuild/create_string_rc.py
,
Dec 4
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Windows%20deterministic/10949 |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by ortuno@chromium.org
, Nov 30