From 33ebaea6003b115c391b175dbb2367154cbfc756 Mon Sep 17 00:00:00 2001 From: Michal Nowak Date: Tue, 5 Dec 2023 11:55:03 +0100 Subject: [PATCH 1/3] Replace danger-python with Hazard Hazard is a minimal danger-python replacement. (cherry picked from commit 08ce1bc45fbe00db5ac5ec77335b38c8ce7d1990) --- .gitlab-ci.yml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index f1cf9316e9..c2a7de026b 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -621,9 +621,15 @@ shfmt: danger: <<: *precheck_job + # Keep the GIT_DEPTH environment variable set to a "high number" before + # https://github.com/libgit2/libgit2/pull/6662 is addressed and integrated + # into pygit2. + variables: + GIT_DEPTH: 1000 needs: [] script: - - danger-python ci -f + - pip install git+https://gitlab.isc.org/isc-projects/hazard.git + - hazard only: refs: - merge_requests From a4fbb8edd09a30af2d5c6001b4e84e3d499f72e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Thu, 7 Dec 2023 13:23:22 +0100 Subject: [PATCH 2/3] Improve determining the lines added by a branch Since the list of lines added to Git-tracked text files in a given branch is not part of the Danger DSL [1], it is determined using custom code in dangerfile.py. The current implementation of that logic is less than perfect as it examines the diff between the current tip of the target branch and the source branch rather than the diff between the merge base of the two branches and the source branch. Consider a Git history like this: * F (target) ... * E * D * C | * B (source) |/ * A (merge base) If danger-python or Hazard are run for commit B, the current logic for determining the list of added lines in dangerfile.py examines the diff between commits F and B rather than between commits A and B. Therefore, the added_lines() function returns not just the lines added by commit B on top of commit A, but also the list of lines that were removed between commits A and F, which leads to confusing results. Fix by using the triple-dot diff operator in the Git invocation whose output is used as the source of information for determining the list of lines added by a given branch. Since Hazard fetches the target branch itself when it is run, remove the explicit "git fetch" invocation that fetches the target branch from GitLab (shortening its local history to a single commit in the process) before "git diff" is invoked. [1] https://danger.systems/js/reference.html#GitDSL (cherry picked from commit 43126e81e620deb63889d0812b2186af22da82bd) --- dangerfile.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/dangerfile.py b/dangerfile.py index ee77f13cd1..531e05bb18 100644 --- a/dangerfile.py +++ b/dangerfile.py @@ -23,11 +23,13 @@ import gitlab def added_lines(target_branch, paths): import subprocess - subprocess.check_output( - ["/usr/bin/git", "fetch", "--depth", "1", "origin", target_branch] - ) + # Hazard fetches the target branch itself, so there is no need to fetch it + # explicitly using `git fetch --depth 1000 origin `. The + # refs/remotes/origin/ ref is also expected to be readily + # usable by the time this file is executed. + diff = subprocess.check_output( - ["/usr/bin/git", "diff", "FETCH_HEAD..", "--"] + paths + ["/usr/bin/git", "diff", f"origin/{target_branch}...", "--"] + paths ) added_lines = [] for line in diff.splitlines(): From e97f4c078400f28395386ead8233a63e7fffc98b Mon Sep 17 00:00:00 2001 From: Tom Krizek Date: Fri, 8 Dec 2023 16:31:46 +0100 Subject: [PATCH 3/3] Handle empty MR description in dangerfile A merge request might have no description at all (i.e. None, rather than an empty string). This might happen when the MR is created via an API. Check a description is present before trying to find a backport string in it. (cherry picked from commit 4f70f5bd7cbd79e54d8c37a53267eae62a633c36) --- dangerfile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dangerfile.py b/dangerfile.py index 531e05bb18..2c1e7de14c 100644 --- a/dangerfile.py +++ b/dangerfile.py @@ -186,7 +186,6 @@ BACKPORT_OF_RE = re.compile( r"Backport\s+of.*(merge_requests/|!)([0-9]+)", flags=re.IGNORECASE ) VERSION_LABEL_RE = re.compile(r"v9.([0-9]+)(-S)?") -backport_desc = BACKPORT_OF_RE.search(danger.gitlab.mr.description) version_labels = [l for l in mr_labels if l.startswith("v9.")] affects_labels = [l for l in mr_labels if l.startswith("Affects v9.")] if is_backport: @@ -205,6 +204,7 @@ if is_backport: "Backport MRs must have their target version in the title. " f"Please put `[9.{minor_ver}{edition}]` at the start of the MR title." ) + backport_desc = BACKPORT_OF_RE.search(danger.gitlab.mr.description or "") if backport_desc is None: fail( "Backport MRs must link to the original MR. Please put "