libs/template: don't create directories that a later {{skip}} names - #6348
libs/template: don't create directories that a later {{skip}} names#6348TangoEnSkai wants to merge 1 commit into
Conversation
The walk records each directory it visits and persistToDisk creates them all,
so a template ends up with the directory structure even where every file was
skipped. That is deliberate, but it tests the skip patterns only at the moment
the walk reaches a directory.
{{skip}} directives are registered while rendering files, so a file processed
after a directory has been visited can name that directory. The walk has
already recorded it by then and the directory is created regardless -- empty,
because its own files do match the pattern and are dropped.
Re-test the accumulated patterns when materializing directories. Directories
whose files merely happened to be skipped are unaffected: those patterns match
the files, not the directory, which keeps the existing behaviour of preserving
a template's structure.
Approval status: pending
|
|
An authorized user can trigger integration tests manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
|
@lennartkats-db tagging you as the bot's suggestion for The one judgement call worth your eyes: I deliberately kept the "create directories whose files were all skipped" behaviour intact, and only suppress a directory that a If you would rather that a directory be dropped whenever every one of its files was skipped, that is a different and larger behaviour change and I would leave it out of this PR. The new test fails on |
1 participant
Changes
renderer.walkrecords every directory it visits, andpersistToDiskcreatesthem all so a template's structure survives even where files were skipped.
That is deliberate. But the walk tests the skip patterns only at the moment it
reaches a directory.
{{skip}}directives are registered while rendering files, so a fileprocessed after a directory has already been visited can name that directory.
By then the walk has recorded it, and the directory is created anyway — empty,
because its own files do match the pattern and are dropped.
This re-tests the accumulated patterns when directories are materialized.
Directories whose files merely happened to be skipped are unaffected: those
patterns (
dir/*) match the files, not the directory, sodiris stillcreated. That keeps the behaviour the surrounding comment asks for — "Only
explicit
{{skip}}directives should prevent directory creation" — which isexactly what is not true today.
Why
The current behaviour contradicts that comment: an explicit
{{skip}}naminga directory is honoured only if it happens to be registered before the walk
reaches it. Whether it is depends on BFS ordering — which sibling directory
sorts first, and which file the directive lives in — so the same directive
works or silently leaves an empty directory depending on where it is written.
This looks like the mechanism behind #6142, which reports
bundle initleaving a stray empty directory next to the correctly generated bundle. I
could not confirm that end to end, since the template in that report is not
public, so I have not marked this as fixing it — but the symptom matches, and
the ordering dependence above would explain why it reproduces with some
template variants and not others.
Tests
go test ./libs/template/... ./cmd/bundle/...— all green.Added
TestRendererSkipDirectoryVisitedBeforeSkipDirectivewith a fixturewhere
dir2/file2.tmplskipsdir1, which the BFS walk visits first. Thetest asserts
dir1is absent anddir2/file2is written; it fails onmainwith
directory ".../dir1" exists.