Skip to content

Explicit Symlink Exceptions for the Vendored Maintainer Merge Tool #2175

Description

@da2ce7

Goal

Allow the vendored maintainer merge tool to accept symbolic links that a repository has declared explicitly, in an auditable file that names each accepted link, its exact target, and the reason it exists, at a tree path the invocation names rather than one the tool knows, read from the merged tree the tool has just produced rather than from the maintainer's working directory, and applied to the links in every commit the merge brings in rather than to the final tree alone, while every undeclared or mismatched link keeps failing exactly as it does today.

Background

contrib/dev-tools/git/github-merge.py refuses to continue when the merged tree contains any symbolic link: get_symlink_files() (lines 151-157) lists every git ls-tree --full-tree -r HEAD entry whose mode satisfies (mode & 0o170000) == 0o120000, and its caller (lines 393-397) prints ERROR: File '<path>' was a symlink per entry and calls sys.exit(4). The check runs after the unsigned local merge commit exists and before tree_sha512sum() takes the tree hash, so a repository containing a symbolic link cannot reach signing at all.

The check is sound: a symbolic link in a merged tree can make a reviewed path resolve somewhere else, and refusing the whole class keeps that out of the merge path without asking the maintainer to judge each case. What it lacks is any way to admit a link a repository has chosen deliberately. The refusal is unconditional, so on such a repository every merge stops, and the only workaround today is deleting the link.

The first repository this blocks is the sibling torrust/torrust-index. Its tree carries exactly one symbolic link, .dockerignore, whose literal content is .containerignore, introduced deliberately in its ADR-T-009 container-infrastructure work because Docker reads only .dockerignore while Podman and Buildah prefer .containerignore; one link keeps a single ignore list for both toolchains. That repository runs the same tool with githubmerge.repository set to torrust/torrust-index; a merge run against torrust/torrust-index#884 created the local merge commit and then stopped with ERROR: File '.dockerignore' was a symlink.

The tracker's own tree contains no symbolic links today, so this change alters no tracker merge until the tracker declares a link, and it is a no-op for any repository that ships no declaration file. The vendored copy, the merge-pull-request.sh wrapper, its test suite, and the vendoring policy all live here, which is why the tool-side change belongs in this repository.

Proposal

A repository-level declaration file, listing each accepted link with its exact target and the reason it exists. Its name and location are a repository convention rather than anything the tool knows; this repository keeps it at .symlinks.json in the repository root:

{
  "namespace": "com.torrust.repository.symlinks",
  "version": [1, 0, 0],
  "symlinks": [
    {
      "path": ".dockerignore",
      "target": ".containerignore",
      "reason": "Docker reads only .dockerignore, while Podman and Buildah prefer .containerignore. One link keeps a single ignore list for both toolchains."
    }
  ]
}

Rules:

  • The declaration is read from the merged tree, out of the local merge commit the tool has just created (git show HEAD:.symlinks.json or the equivalent git cat-file), never from the working directory, the index, the base branch, or any path outside the repository. The merge commit already exists at the check site (checkout of pull/<n>/local-merge, git merge --commit --no-ff, subject guard, then get_symlink_files() on HEAD), so the links and the declaration that admits them come from one and the same tree.
  • A declaration that exists only in the maintainer's working tree, only on the base branch, or only on the pull-request branch without reaching the merged result exempts nothing. Only what the merge produced counts, so the exception is a reviewed, versioned part of the history the merge creates, and a pull request that adds a link is exempt only if the merged result also carries a declaration covering it.
  • The check covers every commit the merge introduces: the pull request's own commits (pull/<n>/base..pull/<n>/head, a range the tool already fetches and enumerates for the merge message) plus the local merge commit itself. Each commit's tree is listed under the same mode mask, and every link found in any of them must be declared in the final merged tree's manifest with a byte-for-byte matching target. Commits already reachable from the base branch are not walked; nothing is checked retroactively. A merge publishes history, not a tip: every introduced commit is a checkout, archive, bisect and review target, so a link that appears mid-range and vanishes before the tip would pass a tip-only check while still resolving on every checkout of the commit that carries it, and a manifest that changed mid-range would let earlier commits be admitted by a statement the merge result has withdrawn. The final manifest is therefore the single authority for the whole range.
  • path is repository-relative and must name a symbolic link in at least one checked commit; target must equal the link's literal content byte for byte in every checked commit that carries that link.
  • A target that is absolute, or that contains a .. segment, is never accepted, whatever the declaration says.
  • An undeclared link, or a link declared with a different target, in any checked commit produces ERROR: File '<path>' was a symlink in commit <hash> and exit code 4, naming the commit that carries it. A declaration entry matching no link in any checked commit exempts nothing and is reported as stale, not refused.
  • Consequence: removing a declared link takes two changes. The pull request that deletes the link must keep the manifest entry in the final merged tree, because its pre-deletion commits are judged against that manifest; the entry is then reported as stale, and a later change drops it once no introduced commit carries the link.
  • A declaration absent from the merged tree, or a run without the argument, behaves exactly as today.
  • The namespace identifies the declaration format, not the repository, so every Torrust repository uses the same value, com.torrust.repository.symlinks; tree sourcing already binds a declaration to its own repository.

