Skip to content

[FEATURE] Report exclude entries which did not take effect - #108

Draft
CybotTM wants to merge 1 commit into
TYPO3:mainfrom
CybotTM:feature/warn-ineffective-excludes
Draft

[FEATURE] Report exclude entries which did not take effect#108
CybotTM wants to merge 1 commit into
TYPO3:mainfrom
CybotTM:feature/warn-ineffective-excludes

Conversation

@CybotTM

@CybotTM CybotTM commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #106 and #107, which fixed two ways an exclude entry could silently fail. This adds the reporting that would have made both of them visible at release time instead of after the fact.

The problem

An exclude entry that matches nothing looks exactly like one that works: no error, no warning, exit code 0, and an archive that carries the directory the configuration was supposed to keep out. That is how the 36 build files ended up in the archive linked in #106, and how the escaped-slash notation broke in #107 without anyone noticing until Elias read the diff.

The notations that read as valid configuration and quietly do nothing today, all measured against main at 7faa2d2:

entry excludes Resources/Private/Build?
Resources/Private/Build yes
Resources\/Private\/Build yes, kept working by #107
Resources/Private/Build/ no
./Resources/Private/Build no
Resources\Private\Build no

None of these is a regression - the first two were broken before #106 as well, with a preg_match(): Unknown modifier warning that at least showed something was wrong. Since #106 they fail in complete silence.

What this does

After the tree has been walked, every configured entry is compared against the paths that actually went into the archive. An entry is reported when the archive still contains what the entry names, with a hint about the most likely reason:


 [WARNING] The exclude entry "Resources/Private/Build/" did not take effect, the
           archive contains "Resources/Private/Build/gulpfile.js". Directory    
           names are matched without a trailing slash, remove it.               
                                                                                
           The exclude entry "Resources\/Private\/Frontend" escapes its slashes.
           This is no longer required, write it as "Resources/Private/Frontend".
                                                                                
           The exclude entry "Resources/Private/Build/gulpfile.js" did not take 
           effect, the archive contains "Resources/Private/Build/gulpfile.js".  
           File entries are matched against the filename, they can not contain a
           path.                                                                

 [OK] Extension artefact successfully generated:                                
      /tmp/smoke/tailor-version-artefact/my_ext_1.0.0.zip

It is advisory: the warnings go to both create-artefact and ter:publish, the exit code stays 0 and nothing about the resulting archive changes.

Why report instead of accepting every notation

Normalising the input away - trim('/'), strip ./, translate backslashes - would fix the three rows in the table above and I would rather not, for four reasons.

Normalisation only covers the variants someone already thought of. A trim() fixes the trailing slash and does nothing for a directory renamed since the config was written, for a files entry carrying a path, or for a typo. The comparison against the packaged paths caught all of those in the smoke run without knowing any of them in advance, because it measures the effect rather than the shape of the input.

Every normalisation is a silent guess, and a wrong guess is the same silent failure again. #106 and #107 are the proof: quoting was right for the documented notation and broke the undocumented one; stripping \/ fixed that and would break a directory whose name really contains a backslash. Each rule quietly widens what counts as valid input without telling anyone which reading was picked.

Accepted-but-undocumented variants become the contract. Once Resources/Private/Build/ just works, the README sentence "plain directory and file names" is no longer true and every future refactoring has to preserve the accident. A warning keeps one documented notation and makes the deviation visible where it is cheap to fix: in the release log, before the archive goes to TER.

It is not either/or. Normalising the trailing slash later is a one-line change and the warning would still say that it happened. Reporting first is the smaller step and it closes the whole class, not the three cases in the table.

Why it compares against the packaged paths

The straightforward implementation - remember which entries matched during the walk, report the rest - is too noisy to survive contact with a real configuration. Custom configurations are curated copies of the shipped default plus a few extras, and most of their entries legitimately match nothing in any single extension.

Measured on xima-media/xima-typo3-frontend-edit 2.5.0 with its own packaging_exclude.php (35 entries): reporting every unmatched entry produces 24 warnings, including two false ones for entries that are perfectly fine. Comparing against what was packaged produces 2, both of them the escaped-slash notice this PR wants that extension to see:


 [WARNING] The exclude entry "resources\/private\/frontend" escapes its slashes.
           This is no longer required, write it as "resources/private/frontend".
                                                                                
           The exclude entry "resources\/private\/libs\/build" escapes its      
           slashes. This is no longer required, write it as                     
           "resources/private/libs/build".                                      

The same comparison also keeps a redundant entry quiet: Resources/Private next to Resources/Private/Build leaves nothing of the second one in the archive, so there is nothing to report.

Escaped slashes

Entries written as Resources\/Private\/Build still work and are now pointed out once per release, because the escaping is only accepted for compatibility. The three extensions using it - xima-typo3-frontend-edit, t3x-nr-saml-auth, hide_sys_template - carry the same two lines, so this looks like one copied template rather than three independent decisions. Without the notice the double notation in the README stays forever; with it, the escaping can be dropped in a future major version.

The README now documents one notation and mentions the escaped one as accepted, not as an alternative.

Tests

Nine new tests, tests/Unit/Service/VersionServiceTest.php and both packaging commands:

  • an entry with a trailing slash is reported, and the archive really does contain the file it names
  • a files entry containing a path is reported
  • an entry that took effect is not reported
  • an entry naming something the extension does not contain is not reported
  • an entry covered by a shorter entry next to it is not reported
  • packaging with the shipped default configuration produces no warnings at all
  • an entry with escaped slashes is reported once, and still excludes its directory
  • the warnings reach the output of create-artefact and of ter:publish

Each of them was shown to fail: seven mutations of the new code (warnings suppressed, evidence check bypassed, packaged paths not collected, each hint removed, the notation notice removed, the command output removed) each redden exactly the tests that cover them, and every one of the nine appears in at least one of those runs.

composer tests:unit is green (163 tests, 449 assertions) and composer cs reports no findings.

Assisted by claude-code:claude-opus-5 — Session

An exclude entry that quietly matches nothing is the failure the whole
filter exists to prevent: a published archive carrying the very directory
the configuration was supposed to keep out. `Resources/Private/Build/`
written with a trailing slash, with a leading `./` or with backslashes as
separators reads like a valid exclude and packages the directory anyway,
without a warning and without an error.

Report an entry when the created archive still contains what the entry
names, together with a hint about the most likely reason. Comparing
against the packaged paths rather than counting pattern matches keeps
two cases quiet which are not a problem: an entry for a directory the
extension does not have - exclude configurations are shared between
extensions and the shipped default covers a lot of them - and an entry
already covered by a shorter one next to it.

Entries still written with escaped slashes are reported as well, since
the escaping is only accepted for compatibility since TYPO3#107.

Assisted-by: claude-code:claude-opus-5
Agent-Session: https://claude.ai/code/session_01CFdZJzsCjJ7rmT1u9snkiv
Agent-Host: 0493f0
Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
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.

1 participant