diff --git a/doc/source/config/project.rst b/doc/source/config/project.rst index 1aa570f41d..af3faa943b 100644 --- a/doc/source/config/project.rst +++ b/doc/source/config/project.rst @@ -105,12 +105,16 @@ pipeline. not appear in a :ref:`project-template` definition. .. attr:: merge-mode - :default: merge-resolve + :default: (driver specific) The merge mode which is used by Git for this project. Be sure this matches what the remote system which performs merges (i.e., - Gerrit). The requested merge mode will be used by the Github driver - when performing merges. + Gerrit). The requested merge mode will also be used by the + GitHub and GitLab drivers when performing merges. + + The default is :value:`project.merge-mode.merge` for all drivers + except Gerrit, where the default is + :value:`project.merge-mode.merge-resolve`. Each project may only have one ``merge-mode`` therefore Zuul will use the first value that it encounters for a given project @@ -122,24 +126,24 @@ pipeline. .. value:: merge Uses the default git merge strategy (recursive). This maps to - the merge mode ``merge`` in Github and Gitlab. + the merge mode ``merge`` in GitHub and GitLab. .. value:: merge-resolve Uses the resolve git merge strategy. This is a very conservative merge strategy which most closely matches the behavior of Gerrit. This maps to the merge mode ``merge`` in - Github and Gitlab. + GitHub and GitLab. .. value:: cherry-pick Cherry-picks each change onto the branch rather than - performing any merges. This is not supported by Github and Gitlab. + performing any merges. This is not supported by Github and GitLab. .. value:: squash-merge Squash merges each change onto the branch. This maps to the - merge mode ``squash`` in Github and Gitlab. + merge mode ``squash`` in GitHub and GitLab. .. attr:: vars :default: None diff --git a/releasenotes/notes/driver-merge-mode-9e8ef35006b3ae6d.yaml b/releasenotes/notes/driver-merge-mode-9e8ef35006b3ae6d.yaml new file mode 100644 index 0000000000..ee4a62a1e1 --- /dev/null +++ b/releasenotes/notes/driver-merge-mode-9e8ef35006b3ae6d.yaml @@ -0,0 +1,10 @@ +--- +upgrade: + - | + The default merge mode used by Zuul for preparing git repos was + previously :value:`project.merge-mode.merge-resolve` in all cases, + but is now :value:`project.merge-mode.merge` for all drivers except + Gerrit, where :value:`project.merge-mode.merge-resolve` is still + the default. This makes the merge operations performed by Zuul + more closely match the operations that will be performed by the + code review systems. diff --git a/tests/fixtures/layouts/merge-mode-default.yaml b/tests/fixtures/layouts/merge-mode-default.yaml new file mode 100644 index 0000000000..ff12638b67 --- /dev/null +++ b/tests/fixtures/layouts/merge-mode-default.yaml @@ -0,0 +1,41 @@ +- pipeline: + name: check + manager: independent + trigger: + gerrit: + - event: patchset-created + success: + gerrit: + Verified: 1 + failure: + gerrit: + Verified: -1 + +- project: + name: '^org/regex-empty.*' + +- project: + name: '^org/regex-cherry.*' + merge-mode: cherry-pick + +- project: + # This should be the driver default + name: org/project-empty + +- project: + # This should be the driver default + name: org/regex-empty-project-empty + +- project: + # This should be squash because the regex doesn't specify + name: org/regex-empty-project-squash + merge-mode: squash-merge + +- project: + # This should be cherry-pick because of the regex + name: org/regex-cherry-project-empty + +- project: + # This should be squash because it is more specific than the regex + name: org/regex-cherry-project-squash + merge-mode: squash-merge diff --git a/tests/unit/test_configloader.py b/tests/unit/test_configloader.py index 571df75500..5b6c46ccf3 100644 --- a/tests/unit/test_configloader.py +++ b/tests/unit/test_configloader.py @@ -27,7 +27,7 @@ from zuul.configloader import ( from zuul.model import Abide, MergeRequest, SourceContext from zuul.zk.locks import tenant_read_lock -from tests.base import iterate_timeout, ZuulTestCase +from tests.base import iterate_timeout, ZuulTestCase, simple_layout class TestConfigLoader(ZuulTestCase): @@ -1211,3 +1211,35 @@ class TestTenantDuplicate(TenantParserTestCase): def test_tenant_dupe(self): # The magic is in setUp pass + + +class TestMergeMode(ZuulTestCase): + config_file = 'zuul-connections-gerrit-and-github.conf' + + def _test_default_merge_mode(self, driver_default, host): + layout = self.scheds.first.sched.abide.tenants.get('tenant-one').layout + md = layout.getProjectMetadata( + f'{host}/org/project-empty') + self.assertEqual(driver_default, md.merge_mode) + md = layout.getProjectMetadata( + f'{host}/org/regex-empty-project-empty') + self.assertEqual(driver_default, md.merge_mode) + md = layout.getProjectMetadata( + f'{host}/org/regex-empty-project-squash') + self.assertEqual(model.MERGER_SQUASH_MERGE, md.merge_mode) + md = layout.getProjectMetadata( + f'{host}/org/regex-cherry-project-empty') + self.assertEqual(model.MERGER_CHERRY_PICK, md.merge_mode) + md = layout.getProjectMetadata( + f'{host}/org/regex-cherry-project-squash') + self.assertEqual(model.MERGER_SQUASH_MERGE, md.merge_mode) + + @simple_layout('layouts/merge-mode-default.yaml') + def test_default_merge_mode_gerrit(self): + self._test_default_merge_mode(model.MERGER_MERGE_RESOLVE, + 'review.example.com') + + @simple_layout('layouts/merge-mode-default.yaml', driver='github') + def test_default_merge_mode_github(self): + self._test_default_merge_mode(model.MERGER_MERGE, + 'github.com') diff --git a/tests/unit/test_web.py b/tests/unit/test_web.py index d8901fabb8..4443937589 100644 --- a/tests/unit/test_web.py +++ b/tests/unit/test_web.py @@ -844,6 +844,11 @@ class TestWeb(BaseTestWeb): 'canonical_name': 'review.example.com/org/project1', 'connection_name': 'gerrit', 'name': 'org/project1', + 'metadata': { + 'default_branch': 'master', + 'merge_mode': 'merge-resolve', + 'queue_name': 'integrated', + }, 'configs': [{ 'source_context': {'branch': 'master', 'path': 'zuul.yaml', @@ -851,7 +856,7 @@ class TestWeb(BaseTestWeb): 'templates': [], 'default_branch': 'master', 'queue_name': 'integrated', - 'merge_mode': 'merge-resolve', + 'merge_mode': None, 'pipelines': [{ 'name': 'check', 'jobs': jobs, diff --git a/web/src/containers/project/ProjectVariant.jsx b/web/src/containers/project/ProjectVariant.jsx index 51a093a327..36d2c09ce5 100644 --- a/web/src/containers/project/ProjectVariant.jsx +++ b/web/src/containers/project/ProjectVariant.jsx @@ -22,7 +22,9 @@ function ProjectVariant(props) { const { tenant, variant } = props const rows = [] - rows.push({label: 'Merge mode', value: variant.merge_mode}) + if (variant.merge_mode) { + rows.push({label: 'Merge mode', value: variant.merge_mode}) + } if (variant.templates.length > 0) { const templateList = ( diff --git a/zuul/configloader.py b/zuul/configloader.py index 2b696a8325..ae05779ed4 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -1178,8 +1178,9 @@ class ProjectParser(object): if name not in project_config.templates: project_config.templates.append(name) - mode = conf.get('merge-mode', 'merge-resolve') - project_config.merge_mode = model.MERGER_MAP[mode] + mode = conf.get('merge-mode') + if mode is not None: + project_config.merge_mode = model.MERGER_MAP[mode] default_branch = conf.get('default-branch', 'master') project_config.default_branch = default_branch @@ -2481,9 +2482,9 @@ class TenantParser(object): # Now that all the project pipelines are loaded, fixup and # verify references to other config objects. - self._validateProjectPipelineConfigs(layout) + self._validateProjectPipelineConfigs(tenant, layout) - def _validateProjectPipelineConfigs(self, layout): + def _validateProjectPipelineConfigs(self, tenant, layout): # Validate references to other config objects def inner_validate_ppcs(ppc): for jobs in ppc.job_list.jobs.values(): @@ -2511,6 +2512,14 @@ class TenantParser(object): inner_validate_ppcs(ppc) for ppc in project_config.pipelines.values(): inner_validate_ppcs(ppc) + # Set a merge mode if we don't have one for this project. + # This can happen if there are only regex project stanzas + # but no specific project stanzas. + project_metadata = layout.getProjectMetadata(project_name) + if project_metadata.merge_mode is None: + (trusted, project) = tenant.getProject(project_name) + mode = project.source.getProjectDefaultMergeMode(project) + project_metadata.merge_mode = model.MERGER_MAP[mode] def _parseLayout(self, tenant, data, loading_errors, layout_uuid=None): # Don't call this method from dynamic reconfiguration because diff --git a/zuul/driver/gerrit/gerritsource.py b/zuul/driver/gerrit/gerritsource.py index f1d74037e1..96c0816b5e 100644 --- a/zuul/driver/gerrit/gerritsource.py +++ b/zuul/driver/gerrit/gerritsource.py @@ -186,6 +186,12 @@ class GerritSource(BaseSource): def getProjectOpenChanges(self, project): return self.connection.getProjectOpenChanges(project) + def getProjectDefaultMergeMode(self, project): + # The gerrit jgit merge operation is most closely approximated + # by "git merge -s resolve", so we return that as the default + # for the Gerrit driver. + return 'merge-resolve' + def getProjectBranches(self, project, tenant, min_ltime=-1): return self.connection.getProjectBranches(project, tenant, min_ltime) diff --git a/zuul/source/__init__.py b/zuul/source/__init__.py index 5ccfa07b6d..faf97b48b5 100644 --- a/zuul/source/__init__.py +++ b/zuul/source/__init__.py @@ -144,6 +144,16 @@ class BaseSource(object, metaclass=abc.ABCMeta): def getProjectOpenChanges(self, project): """Get the open changes for a project.""" + def getProjectDefaultMergeMode(self, project): + """Return the default merge mode for this project. + + If users do not specify the merge mode for a project, this + mode will be used. It may be a driver-specific default, + or the driver may use data from the remote system to provide + a project-specific default. + """ + return 'merge' + @abc.abstractmethod def getGitUrl(self, project): """Get the git url for a project.""" diff --git a/zuul/web/__init__.py b/zuul/web/__init__.py index 09706057b0..3145e2c14e 100755 --- a/zuul/web/__init__.py +++ b/zuul/web/__init__.py @@ -46,6 +46,7 @@ from zuul.lib.jsonutil import ZuulJSONEncoder from zuul.lib.keystorage import KeyStorage from zuul.lib.monitoring import MonitoringServer from zuul.lib.re2util import filter_allowed_disallowed +from zuul import model from zuul.model import ( Abide, BuildSet, @@ -1214,6 +1215,12 @@ class ZuulWebAPI(object): result = project.toDict() result['configs'] = [] + md = tenant.layout.getProjectMetadata(project.canonical_name).toDict() + for k, v in model.MERGER_MAP.items(): + if v == md['merge_mode']: + md['merge_mode'] = k + break + result['metadata'] = md configs = tenant.layout.getAllProjectConfigs(project.canonical_name) for config_obj in configs: config = config_obj.toDict()