upload.py: improve git support; add revision range support.

This commit improves upload.py by:

a) automatically creating a subject line and description from the git commit message when --rev <git commit hash> is used on the command line.

b) allowing specification of commit ranges (ie --rev origin..HEAD) and automatically titling the patches in the standard format ([PATCH 1/4], etc).

c) using the .git/description file to add an extra prefix (if set), to ease the use of a single reviews instance for code from multiple git repositories.

By filling in implementations of the new abstract methods added to VersionControlSystem, these features can be added for other version control systems as well.

Documentation:

*  To upload a specific git commit for review, use the --rev option, for example: 

$ upload.py --rev 960831af2317f0b0b52ee4d13f980d2a56e91cd5

This will make the subject line default to the git log subject (first line of the commit message) and the description default to the git log body (all
but first line of the commit message) with a terse git commitid. If you set .git/description in your checkout, it will add this to the subject line, which helps distinguish patches to different components. For example, I've
set mini/.git/description to "mini", so my patch subject lines for commits to mini start "[PATCH] mini: ".

* You can use ranges in your --rev specification; this will create multiple issues for review, one per commit in the range. For example: 

$ upload.py --rev master...my-branch

or

$ upload.py --rev origin..HEAD

The subject lines for each commit begin "[PATCH 1/4]", "[PATCH 2/4]", etc.

[C. Scott Ananian, 2009-08-05 18:33:50]

This patch is for http://code.google.com/p/rietveld/issues/detail?id=140

[GvR, 2009-08-05 20:13:03]

I'm afraid Andi will have to review this; I don't even have git installed so I
cannot test this.

[Andi Albrecht, 2009-08-17 21:40:17]

This is not a full review. I'm adding Evan as this patch would change
upload.py's default behavior, esp. when used with git.

Evan, what do you think about the changed command line options and how they
work? I assume you know more use cases of upload.py+git than I :)

http://codereview.appspot.com/101056/diff/1/2
File static/upload.py (right):

http://codereview.appspot.com/101056/diff/1/2#newcode1397
Line 1397: for (i,r) in zip(xrange(1, sys.maxint), rev_range):
This would change the current mean of options.revision. You can use --rev
start:end to specify that you want to upload *one* diff containing changes
between start and end.

http://codereview.appspot.com/101056/diff/1/2#newcode1429
Line 1429: message = vcs.GetSubject()
Again, this changes the current behaviour. I'm not sure if it's the right thing
to assume that a user wants to use the commit message when "-m" is not given on
the command line (but "--rev" is present).

http://codereview.appspot.com/101056/diff/1/2#newcode1460
Line 1460: description = vcs.GetDescription()
The same here.

[C. Scott Ananian, 2009-08-19 23:08:48]

Updated patch to handle move-of-an-unmodified file better (but see issue 115).

On 2009/08/17 21:40:17, Andi Albrecht wrote:
> Evan, what do you think about the changed command line options and how they
> work? I assume you know more use cases of upload.py+git than I :)
> 
> http://codereview.appspot.com/101056/diff/1/2
> File static/upload.py (right):
> 
> http://codereview.appspot.com/101056/diff/1/2#newcode1397
> Line 1397: for (i,r) in zip(xrange(1, sys.maxint), rev_range):
> This would change the current mean of options.revision. You can use --rev
> start:end to specify that you want to upload *one* diff containing changes
> between start and end.

It wouldn't change the current meaning of --rev for any backend but git.  --rev
start:end certainly wouldn't do anything for the git backend (although it's
unfortunate that you'd get opposite behavior for the different backends: to
generate one patch for a range you'd use start..end *without* the --rev flag,
both with current upload.py and with my patches).

> http://codereview.appspot.com/101056/diff/1/2#newcode1429
> Line 1429: message = vcs.GetSubject()
> Again, this changes the current behaviour. I'm not sure if it's the right
thing
> to assume that a user wants to use the commit message when "-m" is not given
on
> the command line (but "--rev" is present).
> 
> http://codereview.appspot.com/101056/diff/1/2#newcode1460
> Line 1460: description = vcs.GetDescription()
> The same here.

Both of these methods return None in the default implementation, so this would
be changing the behavior for the git backend only.  (Although you could
implement them appropriately in the other backends if you wanted consistent
behavior.)

[Evan Martin, 2009-08-21 16:10:21]

I'm not opposed to this in principle, as git-cl doesn't use the --rev flag.  I
have a number of style and design comments that should happen after you rebase.

[Evan Martin, 2009-08-21 16:10:42]

On 2009/08/21 16:10:21, Evan Martin wrote:
> I'm not opposed to this in principle, as git-cl doesn't use the --rev flag.  I
> have a number of style and design comments that should happen after you
rebase.

Arrrgh, switched usernames and lost comments, let me recreate.

[Evan Martin, 2009-08-21 16:13:12]

http://codereview.appspot.com/101056/diff/4001/4002
File static/upload.py (right):

http://codereview.appspot.com/101056/diff/4001/4002#newcode1049
Line 1049: (["git", "log", "--pretty=format:%H", range_spec])
why not rev-list?

http://codereview.appspot.com/101056/diff/4001/4002#newcode1064
Line 1064: if not d.startswith("Unnamed") and len(d) < 20:
this "20" feels pretty arbitrary

http://codereview.appspot.com/101056/diff/4001/4002#newcode1092
Line 1092: # the second filename.
This will conflict with changes I've landed upstream, so please rebase and
reupload.

http://codereview.appspot.com/101056/diff/4001/4002#newcode1413
Line 1413: for (i,r) in zip(xrange(1, sys.maxint), rev_range):
enumerate()  ?

[Tommi Komulainen, 2010-03-05 12:27:36]

When uploading multiple patches you get the

The following files are not added to version control:
[...]
Are you sure to continue?(y/N)

confirmation for every patch, which is pretty pointless. Once per upload should
be sufficient.


I think there should also be a confirmation such as:
About to send 3 patches for review:
add foo
fix bar
remove bug
Are you sure to continue?(y/N)

to be confident you got the patch range / branch / whatever right.