Skip to content

fix(archiver): restore the documented semantics of upload exclusions - #80

Open
loicsapone wants to merge 1 commit into
automatephp:mainfrom
loicsapone:fix/upload-exclude-regression
Open

fix(archiver): restore the documented semantics of upload exclusions#80
loicsapone wants to merge 1 commit into
automatephp:mainfrom
loicsapone:fix/upload-exclude-regression

Conversation

@loicsapone

Copy link
Copy Markdown
Contributor

Switching from Finder to tar in 0ec097a also switched the exclude patterns from
Finder expressions to tar globs, without the documentation following.

Plain names such as node_modules kept working by coincidence — tar matches them as a
path 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:

exclude:
  - '/^folder/subfolder/'   # not a valid regexp: unescaped delimiter

Under the pre-0ec097a Finder this raised preg_match(): Unknown modifier 'b', and
since bin/automate turns warnings into ErrorException, 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 from Finder.

The reference example is fixed and the matching rules are now spelled out:

  • a plain string excludes every path containing it;
  • a regexp, delimiters included, is matched against the path relative to the uploaded
    directory.

Tests

ArchiverTest::testArchiveExclude, 5 data sets (folder name, file name, dot folder,
anchored regexp, several patterns). The regexp set fails on main.

testArchive now reads the members with tar -tzf instead of PharData: PharData
caches 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 exclude patterns
and checking what is actually present on the servers.

@julienj

julienj commented Aug 9, 2026

Copy link
Copy Markdown
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:

  1. Directories are gone from the archive. ->files() only lists files, so empty dirs and symlinks to dirs no longer get deployed (I get 60300 entries vs 61503 with plain tar). If you add dirs back to the list, you'll need --no-recursion too — otherwise tar walks into them and re-adds the excluded files.
  2. Invalid patterns are noisy. '/^folder/subfolder/' throws a preg_match(): Unknown modifier warning per file and silently doesn't exclude. Could we validate the patterns upfront and fail with a clear message, like we did in fix: report a clear error when a password cannot be resolved #77?
  3. The Process still has Symfony's default 60s timeout — 60k files already take ~26s to compress here, so a big project will hit it. It predates your change, but since we're in Archiver anyway, could you drop a setTimeout() in this PR?

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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants