999: post-review broken for git when working on a remote SVN branch (not trunk)

sanforda********@gmai***** (Google Code) (Is this you? Claim this profile.)
July 4, 2011 
What steps will reproduce the problem?
1. Make a branch in SVN, say "1.0" for this example.
2. Add a new file in trunk, say ".gitignore" for this example.
3. In git, have remote branch git-svn/branches/1.0 and local branch 1.0.
4. git checkout -b new-1.0-fix 1.0
5. Hack hack hack, commit commit commit, decide you are ready for review now

At this point, I have tried two ways of using post-review:

6a. run post-review

6b. run post-review --parent=1.0

What is the expected output?

With one of these options, I'd like to have a review posted with a diff of
my changes compared to the 1.0 SVN branch.

What do you see instead?

a. Review is posted, but because the diff was taken against master, not
only is it incorrect, but because of issue 804 the diff is not viewable
because it shows .gitignore as deleted (because it was created on trunk
after the branch).

b. Here is the command line output:

[mono] ~/git-uia2atk-clone-test/gitorious-svn-test @ post-review
--parent=1.0 -d
>>> svn info
>>> git rev-parse --git-dir
>>> git svn info
>>> repository info: Path: svn+ssh://mono-cvs.ximian.com/source, Base path:
/branches/uia2atk/1.0, Supports changesets: False
>>> git diff --no-color --no-prefix -r -u 1.0..
>>> git svn find-rev master
>>> git diff --no-color --no-prefix -r -u master..1.0
>>> git svn find-rev master
>>> Looking for 'reviews.mono-a11y.org /' cookie in
/home/sandy/.post-review-cookies.txt
>>> Loaded valid cookie -- no login required
>>> Attempting to create review request for None
>>> HTTP POSTing to
http://reviews.mono-a11y.org/api/json/reviewrequests/new/:
{'repository_path': 'svn+ssh://mono-cvs.ximian.com/source'}
>>> Review request created
>>> Uploading diff, size: 345
>>> Uploading parent diff, size: 225010
>>> HTTP POSTing to
http://reviews.mono-a11y.org/api/json/reviewrequests/59/diff/new/:
{'basedir': '/branches/uia2atk/1.0'}
Error uploading diff: The file was not found in the repository (207)
>>> {'stat': 'fail', 'file': '/branches/uia2atk/1.0/.gitignore', 'err':
{'msg': 'The file was not found in the repository', 'code': 207},
'revision': '130499'}
Your review request still exists, but the diff is not attached.

Honestly I don't know if this is me misusing --parent, or an actual bug.


Please provide any additional information below.

Because I'm not sure if I'm misusing parent, I think I'm going to work up a
patch that adds the --gitsvnparent option, which lets me just make the diff
against my 1.0 branch, without dealing with this parentdiff stuff.  I'd
like to be able to work on SVN branches via git-svn, and have post-review
work as well as it does from an SVN checkout.
#1 sanforda********@gmai***** (Google Code) (Is this you? Claim this profile.) 
As I said, I'm not sure if I'm using --parent incorrectly, so here's a patch that
adds a --gitsvnparent option.  It simplifies things for me by disposing with the
parent diff step (which to me makes no sense in the context of --parent=some-svn-branch).

So now I can use --gitsvnparent=1.0, and it will simply make a diff against my local
1.0 branch (which for me represents my SVN 1.0 branch).  Everything just works
because post-review already knows via `git svn info` how to do the svn diff correctly.

I think a better approach would be to make --parent a little smarter when dealing
with git-svn branches, but I can't think of a way to tell that some git branch
represents a clean sync with an SVN branch instead of yet another local branch that
has an SVN branch in its ancestry.  That's why I opted to make a totally new option
  • +
    gitsvnparent-option.patch 
    Index: rbtools/scripts/post-review
    ===================================================================
    --- rbtools/scripts/post-review	(revision 1885)
    +++ rbtools/scripts/post-review	(working copy)
    @@ -1736,12 +1724,12 @@
             Performs a diff across all modified files in the branch, taking into
             account a parent branch.
             """
    -        parent_branch = options.parent_branch or "master"
    +        parent_branch = options.parent_branch or options.gitsvn_parent_branch or "master"
     
             diff_lines = self.make_diff(parent_branch)
     
    -        if parent_branch != "master":
    -            parent_diff_lines = self.make_diff("master", parent_branch)
    +        if options.parent_branch:
    +            parent_diff_lines = self.make_diff("master", options.parent_branch)
             else:
                 parent_diff_lines = None
     
    @@ -2141,6 +2129,11 @@
                           help="the parent branch this diff should be against "
                                "(only available if your repository
#2 sanforda********@gmai***** (Google Code) (Is this you? Claim this profile.) 
I know this patch is unlikely to be accepted as-is, but I wanted to get used to the
proper patch submission process so I opened a review here:

http://reviews.review-board.org/r/793/
david
#3 david
  • +Component-RBTools
#4 tsto****@gmai***** (Google Code) (Is this you? Claim this profile.) 
I also hit this issue, and hacked around it by parsing git svn log lines looking for the nearest svn parent, and using --revision-range to specify it explicitly. Very annoying.
david
#5 david 
This was fixed a while ago.
  • +Fixed