An optional argument --symlinks <path> on the merge tool naming a repository-relative path inside the merged tree, with no default value, which exempts exactly the matching links and prints each accepted link with its path, target, and reason into the merge output before the maintainer is asked to sign. A run that omits the argument reads no declaration and refuses every link, whatever the merged tree contains. merge-pull-request.sh passes --symlinks .symlinks.json unconditionally and performs no filesystem check, since existence is a property of a tree the wrapper cannot observe before the merge commit exists; that line is the only place this repository's declaration path is stated.

Architectural decision (AD1, resolved): the merge tool is maintained here

github-merge.py arrived as a byte-identical copy of the Bitcoin Core developers' github-merge.py (MIT, retained in github-merge-COPYING), SHA-256 e390eb014131f3183a2cba642134974a6b09b19a65322d17dd7c81cf4ffbaad2, in commit 833a416 ("feat(git): vendor maintainer merge workflow", 2026-07-23), and has not been modified since. This feature cannot be confined to the wrapper because the refusal lives inside the tool, so the decision is about what the file is: a mirror that must stay byte-identical to upstream, or an ordinary repository file that happens to have come from there. It is the latter. README-github-merge.md keeps exactly that one provenance statement, fixed for good, and the tool is modified under ordinary review like any other file: no re-hashing after each change, no divergence log, no byte-identity requirement, no re-vendoring plan, no ADR. A provenance statement answers the only question a reader has about a vendored file; byte-identity plus re-hashing answers whether the copy still matches upstream, which only matters for a copy meant to track upstream, and this one is not: its consumers are Torrust repositories whose needs Bitcoin Core does not share. Offering the feature upstream first was considered and declined (nothing depends on byte-identity once the copy is no longer tracked, and the sibling repository would stay blocked on upstream's schedule); contributing the idea later on its own merits remains possible.

Scope

In scope: the declaration format and rules; tree-sourced reading; the range check over every introduced commit with per-commit reporting; the --symlinks argument and accepted-link output; unconditional wrapper delegation; wrapper tests covering delegation, acceptance, and each refusal case (undeclared, mismatched target, .., absolute, out-of-tree declaration); the README's provenance rewrite to the single statement and a re-check of its known-upstream-issues section (those defects become ordinarily fixable in follow-ups); a note in the merge-pull-request skill.

Out of scope: linter enforcement of .symlinks.json; any check over commits already on the base branch; reading the declaration from any other source; changing the semantics for undeclared links; any symlink policy for the tracker's own tree; other vendored behavior; mirroring the change into other repositories.

Acceptance criteria

  • AC1: .symlinks.json has a documented shape and rule set, including the target forms that are never accepted.
  • AC2: The tool reads the declaration from the final merged tree and from no other source; a declaration present only in the working directory, on the base branch, or in an intermediate commit exempts nothing.
  • AC3: The tool checks the links in every commit the merge introduces (pull/<n>/base..pull/<n>/head plus the merge commit) against that single final declaration, and does not walk commits already reachable from the base branch.
  • AC4: --symlinks <path> is a repository-relative tree path with no default value, and exempts exactly the declared links whose target matches the link's content byte for byte, on the bytes the tree carries, in every checked commit that carries them.
  • AC5: Every accepted link is printed with its path, target, and reason before sign-off.
  • AC6: Undeclared links, mismatched targets, absolute targets, and .. targets each refuse with ERROR: File '<path>' was a symlink in commit <hash> and exit code 4.
  • AC7: A pull request that deletes a declared link and keeps its manifest entry passes, with the retained entry reported as stale.
  • AC8: With no declaration in the final merged tree, or no argument, the tool behaves exactly as today for every checked commit.
  • AC9: merge-pull-request.sh passes --symlinks .symlinks.json unconditionally with no filesystem check.
  • AC10: README-github-merge.md carries the single provenance statement (origin, license, original SHA-256, introducing commit) and no longer requires re-hashing or byte-identity.

Specification

Folder-style specification drafted at docs/issues/drafts/merge-tool-symlink-exceptions/ISSUE.md (spec-first per the create-issue skill); it moves to docs/issues/open/ under this issue's number in a spec-only PR.

Related issues: #2022 vendored and documented the maintainer merge workflow this extends; #2003 is the automation umbrella EPIC. Neither is a parent of this issue.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions