landing-rev: fixes #4102, use branches instead of landing tip refs by default.

- also changed how readme is rendered based on landing commit to minimize locking for often changing repositories.

rhodecode/api/tests/utils.py

@@ -20,11 +20,16 b''
 
 
 import random
+import pytest
 
 from rhodecode.api.utils import get_origin
 from rhodecode.lib.ext_json import json
 
 
+def jsonify(obj):
+    return json.loads(json.dumps(obj))
+
+
 API_URL = '/_admin/api'
 
 
@@ -42,12 +47,16 b' def assert_call_ok(id_, given):'
 
 
 def assert_ok(id_, expected, given):
+    given = json.loads(given)
+    if given.get('error'):
+        pytest.fail("Unexpected ERROR in success response: {}".format(given['error']))
+
     expected = jsonify({
         'id': id_,
         'error': None,
         'result': expected
     })
-    given = json.loads(given)
+
     assert expected == given
 
 
@@ -61,10 +70,6 b' def assert_error(id_, expected, given):'
     assert expected == given
 
 
-def jsonify(obj):
-    return json.loads(json.dumps(obj))
-
-
 def build_data(apikey, method, **kw):
     """
     Builds API data with given random ID

rhodecode/api/views/repo_api.py

@@ -711,7 +711,7 b' def create_repo('
         private=Optional(False),
         clone_uri=Optional(None),
         push_uri=Optional(None),
-        landing_rev=Optional('rev:tip'),
+        landing_rev=Optional(None),
         enable_statistics=Optional(False),
         enable_locking=Optional(False),
         enable_downloads=Optional(False),
@@ -746,7 +746,7 b' def create_repo('
     :type clone_uri: str
     :param push_uri: set push_uri
     :type push_uri: str
-    :param landing_rev: <rev_type>:<rev>
+    :param landing_rev: <rev_type>:<rev>, e.g branch:default, book:dev, rev:abcd
     :type landing_rev: str
     :param enable_locking:
     :type enable_locking: bool
@@ -790,7 +790,6 b' def create_repo('
     copy_permissions = Optional.extract(copy_permissions)
     clone_uri = Optional.extract(clone_uri)
     push_uri = Optional.extract(push_uri)
-    landing_commit_ref = Optional.extract(landing_rev)
 
     defs = SettingsModel().get_default_repo_settings(strip_prefix=True)
     if isinstance(private, Optional):
@@ -804,8 +803,15 b' def create_repo('
     if isinstance(enable_downloads, Optional):
         enable_downloads = defs.get('repo_enable_downloads')
 
+    landing_ref, _label = ScmModel.backend_landing_ref(repo_type)
+    ref_choices, _labels = ScmModel().get_repo_landing_revs(request.translate)
+    ref_choices = list(set(ref_choices + [landing_ref]))
+
+    landing_commit_ref = Optional.extract(landing_rev) or landing_ref
+
     schema = repo_schema.RepoSchema().bind(
         repo_type_options=rhodecode.BACKENDS.keys(),
+        repo_ref_options=ref_choices,
         repo_type=repo_type,
         # user caller
         user=apiuser)
@@ -955,7 +961,7 b' def update_repo('
         owner=Optional(OAttr('apiuser')), description=Optional(''),
         private=Optional(False),
         clone_uri=Optional(None), push_uri=Optional(None),
-        landing_rev=Optional('rev:tip'), fork_of=Optional(None),
+        landing_rev=Optional(None), fork_of=Optional(None),
         enable_statistics=Optional(False),
         enable_locking=Optional(False),
         enable_downloads=Optional(False), fields=Optional('')):
@@ -990,7 +996,7 b' def update_repo('
     :type private: bool
     :param clone_uri: Update the |repo| clone URI.
     :type clone_uri: str
-    :param landing_rev: Set the |repo| landing revision. Default is ``rev:tip``.
+    :param landing_rev: Set the |repo| landing revision. e.g branch:default, book:dev, rev:abcd
     :type landing_rev: str
     :param enable_statistics: Enable statistics on the |repo|, (True | False).
     :type enable_statistics: bool
@@ -1046,8 +1052,10 b' def update_repo('
         repo_enable_downloads=enable_downloads
         if not isinstance(enable_downloads, Optional) else repo.enable_downloads)
 
+    landing_ref, _label = ScmModel.backend_landing_ref(repo.repo_type)
     ref_choices, _labels = ScmModel().get_repo_landing_revs(
         request.translate, repo=repo)
+    ref_choices = list(set(ref_choices + [landing_ref]))
 
     old_values = repo.get_api_data()
     repo_type = repo.repo_type
@@ -1125,7 +1133,7 b' def fork_repo(request, apiuser, repoid, '
               description=Optional(''),
               private=Optional(False),
               clone_uri=Optional(None),
-              landing_rev=Optional('rev:tip'),
+              landing_rev=Optional(None),
               copy_permissions=Optional(False)):
     """
     Creates a fork of the specified |repo|.
