Review fixes - #1
Draft
rabarbra wants to merge 22 commits into
Draft
Conversation
The initial managed-release link was committed with os.Link on a symlink, which returns EPERM on macOS and BSD, so creating a first local release failed on every non-Linux platform. os.Symlink is itself an atomic create-if-absent commit, so the reserved-symlink dance is unnecessary there.
The hashed broker snapshot was unlinked and executed through /proc/self/fd/3, or /dev/fd/3 outside Linux. Only Linux resolves that path to the open file itself; darwin reopens it by name, so an unlinked snapshot fails to exec with EACCES and private S3 repositories did not work there at all.
redactCredential derived base64(username+":"+password) even with no credential, producing the constant "Og==" and rewriting it throughout unrelated pip output on every public repository.
Validating authoritative files spawned three Git processes per file: rev-parse --show-prefix, rev-parse <rev>:<path>, and hash-object. A plan over eleven repositories cost 44 processes, and apply repeats the whole validation inside its per-repository loop. Resolve both sides in one batch each, cat-file --batch-check for the committed blobs and hash-object --stdin-paths for the worktree, and cache the repository prefix per root. The same eleven-repository plan now costs 20 processes and no longer grows with the file count. hash-object --path also applies the clean filters that produced the committed blob, which fixes a real defect: comparing raw worktree bytes reported every authoritative file as changed in any repository that converts line endings, making such a workspace impossible to plan. Hashing the files rather than consulting the index keeps assume-unchanged from hiding a modification, as before. Status output is now read NUL-delimited, so core.quotePath cannot change the path bytes and a rename carries its original path as its own field rather than an ambiguous "old -> new" string that matched no allowlist entry. Neutralise the config that alters output this package parses, using -c overrides so safe.directory keeps working, and sort the replacement keys so a child environment is a deterministic function of its inputs.
The replay walked every ref. A ledger version that only ever existed on an abandoned branch or an old tag was folded into the immutable-record set and the current worktree ledger was then reported as having removed it, so an unrelated stale branch made planning impossible. Scope the walk to HEAD, which is the line of history the records belong to. It also spawned a `git show` per historical commit, per repository, on add, plan, apply and check, so the cost grew without bound as a workspace accumulated publications. Read every version through one cat-file batch and parse each distinct blob once. Naming a revision fails on an unborn HEAD where a whole-repository walk was simply empty, so a workspace that records artifacts before its first commit now reports no history rather than an error.
Building with -tags nos3 drops the stripped binary from 20.6 MB to 12.8 MB. The tag is opt-out so the default build and the shipped image keep S3; excluded builds report that rather than failing obscurely.
Signed-off-by: Polina Simonenko <rabarbrablad@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.