Keep upstream files on false rename/delete; add no-vendor go modules hook

README.md

@@ -98,6 +98,8 @@ It's useful only with Golang repositories, which require a `vendor` folder with
 
 *Note: Internally this is implemented using lifecycle hook script and is equivalent to passing `--post-rebase-hook _BUILTIN_/update_go_modules.sh` parameter.*
 
+For repositories without a `vendor/` tree, use `--post-rebase-hook _BUILTIN_/update_go_modules_no_vendor.sh` instead (tidy/sync only, no vendoring).
+
 ### Slack Webhook
 
 If you want to be notified in Slack about the status of recent rebases, you can set ``--slack-webhook` option. The value here is the path to a local file with the webhook url.

rebasebot/bot.py

@@ -61,6 +61,10 @@ class PullRequestUpdateException(Exception):
 _MERGE_COMMIT_PARENT_COUNT = 2
 _LOST_LINE_LOG_LIMIT = 10
 _GO_MODULES_CARRY_COMMIT_MESSAGE = "UPSTREAM: <carry>: Updating and vendoring go modules after an upstream rebase"
+# Raise rename similarity on cherry-picks so weak vendor/header matches are less
+# likely; keep-versus-delete still uses the picked commit's real path list.
+_CHERRY_PICK_FIND_RENAMES = "-Xfind-renames=70"
+_CHERRY_PICK_STRATEGY_OPTIONS = ("-Xtheirs", _CHERRY_PICK_FIND_RENAMES)
 
 
 @dataclass(frozen=True)
@@ -364,8 +368,9 @@ def _detect_conflicting_files(gitwd: git.Repo, sha: str) -> set:
     """
     Probe a cherry-pick without -Xtheirs to detect which files conflict.
 
-    Attempts the cherry-pick with --no-commit (no merge strategy), records
-    any unmerged files, then resets to the original state.
+    Attempts the cherry-pick with --no-commit (no "theirs" strategy) using the
+    same find-renames threshold as the real apply, records any unmerged files,
+    then resets to the original state.
 
     Returns a set of filenames that had merge conflicts, or an empty set
     if the cherry-pick would apply cleanly.
@@ -374,7 +379,7 @@ def _detect_conflicting_files(gitwd: git.Repo, sha: str) -> set:
     conflicted = set()
 
     try:
-        gitwd.git.cherry_pick(sha, "--no-commit")
+        gitwd.git.cherry_pick(sha, "--no-commit", _CHERRY_PICK_FIND_RENAMES)
     except git.GitCommandError:
         # Conflicts exist — record which files are unmerged
         try:
@@ -396,6 +401,20 @@ def _detect_conflicting_files(gitwd: git.Repo, sha: str) -> set:
     return conflicted
 
 
+def _unescape_git_path(path: str) -> str:
+    """Decode a git C-quoted path from --name-only / status --porcelain output."""
+    if path.startswith('"') and path.endswith('"'):
+        path = path[1:-1]
+        path = path.encode("ascii").decode("unicode_escape").encode("latin1").decode(git.compat.defenc)
+    return path
+
+
+def _picked_commit_paths(gitwd: git.Repo, sha: str) -> set[str]:
+    """Return paths touched by sha with rename detection disabled."""
+    output = gitwd.git.diff_tree("--no-renames", "--no-commit-id", "--name-only", "-r", sha)
+    return {_unescape_git_path(line) for line in output.splitlines() if line}
+
+
 def _check_upstream_content_loss(gitwd: git.Repo, source_branch: str, only_files: set | None = None) -> list:
     """
     After a cherry-pick with -Xtheirs, check whether any upstream content
@@ -455,11 +474,11 @@ def _safe_cherry_pick(
     if conflict_policy != "auto":
         conflicted_files = _detect_conflicting_files(gitwd, sha)
 
-    # Phase 2: actual cherry-pick with -Xtheirs
+    # Phase 2: actual cherry-pick with -Xtheirs and find-renames threshold
     try:
-        gitwd.git.cherry_pick(f"{sha}", "-Xtheirs")
+        gitwd.git.cherry_pick(f"{sha}", *_CHERRY_PICK_STRATEGY_OPTIONS)
     except git.GitCommandError as ex:
-        if not _resolve_rebase_conflicts(gitwd):
+        if not _resolve_rebase_conflicts(gitwd, sha):
             raise RepoException(f"Git rebase failed: {ex}") from ex
 
     created_commit = gitwd.head.commit.hexsha != start_head
@@ -654,7 +673,7 @@ def _prepare_rebase_branch(gitwd: git.Repo, source: GitHubBranch, dest: GitHubBr
     gitwd.git.checkout("-b", "rebase", commit)
 
 
-def _resolve_conflict(gitwd: git.Repo) -> bool:
+def _resolve_conflict(gitwd: git.Repo, sha: str) -> bool:
     status = gitwd.git.status(porcelain=True)
 
     if not status:
@@ -663,7 +682,8 @@ def _resolve_conflict(gitwd: git.Repo) -> bool:
         return True
 
     # Conflict prefixes in porcelain mode that we can fix.
-    # In all next cases we delete the conflicting files.
+    # Delete-shaped conflicts: remove only if the path is in the picked commit's
+    # real (no-rename) path list; otherwise keep HEAD's version.
     # UD - Modified/Deleted
     # DU - Deleted/Modified
     # AU - Renamed/Deleted
@@ -674,25 +694,43 @@ def _resolve_conflict(gitwd: git.Repo) -> bool:
     # Non-conflict status prefixes that we should ignore
     allowed_status_prefixes = ["M  ", "D  ", "A  ", "R  ", "C  "]
 
+    picked_paths = _picked_commit_paths(gitwd, sha)
     unresolvable = False
     files_to_delete = []
+    files_to_keep = []
     for line in status.splitlines():
         logging.info("Resolving conflict: %s", line)
         file_status = line[:3]
         if file_status in allowed_status_prefixes:
-            # There is a conflict we can't resolve
+            # Already staged non-conflict change — leave alone
             continue
         if file_status not in allowed_conflict_prefixes:
             # There is a conflict we can't resolve
             logging.info("Unresolvable conflict: %s", line)
             unresolvable = True
-        filename = line[3:].rstrip("\n")
-        # Special characters are escaped
-        if filename[0] == filename[-1] == '"':
-            filename = filename[1:-1]
-            filename = filename.encode("ascii").decode("unicode_escape").encode("latin1").decode(git.compat.defenc)
-        files_to_delete.append(filename)
-        logging.info("Deleting conflicting file: %s", filename)
+            continue
+        filename = _unescape_git_path(line[3:].rstrip("\n"))
+        if filename in picked_paths:
+            files_to_delete.append(filename)
+        else:
+            files_to_keep.append(filename)
+
+    if files_to_keep:
+        logging.info(
+            "Keeping paths not in picked commit %s (false rename/delete): %s",
+            sha,
+            ", ".join(files_to_keep),
+        )
+    if files_to_delete:
+        logging.info(
+            "Deleting paths present in picked commit %s: %s",
+            sha,
+            ", ".join(files_to_delete),
+        )
+
+    for keep_file in files_to_keep:
+        gitwd.git.checkout("HEAD", "--", keep_file)
+        gitwd.git.add(keep_file)
 
     for ud_file in files_to_delete:
         gitwd.git.rm(ud_file)
@@ -703,21 +741,29 @@ def _resolve_conflict(gitwd: git.Repo) -> bool:
         logging.error("Unresolvable conflict. Aborting rebase.")
         return False
 
+    # If resolution left the index identical to HEAD, skip rather than
+    # creating an empty commit (common when vendor deletes are already applied
+    # and only false rename/delete conflicts remained).
+    if not gitwd.git.diff("HEAD") and not gitwd.git.diff("--cached"):
+        logging.info("Conflict resolution left no changes versus HEAD; skipping pick %s", sha)
+        gitwd.git.cherry_pick("--skip")
+        return True
+
     gitwd.git.commit("--no-edit")
 
     return True
 
 
-def _resolve_rebase_conflicts(gitwd: git.Repo) -> bool:
+def _resolve_rebase_conflicts(gitwd: git.Repo, sha: str) -> bool:
     try:
-        if not _resolve_conflict(gitwd):
+        if not _resolve_conflict(gitwd, sha):
             return False
 
         logging.info("Conflict has been resolved. Continue rebase.")
 
         return True
     except git.GitCommandError:
-        return _resolve_rebase_conflicts(gitwd)
+        return _resolve_rebase_conflicts(gitwd, sha)
 
 
 def _cherrypick_art_pull_request(

rebasebot/builtin-hooks/update_go_modules_no_vendor.sh

@@ -0,0 +1,96 @@
+#!/bin/bash
+
+set -e  # Exit immediately if a command exits with a non-zero status
+set -o pipefail  # Return the exit status of the last command in the pipe that failed
+
+stage_and_commit(){
+    # If committer email and name is passed as environment variable then use it.
+    if [[ -z "$REBASEBOT_GIT_USERNAME" || -z "$REBASEBOT_GIT_EMAIL" ]]; then
+        author_flag=()
+    else
+        author_flag=(--author="$REBASEBOT_GIT_USERNAME <$REBASEBOT_GIT_EMAIL>")
+    fi
+
+    if [[ -n $(git status --porcelain) ]]; then
+        git add -A
+        git commit "${author_flag[@]}" -q -m "UPSTREAM: <drop>: Updating go modules after an upstream rebase"
+    fi
+}
+
+reset_go_mod_files() {
+    while IFS= read -r -d '' go_mod_file; do
+        local module_base_path
+        module_base_path=$(dirname "$go_mod_file")
+
+        # Reset go.mod and go.sum to make sure they are the same as in the source
+        for filename in "go.mod" "go.sum"; do
+            local full_path="$module_base_path/$filename"
+            if [[ ! -f "$full_path" ]]; then
+                continue
+            fi
+            if ! git checkout "source/$REBASEBOT_SOURCE" -- "$full_path"; then
+                echo "go module at $module_base_path is downstream only, skip its resetting"
+                break
+            fi
+        done
+    done < <(find . -name 'go.mod' -print0)
+}
+
+process_go_workspace_updates() {
+    echo "Performing go workspace modules update"
+
+    for filename in "go.work" "go.work.sum"; do
+        if [[ ! -f "$filename" ]]; then
+            continue
+        fi
+        if ! git checkout "source/$REBASEBOT_SOURCE" -- "$filename"; then
+            echo "go.work is downstream only, which is not supported" >&2
+            exit 1
+        fi
+    done
+
+    reset_go_mod_files
+
+    echo "Running go work sync"
+    if ! go work sync; then
+        echo "Unable to run 'go work sync'" >&2
+        exit 1
+    fi
+
+    stage_and_commit
+}
+
+process_go_mod_updates() {
+    echo "Performing go modules update"
+
+    reset_go_mod_files
+
+    while IFS= read -r -d '' go_mod_file; do
+        local module_base_path
+        module_base_path=$(dirname "$go_mod_file")
+
+        pushd "$module_base_path" > /dev/null || { echo "Failed to cd to $module_base_path" >&2; exit 1; }
+
+        echo "Running go mod tidy for $module_base_path"
+        if ! go mod tidy; then
+            echo "Unable to run 'go mod tidy' in $module_base_path" >&2
+            exit 1
+        fi
+
+        popd > /dev/null
+    done < <(find . -name 'go.mod' -print0)
+
+    stage_and_commit
+}
+
+# Check if the source branch environment variable is set
+if [[ -z "$REBASEBOT_SOURCE" ]]; then
+    echo "The environment variable REBASEBOT_SOURCE is not set." >&2
+    exit 1
+fi
+
+if [[ -f "go.work" ]]; then
+    process_go_workspace_updates
+else
+    process_go_mod_updates
+fi

tests/test_bot.py

@@ -140,6 +140,90 @@ def test_update_and_commit_empty(self, tmp_go_app_repo, monkeypatch):
         assert commits[0].message == "tidy and vendor go stuff\n"
 
 
+class TestGoModNoVendor:
+    _COMMIT_MESSAGE = "UPSTREAM: <drop>: Updating go modules after an upstream rebase\n"
+
+    def _args_stub(_, repo_dir, source) -> MagicMock:
+        args = MagicMock()
+        args.source = source
+        args.dest = GitHubBranch(repo_dir, "example", "foo", "dest")
+        args.rebase = GitHubBranch(repo_dir, "example", "foo", "rebase")
+        args.working_dir = repo_dir
+        args.git_username = "unittest"
+        args.git_email = "unit@test.org"
+        return args
+
+    def test_update_and_commit_without_vendor(self, tmp_go_app_repo, monkeypatch):
+        repo_dir, repo = tmp_go_app_repo
+
+        monkeypatch.chdir(repo_dir)
+        os.system("go mod init example.com/foo")
+        repo.git.add(all=True)
+        repo.git.commit("-m", "Init go module")
+
+        source = GitHubBranch(repo_dir, "example", "foo", repo.active_branch.name)
+        repo.create_remote("source", source.url)
+        repo.remotes.source.fetch(source.branch)
+
+        lifecycle_hooks._setup_environment_variables(self._args_stub(repo_dir, source))
+        script = lifecycle_hooks.LifecycleHookScript("_BUILTIN_/update_go_modules_no_vendor.sh")
+        result = script()
+
+        assert result.return_code == 0
+        commits = list(repo.iter_commits())
+        assert len(commits) == 3
+        assert commits[0].message == self._COMMIT_MESSAGE
+        assert not os.path.isdir(os.path.join(repo_dir, "vendor"))
+
+    def test_update_and_commit_go_workspace_without_vendor(self, tmp_go_app_repo, monkeypatch):
+        repo_dir, repo = tmp_go_app_repo
+
+        monkeypatch.chdir(repo_dir)
+        os.system("go mod init example.com/foo")
+        os.system("go work init .")
+        repo.git.add(all=True)
+        repo.git.commit("-m", "Init go workspace")
+
+        source = GitHubBranch(repo_dir, "example", "foo", repo.active_branch.name)
+        repo.create_remote("source", source.url)
+        repo.remotes.source.fetch(source.branch)
+
+        lifecycle_hooks._setup_environment_variables(self._args_stub(repo_dir, source))
+        script = lifecycle_hooks.LifecycleHookScript("_BUILTIN_/update_go_modules_no_vendor.sh")
+        result = script()
+
+        assert result.return_code == 0
+        assert not os.path.isdir(os.path.join(repo_dir, "vendor"))
+        commits = list(repo.iter_commits())
+        # go work sync may or may not dirty the tree; never vendor.
+        if len(commits) == 3:
+            assert commits[0].message == self._COMMIT_MESSAGE
+        else:
+            assert len(commits) == 2
+            assert commits[0].message == "Init go workspace\n"
+
+    def test_update_fails_on_broken_go_mod(self, tmp_go_app_repo, monkeypatch):
+        repo_dir, repo = tmp_go_app_repo
+
+        monkeypatch.chdir(repo_dir)
+        os.system("go mod init example.com/foo")
+        with open(os.path.join(repo_dir, "go.mod"), "w") as f:
+            f.write("this is not a valid go.mod\n")
+        repo.git.add(all=True)
+        repo.git.commit("-m", "Init broken go module")
+
+        source = GitHubBranch(repo_dir, "example", "foo", repo.active_branch.name)
+        repo.create_remote("source", source.url)
+        repo.remotes.source.fetch(source.branch)
+
+        lifecycle_hooks._setup_environment_variables(self._args_stub(repo_dir, source))
+        script = lifecycle_hooks.LifecycleHookScript("_BUILTIN_/update_go_modules_no_vendor.sh")
+        result = script()
+
+        assert result.return_code != 0
+        assert not os.path.isdir(os.path.join(repo_dir, "vendor"))
+
+
 class TestCommitMessageTags:
     @pytest.mark.parametrize(
         "pr_is_merged,commit_message,tag_policy,expected",