@@ -1155,7 +1163,7 b' def fork_repo(request, apiuser, repoid, '
     :type copy_permissions: bool
     :param private: Make the fork private. The default is False.
     :type private: bool
-    :param landing_rev: Set the landing revision. The default is tip.
+    :param landing_rev: Set the landing revision. E.g branch:default, book:dev, rev:abcd
 
     Example output:
 
@@ -1207,11 +1215,17 b' def fork_repo(request, apiuser, repoid, '
     description = Optional.extract(description)
     copy_permissions = Optional.extract(copy_permissions)
     clone_uri = Optional.extract(clone_uri)
-    landing_commit_ref = Optional.extract(landing_rev)
+
+    landing_ref, _label = ScmModel.backend_landing_ref(repo.repo_type)
+    ref_choices, _labels = ScmModel().get_repo_landing_revs(request.translate)
+    ref_choices = list(set(ref_choices + [landing_ref]))
+    landing_commit_ref = Optional.extract(landing_rev) or landing_ref
+
     private = Optional.extract(private)
 
     schema = repo_schema.RepoSchema().bind(
         repo_type_options=rhodecode.BACKENDS.keys(),
+        repo_ref_options=ref_choices,
         repo_type=repo.repo_type,
         # user caller
         user=apiuser)

rhodecode/apps/admin/views/repositories.py

@@ -59,8 +59,6 b' class AdminReposView(BaseAppView, DataGr'
                                    perm_set=['group.write', 'group.admin'])
         c.repo_groups = RepoGroup.groups_choices(groups=acl_groups)
         c.repo_groups_choices = map(lambda k: safe_unicode(k[0]), c.repo_groups)
-        c.landing_revs_choices, c.landing_revs = \
-            ScmModel().get_repo_landing_revs(self.request.translate)
         c.personal_repo_group = self._rhodecode_user.personal_repo_group
 
     @LoginRequired()
@@ -150,8 +148,7 b' class AdminReposView(BaseAppView, DataGr'
         try:
             # CanWriteToGroup validators checks permissions of this POST
             form = RepoForm(
-                self.request.translate, repo_groups=c.repo_groups_choices,
-                landing_revs=c.landing_revs_choices)()
+                self.request.translate, repo_groups=c.repo_groups_choices)()
             form_result = form.to_python(dict(self.request.POST))
             copy_permissions = form_result.get('repo_copy_permissions')
             # create is done sometimes async on celery, db transaction

rhodecode/apps/repository/views/repo_forks.py

@@ -55,9 +55,7 b' class RepoForksView(RepoAppView, DataGri'
             perm_set=['group.write', 'group.admin'])
         c.repo_groups = RepoGroup.groups_choices(groups=acl_groups)
         c.repo_groups_choices = map(lambda k: safe_unicode(k[0]), c.repo_groups)
