Support rsync protect/hide rules in the deploy ignore list - #22
Open
msaggiorato wants to merge 3 commits into
Open
Support rsync protect/hide rules in the deploy ignore list#22msaggiorato wants to merge 3 commits into
msaggiorato wants to merge 3 commits into
Conversation
An exclude is symmetric: it stops us sending a path AND stops --delete
removing it. That leaves whatever was last pushed to a newly-ignored path
sitting on the server forever. Adds four rsync rule types as line prefixes
so a deploy can say what it actually means:
protect / P keep sending ours, never delete theirs
risk / R an exception to a protect
hide / H stop sending, and DO let --delete clean up what we pushed
show / S an exception to a hide
check-against-manifest.sh needed rework to go with it: protect makes rsync
skip a deletion git asked for, and hide makes rsync delete a file git never
knew about. Both would fail the consistency check on the first deploy that
used them. It now reconciles each class of mismatch against its own derived
gitignore view of the rules.
Also fixes three bugs found on the way:
- rule.replace('!','') stripped the first "!" anywhere in a pattern, so
/weird!name.php became /weirdname.php
- re-rooting a negation produced "!/wp-contentvendor/composer" (missing
slash), breaking every negation in the manifest check whenever
env-local-root was a repo subdirectory
- the sort comparator never returned 0, so equal-specificity rules ordered
unpredictably
The specificity sort is kept and now documented as load bearing: "!dir/"
expands to a whole-subtree include, and specificity ordering is what keeps
narrower rules ahead of it. Verified against all 49 real SSH_IGNORE_LIST
values in the fleet - filter output is byte-identical before and after.
Tests drive real rsync, asserting what actually transfers and what survives
--delete rather than the text of the filter file.
reroot() recomputed specificity from the prefixed pattern. Every anchored rule gains the same prefix, so this only changed how anchored rules ranked against unanchored ones — and main.js builds the rsync filter from the un-rerooted rules while building the gitignore views from the rerooted ones. The two then disagreed, so the manifest check stopped matching what rsync actually did and failed the deploy. Demonstrated with "!/plugins/" plus "node_modules/" against a subdirectory deploy root: rsync skips plugins/acme/node_modules/dep.js, the manifest keeps it, mismatch. The old code got this right; this was a regression. Prefix keywords are now matched case-insensitively. "Protect /mu-plugins/" previously fell through to an exclude whose pattern contained a space, which matches nothing — so a capitalised keyword silently dropped the protection and let --delete remove the very files it was meant to keep. A line shaped like `keyword pattern` that names no known keyword now warns, since a real path can contain a space and hard-failing would be wrong. Test gaps closed: - run.js now refuses a case that asserts on `remote` while nothing exists locally. Against an empty source, --delete wipes the target wholesale and every DELETED assertion passes for free. Five cases were vacuous this way, two of them added in the same session that wrote the guard. - survivors() runs with -c, matching the production `avrcz`. Without it, rsync's size+mtime quick check skips same-size files and an overwrite silently no-ops — which is why "protect overwrites ours" could not be asserted before. - New OVERWRITTEN assertion distinguishes "we replaced their copy" from "we left it alone"; KEPT alone could not. - Cover: the precedence tie-break (previously deletable with the whole suite still green), show vs include (S is sender-side only), show's subtree expansion, the -x in grep -vxF, hide on the send side of the manifest, and the asterisks-only specificity score. - manifest.sh no longer swallows a node failure when the rules start with "-", which silently disabled all filtering. 15 of 16 mutations are now killed; the survivor is an equivalent mutant. Fleet output is still byte-identical across all 49 real ignore lists.
The audit hashed the generated filter file, which answers the wrong question: a reordering that changes no outcome looked like a regression, and one that changed an outcome could look benign. It now runs real rsync for each of the 49 live ignore lists against a representative wp-content tree and records what the deploy would send, what survives --delete, and the three gitignore views the manifest check reconciles against. Still hashes only -- the lists carry client staging hostnames and plugin inventories, and this repo is public. Two things this exposed: - The tree was wp-content-rooted, but a few sites deploy at the web root and write /wp-content/... rules, so half the fleet's patterns matched nothing. Every path is now mirrored into both spaces. - The manifest views were only audited for repos with a subdirectory deploy root. The manifest check runs on every deploy, and rsync tolerates things git does not -- a stray space in a pattern is invisible to the transfer and still breaks reconciliation. They are hashed for every repo now. Checked against the real thing: the reroot regression fixed in the previous commit is caught, and it lands on talkbox, the one repo in the fleet that sets SSH_LOCAL_ROOT. Removing the specificity sort moves 46 repos. Also covers the two rule shapes the fleet uses that no test exercised: wildcard directory segments (/saucal_migration_*/, 31 uses) and a wildcard mid-path.
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.
Why
An exclude is symmetric: it stops us sending a path and stops
--deleteremoving it. So whatever was last pushed to a path you later start ignoring sits on the server forever, and there is no way to say "deploy here, but never delete what's already there".This adds four rsync rule types as line prefixes so the ignore list can say what it actually means:
protectPriskRprotecthideH--deleteclean up what we pushedshowShideBare lines and
!are unchanged, so existing lists behave exactly as before.The part that isn't optional
check-against-manifest.shneeded rework to go with it.protectmakes rsync skip a deletion git asked for;hidemakes rsync delete a file git never knew about. Either would fail the consistency check on the first deploy that used them. It now reconciles each class of mismatch against its own derived gitignore view of the same rules, and three negative tests confirm real drift still fails.Bugs fixed along the way
rule.replace('!','')stripped the first!anywhere in a pattern, so/weird!name.phpbecame- /weirdname.php.!/wp-contentvendor/composer(missing slash), breaking every negation in the manifest check wheneverenv-local-rootwas a repo subdirectory.0, so equal-specificity rules ordered unpredictably.Protect /mu-plugins/silently became an exclude whose pattern contained a space — it matches nothing, so the protection vanished and--deleteremoved the very files it was meant to keep. Keywords are now case-insensitive, and akeyword patternline naming no known keyword warns.The specificity sort stays, and is now documented as load-bearing
It looks like a bug (gitignore is last-match-wins, rsync is first-match-wins, this is neither) but
!dir/expands to a whole-subtree include and specificity ordering is the only thing keeping narrower rules ahead of it.Measured against all 49 live
SSH_IGNORE_LISTvalues: the current sort diverges fromgit check-ignoretwice; switching to true gitignore order diverges 17 times across 2 production repos (inside,maintenance-flyingtech), both of which would silently start deploying files. Ties now break by rule precedence, then reverse authoring order.Verification
Tests drive real rsync — they assert what actually transfers and what survives
--delete, not the text of the filter file. One run spawns 39 rsync processes.npm test→ 41 behavioural + 10 manifesttests/fleet-audit.jsreplays all 49 live ignore lists through real rsync against a representativewp-contenttree and fails if any repo would deploy differently. No change in deploy behaviour.test-consistency-diff.shstill passes —consistency-diff.shnow consumes the derivednot-sentview instead of the raw list.Reviewed by three independent passes (rigor, coverage, rsync semantics). They caught the re-rooting regression — which lands on
talkbox, the one repo in the fleet settingSSH_LOCAL_ROOT— and five test cases that were passing vacuously because an empty source tree makes--deletewipe everything.tests/run.jsnow refuses such a case outright.Note for the reviewer
-cinssh-flagsoverrides--size-only, so the deploy is checksum-based andsize-only ignore-timesin the defaultssh-extra-optionsare dead weight. The comments inmain.jsexplaining the diff in terms of size-only are wrong about the mechanism. Pre-existing and untouched here — worth a separate PR.🤖 Generated with Claude Code
https://claude.ai/code/session_014hL1GTv1SHsSyyqSQvoT8v