2533: post-review prefers /home/login/.git over /home/login/svn/.svn

urkud******@gmai***** (Google Code) (Is this you? Claim this profile.) 
What version are you running?
RBTools 0.4.1

What's the URL of the page containing the problem?
none

What steps will reproduce the problem?
1. git clone /some/repo.git; cd repo
2. svn co /some/svn/project; cd project
3. change some file
3. post-review

What is the expected output? What do you see instead?
Expected output: post a review.
I see: There don't seem to be any diffs!
strace tells me that post-review looks into ~/.git/svn but not into $PWD/.svn

What operating system are you using? What browser?
GNU/Linux
chipx86
#1 chipx86 
post-review is scanning with several different apps (git, svn, p4, etc.) to try to find a repository. Right now, the first one wins.

What we probably should be doing is taking all the results and finding the closest parent, which should almost always be right.

If you have any Python experience and want to help with this, since you have a prety good test case and desire to have it in, we can get that into the next RBTools release sooner.
  • +Confirmed
  • -Priority-Medium
    +Priority-High
    +Milestone-RBTools-Release1.0
    +Component-RBTools
#2 urkud******@gmai***** (Google Code) (Is this you? Claim this profile.) 
I have no python experience but I'll try to fix it. Am I right that  rbtools/clients/__init__.py#scan_usable_client is the function I should fix?
chipx86
#3 chipx86 
Yep, that's the one. Though, I realized it's more complicated than that. Right now, RepositoryInfo doesn't have any information on where the base of the repository is, locally. That would have to be added to every supported SCMClient, and that may differ depending on what output the tool provides.

So while that's how I'd prefer this to be done, a simpler (though still touching each class) modification that would also work is to provide a new option/.reviewboardrc variable for specifying the type of repository.

I'd call this REPOSITORY_TYPE and have it take a string. One of "cvs", "clearcase", "git", "mercurial", "perforce", "plastic", or "svn".

What you'd then need to do is modify the SCMClient class (same file) to have a 'scm_type = None' in the class body (before __init__), and then take each SCMClient subclass in clients/ and override that variable to be one of the above strings.

Then in postreview.py, in parse_options, add a "--repository-type" option that takes one of these, defaulting to 'get_config_value(configs, 'REPOSITORY_TYPE').

Then in scan_usable_client, in the "for tool in SCMCLIENTS' loop, before getting the repository info, just do a check like:

    if not options.repository_type or options.repository_type == tool.scm_type:
        # Do the rest of the body for repository_info here.

Touches many files, but it's a bit simpler to do and will actually be faster in many cases for users who use it (since it won't have to call into every tool on the system that it finds).  It's mostly some copy and paste for the strings and add_option.

I'll be happy to answer any questions on this.
#4 marcelo********@gmai***** (Google Code) (Is this you? Claim this profile.) 
I'm having the same problem but I'm using RBTools 0.4.2
Is there a plan on fixing this soon?

If that is not the case, could you please elaborate on what step by step would be the hacky solution?

Thanks
#5 kiw****@gmai***** (Google Code) (Is this you? Claim this profile.) 
A more hacky but simpler solution: move SVNClient(options=options) at the top of SCMCLIENTS list in rbtools/clients/__init__.py

For a clean solution, both proposals in comments #1 and #3 could be useful in some cases, and neither can cover all cases of the other.
david
#6 david
  • -Milestone-RBTools-Release1.0
chipx86
#7 chipx86
  • +Project-RBTools
david
#8 david
  • -reviewboard
    +rbtools
  • -Project:RBTools
david
#9 david 
Fixed in RBTools 1.0
  • -Confirmed
    +Fixed