-        choices, c.landing_revs = ScmModel().get_repo_landing_revs(
-            self.request.translate)
-        c.landing_revs_choices = choices
+
         c.personal_repo_group = c.rhodecode_user.personal_repo_group
 
         return c
@@ -212,9 +210,9 b' class RepoForksView(RepoAppView, DataGri'
         _ = self.request.translate
         c = self.load_default_context()
 
-        _form = RepoForkForm(self.request.translate, old_data={'repo_type': self.db_repo.repo_type},
-                             repo_groups=c.repo_groups_choices,
-                             landing_revs=c.landing_revs_choices)()
+        _form = RepoForkForm(self.request.translate,
+                             old_data={'repo_type': self.db_repo.repo_type},
+                             repo_groups=c.repo_groups_choices)()
         post_data = dict(self.request.POST)
 
         # forbid injecting other repo by forging a request

rhodecode/apps/repository/views/repo_summary.py

@@ -20,6 +20,8 b''
 
 import logging
 import string
+import time
+
 import rhodecode
 
 from pyramid.view import view_config
@@ -53,24 +55,25 b' class RepoSummaryView(RepoAppView):'
         return c
 
     def _get_readme_data(self, db_repo, renderer_type):
-
         log.debug('Looking for README file')
+        landing_commit = db_repo.get_landing_commit()
+        if isinstance(landing_commit, EmptyCommit):
+            return None, None
 
         cache_namespace_uid = 'cache_repo_instance.{}_{}'.format(
             db_repo.repo_id, CacheKey.CACHE_TYPE_README)
-        invalidation_namespace = CacheKey.REPO_INVALIDATION_NAMESPACE.format(
-            repo_id=self.db_repo.repo_id)
         region = rc_cache.get_or_create_region('cache_repo_longterm', cache_namespace_uid)
+        start = time.time()
 
         @region.conditional_cache_on_arguments(namespace=cache_namespace_uid)
-        def generate_repo_readme(repo_id, _repo_name, _renderer_type):
+        def generate_repo_readme(repo_id, commit_id, _repo_name, _renderer_type):
             readme_data = None
-            readme_node = None
             readme_filename = None
-            commit = self._get_landing_commit_or_none(db_repo)
-            if commit:
-                log.debug("Searching for a README file.")
-                readme_node = ReadmeFinder(_renderer_type).search(commit)
+
+            commit = db_repo.get_commit(commit_id)
+            log.debug("Searching for a README file at commit %s.", commit_id)
+            readme_node = ReadmeFinder(_renderer_type).search(commit)
+
             if readme_node:
                 log.debug('Found README node: %s', readme_node)
                 relative_urls = {
@@ -81,42 +84,19 b' class RepoSummaryView(RepoAppView):'
                         'repo_files', repo_name=_repo_name,
                         commit_id=commit.raw_id, f_path=readme_node.path),
                 }
-                readme_data = self._render_readme_or_none(
-                    commit, readme_node, relative_urls)
+                readme_data = self._render_readme_or_none(commit, readme_node, relative_urls)
                 readme_filename = readme_node.unicode_path
 
             return readme_data, readme_filename
 
-        inv_context_manager = rc_cache.InvalidationContext(
-            uid=cache_namespace_uid, invalidation_namespace=invalidation_namespace)
-        with inv_context_manager as invalidation_context:
-            args = (db_repo.repo_id, db_repo.repo_name, renderer_type,)
-            # re-compute and store cache if we get invalidate signal
-            if invalidation_context.should_invalidate():
-                instance = generate_repo_readme.refresh(*args)
-            else:
-                instance = generate_repo_readme(*args)
-
-            log.debug(
-                'Repo readme generated and computed in %.4fs',
-                inv_context_manager.compute_time)
-            return instance
-
-    def _get_landing_commit_or_none(self, db_repo):
-        log.debug("Getting the landing commit.")
-        try:
-            commit = db_repo.get_landing_commit()
-            if not isinstance(commit, EmptyCommit):
-                return commit
-            else:
-                log.debug("Repository is empty, no README to render.")
-        except CommitError:
-            log.exception(
-                "Problem getting commit when trying to render the README.")
+        readme_data, readme_filename = generate_repo_readme(
+            db_repo.repo_id, landing_commit.raw_id, db_repo.repo_name, renderer_type,)
+        compute_time = time.time() - start
+        log.debug('Repo readme generated and computed in %.4fs', compute_time)
+        return readme_data, readme_filename
 
     def _render_readme_or_none(self, commit, readme_node, relative_urls):
