From fa9922989eeb5dff1dbaef4657940d9720c383c4 Mon Sep 17 00:00:00 2001 From: Rainer Niedermayr Date: Wed, 29 Nov 2023 09:35:52 +0100 Subject: [PATCH 1/5] ci: ancestor image resolution: refactoring --- misc/python/materialize/docker.py | 110 ++++++++++++++++-------------- 1 file changed, 57 insertions(+), 53 deletions(-) diff --git a/misc/python/materialize/docker.py b/misc/python/materialize/docker.py index d6fc2477fc28b..79e6d6fe059c2 100644 --- a/misc/python/materialize/docker.py +++ b/misc/python/materialize/docker.py @@ -22,71 +22,75 @@ def resolve_ancestor_image_tag() -> str: def _resolve_ancestor_image_tag() -> tuple[str, str]: if buildkite.is_in_buildkite(): - if buildkite.is_in_pull_request(): - # return the merge base - common_ancestor_commit = buildkite.get_merge_base() - if _image_of_commit_exists(common_ancestor_commit): - return ( - _commit_to_image_tag(common_ancestor_commit), - "merge base of pull request", - ) - else: - return ( - _version_to_image_tag(get_latest_published_version()), - "latest release because image of merge base of pull request not available", - ) - elif git.is_on_release_version(): - # return the previous release - tagged_release_version = git.get_tagged_release_version( - version_type=MzVersion - ) - assert tagged_release_version is not None - previous_release_version = get_previous_published_version( - tagged_release_version - ) + return _resolve_ancestor_image_tag_when_in_buildkite() + else: + return _resolve_ancestor_image_tag_when_running_locally() + + +def _resolve_ancestor_image_tag_when_in_buildkite() -> tuple[str, str]: + if buildkite.is_in_pull_request(): + # return the merge base + common_ancestor_commit = buildkite.get_merge_base() + if _image_of_commit_exists(common_ancestor_commit): return ( - _version_to_image_tag(previous_release_version), - f"previous release because on release branch {tagged_release_version}", + _commit_to_image_tag(common_ancestor_commit), + "merge base of pull request", ) else: - # return the latest release return ( _version_to_image_tag(get_latest_published_version()), - "latest release because not in a pull request and not on a release branch", + "latest release because image of merge base of pull request not available", ) + elif git.is_on_release_version(): + # return the previous release + tagged_release_version = git.get_tagged_release_version(version_type=MzVersion) + assert tagged_release_version is not None + previous_release_version = get_previous_published_version( + tagged_release_version + ) + return ( + _version_to_image_tag(previous_release_version), + f"previous release because on release branch {tagged_release_version}", + ) else: - if git.is_on_release_version(): - # return the previous release - tagged_release_version = git.get_tagged_release_version( - version_type=MzVersion - ) - assert tagged_release_version is not None - previous_release_version = get_previous_published_version( - tagged_release_version - ) + # return the latest release + return ( + _version_to_image_tag(get_latest_published_version()), + "latest release because not in a pull request and not on a release branch", + ) + + +def _resolve_ancestor_image_tag_when_running_locally() -> tuple[str, str]: + if git.is_on_release_version(): + # return the previous release + tagged_release_version = git.get_tagged_release_version(version_type=MzVersion) + assert tagged_release_version is not None + previous_release_version = get_previous_published_version( + tagged_release_version + ) + return ( + _version_to_image_tag(previous_release_version), + f"previous release because on local release branch {tagged_release_version}", + ) + elif git.is_on_main_branch(): + # return the latest release + return ( + _version_to_image_tag(get_latest_published_version()), + "latest release because on local main branch", + ) + else: + # return the merge base + common_ancestor_commit = buildkite.get_merge_base() + if _image_of_commit_exists(common_ancestor_commit): return ( - _version_to_image_tag(previous_release_version), - f"previous release because on local release branch {tagged_release_version}", + _commit_to_image_tag(common_ancestor_commit), + "merge base of local non-main branch", ) - elif git.is_on_main_branch(): - # return the latest release + else: return ( _version_to_image_tag(get_latest_published_version()), - "latest release because on local main branch", + "latest release because image of merge base of local non-main branch not available", ) - else: - # return the merge base - common_ancestor_commit = buildkite.get_merge_base() - if _image_of_commit_exists(common_ancestor_commit): - return ( - _commit_to_image_tag(common_ancestor_commit), - "merge base of local non-main branch", - ) - else: - return ( - _version_to_image_tag(get_latest_published_version()), - "latest release because image of merge base of local non-main branch not available", - ) def get_latest_published_version() -> MzVersion: From 715d6fdf6c351f803bbf84040b626b024aead31d Mon Sep 17 00:00:00 2001 From: Rainer Niedermayr Date: Wed, 29 Nov 2023 11:10:08 +0100 Subject: [PATCH 2/5] spawn: add run_and_get_return_code --- misc/python/materialize/spawn.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/misc/python/materialize/spawn.py b/misc/python/materialize/spawn.py index 807dc28dec89a..5f1118938f5df 100644 --- a/misc/python/materialize/spawn.py +++ b/misc/python/materialize/spawn.py @@ -125,3 +125,17 @@ def capture( return subprocess.check_output( args, cwd=cwd, env=env, input=input, stdin=stdin, stderr=stderr, text=True ) + + +def run_and_get_return_code( + args: Sequence[Path | str], + *, + cwd: Path | None = None, + env: dict[str, str] | None = None, +) -> int: + """Run a subprocess and return the return code.""" + try: + capture(args, cwd=cwd, env=env, stderr=subprocess.DEVNULL) + return 0 + except CalledProcessError as e: + return e.returncode From 90621af9c6ac4b65a079745cc527b0dfcc46d705 Mon Sep 17 00:00:00 2001 From: Rainer Niedermayr Date: Wed, 29 Nov 2023 11:10:26 +0100 Subject: [PATCH 3/5] git: add contains_commit --- misc/python/materialize/git.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/misc/python/materialize/git.py b/misc/python/materialize/git.py index 4f65c9ec3899e..b1b9dd33036f8 100644 --- a/misc/python/materialize/git.py +++ b/misc/python/materialize/git.py @@ -278,6 +278,12 @@ def is_on_main_branch() -> bool: return get_branch_name() == "main" +def contains_commit(commit_sha: str, target: str = "HEAD") -> bool: + command = ["git", "merge-base", "--is-ancestor", commit_sha, target] + return_code = spawn.run_and_get_return_code(command) + return return_code == 0 + + def get_tagged_release_version(version_type: type[VERSION_TYPE]) -> VERSION_TYPE | None: """ This returns the release version if exactly this commit is tagged. From 034fe030f62e004b4fdc35eb37a4dca6d1dd3d93 Mon Sep 17 00:00:00 2001 From: Rainer Niedermayr Date: Wed, 29 Nov 2023 11:46:02 +0100 Subject: [PATCH 4/5] ci: provide mechanism to override ancestor release --- misc/python/materialize/docker.py | 60 +++++++++++++++++++++++++++++-- 1 file changed, 58 insertions(+), 2 deletions(-) diff --git a/misc/python/materialize/docker.py b/misc/python/materialize/docker.py index 79e6d6fe059c2..4321167902a1d 100644 --- a/misc/python/materialize/docker.py +++ b/misc/python/materialize/docker.py @@ -13,6 +13,15 @@ from materialize import buildkite, git from materialize.mz_version import MzVersion +""" +Git revisions that are based on commits listed as keys require at least the version specified in the value. +Note that specified versions do not necessarily need to be already published. +Commits must be ordered descending by their date. +""" +MIN_ANCESTOR_MZ_VERSION_PER_COMMIT: dict[str, MzVersion] = { + # insert newer commits at the top +} + def resolve_ancestor_image_tag() -> str: image_tag, context = _resolve_ancestor_image_tag() @@ -53,9 +62,21 @@ def _resolve_ancestor_image_tag_when_in_buildkite() -> tuple[str, str]: f"previous release because on release branch {tagged_release_version}", ) else: + latest_published_version = get_latest_published_version() + override_commit = _get_override_commit_instead_of_version( + latest_published_version + ) + + if override_commit is not None: + # use the commit instead of the latest release + return ( + _commit_to_image_tag(override_commit), + f"commit override instead of latest release ({latest_published_version})", + ) + # return the latest release return ( - _version_to_image_tag(get_latest_published_version()), + _version_to_image_tag(latest_published_version), "latest release because not in a pull request and not on a release branch", ) @@ -74,8 +95,20 @@ def _resolve_ancestor_image_tag_when_running_locally() -> tuple[str, str]: ) elif git.is_on_main_branch(): # return the latest release + latest_published_version = get_latest_published_version() + override_commit = _get_override_commit_instead_of_version( + latest_published_version + ) + + if override_commit is not None: + # use the commit instead of the latest release + return ( + _commit_to_image_tag(override_commit), + f"commit override instead of latest release ({latest_published_version})", + ) + return ( - _version_to_image_tag(get_latest_published_version()), + _version_to_image_tag(latest_published_version), "latest release because on local main branch", ) else: @@ -125,6 +158,29 @@ def get_previous_published_version(release_version: MzVersion) -> MzVersion: excluded_versions.add(previous_published_version) +def _get_override_commit_instead_of_version( + latest_published_version: MzVersion, +) -> str | None: + """ + If a commit specifies a mz version as prerequisite (to avoid regressions) that is newer than the + provided latest version (i.e., prerequisite not satisfied by the latest version), then return + that commit's hash if the commit contained in the current state. + Otherwise, return none. + """ + for ( + commit_hash, + min_required_mz_version, + ) in MIN_ANCESTOR_MZ_VERSION_PER_COMMIT.items(): + if latest_published_version >= min_required_mz_version: + continue + + if git.contains_commit(commit_hash): + # commit would require at least min_required_mz_version + return commit_hash + + return None + + def _image_of_release_version_exists(version: MzVersion) -> bool: return _mz_image_tag_exists(_version_to_image_tag(version)) From edf444df411a1756e90e94ff8c4f3e8e9fe6fb52 Mon Sep 17 00:00:00 2001 From: Rainer Niedermayr Date: Wed, 29 Nov 2023 11:47:45 +0100 Subject: [PATCH 5/5] ci: ancestor release override: specify 0.79.0 as minimal compare target for commits based on PR#23421 --- misc/python/materialize/docker.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/misc/python/materialize/docker.py b/misc/python/materialize/docker.py index 4321167902a1d..3794c9f33df3c 100644 --- a/misc/python/materialize/docker.py +++ b/misc/python/materialize/docker.py @@ -20,6 +20,8 @@ """ MIN_ANCESTOR_MZ_VERSION_PER_COMMIT: dict[str, MzVersion] = { # insert newer commits at the top + # PR#23421 (coord: smorgasbord of improvements for the crdb-backed timestamp oracle) introduces regressions against 0.78.13 + "5179ebd39aea4867622357a832aaddcde951b411": MzVersion.parse_mz("v0.79.0") }