# HG changeset patch # User Marcin Kuzminski # Date 2017-08-24 20:36:34 # Node ID 678ed37822036409ea1427e748e1b59fc9c9b5b0 # Parent 5bbc687359e44df88de6ccecfd3565bac57cc657 pull-requests: use close action with proper --close-commit solution. - Mercurial settings are no long as LABS - Unified some code between GIT and HG on rebase/close/delete feature branches diff --git a/docs/api/methods/repo-methods.rst b/docs/api/methods/repo-methods.rst --- a/docs/api/methods/repo-methods.rst +++ b/docs/api/methods/repo-methods.rst @@ -534,6 +534,8 @@ get_repo_settings "phases_publish": "True", "rhodecode_hg_use_rebase_for_merging": true, "rhodecode_hg_close_branch_before_merging": false, + "rhodecode_git_use_rebase_for_merging": true, + "rhodecode_git_close_branch_before_merging": false, "rhodecode_pr_merge_enabled": true, "rhodecode_use_outdated_comments": true } diff --git a/rhodecode/lib/vcs/backends/hg/repository.py b/rhodecode/lib/vcs/backends/hg/repository.py --- a/rhodecode/lib/vcs/backends/hg/repository.py +++ b/rhodecode/lib/vcs/backends/hg/repository.py @@ -558,6 +558,7 @@ class MercurialRepository(BaseRepository """ Update the working copty to the specified revision. """ + log.debug('Doing checkout to commit: `%s` for %s', revision, self) self._remote.update(revision, clean=clean) def _identify(self): @@ -654,8 +655,8 @@ class MercurialRepository(BaseRepository Returns the commit id of the close and a boolean indicating if the commit needs to be pushed. """ - self._update(target_ref.commit_id) - message = close_message or "Closing branch" + self._update(source_ref.commit_id) + message = close_message or "Closing branch: `{}`".format(source_ref.name) try: self._remote.commit( message=safe_str(message), @@ -732,16 +733,26 @@ class MercurialRepository(BaseRepository False, False, None, MergeFailureReason.MISSING_SOURCE_REF) merge_ref = None + merge_commit_id = None + close_commit_id = None merge_failure_reason = MergeFailureReason.NONE - if close_branch and not use_rebase: + # enforce that close branch should be used only in case we source from + # an actual Branch + close_branch = close_branch and source_ref.type == 'branch' + + # don't allow to close branch if source and target are the same + close_branch = close_branch and source_ref.name != target_ref.name + + needs_push_on_close = False + if close_branch and not use_rebase and not dry_run: try: - close_commit_id, needs_push = shadow_repo._local_close( + close_commit_id, needs_push_on_close = shadow_repo._local_close( target_ref, merger_name, merger_email, source_ref) - target_ref.commit_id = close_commit_id merge_possible = True except RepositoryError: - log.exception('Failure when doing close branch on hg shadow repo') + log.exception( + 'Failure when doing close branch on hg shadow repo') merge_possible = False merge_failure_reason = MergeFailureReason.MERGE_FAILED else: @@ -754,9 +765,13 @@ class MercurialRepository(BaseRepository source_ref, use_rebase=use_rebase) merge_possible = True - # Set a bookmark pointing to the merge commit. This bookmark may be - # used to easily identify the last successful merge commit in the - # shadow repository. + # read the state of the close action, if it + # maybe required a push + needs_push = needs_push or needs_push_on_close + + # Set a bookmark pointing to the merge commit. This bookmark + # may be used to easily identify the last successful merge + # commit in the shadow repository. shadow_repo.bookmark('pr-merge', revision=merge_commit_id) merge_ref = Reference('book', 'pr-merge', merge_commit_id) except SubrepoMergeError: @@ -776,7 +791,6 @@ class MercurialRepository(BaseRepository if target_ref.type == 'book': shadow_repo.bookmark( target_ref.name, revision=merge_commit_id) - try: shadow_repo_with_hooks = self._get_shadow_instance( shadow_repository_path, @@ -788,6 +802,12 @@ class MercurialRepository(BaseRepository shadow_repo_with_hooks._local_push( merge_commit_id, self.path, push_branches=True, enable_hooks=True) + + # maybe we also need to push the close_commit_id + if close_commit_id: + shadow_repo_with_hooks._local_push( + close_commit_id, self.path, push_branches=True, + enable_hooks=True) merge_succeeded = True except RepositoryError: log.exception( diff --git a/rhodecode/model/forms.py b/rhodecode/model/forms.py --- a/rhodecode/model/forms.py +++ b/rhodecode/model/forms.py @@ -394,6 +394,8 @@ class _BaseVcsSettingsForm(formencode.Sc # git vcs_git_lfs_enabled = v.StringBoolean(if_missing=False) + rhodecode_git_use_rebase_for_merging = v.StringBoolean(if_missing=False) + rhodecode_git_close_branch_before_merging = v.StringBoolean(if_missing=False) # svn vcs_svn_proxy_http_requests_enabled = v.StringBoolean(if_missing=False) diff --git a/rhodecode/model/pull_request.py b/rhodecode/model/pull_request.py --- a/rhodecode/model/pull_request.py +++ b/rhodecode/model/pull_request.py @@ -1424,12 +1424,26 @@ class PullRequestModel(BaseModel): pull_request, 'rhodecode_pr_merge_enabled') def _use_rebase_for_merging(self, pull_request): - return self._get_general_setting( - pull_request, 'rhodecode_hg_use_rebase_for_merging') + repo_type = pull_request.target_repo.repo_type + if repo_type == 'hg': + return self._get_general_setting( + pull_request, 'rhodecode_hg_use_rebase_for_merging') + elif repo_type == 'git': + return self._get_general_setting( + pull_request, 'rhodecode_git_use_rebase_for_merging') + + return False def _close_branch_before_merging(self, pull_request): - return self._get_general_setting( - pull_request, 'rhodecode_hg_close_branch_before_merging') + repo_type = pull_request.target_repo.repo_type + if repo_type == 'hg': + return self._get_general_setting( + pull_request, 'rhodecode_hg_close_branch_before_merging') + elif repo_type == 'git': + return self._get_general_setting( + pull_request, 'rhodecode_git_close_branch_before_merging') + + return False def _get_general_setting(self, pull_request, settings_key, default=False): settings_model = VcsSettingsModel(repo=pull_request.target_repo) diff --git a/rhodecode/model/settings.py b/rhodecode/model/settings.py --- a/rhodecode/model/settings.py +++ b/rhodecode/model/settings.py @@ -409,7 +409,9 @@ class VcsSettingsModel(object): 'use_outdated_comments', 'pr_merge_enabled', 'hg_use_rebase_for_merging', - 'hg_close_branch_before_merging') + 'hg_close_branch_before_merging', + 'git_use_rebase_for_merging', + 'git_close_branch_before_merging') HOOKS_SETTINGS = ( ('hooks', 'changegroup.repo_size'), diff --git a/rhodecode/templates/base/vcs_settings.mako b/rhodecode/templates/base/vcs_settings.mako --- a/rhodecode/templates/base/vcs_settings.mako +++ b/rhodecode/templates/base/vcs_settings.mako @@ -145,34 +145,6 @@ - ## LABS for HG - % if c.labs_active: -
-
-