-        log.debug(
-            'Found README file `%s` rendering...', readme_node.path)
+        log.debug('Found README file `%s` rendering...', readme_node.path)
         renderer = MarkupRenderer()
         try:
             html_source = renderer.render(

rhodecode/lib/celerylib/tasks.py

@@ -125,6 +125,7 b' def send_email(recipients, subject, body'
 def create_repo(form_data, cur_user):
     from rhodecode.model.repo import RepoModel
     from rhodecode.model.user import UserModel
+    from rhodecode.model.scm import ScmModel
     from rhodecode.model.settings import SettingsModel
 
     log = get_logger(create_repo)
@@ -139,7 +140,6 b' def create_repo(form_data, cur_user):'
     private = form_data['repo_private']
     clone_uri = form_data.get('clone_uri')
     repo_group = safe_int(form_data['repo_group'])
-    landing_rev = form_data['repo_landing_rev']
     copy_fork_permissions = form_data.get('copy_permissions')
     copy_group_permissions = form_data.get('repo_copy_permissions')
     fork_of = form_data.get('fork_parent_id')
@@ -154,6 +154,9 b' def create_repo(form_data, cur_user):'
     enable_downloads = form_data.get(
         'enable_downloads', defs.get('repo_enable_downloads'))
 
+    # set landing rev based on default branches for SCM
+    landing_ref, _label = ScmModel.backend_landing_ref(repo_type)
+
     try:
         RepoModel()._create_repo(
             repo_name=repo_name_full,
@@ -163,7 +166,7 b' def create_repo(form_data, cur_user):'
             private=private,
             clone_uri=clone_uri,
             repo_group=repo_group,
-            landing_rev=landing_rev,
+            landing_rev=landing_ref,
             fork_of=fork_of,
             copy_fork_permissions=copy_fork_permissions,
             copy_group_permissions=copy_group_permissions,

rhodecode/model/forms.py

@@ -244,12 +244,10 b' def PasswordResetForm(localizer):'
     return _PasswordResetForm
 
 
-def RepoForm(localizer, edit=False, old_data=None, repo_groups=None,
-             landing_revs=None, allow_disabled=False):
+def RepoForm(localizer, edit=False, old_data=None, repo_groups=None, allow_disabled=False):
     _ = localizer
     old_data = old_data or {}
     repo_groups = repo_groups or []
-    landing_revs = landing_revs or []
     supported_backends = BACKENDS.keys()
 
     class _RepoForm(formencode.Schema):
@@ -263,7 +261,6 b' def RepoForm(localizer, edit=False, old_'
                             if_missing=old_data.get('repo_type'))
         repo_description = v.UnicodeString(strip=True, min=1, not_empty=False)
         repo_private = v.StringBoolean(if_missing=False)
-        repo_landing_rev = v.OneOf(landing_revs, hideList=True)
         repo_copy_permissions = v.StringBoolean(if_missing=False)
         clone_uri = All(v.UnicodeString(strip=True, min=1, not_empty=False))
 
@@ -333,12 +330,10 b' def RepoFieldForm(localizer):'
 
 
 def RepoForkForm(localizer, edit=False, old_data=None,
-                 supported_backends=BACKENDS.keys(), repo_groups=None,
-                 landing_revs=None):
+                 supported_backends=BACKENDS.keys(), repo_groups=None):
     _ = localizer
     old_data = old_data or {}
     repo_groups = repo_groups or []
