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

feat: precompile imports and fix gnodev test #431

Merged
merged 9 commits into from
Jan 11, 2023

Conversation

harry-hov
Copy link
Contributor

@harry-hov harry-hov commented Dec 13, 2022

Changes

  • Now gnodev precompile and gnodev test --precompile support precompiling imports of the input file or package.

    • gnodev precompile: It is enabled by default. You need to pass addition flag --skip-imports to disable it.
    • gnodev test --precompile precompile imports automatically.
  • It also fixes the flag --output flag in gnodev precompile. Currently, it assumes that the output dir contains the pkg path. Which results in an error.

  • Fix error cannot find package... while running gnodev test by creating temporary go.mod file in the temporary root directory.

  • Try to find the nearest go.mod file, run go build in the same folder.

  • Currently, stdlibs/stdshim contains generated *.gen.go files. While other packages in the stdlibs contains *.gno files. Replaced generated files with *.gno files.

How has this been tested?

go run ./cmd/gnodev precompile ./examples/gno.land/p/demo/acl && go run ./cmd/gnodev build ./examples/gno.land/p/demo/acl

The above command precompiles acl and its imports.

Addresses #415

@harry-hov harry-hov changed the title feat: precompile imports and fix gnodev test [WIP] feat: precompile imports and fix gnodev test Dec 13, 2022
@harry-hov harry-hov force-pushed the hariom/precompile-imports branch 2 times, most recently from 45aa668 to 517941f Compare December 15, 2022 14:02
@harry-hov harry-hov marked this pull request as ready for review December 15, 2022 14:15
@harry-hov harry-hov changed the title [WIP] feat: precompile imports and fix gnodev test feat: precompile imports and fix gnodev test Dec 15, 2022
@harry-hov harry-hov self-assigned this Dec 16, 2022
@moul moul removed the enhancement label Dec 16, 2022
Copy link
Member

@zivkovicmilos zivkovicmilos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've left a few comments that need to be addressed, otherwise I think it looks alright 💯

I would've loved to have had more context in the PR description as to what this was fixing, and how it fixed the problem.

The PR didn't include a unit test that verified the problem and the fix, and also didn't include unit tests for introduced logic flows and helpers - we should start focusing on preventing future problems with good coverage

cmd/gnodev/precompile.go Outdated Show resolved Hide resolved
cmd/gnodev/precompile.go Outdated Show resolved Hide resolved
cmd/gnodev/precompile.go Show resolved Hide resolved
cmd/gnodev/util.go Outdated Show resolved Hide resolved
cmd/gnodev/util.go Outdated Show resolved Hide resolved
cmd/gnodev/precompile.go Outdated Show resolved Hide resolved
pkgs/gnolang/precompile.go Outdated Show resolved Hide resolved
pkgs/gnolang/precompile.go Outdated Show resolved Hide resolved
pkgs/gnolang/precompile.go Outdated Show resolved Hide resolved
pkgs/gnolang/precompile.go Show resolved Hide resolved
Now `gnodev precompile` and `gnodev test --precompile`
support precompiling imports of the input file or
package.

- `gnodev precompile`: It is enabled by default. You need to pass
addition flag `--skip-imports` to disable it.

- `gnodev test --precompile` precompile imports automatically.

Signed-off-by: Hariom Verma <hariom.verma@tendermint.com>
Fix error `cannot find package...` while running `gnodev test`
by creating temporary go.mod file in the temporary root directory.

Signed-off-by: Hariom Verma <hariom.verma@tendermint.com>
Try to find the nearest go.mod file, run `go build` in the same
folder.

Signed-off-by: Hariom Verma <hariom.verma@tendermint.com>
Currently, `stdlibs/stdshim` contains generated `*.gen.go` files.
While other packages in the stdlibs contains `*.gno` files. Lets
replace generated files with `*.gno` files.

Signed-off-by: Hariom Verma <hariom.verma@tendermint.com>
Signed-off-by: Hariom Verma <hariom.verma@tendermint.com>
@harry-hov harry-hov requested a review from a team as a code owner January 9, 2023 10:36
Copy link
Member

@zivkovicmilos zivkovicmilos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great 💯

Thank you for adding in the suggestions, I think it's much cleaner now

cmd/gnodev/precompile.go Outdated Show resolved Hide resolved
cmd/gnodev/precompile.go Outdated Show resolved Hide resolved
cmd/gnodev/precompile.go Outdated Show resolved Hide resolved
cmd/gnodev/precompile.go Outdated Show resolved Hide resolved
cmd/gnodev/util.go Outdated Show resolved Hide resolved
cmd/gnodev/util.go Outdated Show resolved Hide resolved
cmd/gnodev/precompile.go Outdated Show resolved Hide resolved
cmd/gnodev/test.go Outdated Show resolved Hide resolved
cmd/gnodev/precompile.go Outdated Show resolved Hide resolved
cmd/gnodev/precompile.go Outdated Show resolved Hide resolved
Signed-off-by: Hariom Verma <hariom.verma@tendermint.com>
Signed-off-by: Hariom Verma <hariom.verma@tendermint.com>
@harry-hov harry-hov requested review from moul and a team January 10, 2023 10:45
@moul moul merged commit 7c18c0d into gnolang:master Jan 11, 2023
@harry-hov harry-hov mentioned this pull request Jan 16, 2023
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

None yet

3 participants