Change merge mode default based on driver
The default merge mode is 'merge-resolve' because it has been observed that it more closely matches the behavior of jgit in Gerrit (or, at least it did the last time we looked into this). The other drivers are unlikely to use jgit and more likely to use the default git merge strategy. This change allows the default to differ based on the driver, and changes the default for all non-gerrit drivers to 'merge'. The implementation anticipates that we may want to add more granularity in the future, so the API accepts a project as an argument, and in the future, drivers could provide a per-project default (which they may obtain from the remote code review system). That is not implemented yet. This adds some extra data to the /projects endpoint in the REST api. It is currently not easy (and perhaps not possible) to determine what a project's merge mode is through the api. This change adds a metadata field to the output which will show the resulting value computed from all of the project stanzas. The project stanzas themselves may have null values for the merge modes now, so the web app now protects against that. Change-Id: I9ddb79988ca08aba4662cd82124bd91e49fd053c
This commit is contained in:
James E. Blair
parent
4bda3e1969
commit
e2a472bc97
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
@ -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
|
||||
|
|
@ -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')
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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 = (
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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."""
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
Loading…
Reference in New Issue