fix(archiver): restore the documented semantics of upload exclusions - #80
Open
loicsapone wants to merge 1 commit into
Open
fix(archiver): restore the documented semantics of upload exclusions#80loicsapone wants to merge 1 commit into
loicsapone wants to merge 1 commit into
Conversation
Member
|
Thanks for this — the bug is real, regexp excludes have been silently shipping files since 4.2.0. I benchmarked the Finder approach on ~60k files: +0.5s and 10MB peak, so no perf concern there. A couple of things before I merge:
I'll release this as 4.3.0 with an upgrade note: excludes match both ways again, so a plain node_modules now also drops old_node_modules/ — back to pre-4.2 behaviour, but worth calling out for anyone who wrote their config against the tar globs. |
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
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.
Switching from
Findertotarin 0ec097a also switched theexcludepatterns fromFinder expressions to tar globs, without the documentation following.
Plain names such as
node_moduleskept working by coincidence — tar matches them as apath component glob. But the regexp form documented in the reference is now handed
to tar verbatim and silently matches nothing, so files meant to stay local are shipped
to the servers with no warning.
The reference example was itself invalid, and had been for a while:
Under the pre-0ec097a Finder this raised
preg_match(): Unknown modifier 'b', andsince
bin/automateturns warnings intoErrorException, it aborted the deployment.Fix
tar is given an explicit member list built by
Finder(--null --files-from=-),which restores the documented behaviour for both forms while keeping tar responsible
for the archiving itself — the memory problem 0ec097a was solving came from
PharData, not fromFinder.The reference example is fixed and the matching rules are now spelled out:
directory.
Tests
ArchiverTest::testArchiveExclude, 5 data sets (folder name, file name, dot folder,anchored regexp, several patterns). The regexp set fails on
main.testArchivenow reads the members withtar -tzfinstead ofPharData: PharDatacaches archives by file name, and every archive built by this test class shares the
same one, so the second read in a single process returns stale contents.
Upgrade note
Worth a line in the release notes: anyone using the regexp form has been deploying
files they believed were excluded, since 0ec097a. Suggest reviewing
excludepatternsand checking what is actually present on the servers.