-    landing_revs = landing_revs or []
 
     class _RepoForkForm(formencode.Schema):
         allow_extra_fields = True
@@ -353,7 +348,6 b' def RepoForkForm(localizer, edit=False, '
         copy_permissions = v.StringBoolean(if_missing=False)
         fork_parent_id = v.UnicodeString()
         chained_validators = [v.ValidForkName(localizer, edit, old_data)]
-        landing_rev = v.OneOf(landing_revs, hideList=True)
     return _RepoForkForm
 
 

rhodecode/model/scm.py

@@ -891,6 +891,21 b' class ScmModel(BaseModel):'
     def get_unread_journal(self):
         return self.sa.query(UserLog).count()
 
+    @classmethod
+    def backend_landing_ref(cls, repo_type):
+        """
+        Return a default landing ref based on a repository type.
+        """
+
+        landing_ref = {
+            'hg': ('branch:default', 'default'),
+            'git': ('branch:master', 'master'),
+            'svn': ('rev:tip', 'latest tip'),
+            'default': ('rev:tip', 'latest tip'),
+        }
+
+        return landing_ref.get(repo_type) or landing_ref['default']
+
     def get_repo_landing_revs(self, translator, repo=None):
         """
         Generates select option with tags branches and bookmarks (for hg only)
@@ -901,41 +916,56 b' class ScmModel(BaseModel):'
         _ = translator
         repo = self._get_repo(repo)
 
-        hist_l = [
-            ['rev:tip', _('latest tip')]
+        if repo:
+            repo_type = repo.repo_type
+        else:
+            repo_type = 'default'
+
+        default_landing_ref, landing_ref_lbl = self.backend_landing_ref(repo_type)
+
+        default_ref_options = [
+            [default_landing_ref, landing_ref_lbl]
         ]
-        choices = [
-            'rev:tip'
+        default_choices = [
+            default_landing_ref
         ]
 
         if not repo:
-            return choices, hist_l
+            return default_choices, default_ref_options
 
         repo = repo.scm_instance()
 
-        branches_group = (
-            [(u'branch:%s' % safe_unicode(b), safe_unicode(b))
-                for b in repo.branches],
-            _("Branches"))
-        hist_l.append(branches_group)
+        ref_options = [('rev:tip', 'latest tip')]
+        choices = ['rev:tip']
+
+        # branches
+        branch_group = [(u'branch:%s' % safe_unicode(b), safe_unicode(b)) for b in repo.branches]
+        if not branch_group:
+            # new repo, or without maybe a branch?
+            branch_group = default_ref_options
+
+        branches_group = (branch_group, _("Branches"))
+        ref_options.append(branches_group)
         choices.extend([x[0] for x in branches_group[0]])
 
+        # bookmarks for HG
         if repo.alias == 'hg':
             bookmarks_group = (
                 [(u'book:%s' % safe_unicode(b), safe_unicode(b))
                     for b in repo.bookmarks],
                 _("Bookmarks"))
-            hist_l.append(bookmarks_group)
+            ref_options.append(bookmarks_group)
             choices.extend([x[0] for x in bookmarks_group[0]])
 
+        # tags
         tags_group = (
             [(u'tag:%s' % safe_unicode(t), safe_unicode(t))
                 for t in repo.tags],
             _("Tags"))
-        hist_l.append(tags_group)
+        ref_options.append(tags_group)
         choices.extend([x[0] for x in tags_group[0]])
 
-        return choices, hist_l
+        return choices, ref_options
 
     def get_server_info(self, environ=None):
         server_info = get_system_info(environ)

rhodecode/model/validation_schema/schemas/repo_schema.py

@@ -26,6 +26,11 b' from rhodecode.model.validation_schema.u'
 from rhodecode.model.validation_schema import validators, preparers, types
 
 DEFAULT_LANDING_REF = 'rev:tip'
+DEFAULT_BACKEND_LANDING_REF = {
+    'hg': 'branch:default',
+    'git': 'branch:master',
+    'svn': 'rev:tip',
+}
 
 
 def get_group_and_repo(repo_name):
@@ -74,8 +79,14 b' def deferred_sync_uri_validator(node, kw'
 
 @colander.deferred
 def deferred_landing_ref_widget(node, kw):
-    items = kw.get(
-        'repo_ref_items', [(DEFAULT_LANDING_REF, DEFAULT_LANDING_REF)])
+    repo_type = kw.get('repo_type')
+    default_opts = []
+    if repo_type:
+        default_opts.append(
+            (DEFAULT_BACKEND_LANDING_REF[repo_type],
+             DEFAULT_BACKEND_LANDING_REF[repo_type]))
+
+    items = kw.get('repo_ref_items', default_opts)
     items = convert_to_optgroup(items)
     return deform.widget.Select2Widget(values=items)
 

rhodecode/templates/admin/repos/repo_add_base.mako

@@ -39,15 +39,6 b''
             </div>
         </div>
         <div class="field">
-            <div class="label">
-                <label for="repo_type">${_('Type')}:</label>
-            </div>
-            <div class="select">
-                ${h.select('repo_type','hg',c.backends)}
-                <span class="help-block">${_('Set the type of repository to create.')}</span>
-            </div>
-        </div>
-        <div class="field">
              <div class="label">
                  <label for="repo_group">${_('Repository group')}:</label>
              </div>
@@ -63,6 +54,15 b''
         </div>
         <div class="field">
             <div class="label">
+                <label for="repo_type">${_('Type')}:</label>
+            </div>
+            <div class="select">
+                ${h.select('repo_type','hg',c.backends)}
+                <span class="help-block">${_('Set the type of repository to create.')}</span>
+            </div>
+        </div>
+        <div class="field">
+            <div class="label">
                 <label for="repo_description">${_('Description')}:</label>
             </div>
             <div class="textarea editor">
@@ -75,15 +75,6 b''
                 </span>
             </div>
         </div>
-        <div class="field">
-            <div class="label">
-                <label for="repo_landing_rev">${_('Landing commit')}:</label>
-            </div>
-            <div class="select">
-                ${h.select('repo_landing_rev','',c.landing_revs,class_="medium")}
-                <span class="help-block">${_('The default commit for file pages, downloads, full text search index, and README generation.')}</span>
-            </div>
-        </div>
         <div id="copy_perms" class="field">
             <div class="label label-checkbox">
                 <label for="repo_copy_permissions">${_('Copy Parent Group Permissions')}:</label>
@@ -147,11 +138,7 b''
             'dropdownCssClass': "drop-menu-dropdown",
             'minimumResultsForSearch': -1,
         });
-        $("#repo_landing_rev").select2({
-            'containerCssClass': "drop-menu",
-            'dropdownCssClass': "drop-menu-dropdown",
-            'minimumResultsForSearch': -1,
-        });
+
         $('#repo_name').focus();
 
         $('#select_my_group').on('click', function(e){

rhodecode/templates/forks/fork.mako

@@ -67,16 +67,6 b''
             </div>
 
             <div class="field">
-                <div class="label">
-                    <label for="landing_rev">${_('Landing commit')}:</label>
-                </div>
-                <div class="select">
-                    ${h.select('landing_rev','',c.landing_revs,class_="medium")}
-                    <span class="help-block">${_('The default commit for file pages, downloads, full text search index, and README generation.')}</span>
-                </div>
-            </div>
-
-            <div class="field">
                 <div class="label label-checkbox">
                     <label for="private">${_('Copy permissions')}:</label>
                 </div>