post-review: teach GitClient to be flexible

Review Request #935 — Created July 26, 2009 and discarded

Information

djs
Review Board SVN (deprecated)

Reviewers

MOVED TO http://reviews.review-board.org/r/985/


This adds more robust support for git to post-review, so now I'll actually start using it myself. You should now be able to provide any arbitrary diff for review as long as you don't do something crazy like modify your local upstream to not match the remote.

 * Add --upstream option to select the upstream repository, instead of hard-coding origin (origin is default)
 * Remove all references to master branch. We now diff against the remote name
 * diff_between_revisions is implemented
 * git-svn now diffs against git-svn remote (equivalent to upstream in pure git) instead of 'master'

I don't use git-svn much, so someone with more experience should test this out. I also noticed after the fact that there are several related changes under review:
http://reviews.review-board.org/r/793/
http://reviews.review-board.org/r/889/
Manual testing:

 * post-review with no options
 * post-review with --upstream specified
 * post-review with --parent specified
 * post-review with --revision-range specified
 * Posted this diff with this version :)
DJ
DJ
HE
  1. Do you have a git remote branch that I can pull from?
    1. http://gitorious.org/~djs/review-board/djs-dev
      
      I was going to merge all the various patches in, but I won't get to it until next week.
  2. 
      
chipx86
  1. I just tested this patch, and I'm having trouble using it. I can't do a git diff between "origin" (in the plain git case) or "git-svn" (in the git-svn) case and anything else. Git complains:
    
    fatal: ambiguous argument 'origin': unknown revision or path not in the working tree.
    Use '--' to separate paths from revisions
    1. Can you post the debug output? Are you getting the 'origin' error even with git-svn?
      
      Here's a sample for the git case:
      >>> svn info
      >>> git rev-parse --git-dir
      >>> git svn info
      >>> git svn --version
      >>> git config --get svn-remote.svn.url
      >>> git remote show origin
      >>> repository info: Path: git://git.kernel.org/pub/scm/git/git.git, Base path: , Supports changesets: False
      >>> git diff --no-color origin..
      
      And, for reference:
      $ git remote -v
      origin	git://git.kernel.org/pub/scm/git/git.git
      
      If I don't have an origin:
      
      ~/dev/git$ git remote rename origin foo
      ~/dev/git$ ~/reviewboard-git/trunk/rbtools/rbtools/postreview.py -d --server=http://localhost/
      >>> svn info
      >>> git rev-parse --git-dir
      >>> git svn info
      >>> git svn --version
      >>> git config --get svn-remote.svn.url
      >>> git remote show origin
      Failed to execute command: ['git', 'remote', 'show', 'origin']
      fatal: 'origin' does not appear to be a git repository
      fatal: The remote end hung up unexpectedly
      
      But, I can specify an upstream with --upstream:
      ~/dev/git$ ~/reviewboard-git/trunk/rbtools/rbtools/postreview.py -d --server=http://localhost/ --upstream=foo
      >>> svn info
      >>> git rev-parse --git-dir
      >>> git svn info
      >>> git svn --version
      >>> git config --get svn-remote.svn.url
      >>> git remote show foo
      >>> repository info: Path: git://git.kernel.org/pub/scm/git/git.git, Base path: , Supports changesets: False
      >>> git diff --no-color foo..
      
      
      For git-svn, I don't use this regularly and I think there are a number of options for cloning an svn repo. I cloned the whole reviewboard repository as one git repository and git-svn resolves to the remote HEAD in svn. So, with default options:
      
      >>> svn info
      >>> git rev-parse --git-dir
      >>> git svn info
      >>> repository info: Path: http://reviewboard.googlecode.com/svn, Base path: /trunk, Supports changesets: False
      >>> git config --get reviewboard.url
      >>> svn propget reviewboard:url http://reviewboard.googlecode.com/svn
      >>> git diff --no-color --no-prefix -r -u git-svn..
      >>> git svn find-rev git-svn
      
      
      The error you mentioned would probably come from git diff, which means that origin did resolve for the git remote show. Is it possible you have a remote named 'origin' but you've never fetched anything from it?
    2. In testing this from my Review Board git-svn clone (which is from a while back and uses an upstream of "trunk" rather than "git-svn"):
      
      post-review -d --server=http://localhost:8080 --upstream=trunk
      >>> svn info
      >>> git rev-parse --git-dir
      >>> git svn info
      >>> repository info: Path: https://reviewboard.googlecode.com/svn, Base path: /trunk, Supports changesets: False
      >>> git diff --no-color --no-prefix -r -u trunk..
      Failed to execute command: ['git', 'diff', '--no-color', '--no-prefix', '-r', '-u', 'trunk..']
      ["fatal: ambiguous argument 'trunk..': unknown revision or path not in the working tree.\n", "Use '--' to separate paths from revisions\n"]
      
      
      From my win32-installer Git clone, with an upstream of origin:
      
      post-review -d --server=http://localhost:8080
      >>> svn info
      >>> git rev-parse --git-dir
      >>> git svn info
      >>> git svn --version
      >>> git config --get svn-remote.svn.url
      >>> git remote show origin
      >>> repository info: Path: git@github.com:reviewboard/win32-installer.git, Base path: , Supports changesets: False
      >>> git diff --no-color --full-index origin..
      Failed to execute command: ['git', 'diff', '--no-color', '--full-index', 'origin..']
      fatal: ambiguous argument 'origin..': unknown revision or path not in the working tree.
      Use '--' to separate paths from revisions
      
      
      For the record, I'm using git v1.5.4.4.
  2. 
      
DJ
Review request changed
Change Summary:
close out
Description:
  +

MOVED TO http://reviews.review-board.org/r/985/

  +
  +

  +
   

This adds more robust support for git to post-review, so now I'll actually start using it myself. You should now be able to provide any arbitrary diff for review as long as you don't do something crazy like modify your local upstream to not match the remote.

   
   
  • Add --upstream option to select the upstream repository, instead of hard-coding origin (origin is default)
   
  • Remove all references to master branch. We now diff against the remote name
   
  • diff_between_revisions is implemented
   
  • git-svn now diffs against git-svn remote (equivalent to upstream in pure git) instead of 'master'
   
   

I don't use git-svn much, so someone with more experience should test this out. I also noticed after the fact that there are several related changes under review:

    http://reviews.review-board.org/r/793/
    http://reviews.review-board.org/r/889/