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.

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

proposal: cmd/go: vendor: only copy files that the Go build system recognizes #67233

Open
ianlancetaylor opened this issue May 7, 2024 · 10 comments
Labels
Milestone

Comments

@ianlancetaylor
Copy link
Contributor

Proposal Details

Currently go mod vendor copies all files in a package except:

  • files whose name ends with _test.go
  • files named go.mod and go.sum
  • files whose names ends with .go and contain a go:build ignore constraint

This means that other random files are copied into the vendored directory. For example, the vendored golang.org/x/sys/unix in the main repo includes mkall.sh, mkerrors.sh, and README.md. There is no reason to vendor those files.

I propose that we change go mod vendor to only copy files that go build recognizes. This is files that end with .go, .c, .cxx, .swig, and so forth. Making this change would also require go mod vendor to look for go:embed directives and copy over all files that they mention (for example, the vendored files in GOROOT/src/cmd/vendor/github.com/google/pprof/internal/driver/html).

@seankhliao
Copy link
Member

given #26366, isn't it just a bug that cmd/go copies more than necessary now?

@ianlancetaylor
Copy link
Contributor Author

Maybe? I wrote this as a proposal because it's clearly a user-visible change. But if we want to just call the current behavior a bug, that is fine with me.

CC @matloob

@bjorndm

This comment was marked as off-topic.

@matloob
Copy link
Contributor

matloob commented May 8, 2024

cc @samthanawalla

I think there are some subtleties here that make it more than just a bug. For instance, in the linked issue, #26366 (comment) there's a user who wants to make sure that the LICENSE files are copied in the vendor directory.

@bjorndm

This comment was marked as off-topic.

@ianlancetaylor
Copy link
Contributor Author

@bjorndm I'm suggesting that we copy all files that the Go build system recognizes, which includses all files with extensions .c and .h.

@matloob We already copy all files that start with metaPrefixes, including LICENSE; I don't think that should change.

@bjorndm

This comment was marked as off-topic.

@matloob
Copy link
Contributor

matloob commented May 8, 2024

@ianlancetaylor Ah, got it.

@ianlancetaylor
Copy link
Contributor Author

@bjorndm Are you saying that that does not work today? That seems like a separate issue from this one.

@bjorndm
Copy link

bjorndm commented May 8, 2024

@ianlancetaylor The file I linked above is an example of the hack that is necessary to get go mod vendor to keep C and H files in subdirectories. I'm here to make sure that this hack keeps on working at least. But you are right, not needing this hack is a separate issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Incoming
Development

No branches or pull requests

5 participants