Skip to content

fix: MemMapFs.OpenFile checks parent dir exists before creating file#572

Open
veeceey wants to merge 1 commit into
spf13:masterfrom
veeceey:fix/issue-270-openfile-parent-dir-check
Open

fix: MemMapFs.OpenFile checks parent dir exists before creating file#572
veeceey wants to merge 1 commit into
spf13:masterfrom
veeceey:fix/issue-270-openfile-parent-dir-check

Conversation

@veeceey

@veeceey veeceey commented Feb 23, 2026

Copy link
Copy Markdown

Fixes #270

MemMapFs.OpenFile with O_CREATE was silently creating parent directories that don't exist. This meant WriteFile would happily write to non-existent paths in MemMapFs while OsFs would correctly return an error -- inconsistent behavior.

The fix adds a parent directory existence check in OpenFile before calling Create. If the parent doesn't exist, it returns os.ErrNotExist, matching os.OpenFile semantics.

Updated existing tests that relied on the implicit directory creation to explicitly create directories first (which is what you'd need to do on a real filesystem anyway). Added a dedicated test covering the bug.

… file

MemMapFs.OpenFile with O_CREATE was silently creating parent directories
that don't exist, diverging from os.OpenFile semantics. This caused
WriteFile to succeed when writing to non-existent directories in
MemMapFs, while OsFs correctly returned an error.

Now OpenFile verifies the parent directory exists before creating a new
file, returning os.ErrNotExist if it doesn't. This matches the behavior
of the real filesystem.

Fixes spf13#270
@CLAassistant

CLAassistant commented Feb 23, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@veeceey

veeceey commented Feb 23, 2026

Copy link
Copy Markdown
Author

Test Results

Full test suite passes:

$ go test -count=1 ./...
ok  	github.com/spf13/afero	4.528s
ok  	github.com/spf13/afero/mem	0.195s
ok  	github.com/spf13/afero/tarfs	0.648s
ok  	github.com/spf13/afero/zipfs	0.506s

Verified the fix with the new test TestOpenFileNonExistentDirectory:

$ go test -count=1 -v -run TestOpenFileNonExistentDirectory ./...
=== RUN   TestOpenFileNonExistentDirectory
--- PASS: TestOpenFileNonExistentDirectory (0.00s)

Also confirmed manually that WriteFile to a non-existent directory now returns os.ErrNotExist, matching OsFs behavior.

@veeceey

veeceey commented Mar 12, 2026

Copy link
Copy Markdown
Author

gentle ping when you have a moment

@spf13

spf13 commented May 7, 2026

Copy link
Copy Markdown
Owner

The fix is correct — MemMapFs should have matched os.OpenFile semantics here from the start, and the implementation is clean.

However, this is a breaking change for downstream consumers and we cannot merge it without coordination. I checked the major consumers:

Hugo (gohugoio/hugo) breaks in at least 4 test files: hugofs/rootmapping_fs_test.go (multiple test functions), hugolib/filesystems/basefs_test.go (TestStaticFs, TestStaticFsMultihost, setConfigAndWriteSomeFilesTo helper), and hugolib/integrationtest_builder.go (writeToFs/AddFiles). The pattern is afero.WriteFile on multi-level paths with no preceding MkdirAll.

Grafana k6 (grafana/k6:lib/fsext/trimpathseparator_test.go) explicitly tests that WriteFile succeeds without pre-created directories — directly the behavior this PR removes.

Viper and Cobra are safe (Viper uses fs.Create() not WriteFile; Cobra does not use MemMapFs).

Holding this until we can do a coordinated release that fixes Hugo's test files simultaneously. The migration path for callers is straightforward: add MkdirAll(filepath.Dir(path), perm) before WriteFile, or use afero.WriteReader which already does this correctly.

@spf13

spf13 commented Jun 9, 2026

Copy link
Copy Markdown
Owner

The fix here is correct — MemMapFs should require parent directories to exist before creating a file, matching os.OpenFile semantics. However, this is an observable behavior change for existing users: code that currently does WriteFile(memFs, "/some/path/file.txt", ...) without a preceding MkdirAll will silently break. MemMapFs is widely used as a frictionless test double, and many codebases rely on this permissiveness.

Before merging we need to assess the blast radius. A few questions:

  1. How common is this pattern in the wild? (A quick grep of open-source afero users would tell us.)
  2. Should this be gated behind a version bump with a clear migration note?
  3. Is there an opt-in flag (e.g. StrictMode) that would let users choose the behavior, rather than a hard cut?

The implementation and tests are good. This is a policy/impact question, not a correctness question.

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.

WriteFile creates non-existing directory for MemMapFs but not for OsFs

3 participants