Changeset - 5fba3778431c
[Not reviewed]
beta
0 5 0
Marcin Kuzminski - 13 years ago 2012-10-03 18:41:57
marcin@python-works.com
#590 Add GET flag that controls the way the diff are generated, for pull requests we want to use non-bundle based diffs,
That are far better for doing code reviews. The /compare url still uses bundle compare for full comparision including the incoming changesets.
- Fixed tests
5 files changed with 144 insertions and 12 deletions:
0 comments (0 inline, 0 general)
rhodecode/controllers/compare.py
Show inline comments
 
@@ -19,120 +19,130 @@
 
# This program is distributed in the hope that it will be useful,
 
# but WITHOUT ANY WARRANTY; without even the implied warranty of
 
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 
# GNU General Public License for more details.
 
#
 
# You should have received a copy of the GNU General Public License
 
# along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
import logging
 
import traceback
 

	
 
from webob.exc import HTTPNotFound
 
from pylons import request, response, session, tmpl_context as c, url
 
from pylons.controllers.util import abort, redirect
 
from pylons.i18n.translation import _
 

	
 
from rhodecode.lib.vcs.exceptions import EmptyRepositoryError, RepositoryError
 
from rhodecode.lib import helpers as h
 
from rhodecode.lib.base import BaseRepoController, render
 
from rhodecode.lib.auth import LoginRequired, HasRepoPermissionAnyDecorator
 
from rhodecode.lib import diffs
 

	
 
from rhodecode.model.db import Repository
 
from rhodecode.model.pull_request import PullRequestModel
 
from webob.exc import HTTPBadRequest
 
from rhodecode.lib.utils2 import str2bool
 

	
 
log = logging.getLogger(__name__)
 

	
 

	
 
class CompareController(BaseRepoController):
 

	
 
    @LoginRequired()
 
    @HasRepoPermissionAnyDecorator('repository.read', 'repository.write',
 
                                   'repository.admin')
 
    def __before__(self):
 
        super(CompareController, self).__before__()
 

	
 
    def __get_cs_or_redirect(self, rev, repo, redirect_after=True,
 
                             partial=False):
 
        """
 
        Safe way to get changeset if error occur it redirects to changeset with
 
        proper message. If partial is set then don't do redirect raise Exception
 
        instead
 

	
 
        :param rev: revision to fetch
 
        :param repo: repo instance
 
        """
 

	
 
        try:
 
            type_, rev = rev
 
            return repo.scm_instance.get_changeset(rev)
 
        except EmptyRepositoryError, e:
 
            if not redirect_after:
 
                return None
 
            h.flash(h.literal(_('There are no changesets yet')),
 
                    category='warning')
 
            redirect(url('summary_home', repo_name=repo.repo_name))
 

	
 
        except RepositoryError, e:
 
            log.error(traceback.format_exc())
 
            h.flash(str(e), category='warning')
 
            if not partial:
 
                redirect(h.url('summary_home', repo_name=repo.repo_name))
 
            raise HTTPBadRequest()
 

	
 
    def index(self, org_ref_type, org_ref, other_ref_type, other_ref):
 

	
 
        org_repo = c.rhodecode_db_repo.repo_name
 
        org_ref = (org_ref_type, org_ref)
 
        other_ref = (other_ref_type, other_ref)
 
        other_repo = request.GET.get('repo', org_repo)
 
        bundle_compare = str2bool(request.GET.get('bundle', True))
 

	
 
        c.swap_url = h.url('compare_url', repo_name=other_repo,
 
              org_ref_type=other_ref[0], org_ref=other_ref[1],
 
              other_ref_type=org_ref[0], other_ref=org_ref[1],
 
              repo=org_repo)
 
              repo=org_repo, as_form=request.GET.get('as_form'),
 
              bundle=bundle_compare)
 

	
 
        c.org_repo = org_repo = Repository.get_by_repo_name(org_repo)
 
        c.other_repo = other_repo = Repository.get_by_repo_name(other_repo)
 

	
 
        if c.org_repo is None or c.other_repo is None:
 
            log.error('Could not found repo %s or %s' % (org_repo, other_repo))
 
            raise HTTPNotFound
 

	
 
        if c.org_repo.scm_instance.alias != 'hg':
 
            log.error('Review not available for GIT REPOS')
 
            raise HTTPNotFound
 
        partial = request.environ.get('HTTP_X_PARTIAL_XHR')
 
        self.__get_cs_or_redirect(rev=org_ref, repo=org_repo, partial=partial)
 
        self.__get_cs_or_redirect(rev=other_ref, repo=other_repo, partial=partial)
 

	
 
        c.cs_ranges, discovery_data = PullRequestModel().get_compare_data(
 
                                       org_repo, org_ref, other_repo, other_ref
 
                                      )
 
                                    org_repo, org_ref, other_repo, other_ref
 
                                    )
 

	
 
        c.statuses = c.rhodecode_db_repo.statuses([x.raw_id for x in
 
                                                   c.cs_ranges])
 
        c.target_repo = c.repo_name
 
        # defines that we need hidden inputs with changesets
 
        c.as_form = request.GET.get('as_form', False)
 
        if partial:
 
            return render('compare/compare_cs.html')
 

	
 
        if not bundle_compare and c.cs_ranges:
 
            # case we want a simple diff without incoming changesets, just
 
            # for review purposes. Make the diff on the forked repo, with
 
            # revision that is common ancestor
 
            other_ref = ('rev', c.cs_ranges[-1].parents[0].raw_id)
 
            other_repo = org_repo
 

	
 
        c.org_ref = org_ref[1]
 
        c.other_ref = other_ref[1]
 
        # diff needs to have swapped org with other to generate proper diff
 

	
 
        _diff = diffs.differ(other_repo, other_ref, org_repo, org_ref,
 
                             discovery_data)
 
                             discovery_data, bundle_compare=bundle_compare)
 
        diff_processor = diffs.DiffProcessor(_diff, format='gitdiff')
 
        _parsed = diff_processor.prepare()
 

	
 
        c.files = []
 
        c.changes = {}
 

	
 
        for f in _parsed:
 
            fid = h.FID('', f['filename'])
 
            c.files.append([fid, f['operation'], f['filename'], f['stats']])
 
            diff = diff_processor.as_html(enable_comments=False, diff_lines=[f])
 
            c.changes[fid] = [f['operation'], f['filename'], diff]
 

	
 
        return render('compare/compare_diff.html')
