Rebaseline via webkit-patch produces confusing git changes |
||||
Issue descriptionAfter running webkit-patch rebaseline-cl fast/borders/border-antialiasing.html my git status reports this set of changes # Changes to be committed: # (use "git reset HEAD <file>..." to unstage) # # copied: third_party/WebKit/LayoutTests/platform/linux/fast/borders/border-antialiasing-expected.png -> third_party/WebKit/LayoutTests/platform/android/fast/borders/border-antialiasing-expected.png # new file: third_party/WebKit/LayoutTests/platform/linux-precise/fast/borders/border-antialiasing-expected.png # modified: third_party/WebKit/LayoutTests/platform/linux/fast/borders/border-antialiasing-expected.png # new file: third_party/WebKit/LayoutTests/platform/mac-mac10.10/fast/borders/border-antialiasing-expected.png # new file: third_party/WebKit/LayoutTests/platform/mac-mac10.9/fast/borders/border-antialiasing-expected.png # new file: third_party/WebKit/LayoutTests/platform/mac-retina/fast/borders/border-antialiasing-expected.png # modified: third_party/WebKit/LayoutTests/platform/mac/fast/borders/border-antialiasing-expected.png # modified: third_party/WebKit/LayoutTests/platform/win/fast/borders/border-antialiasing-expected.png # new file: third_party/WebKit/LayoutTests/platform/win7/fast/borders/border-antialiasing-expected.png # # Changes not staged for commit: # (use "git add/rm <file>..." to update what will be committed) # (use "git checkout -- <file>..." to discard changes in working directory) # # deleted: third_party/WebKit/LayoutTests/platform/linux-precise/fast/borders/border-antialiasing-expected.png # deleted: third_party/WebKit/LayoutTests/platform/mac-mac10.10/fast/borders/border-antialiasing-expected.png # deleted: third_party/WebKit/LayoutTests/platform/mac-mac10.9/fast/borders/border-antialiasing-expected.png # deleted: third_party/WebKit/LayoutTests/platform/mac-retina/fast/borders/border-antialiasing-expected.png # deleted: third_party/WebKit/LayoutTests/platform/win7/fast/borders/border-antialiasing-expected.png I find this very confusing. There are files that are new in the "Changes to be committed" while also "deleted" in "Changes not staged for commit". Not being a git expert I don't know what will happen if I commit this change. I presume the confusion arises because the tool deletes then adds the files, rather than just leaving them modified in place.
,
Aug 21 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2dc14ad8012cd602f7cee0ecbb0f9be05ed3d76e commit 2dc14ad8012cd602f7cee0ecbb0f9be05ed3d76e Author: qyearsley <qyearsley@chromium.org> Date: Sun Aug 21 00:21:00 2016 Don't run git add/rm after rebaseline commands for rebaseline-cl. BUG= 639410 , 474273 Review-Url: https://codereview.chromium.org/2261833003 Cr-Commit-Position: refs/heads/master@{#413354} [modify] https://crrev.com/2dc14ad8012cd602f7cee0ecbb0f9be05ed3d76e/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py [modify] https://crrev.com/2dc14ad8012cd602f7cee0ecbb0f9be05ed3d76e/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl.py
,
Aug 24 2016
,
Sep 6 2016
,
Sep 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9dc06fa0fd9e3ffb68b4feeef5158ab54f2ee6b6 commit 9dc06fa0fd9e3ffb68b4feeef5158ab54f2ee6b6 Author: qyearsley <qyearsley@chromium.org> Date: Thu Sep 15 23:36:53 2016 Refactor the way that SCM changes are tracked and aggregated in rebaseline.py. This is a preliminary refactoring CL which is intended to consolidate related logic into one place, and prepare for the real change that I want to make: That is, apply git changes only once in _rebaseline after all changes are made, to fix http://crbug.com/639410 . I want to make two follow-up changes after this: (1) Change _rebaseline to apply git changes only once after all commands are run. (2) Refactor and simplify ChangeSet, _serial_commands, and _run_in_parallel. BUG= 639410 Review-Url: https://codereview.chromium.org/2347643002 Cr-Commit-Position: refs/heads/master@{#419038} [modify] https://crrev.com/9dc06fa0fd9e3ffb68b4feeef5158ab54f2ee6b6/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/optimize_baselines.py [modify] https://crrev.com/9dc06fa0fd9e3ffb68b4feeef5158ab54f2ee6b6/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py [modify] https://crrev.com/9dc06fa0fd9e3ffb68b4feeef5158ab54f2ee6b6/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py
,
Sep 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/34fdc9df872c6ce13e6c3c546084435321d4e882 commit 34fdc9df872c6ce13e6c3c546084435321d4e882 Author: qyearsley <qyearsley@chromium.org> Date: Thu Sep 22 21:24:23 2016 In rebaseline.py, update SCM and expectations all at once after all commands. This is meant to fix http://crbug.com/639410 by making it so that if there are files that are both added and deleted (e.g. downloaded and then de-duped), they won't ever be staged since all changes would be aggregated and de-duped before running git add/rm. BUG= 639410 Review-Url: https://codereview.chromium.org/2341643005 Cr-Commit-Position: refs/heads/master@{#420461} [modify] https://crrev.com/34fdc9df872c6ce13e6c3c546084435321d4e882/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/optimize_baselines_unittest.py [modify] https://crrev.com/34fdc9df872c6ce13e6c3c546084435321d4e882/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py [modify] https://crrev.com/34fdc9df872c6ce13e6c3c546084435321d4e882/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl.py [modify] https://crrev.com/34fdc9df872c6ce13e6c3c546084435321d4e882/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py
,
Sep 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/48a6db917e5860b4798c5c16565bbc03304433d3 commit 48a6db917e5860b4798c5c16565bbc03304433d3 Author: qyearsley <qyearsley@chromium.org> Date: Wed Sep 28 17:27:42 2016 Revert of In rebaseline.py, update SCM and expectations all at once after all commands. (patchset #2 id:20001 of https://codereview.chromium.org/2341643005/ ) Reason for revert: Caused rebaseline-o-matic to fail: http://crrev.com/650944 Could be re-landed with a fix. Original issue's description: > In rebaseline.py, update SCM and expectations all at once after all commands. > > This is meant to fix http://crbug.com/639410 by making it so that if there are files that are both added and deleted (e.g. downloaded and then de-duped), they won't ever be staged since all changes would be aggregated and de-duped before running git add/rm. > > BUG= 639410 > > Committed: https://crrev.com/34fdc9df872c6ce13e6c3c546084435321d4e882 > Cr-Commit-Position: refs/heads/master@{#420461} TBR=dpranke@chromium.org,wkorman@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 639410 Review-Url: https://codereview.chromium.org/2380533003 Cr-Commit-Position: refs/heads/master@{#421556} [modify] https://crrev.com/48a6db917e5860b4798c5c16565bbc03304433d3/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/optimize_baselines_unittest.py [modify] https://crrev.com/48a6db917e5860b4798c5c16565bbc03304433d3/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py [modify] https://crrev.com/48a6db917e5860b4798c5c16565bbc03304433d3/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl.py [modify] https://crrev.com/48a6db917e5860b4798c5c16565bbc03304433d3/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py
,
Oct 4 2016
Update about why rebaseline was trying to add files that don't exist ( bug 650944 ): The overall process of what was happening was: 1. Existing baselines may be copied by copy-existing-baselines-internal; this may add new files but not remove any. 2. Baselines may be downloaded by rebaseline-test-internal. This may add new files or modify existing files, but not remove any. In this step and the previous one, the list of files to call git add for is growing. 3. Finally, baselines may be moved or removed by optimize-baselines. If the file has not yet been added with git add, then it's just removed from the filesystem without recording the fact that it is removed, so the list of files to delete is not updated. This way, as the copy-existing-baselines-internal, rebaseline-test-internal, and optimize-baselines are run, the list of files to "add to SCM" (i.e. arguments to git add) keeps growing, and the list of files to remove stays empty. Then, at the end, the list of files to add includes all of the files that were temporarily added then removed. One possible solution for this would be to make it so that in optimize-baselines, the list of files to remove is always updated, so that by the time all of the changes are made, the list of files to add should only include files that were added but never removed. This was what I originally intended to do. But another approach would be: Assuming that we're only using git and we're not adding any files in the repo that we don't actually want to commit, we could just call `git add -A` after all of the rebaselining commands, and forget about tracking SCM changes altogether. This would be simpler and easier to understand.
,
Oct 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ccd977fa5650bf395c59bbbb0afc81f5dbd3531c commit ccd977fa5650bf395c59bbbb0afc81f5dbd3531c Author: qyearsley <qyearsley@chromium.org> Date: Sat Oct 08 17:29:31 2016 Don't track SCM changes in rebaseline commands. There's a fair amount of logic in the rebaseline-related command which is all for the purpose of tracking which files to add and remove to the source control management system. However, since we're now only using git, one possible simpler route is to just call `git add --all`. Pros: - Much simpler, fewer chances for bugs in SCM change tracking logic. Cons: - This would make rebaseline commands stage all new files from the user in the working tree even if they didn't intend to stage these files. One possible way to compensate for this would be to *not* call `git add --all` after all rebaselining, but instead call this seperately when (1) doing a w3c test auto-update or (2) doing a auto-rebaseline on rebaseline-o-matic. This is an alternative to http://crrev.com/2396433004. BUG= 639410 Review-Url: https://codereview.chromium.org/2397573002 Cr-Commit-Position: refs/heads/master@{#424082} [modify] https://crrev.com/ccd977fa5650bf395c59bbbb0afc81f5dbd3531c/third_party/WebKit/Tools/Scripts/webkitpy/common/checkout/baselineoptimizer.py [modify] https://crrev.com/ccd977fa5650bf395c59bbbb0afc81f5dbd3531c/third_party/WebKit/Tools/Scripts/webkitpy/common/checkout/baselineoptimizer_unittest.py [modify] https://crrev.com/ccd977fa5650bf395c59bbbb0afc81f5dbd3531c/third_party/WebKit/Tools/Scripts/webkitpy/common/checkout/scm/git.py [modify] https://crrev.com/ccd977fa5650bf395c59bbbb0afc81f5dbd3531c/third_party/WebKit/Tools/Scripts/webkitpy/common/checkout/scm/scm.py [modify] https://crrev.com/ccd977fa5650bf395c59bbbb0afc81f5dbd3531c/third_party/WebKit/Tools/Scripts/webkitpy/common/checkout/scm/scm_mock.py [modify] https://crrev.com/ccd977fa5650bf395c59bbbb0afc81f5dbd3531c/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/analyze_baselines.py [modify] https://crrev.com/ccd977fa5650bf395c59bbbb0afc81f5dbd3531c/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/auto_rebaseline_unittest.py [modify] https://crrev.com/ccd977fa5650bf395c59bbbb0afc81f5dbd3531c/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/optimize_baselines.py [modify] https://crrev.com/ccd977fa5650bf395c59bbbb0afc81f5dbd3531c/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/optimize_baselines_unittest.py [modify] https://crrev.com/ccd977fa5650bf395c59bbbb0afc81f5dbd3531c/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py [modify] https://crrev.com/ccd977fa5650bf395c59bbbb0afc81f5dbd3531c/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl.py [modify] https://crrev.com/ccd977fa5650bf395c59bbbb0afc81f5dbd3531c/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl_unittest.py [modify] https://crrev.com/ccd977fa5650bf395c59bbbb0afc81f5dbd3531c/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py [modify] https://crrev.com/ccd977fa5650bf395c59bbbb0afc81f5dbd3531c/third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations.py
,
Oct 14 2016
I believe with that with the above change, this should be fixed now. |
||||
►
Sign in to add a comment |
||||
Comment 1 by qyears...@chromium.org
, Aug 19 2016Status: Assigned (was: Untriaged)
Ah, good point! What webkit-patch rebaseline-cl does there is run (in parallel for each builder): webkit-patch rebaseline-test-internal --suffixes png \ --builder <builder> --build-number <build-number> \ --test fast/borders/border-antialiasing.html And then: webkit-patch optimize-baselines --no-modify-scm --suffixes png \ fast/borders/border-antialiasing.html My hypothesis is that it's now calling `git add ...` for each of the intermediary baselines that's added, and then when the baselines are de-duped, it's not doing anything like that. Maybe if --no-modify-scm is used for the first set of commands, then the git status should make more sense.