Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(167)

Issue 1704383002: [GN] Don't rewrite files with the same contents (Closed)

Created:
4 years, 10 months ago by Tomasz Moniuszko
Modified:
4 years, 10 months ago
Reviewers:
brettw
CC:
chromium-reviews, tfarina
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[GN] Don't rewrite files with the same contents Reland of https://codereview.chromium.org/1656253003 with fix. Reason for revert: Need to revert this patch according https://sites.google.com/a/chromium.org/dev/developers/tree-sheriffs/sheriffing-bug-queues. This patch added a flaky test FilesystemUtils.WriteFileIfChanged. ----- It's a test flake: 1) Try to find the patch that caused the flake. It should be recent (e.g. last day or two) in all likelihood. 2) If successful with finding that patch, revert the patch. This is especially true if the flake is from a new test introduced in that patch. 3) Close the bug. ----- The test has failed in the following builds: http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/176911 http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/176911 http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/176735 http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/176715 http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/176715 http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/176561 http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/176462 http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/176375 ----- Example failure: [ RUN ] FilesystemUtils.WriteFileIfChanged ../../tools/gn/filesystem_utils_unittest.cc:610: Failure Expected: (last_modified) != (file_info.last_modified), actual: 2016-02-04 18:06:36.920 UTC vs 2016-02-04 18:06:36.920 UTC [ FAILED ] FilesystemUtils.WriteFileIfChanged (3 ms) [315/315] FilesystemUtils.WriteFileIfChanged (3 ms) Retrying 1 test (retry #2) [ RUN ] FilesystemUtils.WriteFileIfChanged ../../tools/gn/filesystem_utils_unittest.cc:610: Failure Expected: (last_modified) != (file_info.last_modified), actual: 2016-02-04 18:06:36.936 UTC vs 2016-02-04 18:06:36.936 UTC [ FAILED ] FilesystemUtils.WriteFileIfChanged (2 ms) [316/316] FilesystemUtils.WriteFileIfChanged (2 ms) Retrying 1 test (retry #3) [ RUN ] FilesystemUtils.WriteFileIfChanged ../../tools/gn/filesystem_utils_unittest.cc:610: Failure Expected: (last_modified) != (file_info.last_modified), actual: 2016-02-04 18:06:36.952 UTC vs 2016-02-04 18:06:36.952 UTC [ FAILED ] FilesystemUtils.WriteFileIfChanged (2 ms) [317/317] FilesystemUtils.WriteFileIfChanged (2 ms) 1 test failed: FilesystemUtils.WriteFileIfChanged (../../tools/gn/filesystem_utils_unittest.cc:579) ----- More details in http://crbug.com/584548. Original issue's description: > [GN] Don't rewrite files with the same contents > > BUG= > > Committed: https://crrev.com/f8ea5cceefcedd4a01935d5ac4d2ba71e23ac13e > Cr-Commit-Position: refs/heads/master@{#373544} BUG=584548 Committed: https://crrev.com/36deb395534c77295db41cc5f3e6b9627fbc5aa8 Cr-Commit-Position: refs/heads/master@{#376430}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+157 lines, -125 lines) Patch
M tools/gn/filesystem_utils.h View 1 chunk +12 lines, -0 lines 0 comments Download
M tools/gn/filesystem_utils.cc View 1 chunk +80 lines, -0 lines 0 comments Download
M tools/gn/filesystem_utils_unittest.cc View 2 chunks +57 lines, -0 lines 0 comments Download
M tools/gn/function_write_file.cc View 2 chunks +3 lines, -68 lines 0 comments Download
M tools/gn/ninja_target_writer.cc View 1 chunk +1 line, -3 lines 0 comments Download
M tools/gn/visual_studio_writer.cc View 4 chunks +4 lines, -54 lines 0 comments Download

Messages

Total messages: 13 (7 generated)
Tomasz Moniuszko
FilesystemUtils.WriteFileIfChanged test fixed. Now it compares file contents instead of modification time in case when ...
4 years, 10 months ago (2016-02-18 15:27:29 UTC) #3
brettw
lgtm LGTM, can you clean up the commit message? I normally use the original commit ...
4 years, 10 months ago (2016-02-18 21:43:55 UTC) #5
Tomasz Moniuszko
On 2016/02/18 21:43:55, brettw wrote: > lgtm > > LGTM, can you clean up the ...
4 years, 10 months ago (2016-02-19 10:45:34 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1704383002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1704383002/1
4 years, 10 months ago (2016-02-19 10:46:02 UTC) #9
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 10 months ago (2016-02-19 11:16:50 UTC) #11
commit-bot: I haz the power
4 years, 10 months ago (2016-02-19 11:17:59 UTC) #13
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/36deb395534c77295db41cc5f3e6b9627fbc5aa8
Cr-Commit-Position: refs/heads/master@{#376430}

Powered by Google App Engine
This is Rietveld 408576698