archive/{zip,tar}: (*Writer).AddFS omits empty directories

[songgao]

Go version

go1.22.2

Output of go env in your module/workspace:

❯ go env
GO111MODULE=''
GOARCH='arm64'
GOBIN='/Users/songgao/go/bin'
GOCACHE='/Users/songgao/Library/Caches/go-build'
GOENV='/Users/songgao/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/songgao/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/songgao/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/Users/songgao/.go/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/Users/songgao/.go/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.22.2'
GCCGO='gccgo'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/6t/8hmkkzqd79nbdy4m1stjq3140000gn/T/go-build3063187010=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

These modified tests fail:

archive/tar:

func TestWriterAddFS(t *testing.T) {
	fsys := fstest.MapFS{
		"emptyfolder":          {Mode: 0755 | os.ModeDir},
		"file.go":              {Data: []byte("hello")},
		"subfolder":            {Mode: 0755 | os.ModeDir},
		"subfolder/another.go": {Data: []byte("world")},
	}
	var buf bytes.Buffer
	tw := NewWriter(&buf)
	if err := tw.AddFS(fsys); err != nil {
		t.Fatal(err)
	}
	if err := tw.Close(); err != nil {
		t.Fatal(err)
	}

	// Test that we can get the files back from the archive
	tr := NewReader(&buf)

	names := make([]string, 0, len(fsys))
	for name := range fsys {
		names = append(names, name)
	}
	sort.Strings(names)

	entriesLeft := len(fsys)
	for _, name := range names {
		entriesLeft--

		entryInfo, err := fsys.Stat(name)
		if err != nil {
			t.Fatalf("getting entry info error: %v", err)
		}
		hdr, err := tr.Next()
		if err == io.EOF {
			break // End of archive
		}
		if err != nil {
			t.Fatal(err)
		}

		if hdr.Name != name {
			t.Fatalf("test fs has filename %v; archive header has %v",
				name, hdr.Name)
		}

		if entryInfo.Mode() != hdr.FileInfo().Mode() {
			t.Fatalf("%s: test fs has mode %v; archive header has %v",
				name, entryInfo.Mode(), hdr.FileInfo().Mode())
		}

		if entryInfo.IsDir() {
			continue
		}

		data, err := io.ReadAll(tr)
		if err != nil {
			t.Fatal(err)
		}
		origdata := fsys[name].Data
		if string(data) != string(origdata) {
			t.Fatalf("test fs has file content %v; archive header has %v",
				data, origdata)
		}
	}
	if entriesLeft > 0 {
		t.Fatalf("not all entries are in the archive")
	}
}

archive/zip:

func TestWriterAddFS(t *testing.T) {
	buf := new(bytes.Buffer)
	w := NewWriter(buf)
	tests := []WriteTest{
		{
			Name: "emptyfolder",
			Mode: 0755 | os.ModeDir,
		},
		{
			Name: "file.go",
			Data: []byte("hello"),
			Mode: 0644,
		},
		{
			Name: "subfolder",
			Mode: 0755 | os.ModeDir,
		},
		{
			Name: "subfolder/another.go",
			Data: []byte("world"),
			Mode: 0644,
		},
	}
	err := w.AddFS(writeTestsToFS(tests))
	if err != nil {
		t.Fatal(err)
	}

	if err := w.Close(); err != nil {
		t.Fatal(err)
	}

	// read it back
	r, err := NewReader(bytes.NewReader(buf.Bytes()), int64(buf.Len()))
	if err != nil {
		t.Fatal(err)
	}
	for i, wt := range tests {
		testReadFile(t, r.File[i], &wt)
	}
}

What did you see happen?

Tests fail

What did you expect to see?

tests pass

[gopherbot]

Change https://go.dev/cl/578415 mentions this issue: archive/{zip,tar}: fix Writer.AddFS to include empty directories

[cherrymui]

cc @dsnet

[purpleidea]

I can confirm this is a real bug as I just reported it with a short workaround patch for out-of-tree uses in #69459

I guess we're both good at finding bugs!