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
x/exp/cmd/gorelease: ignore .gitignored files when compiling local .zip file #37413
Comments
@jayconrod We are currently using zip.CreateFromDir to create the zip file. Seems like this change should be in that function rather than gorelease, since gorelease doesn't really handle any of this logic. Does that SGTY?
@myitcv Could you elaborate as to how this relates to the zip issue? Feels a bit like a separate FR here, one which I'm not sure is a bit more gray area: I could certainly see the value in running gorelease on uncommitted work to see if you've made changes that will require major version bumps. |
I think reasoning at the time (and indeed still now) was that pseudoversions are defined in terms of commits, something that everyone has come to expect. If FWIW with FWIW this also has relevance as far as the "ignore" problem is concerned. |
Sorry for the slow responses! (This is a Friday project for me)
Could you elaborate further? One can certainly run
I suspect I'm not fully understanding your request, though - would love more information. 😃 |
@jadekler I'm worried about this adding too much complexity and incompatibility to This should probably be implemented in For reference, the |
Sorry for the delay, was on holiday.
I think my suggestion is perhaps more a function of an implementation restriction within |
Change https://golang.org/cl/330769 mentions this issue: |
Change https://golang.org/cl/341930 mentions this issue: |
Updates golang/go#37413 Change-Id: I5ea07a6e4eedc6cb215e4893944f1ab215ea8f2b Reviewed-on: https://go-review.googlesource.com/c/mod/+/330769 Trust: Jean de Klerk <deklerk@google.com> Trust: Jay Conrod <jayconrod@google.com> Run-TryBot: Jean de Klerk <deklerk@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
I don't think always ignoring files ignored by git is a good idea. It might be a great default, but I know people and projects where e.g. the files generated by protoc an such generators are not in the git repository. But those are necessary to publish the module as otherwise it would not compile. |
@nightlyone Is your concern that .gitignore-ed files would not be published in a module? Running |
@jadekler the Go code contained in the module might not even compile when excluding the files mentioned in the .gitignore file (e.g. foo.pb.go might be excluded, because the CI and developer should always generate them locally from foo.proro files). |
@nightlyone The approach we're taking here is to read out files that are checked into the Git repo at a particular commit. If files needed to build packages are not checked in, |
Change https://golang.org/cl/345730 mentions this issue: |
Add ErrUnrecognizedVCS, which allows calling functions (such as gorelease) to fallback: usually to CreateFromDir. Updates golang/go#37413 Change-Id: I846f72b1ce22bfc699e8cd83b28ea4529e73d6e9 Reviewed-on: https://go-review.googlesource.com/c/mod/+/345730 Trust: Jean de Klerk <deklerk@google.com> Run-TryBot: Jean de Klerk <deklerk@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
@myitcv This involved quite a lot of code. I'm fairly sure this WAI now, but if you have some time, would love a double check things work as you expect. =) |
@jadekler thanks for the ping on this! Looks like everything is working as expected, thanks! |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
in a checkout of
govim
which is ~8MB in size at the time of writing.What did you expect to see?
A comparison with
v0.0.29
to be presentedWhat did you see instead?
It appears that
gorelease
is including.gitignore
-d files when creating a local zip file. It should almost certainly ignore those. Indeedgorelease
should probably fail unlessgit status -porcelain
is clean to avoid any ambiguity.cc @jayconrod
The text was updated successfully, but these errors were encountered: