Bug 111668 – webkitpy: change git_commit_from_svn_revision() to work in a pure git checkout

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Dirk Pranke]

right now we call git svn find-rev, which won't work.

[Dirk Pranke]

See the motivating bug 110839 for this ... we should just change it to call _most_recent_log_matching .

[Ryosuke Niwa]

Why is this won't fix? I don't think so.

[Tim 'mithro' Ansell]

I closed it WONTFIX because I won't be fixing it as I've been asked to concentrated on Blink.

[Ryosuke Niwa]

(In reply to comment #3)
> I closed it WONTFIX because I won't be fixing it as I've been asked to concentrated on Blink.

That's not a good reason to close this bug as WONTFIX since this bug is not specific to Chromium at all.

[Dirk Pranke]

(In reply to comment #3)
> I closed it WONTFIX because I won't be fixing it as I've been asked to concentrated on Blink.

In this case you should just un-assign yourself from the bug (use the Reset Assignee to default checkbox), rather than closing the bugs as WontFix.

[Peter Molnar]

Created attachment 215920 [details]
Proposed patch

[Ryosuke Niwa]

Comment on attachment 215920 [details]
Proposed patch

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

Did you run test-webkitpy webkitpy.common.checkout.scm.scm_unittest?

> Tools/Scripts/webkitpy/common/checkout/scm/git.py:313
> +        git_log = self._run_git(['log', '-1', '--grep=' + '^\s*git-svn-id:.*@%s ' % svn_revision])

What's the point of +?

[Peter Molnar]

(In reply to comment #7)
> (From update of attachment 215920 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=215920&action=review
> 
> Did you run test-webkitpy webkitpy.common.checkout.scm.scm_unittest?

Yes, in fact I ran "test-webkitpy --all" that covers all SCM tests as far as I know. All webkitpy tests were passing.

> 
> > Tools/Scripts/webkitpy/common/checkout/scm/git.py:313
> > +        git_log = self._run_git(['log', '-1', '--grep=' + '^\s*git-svn-id:.*@%s ' % svn_revision])
> 
> What's the point of +?

As mentioned earlier in that file:
# We use '--grep=' + foo rather than '--grep', foo because
# git 1.7.0.4 (and earlier) didn't support the separate arg.

[Ryosuke Niwa]

Comment on attachment 215920 [details]
Proposed patch

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

>>> Tools/Scripts/webkitpy/common/checkout/scm/git.py:313
>>> +        git_log = self._run_git(['log', '-1', '--grep=' + '^\s*git-svn-id:.*@%s ' % svn_revision])
>> 
>> What's the point of +?
> 
> As mentioned earlier in that file:
> # We use '--grep=' + foo rather than '--grep', foo because
> # git 1.7.0.4 (and earlier) didn't support the separate arg.

I get that but why don't you just do '--grep=^\s*git-svn-id:.*@%s ' instead?  It seems like ' + ' is a useless noise.

[Peter Molnar]

Created attachment 215947 [details]
Patch updated

I just thought it was easier to read that way. New patch attached.

[WebKit Commit Bot]

Comment on attachment 215947 [details]
Patch updated

Clearing flags on attachment: 215947

Committed r158828: <http://trac.webkit.org/changeset/158828>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.