${_('Mercurial Labs Settings')} (${_('These features are considered experimental and may not work as expected.')})

-
-
- -
- ${h.checkbox('rhodecode_hg_use_rebase_for_merging' + suffix, 'True', **kwargs)} - -
-
- ${_('Use rebase instead of creating a merge commit when merging via web interface.')} -
- -
- ${h.checkbox('rhodecode_hg_close_branch_before_merging' + suffix, 'True', **kwargs)} - -
-
- ${_('Close branch before merging it into destination branch. No effect when rebase strategy is use.')} -
- -
-
- % endif - % endif % if display_globals or repo_type in ['git']: @@ -340,4 +312,59 @@ % endif + % if display_globals or repo_type in ['hg',]: +
+
+

${_('Mercurial Pull Request Settings')}

+
+
+ ## Specific HG settings +
+ ${h.checkbox('rhodecode_hg_use_rebase_for_merging' + suffix, 'True', **kwargs)} + +
+
+ ${_('Use rebase instead of creating a merge commit when merging via web interface.')} +
+ +
+ ${h.checkbox('rhodecode_hg_close_branch_before_merging' + suffix, 'True', **kwargs)} + +
+
+ ${_('Close branch before merging it into destination branch. No effect when rebase strategy is use.')} +
+ + +
+
+ % endif + +## DISABLED FOR GIT FOR NOW as the rebase/close is not supported yet +## % if display_globals or repo_type in ['git']: +##
+##
+##

${_('Git Pull Request Settings')}

