archive/zip: add (*Writer).AddFS

[c9845]

Zipping a directory is unclear and slightly confusing via the archive/zip package, mostly due to the way paths need to be handled and how directories are created. I suggest adding a func to handle zipping a directory simply to aid in implementation. I have had to implement, or educate others on implementing, similar code for various projects over the years. Furthermore, help via StackOverflow, golang-nuts, etc., isn't very clear either. There is clearly somewhat wide-spread confusion regarding zipping a directory.

Pseudo-code is below.

// Dir creates a zip from sourceDirPath and saves it to zipPath.  If a file at zipPath 
// already exists, an error is returned.
func Dir(sourceDirPath, zipPath string) (err error) {
       // Make sure zip file doesn't already exist at zipPath.
	_, err = os.Stat(zipFileAbs)
	if err == nil {
		return os.ErrExist
	} else if !os.IsNotExist(err) {
		return
	}

	// Create the zip file.
	zipFile, err := os.Create(zipFileAbs)
	if err != nil {
		return
	}
	defer zipFile.Close()

	// Initialize the zip writer.
	z := zip.NewWriter(zipFile)
	defer z.Close()

	// Zip up the sourceDirPath, files and directories, recursively.
	err = filepath.WalkDir(sourceDirPath, func(path string, d fs.DirEntry, err error) error {
		// Error with path.
		if err != nil {
			return err
		}

		// Skip the source directory root. This can be ignored because the source
		// directory root is just the root of the zip file.
		if sourceDir == path {
			return nil
		}

		// Skip directories. Directories will be created automatically from paths to
		// each file to zip up.
		if d.IsDir() {
			return nil
		}

		// Handle formatting path name properly for use in zip file. Paths must be 
                // relative, not start with a slash character, and must use forward slashes,
                // even on Windows.  See: https://pkg.go.dev/archive/zip#Writer.Create
		//
                // Directories are created automatically based on the subdirectories provided
                // in each file's path.
		zipPath := strings.Replace(path, sourceDir, "", 1)
		zipPath = strings.TrimPrefix(zipPath, string(filepath.Separator))
		zipPath = filepath.ToSlash(zipPath)

		// Open the path to read from.
		f, err := os.Open(path)
		if err != nil {
			return err
		}
		defer f.Close()

		// Create the path in the zip.
		w, err := z.Create(zipPath)
		if err != nil {
			return err
		}

		// Write the source file into the zip at path from Create().
		_, err = io.Copy(w, f)
		if err != nil {
			return err
		}

		return nil
	})
	if err != nil {
		return
	}

	// Zip file created.
	err = z.Close()
	if err != nil {
		return
	}

	return
}

[seankhliao]

This doesn't look like something that belongs in archive/zip, it shouldn't directly operate on the filesystem, both for reading and writing.
And if we did add something like this, the first thing people will want is control over the file attributes.
Also, the value proposition here is much weaker if you use the io/fs api (error handling elided):

fsys := os.DirFS("src")
zw := zip.NewWriter(w)
defer zw.Close()
fs.WalkDir(fsys, ".", func(p string, d fs.DirEntry, err error) error {
        if err != nil || d.IsDir() {
                return err
        }
        zf, _ := zw.Create(p)
        f, _ := fsys.Open(p)
        defer f.Close()
        _, _ = io.Copy(zf, f)
})

[DeedleFake]

How about a way to simply hand archive/zip an fs.FS and it'll just insert all of its contents into the in-progress zipfile? Something like

zw := zip.NewWriter(w)
defer zw.Close()
err := zw.AddFS(os.DirFS("src"))
// ...

[earthboundkid]

Adding to DeedleFake's proposal, perhaps AddFS(fsys fs.FS, globs ...string) error and it will only add those files matching a glob?

[beoran]

@carlmjohnson if we want a filter,

AddFS(fsys fs.FS, filters ...func(string) (bool, error)) error

Seems like the more flexible solution.

[c9845]

I completely forgot about fs.FS! That makes a lot of sense for this.

Using @DeedleFake's proposed zw.AddFS() also cleans us the func call since the output path to the zip file does not need to be provided to the func argument list.

Regarding filtering the files to include, I can see this going two ways.

1. Skip filtering and make the user/caller/parent func/etc. filter the directory prior to zipping.
2. Create an AddFS and AddFSWithFilter func to make the default case simpler. A ...string of regex glob patterns or ...func could work here, both have pluses and minuses.

---

I have rewritten my proposal pseudo-code below to use fs.FS and skip the filtering part for now.

// AddFS creates a zip archive of a directory by writing files from f to w.
func (w *Writer) AddFS (f fs.FS) (err error) {
	err = fs.WalkDir(f, ".", func(path string, d fs.DirEntry, err error) error {
		// Error with path.
		if err != nil {
			return err
		}

		// Skip directories. Directories will be created automatically from paths to
		// each file to zip up.
		if d.IsDir() {
			return nil
		}

		// Handle formatting path name properly for use in zip file. Paths must
		// use forward slashes, even on Windows. 
                // See: https://pkg.go.dev/archive/zip#Writer.Create
		//
		// Directories are created automatically based on the subdirectories provided
		// in each file's path.
		path = filepath.ToSlash(path)

		// Open the path to read from.
		f, err := f.Open(path)
		if err != nil {
			return err
		}
		defer f.Close()

		// Create the file in the zip.
		w, err := w.Create(path)
		if err != nil {
			return err
		}

		// Write the source file into the zip at path noted in Create().
		_, err = io.Copy(w, f)
		if err != nil {
			return err
		}

		return nil
	})
	if err != nil {
		return
	}

	return
}

[earthboundkid]

If filter is a callback, you don't need to take multiple. Just take one and people can write any filter chaining themselves: AddFS(fsys fs.FS, filter func(string) (bool, error)) error. I think this is probably better than just hard coding glob. The other way to do it might be AddFS(fsys fs.FS, paths []string) error and then you require users to do any path listing/filtering ahead of time. You could special case it so that nil paths means "add everything".

[DeedleFake]

Adding to DeedleFake's proposal, perhaps AddFS(fsys fs.FS, globs ...string) error and it will only add those files matching a glob?

Maybe this would be useful as a more generalized wrapper for the io/fs package? For example, maybe a func FilteredByGlobs(fsys FS, patterns ...string) FS that returns an FS that treats anything that doesn't match the globs as not existing. There could be one that uses a custom filtering function, too, but I'm not sure if that makes sense over just writing a custom filesystem wrapper to do it if necessary. It's not that hard and it'll give more control.

[seankhliao]

there's https://pkg.go.dev/io/fs#Glob

[DeedleFake]

But that just returns a []string of the matches. You can't use it to, for example, filter the contents of an fs.FS before handing it to something that itself wants an fs.FS.

[c9845]

I think we should just skip the "filtering files to zip" functionality altogether and just zip an entire fs.FS. This covers 99% of use cases and is typically what is done when you zip a directory in a desktop GUI.

If a user doesn't want certain files to be zipped, then they can manually write the code to filter and zip, or remove the file(s) from the directory to be zipped.