rhodecode/controllers/pullrequests.py
Show inline comments
 
@@ -251,48 +251,54 @@ class PullrequestsController(BaseRepoCon
 
        :type pull_request:
 
        """
 

	
 
        org_repo = pull_request.org_repo
 
        (org_ref_type,
 
         org_ref_name,
 
         org_ref_rev) = pull_request.org_ref.split(':')
 

	
 
        other_repo = pull_request.other_repo
 
        (other_ref_type,
 
         other_ref_name,
 
         other_ref_rev) = pull_request.other_ref.split(':')
 

	
 
        # despite opening revisions for bookmarks/branches/tags, we always
 
        # convert this to rev to prevent changes after book or branch change
 
        org_ref = ('rev', org_ref_rev)
 
        other_ref = ('rev', other_ref_rev)
 

	
 
        c.org_repo = org_repo
 
        c.other_repo = other_repo
 

	
 
        c.cs_ranges, discovery_data = PullRequestModel().get_compare_data(
 
                                       org_repo, org_ref, other_repo, other_ref
 
                                      )
 
        if c.cs_ranges:
 
            # case we want a simple diff without incoming changesets, just
 
            # for review purposes. Make the diff on the forked repo, with
 
            # revision that is common ancestor
 
            other_ref = ('rev', c.cs_ranges[-1].parents[0].raw_id)
 
            other_repo = org_repo
 

	
 
        c.statuses = org_repo.statuses([x.raw_id for x in c.cs_ranges])
 
        # defines that we need hidden inputs with changesets
 
        c.as_form = request.GET.get('as_form', False)
 

	
 
        c.org_ref = org_ref[1]
 
        c.other_ref = other_ref[1]
 
        # diff needs to have swapped org with other to generate proper diff
 
        _diff = diffs.differ(other_repo, other_ref, org_repo, org_ref,
 
                             discovery_data)
 
        diff_processor = diffs.DiffProcessor(_diff, format='gitdiff')
 
        _parsed = diff_processor.prepare()
 

	
 
        c.files = []
 
        c.changes = {}
 

	
 
        for f in _parsed:
 
            fid = h.FID('', f['filename'])
 
            c.files.append([fid, f['operation'], f['filename'], f['stats']])
 
            diff = diff_processor.as_html(enable_comments=enable_comments,
 
                                          diff_lines=[f])
 
            c.changes[fid] = [f['operation'], f['filename'], diff]
 

	
 
    def show(self, repo_name, pull_request_id):
rhodecode/lib/diffs.py
Show inline comments
 
@@ -7,66 +7,69 @@
 

	
 

	
 
    :created_on: Dec 4, 2011
 
    :author: marcink
 
    :copyright: (C) 2010-2012 Marcin Kuzminski <marcin@python-works.com>
 
    :original copyright: 2007-2008 by Armin Ronacher
 
    :license: GPLv3, see COPYING for more details.
 
"""
 
# This program is free software: you can redistribute it and/or modify
 
# it under the terms of the GNU General Public License as published by
 
# the Free Software Foundation, either version 3 of the License, or
 
# (at your option) any later version.
 
#
 
# This program is distributed in the hope that it will be useful,
 
# but WITHOUT ANY WARRANTY; without even the implied warranty of
 
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 
# GNU General Public License for more details.
 
#
 
# You should have received a copy of the GNU General Public License
 
# along with this program.  If not, see <http://www.gnu.org/licenses/>.
 

	
 
import re
 
import difflib
 
import markupsafe
 
import logging
 

	
 
from itertools import tee, imap
 

	
 
from mercurial import patch
 
from mercurial.mdiff import diffopts
 
from mercurial.bundlerepo import bundlerepository
 

	
 
from pylons.i18n.translation import _
 

	
 
from rhodecode.lib.compat import BytesIO
 
from rhodecode.lib.vcs.utils.hgcompat import localrepo
 
from rhodecode.lib.vcs.exceptions import VCSError
 
from rhodecode.lib.vcs.nodes import FileNode, SubModuleNode
 
from rhodecode.lib.vcs.backends.base import EmptyChangeset
 
from rhodecode.lib.helpers import escape
 
from rhodecode.lib.utils import make_ui
 
from rhodecode.lib.utils2 import safe_unicode
 

	
 
log = logging.getLogger(__name__)
 

	
 

	
 
def wrap_to_table(str_):
 
    return '''<table class="code-difftable">
 
                <tr class="line no-comment">
 
                <td class="lineno new"></td>
 
                <td class="code no-comment"><pre>%s</pre></td>
 
                </tr>
 
              </table>''' % str_
 

	
 

	
 
def wrapped_diff(filenode_old, filenode_new, cut_off_limit=None,
 
                ignore_whitespace=True, line_context=3,
 
                enable_comments=False):
 
    """
 
    returns a wrapped diff into a table, checks for cut_off_limit and presents
 
    proper message
 
    """
 

	
 
    if filenode_old is None:
 
        filenode_old = FileNode(filenode_new.path, '', EmptyChangeset())
 

	
 
    if filenode_old.is_binary or filenode_new.is_binary:
 
        diff = wrap_to_table(_('binary file'))
 
        stats = (0, 0)
 
@@ -553,85 +556,88 @@ class DiffProcessor(object):
 
        _html.append('''</table>''')
 
        if _html_empty:
 
            return None
 
        return ''.join(_html)
 

	
 
    def stat(self):
 
        """
 
        Returns tuple of added, and removed lines for this instance
 
        """
 
        return self.adds, self.removes
 

	
 

	
 
class InMemoryBundleRepo(bundlerepository):
 
    def __init__(self, ui, path, bundlestream):
 
        self._tempparent = None
 
        localrepo.localrepository.__init__(self, ui, path)
 
        self.ui.setconfig('phases', 'publish', False)
 

	
 
        self.bundle = bundlestream
 

	
 
        # dict with the mapping 'filename' -> position in the bundle
 
        self.bundlefilespos = {}
 

	
 

	
 
def differ(org_repo, org_ref, other_repo, other_ref, discovery_data=None):
 
def differ(org_repo, org_ref, other_repo, other_ref, discovery_data=None,
 
           bundle_compare=False):
 
    """
 
    General differ between branches, bookmarks or separate but releated
 
    repositories
 

	
 
    :param org_repo:
 
    :type org_repo:
 
    :param org_ref:
 
    :type org_ref:
 
    :param other_repo:
 
    :type other_repo:
 
    :param other_ref:
 
    :type other_ref:
 
    """
 

	
 
    bundlerepo = None
 
    ignore_whitespace = False
 
    context = 3
 
    org_repo = org_repo.scm_instance._repo
 
    other_repo = other_repo.scm_instance._repo
 
    opts = diffopts(git=True, ignorews=ignore_whitespace, context=context)
 
    org_ref = org_ref[1]
 
    other_ref = other_ref[1]
 

	
 
    if org_repo != other_repo:
 
    if org_repo != other_repo and bundle_compare:
 

	
 
        common, incoming, rheads = discovery_data
 
        other_repo_peer = localrepo.locallegacypeer(other_repo.local())
 
        # create a bundle (uncompressed if other repo is not local)
 
        if other_repo_peer.capable('getbundle') and incoming:
 
            # disable repo hooks here since it's just bundle !
 
            # patch and reset hooks section of UI config to not run any
 
            # hooks on fetching archives with subrepos
 
            for k, _ in other_repo.ui.configitems('hooks'):
 
                other_repo.ui.setconfig('hooks', k, None)
 

	
 
            unbundle = other_repo.getbundle('incoming', common=common,
 
                                            heads=None)
 

	
 
            buf = BytesIO()
 
            while True:
 
                chunk = unbundle._stream.read(1024 * 4)
 
                if not chunk:
 
                    break
 
                buf.write(chunk)
 

	
 
            buf.seek(0)
 
            # replace chunked _stream with data that can do tell() and seek()
 
            unbundle._stream = buf
 

	
 
            ui = make_ui('db')
 
            bundlerepo = InMemoryBundleRepo(ui, path=org_repo.root,
 
                                            bundlestream=unbundle)
 

	
 
        return ''.join(patch.diff(bundlerepo or org_repo,
 
                                  node1=org_repo[org_ref].node(),
 
                                  node2=other_repo[other_ref].node(),
 
                                  opts=opts))
 
    else:
 
        log.debug('running diff between %s@%s and %s@%s'
 
                  % (org_repo, org_ref, other_repo, other_ref))
 
        return ''.join(patch.diff(org_repo, node1=org_ref, node2=other_ref,
 
                                  opts=opts))
rhodecode/templates/pullrequests/pullrequest.html
Show inline comments
 
@@ -120,49 +120,49 @@
 
            <div class="buttons">
 
                ${h.submit('save',_('Send pull request'),class_="ui-btn large")}
 
                ${h.reset('reset',_('Reset'),class_="ui-btn large")}
 
           </div>
 
        </div>
 
    </div>
 
    ${h.end_form()}
 

	
 
</div>
 

	
 
<script type="text/javascript">
 
  var _USERS_AC_DATA = ${c.users_array|n};
 
  var _GROUPS_AC_DATA = ${c.users_groups_array|n};
 
  PullRequestAutoComplete('user', 'reviewers_container', _USERS_AC_DATA, _GROUPS_AC_DATA);
 

	
 
  var other_repos_info = ${c.other_repos_info|n};
 
  
 
  var loadPreview = function(){
 
	  YUD.setStyle(YUD.get('pull_request_overview_url').parentElement,'display','none');
 
      var url = "${h.url('compare_url',
 
    	                 repo_name='org_repo',
 
    	                 org_ref_type='org_ref_type', org_ref='org_ref',
 
                         other_ref_type='other_ref_type', other_ref='other_ref',
 
                         repo='other_repo',
 
                         as_form=True)}";
 
                         as_form=True, bundle=False)}";
 

	
 
      var select_refs = YUQ('#pull_request_form select.refs')
 
      var rev_data = {}; // gather the org/other ref and repo here
 
      for(var i=0;i<select_refs.length;i++){
 
        var select_ref = select_refs[i];
 
        var select_ref_data = select_ref.value.split(':');
 
        var key = null;
 
        var val = null;
 
        
 
        if(select_ref_data.length>1){
 
          key = select_ref.name+"_type";
 
          val = select_ref_data[0];
 
          url = url.replace(key,val);
 
          rev_data[key] = val;
 
          
 
          key = select_ref.name;
 
          val = select_ref_data[1];
 
          url = url.replace(key,val);
 
          rev_data[key] = val;
 
          
 
        }else{
 
          key = select_ref.name;
 
          val = select_ref.value;
 
          url = url.replace(key,val);
rhodecode/tests/functional/test_compare.py
Show inline comments
 
@@ -233,61 +233,171 @@ class TestCompareController(TestControll
 
            cs=cs1, user=TEST_USER_ADMIN_LOGIN,
 
            author=TEST_USER_ADMIN_LOGIN,
 
            message='commit2-fork',
 
            content='file2-line1-from-fork',
 
            f_path='file2-fork'
 
        )
 
        cs3 = ScmModel().create_node(
 
            repo=repo2.scm_instance, repo_name=r2_name,
 
            cs=cs2, user=TEST_USER_ADMIN_LOGIN,
 
            author=TEST_USER_ADMIN_LOGIN,
 
            message='commit3-fork',
 
            content='file3-line1-from-fork',
 
            f_path='file3-fork'
 
        )
 

	
 
        #compare !
 
        rev1 = 'default'
 
        rev2 = 'default'
 
        response = self.app.get(url(controller='compare', action='index',
 
                                    repo_name=r2_name,
 
                                    org_ref_type="branch",
 
                                    org_ref=rev1,
 
                                    other_ref_type="branch",
 
                                    other_ref=rev2,
 
                                    repo=r1_name
 
                                    repo=r1_name,
 
                                    bundle=True,
 
                                    ))
 

	
 
        try:
 
            response.mustcontain('%s@%s -> %s@%s' % (r2_name, rev1, r1_name, rev2))
 
            response.mustcontain("""file1-line1-from-fork""")
 
            response.mustcontain("""file2-line1-from-fork""")
 
            response.mustcontain("""file3-line1-from-fork""")
 

	
 
            #add new commit into parent !
 
            cs0 = ScmModel().create_node(
 
                repo=repo1.scm_instance, repo_name=r1_name,
 
                cs=EmptyChangeset(alias='hg'), user=TEST_USER_ADMIN_LOGIN,
 
                author=TEST_USER_ADMIN_LOGIN,
 
                message='commit2',
 
                content='line1-from-new-parent',
 
                f_path='file2'
 
            )
 
            #compare !
 
            rev1 = 'default'
 
            rev2 = 'default'
 
            response = self.app.get(url(controller='compare', action='index',
 
                                        repo_name=r2_name,
 
                                        org_ref_type="branch",
 
                                        org_ref=rev1,
 
                                        other_ref_type="branch",
 
                                        other_ref=rev2,
 
                                        repo=r1_name,
 
                                        bundle=True,
 
                                        ))
 

	
 
            response.mustcontain('%s@%s -> %s@%s' % (r2_name, rev1, r1_name, rev2))
 
            response.mustcontain("""<a href="#">file2</a>""")  # new commit from parent
 
            response.mustcontain("""line1-from-new-parent""")
 
            response.mustcontain("""file1-line1-from-fork""")
 
            response.mustcontain("""file2-line1-from-fork""")
 
            response.mustcontain("""file3-line1-from-fork""")
 
        finally:
 
            RepoModel().delete(r2_id)
 
            RepoModel().delete(r1_id)
 

	
 
    def test_org_repo_new_commits_after_forking_simple_diff(self):
 
        self.log_user()
 

	
 
        repo1 = RepoModel().create_repo(repo_name='one', repo_type='hg',
 
                                        description='diff-test',
 
                                        owner=TEST_USER_ADMIN_LOGIN)
 

	
 
        Session().commit()
 
        r1_id = repo1.repo_id
 
        r1_name = repo1.repo_name
 

	
 
        #commit something initially !
 
        cs0 = ScmModel().create_node(
 
            repo=repo1.scm_instance, repo_name=r1_name,
 
            cs=EmptyChangeset(alias='hg'), user=TEST_USER_ADMIN_LOGIN,
 
            author=TEST_USER_ADMIN_LOGIN,
 
            message='commit1',
 
            content='line1',
 
            f_path='file1'
 
        )
 
        Session().commit()
 
        self.assertEqual(repo1.scm_instance.revisions, [cs0.raw_id])
 
        #fork the repo1
 
        repo2 = RepoModel().create_repo(repo_name='one-fork', repo_type='hg',
 
                                description='compare-test',
 
                                clone_uri=repo1.repo_full_path,
 
                                owner=TEST_USER_ADMIN_LOGIN, fork_of='one')
 
        Session().commit()
 
        self.assertEqual(repo2.scm_instance.revisions, [cs0.raw_id])
 
        r2_id = repo2.repo_id
 
        r2_name = repo2.repo_name
 

	
 
        #make 3 new commits in fork
 
        cs1 = ScmModel().create_node(
 
            repo=repo2.scm_instance, repo_name=r2_name,
 
            cs=repo2.scm_instance[-1], user=TEST_USER_ADMIN_LOGIN,
 
            author=TEST_USER_ADMIN_LOGIN,
 
            message='commit1-fork',
 
            content='file1-line1-from-fork',
 
            f_path='file1-fork'
 
        )
 
        cs2 = ScmModel().create_node(
 
            repo=repo2.scm_instance, repo_name=r2_name,
 
            cs=cs1, user=TEST_USER_ADMIN_LOGIN,
 
            author=TEST_USER_ADMIN_LOGIN,
 
            message='commit2-fork',
 
            content='file2-line1-from-fork',
 
            f_path='file2-fork'
 
        )
 
        cs3 = ScmModel().create_node(
 
            repo=repo2.scm_instance, repo_name=r2_name,
 
            cs=cs2, user=TEST_USER_ADMIN_LOGIN,
 
            author=TEST_USER_ADMIN_LOGIN,
 
            message='commit3-fork',
 
            content='file3-line1-from-fork',
 
            f_path='file3-fork'
 
        )
 

	
 
        #compare !
 
        rev1 = 'default'
 
        rev2 = 'default'
 
        response = self.app.get(url(controller='compare', action='index',
 
                                    repo_name=r2_name,
 
                                    org_ref_type="branch",
 
                                    org_ref=rev1,
 
                                    other_ref_type="branch",
 
                                    other_ref=rev2,
 
                                    repo=r1_name,
 
                                    bundle=False,
 
                                    ))
 

	
 
        try:
 
            #response.mustcontain('%s@%s -> %s@%s' % (r2_name, rev1, r1_name, rev2))
 

	
 
            #add new commit into parent !
 
            cs0 = ScmModel().create_node(
 
                repo=repo1.scm_instance, repo_name=r1_name,
 
                cs=EmptyChangeset(alias='hg'), user=TEST_USER_ADMIN_LOGIN,
 
                author=TEST_USER_ADMIN_LOGIN,
 
                message='commit2',
 
                content='line1',
 
                f_path='file2'
 
            )
 
            #compare !
 
            rev1 = 'default'
 
            rev2 = 'default'
 
            response = self.app.get(url(controller='compare', action='index',
 
                                        repo_name=r2_name,
 
                                        org_ref_type="branch",
 
                                        org_ref=rev1,
 
                                        other_ref_type="branch",
 
                                        other_ref=rev2,
 
                                        repo=r1_name
 
                                        repo=r1_name,
 
                                        bundle=False
 
                                        ))
 

	
 
            rev2 = cs0.parents[0].raw_id
 
            response.mustcontain('%s@%s -> %s@%s' % (r2_name, rev1, r1_name, rev2))
 
            response.mustcontain("""file1-line1-from-fork""")
 
            response.mustcontain("""file2-line1-from-fork""")
 
            response.mustcontain("""file3-line1-from-fork""")
 
            self.assertFalse("""<a href="#">file2</a>""" in response.body)  # new commit from parent
 
            self.assertFalse("""line1-from-new-parent"""  in response.body)
 
        finally:
 
            RepoModel().delete(r2_id)
 
            RepoModel().delete(r1_id)
 
            RepoModel().delete(r1_id)
 
\ No newline at end of file
0 comments (0 inline, 0 general)