Bug 47940 - webkit-patch doesn't get along with git rm
Summary: webkit-patch doesn't get along with git rm
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Adam Barth
URL:
Keywords:
Depends on:
Blocks: 35460
  Show dependency treegraph
 
Reported: 2010-10-19 15:38 PDT by Tony Gentilcore
Modified: 2010-10-20 14:41 PDT (History)
5 users (show)

See Also:


Attachments
Patch (1.40 KB, patch)
2010-10-19 22:20 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch (1.45 KB, patch)
2010-10-19 22:23 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch (2.77 KB, patch)
2010-10-19 22:50 PDT, Adam Barth
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tony Gentilcore 2010-10-19 15:38:46 PDT
1. $ git rm foo
2. $ webkit-patch pretty-diff
3. You're not on a horse :-(

fatal: ambiguous argument 'foo': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions

Adam, Eric thought this might be a regression caused by http://trac.webkit.org/changeset/70059
Comment 1 Adam Barth 2010-10-19 17:58:29 PDT
Seems likely.  Should be an easy fix in scm.py.  Just add the --.
Comment 2 Eric Seidel (no email) 2010-10-19 20:35:01 PDT
I don't think it's the --.  The problem is we view the deleted file as a changed file.  git won't accept deleted files as arguments to git diff (maybe there is an option to allow it?)
Comment 3 Adam Barth 2010-10-19 22:20:53 PDT
Created attachment 71249 [details]
Patch
Comment 4 Eric Seidel (no email) 2010-10-19 22:22:36 PDT
Comment on attachment 71249 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=71249&action=review

r- for lack of tests (and because I think this isn't a complete fix and may be wrong.)

> WebKitTools/Scripts/webkitpy/common/checkout/scm.py:656
> +        status_command = ['git', 'diff', '-r', '--name-status', '-C', '-M', "--no-ext-diff", "--full-index", "--", self.merge_base(git_commit)]

Um.  Are you sure that's the right place?  I don't think it is.  I think you'd want it after the merge base?  And what about the case where we're generating a patch?
Comment 5 Adam Barth 2010-10-19 22:23:53 PDT
Created attachment 71250 [details]
Patch
Comment 6 Eric Seidel (no email) 2010-10-19 22:24:29 PDT
Comment on attachment 71250 [details]
Patch

Still no tests.  And thus wrong.
Comment 7 Adam Barth 2010-10-19 22:50:52 PDT
Created attachment 71253 [details]
Patch
Comment 8 Eric Seidel (no email) 2010-10-20 09:39:48 PDT
Comment on attachment 71253 [details]
Patch

Thank you!
Comment 9 WebKit Commit Bot 2010-10-20 11:52:51 PDT
The commit-queue encountered the following flaky tests while processing attachment 71253 [details]:



Please file bugs against the tests.  The commit-queue is continuing to process your patch.
Comment 10 WebKit Commit Bot 2010-10-20 11:53:27 PDT
Comment on attachment 71253 [details]
Patch

Rejecting patch 71253 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'land-attachment', '--force-clean', '--ignore-builders', '--quiet', '--non-interactive', '--parent-command=commit-queue', 71253]" exit_code: 2
Last 500 characters of output:
bCore/rendering/RenderBlockLineLayout.cpp
r70158 = 35d2ad5c3fa6f7a0732c477ec1bce4e1655ad6a8 (refs/remotes/trunk)
First, rewinding head to replay your work on top of it...
error: Untracked working tree file 'LayoutTests/plugins/mouse-move-over-plugin-in-frame-expected.txt' would be overwritten by merge.
could not detach HEAD
rebase refs/remotes/trunk: command returned error: 1

Died at WebKitTools/Scripts/update-webkit line 129.

Failed to run "['WebKitTools/Scripts/update-webkit']" exit_code: 2

Full output: http://queues.webkit.org/results/4593014
Comment 11 WebKit Commit Bot 2010-10-20 13:35:48 PDT
The commit-queue encountered the following flaky tests while processing attachment 71253 [details]:

http/tests/security/xssAuditor/script-tag-with-source-double-quote.html

Please file bugs against the tests.  The author(s) of the test(s) have been CCed on this bug.  The commit-queue is continuing to process your patch.
Comment 12 WebKit Commit Bot 2010-10-20 13:36:34 PDT
Comment on attachment 71253 [details]
Patch

Clearing flags on attachment: 71253

Committed r70169: <http://trac.webkit.org/changeset/70169>
Comment 13 WebKit Commit Bot 2010-10-20 13:36:40 PDT
All reviewed patches have been landed.  Closing bug.
Comment 14 Eric Seidel (no email) 2010-10-20 13:45:16 PDT
Committed r70171: <http://trac.webkit.org/changeset/70171>
Comment 15 WebKit Review Bot 2010-10-20 14:41:21 PDT
http://trac.webkit.org/changeset/70169 might have broken GTK Linux 64-bit Debug
The following tests are not passing:
fast/backgrounds/repeat/negative-offset-repeat-transformed.html
fast/borders/border-image-rotate-transform.html
fast/borders/border-image-scale-transform.html
fast/transforms/scrollIntoView-transformed.html
transforms/2d/hindi-rotated.html
Comment 16 WebKit Review Bot 2010-10-20 14:41:39 PDT
http://trac.webkit.org/changeset/70171 might have broken GTK Linux 64-bit Debug
The following tests are not passing:
fast/backgrounds/repeat/negative-offset-repeat-transformed.html
fast/borders/border-image-rotate-transform.html
fast/borders/border-image-scale-transform.html
fast/transforms/scrollIntoView-transformed.html
transforms/2d/hindi-rotated.html