547: post-review doesn't link up with authenticated Mercurial repositories properly

ahkn****@gmai***** (Google Code) (Is this you? Claim this profile.)
david
david
Feb. 13, 2012
1583
What steps will reproduce the problem?
1. Create an HG repo on an HTTPS server.
2. Clone it locally.
3. Add your username and password to the .hg.hgrc file
4. Add this repo to Review Board.
5. Attempt a post-review to it.

What is the expected output? What do you see instead?

It *should* see the right repo.  What it does is attempt to compare the raw URL in "default" to the 
server URL.  It *should* strip the UN/PW from the URL and compare that instead.
chipx86
#1 chipx86
  • +Confirmed
  • +Component-RBTools
    +Milestone-RBTools-Release1.0
#3 xaka****@gmai***** (Google Code) (Is this you? Claim this profile.)
Any updates on this? I looked at code and see that you put empty env when do "hg" call so no USER/HOME variables propogates to. That's because "hg" can't find config file.
#5 mail*****@gmai***** (Google Code) (Is this you? Claim this profile.)
I just realized that it is not possible to use Reviewboard with ssh Mercurial repositories.

I just setup reviewboard using a ssh repository. I created a manual diff using "hg diff" & created a review. When I view the diff, I get an error:
The file 'file-location' (rUNKNOWN) could not be found in the repository: 'sshrepository' object has no attribute 'changectx'
Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/ReviewBoard-1.5.5-py2.7.egg/reviewboard/diffviewer/views.py", line 153, in view_diff
    interdiffset, highlighting, True)
  File "/usr/local/lib/python2.7/dist-packages/ReviewBoard-1.5.5-py2.7.egg/reviewboard/diffviewer/diffutils.py", line 1066, in get_diff_files
    large_data=True)
  File "/usr/local/lib/python2.7/dist-packages/Djblets-0.6.7-py2.7.egg/djblets/util/misc.py", line 166, in cache_memoize
    data = lookup_callable()
  File "/usr/local/lib/python2.7/dist-packages/ReviewBoard-1.5.5-py2.7.egg/reviewboard/diffviewer/diffutils.py", line 1065, in <lambda>
    enable_syntax_highlighting)),
  File "/usr/local/lib/python2.7/dist-packages/ReviewBoard-1.5.5-py2.7.egg/reviewboard/diffviewer/diffutils.py", line 551, in get_chunks
    old = get_original_file(filediff)
  File "/usr/local/lib/python2.7/dist-packages/ReviewBoard-1.5.5-py2.7.egg/reviewboard/diffviewer/diffutils.py", line 364, in get_original_file
    large_data=True)[0]
  File "/usr/local/lib/python2.7/dist-packages/Djblets-0.6.7-py2.7.egg/djblets/util/misc.py", line 166, in cache_memoize
    data = lookup_callable()
  File "/usr/local/lib/python2.7/dist-packages/ReviewBoard-1.5.5-py2.7.egg/reviewboard/diffviewer/diffutils.py", line 363, in <lambda>
    data = cache_memoize(key, lambda: [fetch_file(file, revision)],
  File "/usr/local/lib/python2.7/dist-packages/ReviewBoard-1.5.5-py2.7.egg/reviewboard/diffviewer/diffutils.py", line 342, in fetch_file
    data = tool.get_file(file, revision)
  File "/usr/local/lib/python2.7/dist-packages/ReviewBoard-1.5.5-py2.7.egg/reviewboard/scmtools/hg.py", line 35, in get_file
    return self.client.cat_file(path, str(revision))
  File "/usr/local/lib/python2.7/dist-packages/ReviewBoard-1.5.5-py2.7.egg/reviewboard/scmtools/hg.py", line 192, in cat_file
    raise FileNotFoundError(path, rev, str(e))
FileNotFoundError: The file '...file-location' (rUNKNOWN) could not be found in the repository: 'sshrepository' object has no attribute 'changectx'
#6 amc****@gmai***** (Google Code) (Is this you? Claim this profile.)
It looks like post-review sets HGRCPATH to /dev/null which doesn't seem very friendly or useful. I tried removing this and I get the follow stack trace:


Traceback (most recent call last):
  File "/usr/local/share/python/post-review", line 9, in <module>
    load_entry_point('RBTools==0.3.2', 'console_scripts', 'post-review')()
  File "/usr/local/Cellar/python/2.7.1/lib/python2.7/site-packages/rbtools/postreview.py", line 3773, in main
    diff, parent_diff = tool.diff(args)
  File "/usr/local/Cellar/python/2.7.1/lib/python2.7/site-packages/rbtools/postreview.py", line 2446, in diff
    return self._get_outgoing_diff(files)
  File "/usr/local/Cellar/python/2.7.1/lib/python2.7/site-packages/rbtools/postreview.py", line 2503, in _get_outgoing_diff
    self._get_top_and_bottom_outgoing_revs(outgoing_changesets)
  File "/usr/local/Cellar/python/2.7.1/lib/python2.7/site-packages/rbtools/postreview.py", line 2540, in _get_top_and_bottom_outgoing_revs
    top_rev = max(outgoing_changesets)
ValueError: max() arg is an empty sequence

I am guessing post-review does this to keep problematic hgrc files from breaking it but it seems to have done the opposite for me.
#7 wal***@char**** (Google Code) (Is this you? Claim this profile.)
I do not know whether this fixes the general HTTPS Mercurial issue, but the attached patch permits me to run ReviewBoard against a private Bitbucket repository.

There is an issue with urllib2.AbstractBasicAuthHandler in that it REQUIRES a www-authenticate header in the HTTP response in order to send any credentials. This really should be fixed in the python distribution.

There is also a necessary Repository configuration workaround in ReviewBoard resulting from Reviewboard's ignorance of the Bitbucket REST API.

    Hosting Service: Custom
    Repository type: Mercurial
    Path: https://api.bitbucket.org/1.0/repositories/USER/REPOSITORY/

Obviously substitute the real user name and repository in the path.

In addition, EVERY time I visit the specific Repository admin page, I end up having to re-supply the Bitbucket password when I save any change.
  • +
    diff --git a/reviewboard/scmtools/hg.py b/reviewboard/scmtools/hg.py
    index 9dfd2ec..73fe919 100644
    --- a/reviewboard/scmtools/hg.py
    +++ b/reviewboard/scmtools/hg.py
    @@ -164,6 +164,44 @@ class HgDiffParser(DiffParser):
             else:
                 return False
     
    +class HTTPBasicAuthHandler(urllib2.BaseHandler, urllib2.AbstractBasicAuthHandler):
    +    '''Workaround class for broken AbstractBasicAuthHandler.
    +
    +    urllib2.AbstractBasicAuthHandler requires a 'www-authenticate' header
    +    or it will not attempt to provide authentication credentials. This class
    +    attempts to maintain compatibility if the header is present but fallback to
    +    searching for uri-based credentials otherwise.
    +
    +    '''
    +
    +    auth_header = 'Authorization'
    +
    +    def __init__(self, password_manager = None):
    +        self.retries = 0
    +        self.passwd = password_manager or urllib2.HTTPPasswordMgr()
    +        self.add_password = self.passwd.add_password
    +
    +    def realm_lookup(self, www_auth_value):
    +        
david
#8 david
I just experimented and ssh-based access works now (fixing the issue in comment #5). Not passing the username and password properly when there's no www-authenticate header is a legitimate problem, and one that occurs with other types of servers as well. I'm going to experiment and see if the requests module implements this correctly.
david
#9 david
  • -Confirmed
    +PendingReview
  • +david
david
#10 david
Fixed in release-1.6.x (f591a04) and master. Thanks!
  • -PendingReview
    +Fixed
#11 mail*****@gmai***** (Google Code) (Is this you? Claim this profile.)
I am still seing this issue with the latest release of reviewboard: 1.6.4.1.
david
#12 david
Are you seeing problems with HTTPS or SSH repositories?
#13 mail*****@gmai***** (Google Code) (Is this you? Claim this profile.)
This issue is with ssh repositories. I'm seeing different issues with https based repos.
I tried applying the patch but looks like the hg.py file is different from the time of the patch. Here's the relevant code:
class HgClient(object):
    def __init__(self, repoPath, local_site):
        from mercurial import hg, ui
        from mercurial.__version__ import version

        version_parts = [int(x) for x in version.split(".")]

        if version_parts[0] == 1 and version_parts[1] <= 2:
            hg_ui = ui.ui(interactive=False)
        else:
            hg_ui = ui.ui()
            hg_ui.setconfig('ui', 'interactive', 'off')

        # Check whether ssh is configured for mercurial. Assume that any
        # configured ssh is set up correctly for this repository.
        hg_ssh = hg_ui.config('ui', 'ssh')

        if not hg_ssh:
            logging.debug('Using rbssh for mercurial')
            hg_ui.setconfig('ui', 'ssh', 'rbssh --rb-local-site=%s'
                            % local_site)
        else:
            logging.debug('Found configured ssh for mercurial: %s' % hg_ssh)

        self.repo = hg.repository(hg_ui, path=repoPath)

    def cat_file(self, path, rev="tip"):
        if rev == HEAD:
            rev = "tip"
        elif rev == PRE_CREATION:
            rev = ""

        try:
            return self.repo.changectx(rev).filectx(path).data()
        except Exception, e:
            # LookupError moves from repo to revlog in hg v0.9.4, so we
            # catch the more general Exception to avoid the dependency.
            raise FileNotFoundError(path, rev, str(e))
david
#14 david
Please open a separate bug for ssh repositories. This issue was opened about HTTPS repositories which don't send the WWW-Authenticate header.
#15 mail*****@gmai***** (Google Code) (Is this you? Claim this profile.)
Logs:
2012-03-28 16:24:39,755 - DEBUG - Using rbssh for mercurial
2012-03-28 16:24:40,559 - DEBUG - Using rbssh for mercurial
2012-03-28 16:24:41,192 - DEBUG - Generating diff viewer page for filediff id 9
2012-03-28 16:24:41,195 - DEBUG - Begin: Generating diff file info for diffset id 9
2012-03-28 16:24:41,200 - DEBUG - Using rbssh for mercurial
2012-03-28 16:24:41,890 - DEBUG - End: Generating diff file info for diffset id 9
2012-03-28 16:24:41,891 - DEBUG - Generating diff file info for diffset id 9 took 0.694481 seconds
2012-03-28 16:24:42,059 - DEBUG - Begin: Generating diff file info for diffset id 9, filediff 14
2012-03-28 16:24:42,060 - DEBUG - Using rbssh for mercurial
2012-03-28 16:24:42,743 - INFO - Cache miss for key reviews.example.com:diff-sidebyside-hl-14.
2012-03-28 16:24:42,746 - DEBUG - Using rbssh for mercurial
2012-03-28 16:24:43,384 - INFO - Cache miss for key reviews.example.com:ssh%3A//hg%40hg.example.com/dev/gui:README:UNKNOWN.
2012-03-28 16:24:43,385 - DEBUG - Begin: Fetching file 'README' rUNKNOWN from gui
#16 mail*****@gmai***** (Google Code) (Is this you? Claim this profile.)
ok.
#17 mail*****@gmai***** (Google Code) (Is this you? Claim this profile.)
Filed bug 2556 for ssh based mercurial issues.
Filed bug 2555 for https based mercurial issues.
#19 mail*****@gmai***** (Google Code) (Is this you? Claim this profile.)
I noticed that the patch in this bug doesn't seem applied on hg.py. Is that by design? Maybe the relevant code was refactored out?
#20 mail*****@gmai***** (Google Code) (Is this you? Claim this profile.)
FYI: I tried going to the old hg.py file, manually applied the patch & retried. The fix didn't work for me.