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

fix cd gnovm; make imports #3577

Closed
moul opened this issue Jan 21, 2025 · 3 comments
Closed

fix cd gnovm; make imports #3577

moul opened this issue Jan 21, 2025 · 3 comments
Labels
🐞 bug Something isn't working 📦 🤖 gnovm Issues or PRs gnovm related

Comments

@moul
Copy link
Member

moul commented Jan 21, 2025

#3455 introduced a bug when we run cd gnovm; make imports.

21:39:15 ~  cd go/src/github.com/gnolang/gno/gnovm
21:39:20 gno/gnovm  make imports
go run -modfile ../misc/devdeps/go.mod golang.org/x/tools/cmd/goimports -w .
pkg/gnolang/testdata/corpra/parsefile/a.go:5:1: expected declaration, found ')'
pkg/gnolang/testdata/corpra/parsefile/bug_3014_redefine.go:2:1: illegal character U+FFFC ''
pkg/gnolang/testdata/corpra/parsefile/bug_3014_redefine2.go:2:1: illegal character U+FFFC ''
pkg/gnolang/testdata/corpra/parsefile/bug_3014_redefine3.go:2:1: illegal character U+FFFC ''
pkg/gnolang/testdata/corpra/parsefile/bug_3014_redefine4.go:2:1: illegal character U+FFFC ''
pkg/gnolang/testdata/corpra/parsefile/bug_3014_redefine5.go:2:1: illegal character U+FFFC ''
pkg/gnolang/testdata/corpra/parsefile/bug_3014_redefine6.go:2:1: illegal character U+FFFC ''
exit status 2
make: *** [Makefile:62: imports] Error 1

We need to fix the make imports command to ignore those intentionally invalid files or move them elsewhere so that they are excluded.

Additionally, we need to ensure that the CI checks for make imports result in success, similar to how we handle make fmt.

@moul moul mentioned this issue Jan 21, 2025
2 tasks
@Kouteki Kouteki moved this from Triage to Todo in 🧙‍♂️gno.land core team Jan 22, 2025
@Kouteki Kouteki added 🐞 bug Something isn't working 📦 🤖 gnovm Issues or PRs gnovm related in focus labels Jan 22, 2025
@thehowl
Copy link
Member

thehowl commented Jan 23, 2025

testdata is normally excluded by go tools, so it's weird it's caught by goimports.

@thehowl
Copy link
Member

thehowl commented Jan 23, 2025

Maybe we can turn it into some kind of go list -f '{{ join .BuildFiles "\n" }}' | xargs goimports -w

@moul
Copy link
Member Author

moul commented Jan 28, 2025

Fixed in #3602

We will close this issue until we encounter a similar problem in the future.

@moul moul closed this as completed Jan 28, 2025
moul added a commit that referenced this issue Feb 9, 2025
for the context, i was trying to fix these unrelated PR diffs:
https://github.com/gnolang/gno/pull/3176/files#diff-adbe9861bf11ff27b97aafd5a5e8bbe30dca0474c3008cd066f7fd8863b8b3d0L1,
then I noticed #3577.

This PR:
- reformats the generated files
- updates the makefiles to ensure that `make generate` will also format
the generated files
- updates the CI to check for both formatted and generated files
- [x] Depends on #3577 
- [x] Depends on #3584

---------

Signed-off-by: moul <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working 📦 🤖 gnovm Issues or PRs gnovm related
Projects
Development

No branches or pull requests

3 participants