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

feat: use the new Precompile in gnodev and in the addpkg/execution flow #126

Merged
merged 17 commits into from
Apr 26, 2022
Merged

feat: use the new Precompile in gnodev and in the addpkg/execution flow #126

merged 17 commits into from
Apr 26, 2022

Conversation

moul
Copy link
Member

@moul moul commented Apr 20, 2022 •
edited
Loading

placeholder PR, will be updated soon

  • gnodev tries to compile the translated .go code using go.
  • bootstrap a simple stdshim package.
  • move maths to gno.land/r/maths
  • Precompiler tries to compile the translated .go code using go, before proceeding.
  • way to ensure the go version we're checking against, for distributed determinism.

Note: I needed to fill the examples/go.mod to make go build do the work. I think that as soon as the PR will be merged, then we could rely entirely on standard go modules features.

Continues #119

@moul-bot moul-bot force-pushed the dev/moul/precompile-next branch from 68569af to 6e443f7 Compare April 21, 2022 16:19
@moul
Copy link
Member Author

moul commented Apr 21, 2022

proof that it works -> it detects things like "unused variables".

Screenshot Manfred Macbook 2022-04-21 at 18 21 52@2x

@moul-bot moul-bot force-pushed the dev/moul/precompile-next branch 2 times, most recently from 300d9e5 to 534bf95 Compare April 21, 2022 17:13
@moul
Copy link
Member Author

moul commented Apr 21, 2022

I've fixed all the issues I found except one about the maths package. I think that it shouldn't be placed in stdlibs but in p/maths instead, I'll make a dedicated commit for this and let me know if I can continue the PR or if I shouldn't do this. Thank you.

Screenshot Manfred Macbook 2022-04-21 at 19 13 36@2x

@moul-bot moul-bot force-pushed the dev/moul/precompile-next branch from d7e7619 to acea289 Compare April 21, 2022 17:46
@moul
Copy link
Member Author

moul commented Apr 21, 2022 •
edited
Loading

with the maths package moved, everything builds correctly with gnodev

Screenshot Manfred Macbook 2022-04-21 at 19 48 34@2x

@moul-bot moul-bot force-pushed the dev/moul/precompile-next branch from acea289 to e41af05 Compare April 22, 2022 08:11
jaekwon
jaekwon reviewed Apr 23, 2022
View reviewed changes
examples/go.mod
@@ -0,0 +1,25 @@
module github.com/gnolang/gno/examples
Copy link
Contributor

Choose a reason for hiding this comment

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

is this file needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right now yes, because of this -> https://github.com/gnolang/gno/pull/126/files/e41af05ad59b5ac45a8d192fdda32c52821a72bc#diff-cd249c062d77b4d6a5621756423850d0708ffbe4aae554ce33cdc5b908b0ee39R25

My goal is to make it unnecessary to have any go.mod, but I think that it will requires some adjustments and to merge all the precompile stuff to be able to use go mod’s cache effectively

FYI, I started a standalone repo to experience by myself Gno smart contracts from an external developer; my goal is to make it simpler over tie

Copy link
Contributor

Choose a reason for hiding this comment

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

the link is broken

@moul-bot moul-bot force-pushed the dev/moul/precompile-next branch from e41af05 to 0881b6d Compare April 23, 2022 06:59
@moul-bot moul-bot force-pushed the dev/moul/precompile-next branch from 0881b6d to d197586 Compare April 23, 2022 07:42
moul
moul commented Apr 23, 2022
View reviewed changes
nodes.go
@@ -1080,6 +1080,10 @@ func ReadMemPackage(dir string, pkgPath string) *std.MemPackage {
if file.IsDir() {
continue
}
// skip files starting with a dot.
if strings.HasPrefix(file.Name(), ".") {
Copy link
Member Author

Choose a reason for hiding this comment

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

Not really related to this PR, but I think it’s appropriate

@moul
Copy link
Member Author

moul commented Apr 23, 2022

@jaekwon I’ve refactored gnodev in a way I find pretty good today; and still a good base for improvements.

The simple precompile + syntax check is linked with the gnokey maketx addpkg; fyi, I know perform two different checks:

  • a simple one file-per-file that checks basically that gofmt is working; this check is fast and does not depend on imports
  • a more complete one that checks the full package (all .go in a folder) + run “go build”; this one is more complete, but in our case, it’s totally possible that the import change over time, or is not ready

I suggest to only enforce the simple gofmt check; and let the more complete one in gnokey build

@moul moul marked this pull request as ready for review April 23, 2022 22:36
@jaekwon jaekwon merged commit 540085d into gnolang:master Apr 26, 2022
@moul moul deleted the dev/moul/precompile-next branch July 5, 2022 09:16
@moul moul added this to the 🏗1️⃣ test1.gno.land milestone Oct 20, 2022
@moul moul self-assigned this Oct 11, 2023
@moul moul mentioned this pull request Oct 14, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaekwon jaekwon jaekwon left review comments

Assignees

@moul moul

Labels
None yet
Projects
😎 Manfred's Board
Status: ✅ Done
🚀 The Launch [DEPRECATED]
Archived in project
Milestone
🏗1️⃣ test1.gno.land
Development

Successfully merging this pull request may close these issues.
2 participants
@moul @jaekwon