996: post-review broken when run from root of git repo

sanforda********@gmai***** (Google Code) (Is this you? Claim this profile.)
chipx86
 chipx86
March 29, 2009 
Recently this was added to post-review:

        git_dir = execute(["git", "rev-parse", "--git-dir"],
        ignore_errors=True).strip()

        if git_dir.startswith("fatal:") or not os.path.isdir(git_dir):
        return None

        # post-review in directories other than the top level of
        # of a work-tree would result in broken diffs on the server
        os.chdir(os.path.dirname(git_dir))

The problem with this is that if you're in the root of your repo, git_dir
is set to ".git", and dirname returns an empty string, resulting in this
error when chdir is called:

  File "/home/sandy/bin/post-review", line 1596, in get_repository_info
    os.chdir(git_dir_name)
OSError: [Errno 2] No such file or directory: ''

I'm not a python or git expert, so I fixed this locally by changing that
last part to:

        # post-review in directories other than the top level of
        # of a work-tree would result in broken diffs on the server
        git_dir_name = os.path.dirname(git_dir)
        if git_dir_name != '':
                os.chdir(git_dir_name)
#1 sanforda********@gmai***** (Google Code) (Is this you? Claim this profile.) 
This was caused by the fix for issue 928.  I can confirm that with my change that
issue is still fixed.
#2 sanforda********@gmai***** (Google Code) (Is this you? Claim this profile.) 
I can't seem to register an account on your Reviewboard instance, so I've attached a
patch here.
  • +
    toplevel-git-pr-fix.patch 
    Index: rbtools/scripts/post-review
    ===================================================================
    --- rbtools/scripts/post-review	(revision 1875)
    +++ rbtools/scripts/post-review	(working copy)
    @@ -1648,7 +1648,9 @@
     
             # post-review in directories other than the top level of
             # of a work-tree would result in broken diffs on the server
    -        os.chdir(os.path.dirname(git_dir))
    +        git_dir_name = os.path.dirname(git_dir)
    +        if git_dir_name != '':
    +            os.chdir(git_dir_name)
     
             # We know we have something we can work with. Let's find out
             # what it is. We'll try SVN first.
chipx86
#3 chipx86 
You should be able to register. What are you seeing when you try?
  • +NeedInfo
#4 sanforda********@gmai***** (Google Code) (Is this you? Claim this profile.) 
Incidentally, the original proposed fix for issue 928 [ chdir(git_dir) then
chdir('..') ] does not have this bug.
chipx86
#5 chipx86 
Probably what we wanted was os.chdir(os.path.dirname(os.path.abspath(git_dir)))
chipx86
#6 chipx86 
Fixed in r1876.
  • -NeedInfo
    +Fixed
  • -Priority-Medium
    +Priority-Critical
    +Component-RBTools
    +Milestone-Release1.0
  • +chipx86
#7 sanforda********@gmai***** (Google Code) (Is this you? Claim this profile.) 
Yeah, that fix looks better than mine. :-)  So what happens is whenever I visit this URL:

http://reviews.review-board.org/account/register/

it just forwards me here:

http://reviews.review-board.org/account/login/
chipx86
#8 chipx86 
Oops, had registration turned off :) Try now.