Bug 26914 – bugzilla-tool fails for SVN users

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

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. :(

[Eric Seidel (no email)]

Created attachment 32168 [details]
patch

[Mark Rowe (bdash)]

STOP IT.

[Adam Barth]

Comment on attachment 32168 [details]
patch

I'm no python expert, but this looks reasonable to me.

[David Levin]

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".)

[Eric Seidel (no email)]

(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.

[Eric Seidel (no email)]

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

[Eric Seidel (no email)]

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. :(