Bug 26914 - bugzilla-tool fails for SVN users
Summary: bugzilla-tool fails for SVN users
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-01 19:13 PDT by Eric Seidel (no email)
Modified: 2009-07-02 00:21 PDT (History)
3 users (show)

See Also:


Attachments
patch (10.71 KB, patch)
2009-07-01 19:17 PDT, Eric Seidel (no email)
abarth: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2009-07-01 19:13:21 PDT
bugzilla-tool fails for SVN users

svn is dumb:
svn ci -F -
svn: Reading from stdin is disallowed

Earlier versions of svn have some message about -F - being broken, thus disabled. :)  Now they've just disallowed it.  Sigh.

Either we could write out the message to a file, or we could pass it via -m.  If we pass it via -m we need to not use shell=True.  Since Mark would like us to move off of shell=True anyway, I"ll do that.

The first successful commit with this fix:
http://trac.webkit.org/changeset/45463
(Even though the author of that patch had changed NOBODY(OOPS!) to "NOBODY" so my reviewer update code failed to update the reviewer. :(
Comment 1 Eric Seidel (no email) 2009-07-01 19:17:07 PDT
Created attachment 32168 [details]
patch
Comment 2 Mark Rowe (bdash) 2009-07-01 19:20:56 PDT
STOP IT.
Comment 3 Adam Barth 2009-07-01 21:41:19 PDT
Comment on attachment 32168 [details]
patch

I'm no python expert, but this looks reasonable to me.
Comment 4 David Levin 2009-07-01 22:01:37 PDT
I'll let Adam's r+ stand, but here are some comments to consider:

> diff --git a/WebKitTools/Scripts/modules/scm.py b/WebKitTools/Scripts/modules/scm.py
>  # All git-specific logic should go here.
> @@ -224,51 +234,56 @@ class Git(SCM):
> +    @staticmethod
> +    def commit_success_regexp():
> +        return "^Committed r(?P<svn_revision>\d+)\.$"

This shouldn't have a \. in it.

Here's a sample git svn dcommit message:

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
...
Committed r45448
	M	WebCore/ChangeLog
...
r45448 = 97e3c9608ffcf1f67fe127a738e043e2db154d7e (trunk)


>      def files_changed_summary_for_commit(self, commit_id):
> -        return self.run_command("git diff-tree --shortstat --no-commit-id " + commit_id)
> +        return self.run_command(['git', 'diff-tree', '--short-stat', '--no-commit-id', commit_id])

Why did the parameter change to from "--shortstat" to "--short-stat"?  (I didn't see --short-stat listed as a valid option for "git diff-tree".)
Comment 5 Eric Seidel (no email) 2009-07-01 22:57:12 PDT
(In reply to comment #4)
> I'll let Adam's r+ stand, but here are some comments to consider:

Thank you for the very careful review.  I really really really need testing for this script.  I've filed bug 26916 about that.

I've fixed both of the issues you mentioned.
Comment 6 Eric Seidel (no email) 2009-07-01 23:54:44 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebKitTools/ChangeLog
	M	WebKitTools/Scripts/bugzilla-tool
	M	WebKitTools/Scripts/modules/scm.py
Committed r45469
	M	WebCore/dom/MessageChannel.cpp
	M	WebCore/dom/MessagePortChannel.cpp
	M	WebCore/dom/default/PlatformMessagePortChannel.cpp
	M	WebCore/dom/MessagePortChannel.h
	M	WebCore/ChangeLog
r45467 = a08d526b1bd9d60aa510e837f37ef18283db029a (trunk)
	M	WebCore/ChangeLog
	M	WebCore/rendering/RenderLayerBacking.cpp
	A	LayoutTests/platform/mac/compositing/geometry/root-layer-update-expected.png
	A	LayoutTests/platform/mac/compositing/geometry/root-layer-update-expected.txt
	A	LayoutTests/platform/mac/compositing/geometry/root-layer-update-expected.checksum
	M	LayoutTests/ChangeLog
	A	LayoutTests/compositing/geometry/root-layer-update.html
r45468 = 25496daa3193049bc58b8c6255dbdba7af72f00a (trunk)
	M	WebKitTools/ChangeLog
	M	WebKitTools/Scripts/modules/scm.py
	M	WebKitTools/Scripts/bugzilla-tool
r45469 = 21a28c8eebc35fcfe1b6453a264f3e208128e89c (trunk)
First, rewinding head to replay your work on top of it...
Nothing to do.
http://trac.webkit.org/changeset/45469
Comment 7 Eric Seidel (no email) 2009-07-02 00:21:11 PDT
Sigh.  I need to teach bugzilla-tool how to strip excess junk from the commit message.

https://bugs.webkit.org/show_bug.cgi?id=26920

Sorry. :(