Added "gclient pack" subcommand, which generates a patch relative to...

Added "gclient pack" subcommand, which generates a patch relative to
the root of the source tree. It is similar to "gclient diff", and
shares much of the implementation, but it seems that developers may
want the semantics of each in different situations, which is why it is
being added as a new command.

Generalized SubprocessCallAndCapture into SubprocessCallAndFilter.
Added RunSVNAndFilterOutput; changed RunSVNAndGetFileList to use it.
Fixed problem in presubmit_canned_checks.py where it was not working
on Windows. Updated unit tests for gclient.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=25410

[Ken Russell (Google), 2009-09-03 03:52:07]

Please review this addition of the "pack" subcommand, which is useful for moving
large patches from machine to machine. Thanks.

[M-A Ruel, 2009-09-03 04:12:39]

I don't see why we'd keep gclient diff at all. I think you should implement your
wanted functionality in diff instead. Wdyt?

http://codereview.chromium.org/193004/diff/4001/4002
File gclient.py (right):

http://codereview.chromium.org/193004/diff/4001/4002#newcode215
Line 215: gclient patch > patch.txt
patch?

http://codereview.chromium.org/193004/diff/4001/4002#newcode1067
Line 1067: 'cleanup', 'diff', 'export', 'pack', 'revert', 'status', 'update',
'runhooks'
80 cols

http://codereview.chromium.org/193004/diff/4001/4003
File presubmit_canned_checks.py (right):

http://codereview.chromium.org/193004/diff/4001/4003#newcode326
Line 326: backpath = ['.', input_api.os_path.pathsep.join(['..'] *
(cwd.count('/') + 1))]
80 cols

You're right, I always forgot to fix that.

[Ken Russell (Google), 2009-09-03 05:07:20]

> I don't see why we'd keep gclient diff at all. I think you
> should implement your wanted functionality in diff instead.
> Wdyt?

Steven pointed out that some developers might want to get the diff output for
each solution relative to the base of that solution, to make them easier to
split up and send to multiple project owners. Either way is fine with me.

http://codereview.chromium.org/193004/diff/4001/4002
File gclient.py (right):

http://codereview.chromium.org/193004/diff/4001/4002#newcode215
Line 215: gclient patch > patch.txt
On 2009/09/03 04:12:39, Marc-Antoine Ruel wrote:
> patch?

Whoops. Fixed.

http://codereview.chromium.org/193004/diff/4001/4002#newcode1067
Line 1067: 'cleanup', 'diff', 'export', 'pack', 'revert', 'status', 'update',
'runhooks'
On 2009/09/03 04:12:39, Marc-Antoine Ruel wrote:
> 80 cols

Done.

http://codereview.chromium.org/193004/diff/4001/4003
File presubmit_canned_checks.py (right):

http://codereview.chromium.org/193004/diff/4001/4003#newcode326
Line 326: backpath = ['.', input_api.os_path.pathsep.join(['..'] *
(cwd.count('/') + 1))]
On 2009/09/03 04:12:39, Marc-Antoine Ruel wrote:
> 80 cols
> 
> You're right, I always forgot to fix that.

Done.

[sgk, 2009-09-04 00:25:10]

code lgtm, nice job.

I reconsidered my initial thought from yesterday.  Someone wanting to submit a
patch to an upstream entry can always just cd there and run svn diff.  Duh.

If there's consensus for changing the behavior of "gclient diff" to work like
this, I'm okay with it.

http://codereview.chromium.org/193004/diff/7001/7002
File gclient.py (right):

http://codereview.chromium.org/193004/diff/7001/7002#newcode201
Line 201: 
No extra newline here (consistency with other entries).

http://codereview.chromium.org/193004/diff/7001/7002#newcode488
Line 488: filter(in_line)
Good, more flexible than just the pattern.

[M-A Ruel, 2009-09-04 00:29:25]

lgtm