diff options
| -rw-r--r-- | tvix/nar-bridge/pkg/reader/reader.go | 16 | ||||
| -rw-r--r-- | tvix/nar-bridge/pkg/reader/reader_test.go | 28 | ||||
| -rw-r--r-- | tvix/nar-bridge/testdata/popdirectories.nar | bin | 0 -> 600 bytes |
3 files changed, 39 insertions, 5 deletions
diff --git a/tvix/nar-bridge/pkg/reader/reader.go b/tvix/nar-bridge/pkg/reader/reader.go index 6d34cd168db4..9ff7b3fedb09 100644 --- a/tvix/nar-bridge/pkg/reader/reader.go +++ b/tvix/nar-bridge/pkg/reader/reader.go @@ -190,15 +190,21 @@ func (r *Reader) Import( // Check for valid path transitions, pop from stack if needed // The nar reader already gives us some guarantees about ordering and illegal transitions, // So we really only need to check if the top-of-stack path is a prefix of the path, - // and if it's not, pop from the stack. + // and if it's not, pop from the stack. We do this repeatedly until the top of the stack is + // the subdirectory the new entry is in, or we hit the root directory. // We don't need to worry about the root node case, because we can only finish the root "/" // If we're at the end of the NAR reader (covered by the EOF check) - if len(stack) > 0 && !strings.HasPrefix(hdr.Path, stack[len(stack)-1].path) { - err := popFromStack() - if err != nil { - return nil, fmt.Errorf("unable to pop from stack: %w", err) + for { + // We never want to pop the root directory until we're completely done. + if len(stack) > 1 && !strings.HasPrefix(hdr.Path, stack[len(stack)-1].path+"/") { + err := popFromStack() + if err != nil { + return nil, fmt.Errorf("unable to pop from stack: %w", err) + } + continue } + break } if hdr.Type == nar.TypeSymlink { diff --git a/tvix/nar-bridge/pkg/reader/reader_test.go b/tvix/nar-bridge/pkg/reader/reader_test.go index 6ca1ca2d7444..1ba9c9651cdc 100644 --- a/tvix/nar-bridge/pkg/reader/reader_test.go +++ b/tvix/nar-bridge/pkg/reader/reader_test.go @@ -566,3 +566,31 @@ func TestCallbackErrors(t *testing.T) { require.ErrorIs(t, err, targetErr) }) } + +// TestPopDirectories is a regression test that ensures we handle the directory +// stack properly. +// +// This test case looks like: +// +// / (dir) +// /test (dir) +// /test/tested (file) +// /tested (file) +// +// We used to have a bug where the second `tested` file would appear as if +// it was in the `/test` dir because it has that dir as a string prefix. +func TestPopDirectories(t *testing.T) { + f, err := os.Open("../../testdata/popdirectories.nar") + require.NoError(t, err) + defer f.Close() + + r := reader.New(f) + _, err = r.Import( + context.Background(), + func(fileReader io.Reader) error { return nil }, + func(directory *storev1pb.Directory) error { + return directory.Validate() + }, + ) + require.NoError(t, err) +} diff --git a/tvix/nar-bridge/testdata/popdirectories.nar b/tvix/nar-bridge/testdata/popdirectories.nar new file mode 100644 index 000000000000..74313aca529f --- /dev/null +++ b/tvix/nar-bridge/testdata/popdirectories.nar Binary files differ |