Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/mod/zip: CreateFromDir should not include .git #35372

Closed
jayconrod opened this issue Nov 5, 2019 · 3 comments
Closed

x/mod/zip: CreateFromDir should not include .git #35372

jayconrod opened this issue Nov 5, 2019 · 3 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unreleased

Comments

@jayconrod
Copy link
Contributor

Quoting @dmitshur's comment from CL 202042.

A mini experience report on CreateFromDir.

I read the docs for CreateFromDir, then used it in the most natural way I could think of by pointing it at a checked out repository I had on disk. It created a module zip file without errors. Since my goal was to ensure its output was identical to an algorithm I wrote previously, I noticed the zip was very large. It turned to be due to the ".git" directory being included, which wasn't my intention. I hope that "tools that build zip files" don't miss this.

I (probably) wouldn't have made the mistake of including .git directory if I used Create, because that requires an explicit list of files, and it's more clear all listed files are to be included/processed. However, it wasn't as clear when pointing to an existing directory on disk, given that CreateFromDir felt like a "high level" convenience wrapper.

I don't think there's any scenario where a .git directory should be included in a module zip file. CreateFromDir is indeed intended to be a convenience function, and including these directories significantly diminishes its value. We should exclude these, along with other VCS directory names.
@jayconrod jayconrod added the NeedsFix The path to resolution is known, but the work has not been done. label Nov 5, 2019
@jayconrod jayconrod added this to the Unreleased milestone Nov 5, 2019
@bcmills
Copy link
Contributor

bcmills commented Nov 5, 2019

We should exclude these, along with other VCS directory names.

One of the major use-cases for CreateFromDir is to support VCS systems otherwise unknown to the go command. Would it make sense for CreateFromDir to ignore, say, all top-level directories whose paths start with a leading dot?

@jayconrod
Copy link
Contributor Author

Would it make sense for CreateFromDir to ignore, say, all top-level directories whose paths start with a leading dot?

I think there are too many use cases where people put generated code or examples in directories that start with . or _. Omitting those would be more confusing than helpful.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/205837 mentions this issue: zip: exclude VCS directories from CreateFromDir

@golang golang locked and limited conversation to collaborators Nov 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
3 participants
@jayconrod @bcmills @gopherbot