+##
+##
+##
+## ${h.checkbox('rhodecode_git_use_rebase_for_merging' + suffix, 'True', **kwargs)} +## +##
+##
+## ${_('Use rebase instead of creating a merge commit when merging via web interface.')} +##
+## +##
+## ${h.checkbox('rhodecode_git_close_branch_before_merging' + suffix, 'True', **kwargs)} +## +##
+##
+## ${_('Delete branch after merging it into destination branch. No effect when rebase strategy is use.')} +##
+##
+##
+## % endif + + diff --git a/rhodecode/tests/functional/test_admin_settings.py b/rhodecode/tests/functional/test_admin_settings.py --- a/rhodecode/tests/functional/test_admin_settings.py +++ b/rhodecode/tests/functional/test_admin_settings.py @@ -324,18 +324,6 @@ class TestAdminSettingsVcs(object): setting = SettingsModel().get_setting_by_name(setting_key) assert setting.app_settings_value is new_value - def test_has_a_section_for_labs_settings_if_enabled(self, app): - with mock.patch.dict( - rhodecode.CONFIG, {'labs_settings_active': 'true'}): - response = self.app.get(url('admin_settings_vcs')) - response.mustcontain('Labs Settings') - - def test_has_not_a_section_for_labs_settings_if_disables(self, app): - with mock.patch.dict( - rhodecode.CONFIG, {'labs_settings_active': 'false'}): - response = self.app.get(url('admin_settings_vcs')) - response.mustcontain(no='Labs Settings') - @pytest.mark.parametrize('new_value', [True, False]) def test_allows_to_change_hg_rebase_merge_strategy( self, app, form_defaults, csrf_token, new_value): diff --git a/rhodecode/tests/models/settings/test_vcs_settings.py b/rhodecode/tests/models/settings/test_vcs_settings.py --- a/rhodecode/tests/models/settings/test_vcs_settings.py +++ b/rhodecode/tests/models/settings/test_vcs_settings.py @@ -41,6 +41,9 @@ GENERAL_FORM_DATA = { 'rhodecode_pr_merge_enabled': True, 'rhodecode_use_outdated_comments': True, 'rhodecode_hg_use_rebase_for_merging': True, + 'rhodecode_hg_close_branch_before_merging': True, + 'rhodecode_git_use_rebase_for_merging': True, + 'rhodecode_git_close_branch_before_merging': True, } diff --git a/rhodecode/tests/models/test_pullrequest.py b/rhodecode/tests/models/test_pullrequest.py --- a/rhodecode/tests/models/test_pullrequest.py +++ b/rhodecode/tests/models/test_pullrequest.py @@ -164,7 +164,7 @@ class TestPullRequestModel(object): pull_request.target_ref_parts, pull_request.source_repo.scm_instance(), pull_request.source_ref_parts, self.workspace_id, dry_run=True, - use_rebase=False) + use_rebase=False, close_branch=False) assert pull_request._last_merge_source_rev == self.source_commit assert pull_request._last_merge_target_rev == self.target_commit @@ -193,7 +193,7 @@ class TestPullRequestModel(object): pull_request.target_ref_parts, pull_request.source_repo.scm_instance(), pull_request.source_ref_parts, self.workspace_id, dry_run=True, - use_rebase=False) + use_rebase=False, close_branch=False) assert pull_request._last_merge_source_rev == self.source_commit assert pull_request._last_merge_target_rev == self.target_commit @@ -225,7 +225,7 @@ class TestPullRequestModel(object): pull_request.target_ref_parts, pull_request.source_repo.scm_instance(), pull_request.source_ref_parts, self.workspace_id, dry_run=True, - use_rebase=False) + use_rebase=False, close_branch=False) assert pull_request._last_merge_source_rev is None assert pull_request._last_merge_target_rev is None @@ -299,7 +299,7 @@ class TestPullRequestModel(object): pull_request.source_repo.scm_instance(), pull_request.source_ref_parts, self.workspace_id, user_name=user.username, user_email=user.email, message=message, - use_rebase=False + use_rebase=False, close_branch=False ) self.invalidation_mock.assert_called_once_with( pull_request.target_repo.repo_name) @@ -338,7 +338,7 @@ class TestPullRequestModel(object): pull_request.source_repo.scm_instance(), pull_request.source_ref_parts, self.workspace_id, user_name=user.username, user_email=user.email, message=message, - use_rebase=False + use_rebase=False, close_branch=False ) pull_request = PullRequest.get(pull_request.pull_request_id)