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(gno.land/cmd): auto load and sort pkgs from examples #763

Merged
merged 30 commits into from
Jun 19, 2023

Conversation

harry-hov
Copy link
Contributor

@harry-hov harry-hov commented Apr 19, 2023

Description

This PR and add gno.mod file to every individual pkg inside examples dir. The gno.mod file's purpose is to specify each package's name and dependencies so that the cmd/gnoland tool can automatically load, sort, and add the packages when the chain starts.

Related #743


Note:

  • gno.land/p/demo/rand is marked as draft. Which means pkg gno.land/p/demo/rand and pkg that depends on it are not automatically included when the chain starts. The reason for this is gno.land/p/demo/rand depend on the math/rand package which is missing from stdlibs (see Add the deterministic math/rand helpers to stdlibs #744).

  • gno.land/r/demo/x/upgrade is also marked s draft. Will discuss it in a separate issue.

@harry-hov harry-hov changed the title feat(cmd/gnoland): makeGenesisDoc() auto load pkgs feat(gno.land/cmd): auto load pkgs from examples Apr 21, 2023
@harry-hov harry-hov marked this pull request as ready for review April 21, 2023 09:21
@harry-hov harry-hov requested a review from a team as a code owner April 21, 2023 09:21
@harry-hov harry-hov marked this pull request as draft April 21, 2023 10:39
@harry-hov harry-hov marked this pull request as ready for review April 21, 2023 10:45
@harry-hov harry-hov changed the title feat(gno.land/cmd): auto load pkgs from examples feat(gno.land/cmd): auto load and sort pkgs from examples Apr 21, 2023
gno.land/cmd/gnoland/main.go Outdated Show resolved Hide resolved
@moul
Copy link
Member

moul commented Apr 21, 2023

Can a solution be found to add gno.mod to invalid packages while providing an option to skip them? Asking users to create gno.mod after package creation is undesirable, as opposed to 'go' which begins with go mod init.

@harry-hov
Copy link
Contributor Author

harry-hov commented Apr 24, 2023

Can a solution be found to add gno.mod to invalid packages while providing an option to skip them?

What about marking module as Deprecated? and not include deprecated modules automatically

// Deprecated: does not supported.
module example.com/mod

or

Support something new. maybe Skip: (case-sensitive)?

// Skip: does not supported.
module example.com/mod

Edit: For supporting something new we need to use custom type. Currently we are using *modfile.Module (It contains Deprecated string)

@tbruyelle
Copy link
Contributor

@moul @harry-hov Sorry for the lack of context from me, but what's the point of this deprecated instruction ?

@moul
Copy link
Member

moul commented Apr 26, 2023

A way to exclude modules from automated publishing systems, such as auto-loading the ./examples folder from the main chain or using future features like gno mod publish. This metadata can be used elsewhere as well, but is likely to be infrequently used, primarily by the core team in the monorepo. Any alternative suggestions?

@tbruyelle
Copy link
Contributor

Currently as far as I know we don't automatically publish all the modules from the examples folder. When a new chain is loaded, there's only a couple of gentxs that publish some packages from examples. So potentially, if we want to exclude some of them, we can update the gentxs, and remove the one we want to deprecate.

About gno mod publish, a potential usecase would be to interrupt the command if one of the dependency of the target package is deprecated, but I'm not it's really useful. Is there any other more interesting usecase ?

So maybe I'm wrong but I'm not convinced about the usage of this new instruction.

@harry-hov
Copy link
Contributor Author

@tbruyelle Let me try to convince you with my understanding of the flow.

Currently(in master), we need to manually fill the list of pkgs that we want to include when the chain starts. here:

for _, path := range []string{
"p/demo/ufmt",
"p/demo/avl",
"p/demo/grc/exts",
"p/demo/grc/grc20",
"p/demo/grc/grc721",
"p/demo/grc/grc1155",
"p/demo/maths",
"p/demo/blog",
"r/demo/users",
"r/demo/foo20",
"r/demo/foo1155",
"r/demo/boards",
"r/demo/banktest",
"r/demo/types",
"r/demo/markdown_test",
"r/gnoland/blog",
"r/gnoland/faucet",
"r/system/validators",
"r/system/names",
"r/system/rewards",
"r/demo/deep/very/deep",
} {

It's not simple as adding the pkg to the list. You need to keep check of the dependency. IOW, in the list pkg[i] should not depend on pkg[j], if j > i.

This PR makes it automatic. It loads list of all the pkgs from the example dir (dir consists of gno.mod file is considered as pkg) and then sort them accordingly.

We want to include all the pkgs in the example dir, but there are few exception like gno.land/p/demo/rand, since it depends on math/rand and chain doesn't understand it(yet).

The original idea is not to have gno.mod file in pkgs that we don't want to include. But @moul suggested including gno.mod in all pkgs even if we want to ignore them for now (I agree with it).

Now we need to think of the way to ignore them and still have gno.mod file. I suggested marking pkg as deprecated.
If you have any better way to do this, I'm all ears.


I think it's good to support parsing deprecated even if we are not using it. It makes gno.mod more compatible with go.mod. And last I see no harm in supporting deprecated

phewww.

@tbruyelle
Copy link
Contributor

@harry-hov Thanks for the clarification, indeed I missed that part where the gnoland binary pushes all that list of packages. I understand well the usage now. A better name than deprecated would be good (like experimental ?), but since it's already supported by golang.org/x/mod let's go with that 🙏 .

gno.land/cmd/gnoland/main.go Outdated Show resolved Hide resolved
gno.land/cmd/gnoland/main.go Outdated Show resolved Hide resolved
gno.land/cmd/gnoland/main.go Outdated Show resolved Hide resolved
gno.land/cmd/gnoland/main.go Outdated Show resolved Hide resolved
gno.land/cmd/gnoland/main.go Outdated Show resolved Hide resolved
gno.land/cmd/gnoland/main.go Outdated Show resolved Hide resolved
@moul
Copy link
Member

moul commented Apr 28, 2023

@harry-hov Thanks for the clarification, indeed I missed that part where the gnoland binary pushes all that list of packages. I understand well the usage now. A better name than deprecated would be good (like experimental ?), but since it's already supported by golang.org/x/mod let's go with that 🙏 .

@tbruyelle we can still change the name to something better. What about something self-explaining like draft (hugo), unpublishable, restricted or private (NPM).

Edit: WIP in #790

@moul
Copy link
Member

moul commented May 26, 2023

cc @ilgooz would like your opinion on usage of gno.mod, please.

@zivkovicmilos zivkovicmilos requested review from zivkovicmilos and removed request for tbruyelle June 5, 2023 15:54
@thehowl
Copy link
Member

thehowl commented Jun 5, 2023

Apologies for the late reply; I'll try to some up some of my thoughts and try to sum up also what we discussed earlier today.

gno.mod will serve a purpose whether gno-pkg is part of official repo or exists as standalone repo. The purpose of having a gno.mod file is to ensure consistent behavior for gno-pkg, regardless of whether it is part of the official repository or exists as a standalone repository. By treating every gno-pkg the same, we create a seamless experience when moving internal packages to a standalone repository or making an external package official.

True; however it does make it more redundant to structure "programs" with more than one package; as is often the case for any reasonably complex codebase. While this is not currently the case for most gno programs, which generally have at most one package together with one realm, this eventuality should be considered. I think this also ties reasonably well with your issue on #852.

My thinking is that big refactorings where packages and directory structures are moved around are rare and not necessarily something we should look to facilitate; rather, it is a good structuring of the code that should be facilitated, into writing many individual useful components (packages). Thus, I think that ideally the publish/addpkg command should work on the "module" as a unit, and publish any individual sub-packages (or, in the case of realms, sub-realms) together.

When working on repositories outside monorepo, as we were discussing today, I think there are a couple of reasonable arrangements for the usual single pkg + realm configuration.

structure-1
├── package
│   └── gno.mod # module gno.land/p/structure-1
└── realm
    └── gno.mod # module gno.land/r/structure-1

structure-2
├── gno.mod # module gno.land/p/structure-2
└── realm
    └── gno.mod # module gno.land/r/structure-2

Structure 2 is also valid because, like in Go, I think our tooling should consider any subdirectory which has its own gno.mod as a "separate module" -- and thus not one to be concerned with, ie. not subpackage. We do the same in this monorepo (./misc/devdeps/go.mod). Of course, the realm and package could also be inverted; it depends on what the author thinks is most important to have as a top-level directory in the repository organisation. Note that the latter is kind of akin to some structures in Go repositories - think package in top-level dir and command in cmd/$name.

In the future, as we were discussing today, an idea floating around is that of doing away with the "r/" and "p/", and moving towards "directory extensions", ie. foo.realm and foo.pkg, for instance. Leaving aside the design per se, which I think is up to @moul to propose eventually, the transition to this structure makes it also possible to go from two gno.mods to one. (Though here it's also worth thinking if this is something we do want, mostly because realms and packages have different requirements in how they upgrade themselves and their dependencies -- see-also #694).

In any case, one of the crucial parts is that today gno.mod serves a primary purpose in first of all determining the full import path (= publish path) of the package/realm. I think this is very important as it allows work outside of the monorepo, and we should enrichen it with the possibility of structuring subdirectories; additionally making @/examples the same as everyone else (by just making it a module, with a top-level gno.mod file in @/examples/gno.land). But when we do get to a stage where we can implement versioning, I think it should only reason at a module-level; for instance, updating gno.land/p/morgan/foo necessarily updates gno.land/p/morgan/foo/bar, and most importantly if gno.land/p/morgan/foo calls .../foo/bar, it should never call a version in a different "module version".

It's one of the problem it fixes. I already mentioned my ulterior motive behind adding gno.mod in the root of every gno pkg.

Yes; but I do think the information is redundant, when the dependency graph can be determined just as easily by static code analysis. In other words, it does solve the issue, but I think the better way is understanding package dependencies through the AST rather than gno.mod. This also makes sense long-term, as we want to support multi-pkg modules, as gno.mod should only declare the individual modules, not subpackages.

I'm with moving pkgs that we don't support yet

Just reporting on this as we discussed it today with @moul, but I think a reasonable idea that he had was that of having only these packages with a go.mod, and making the function generating the loading order for gnoland stop at module boundaries. If we do this, I think that for clarity of any observers, we should add the following in the readmes:

This package/realm is currently not correctly loaded by gno.land; a gno.mod file has been added to avoid it from being loaded by the gno.land binary. Should the missing functionality of this package be implemented in the VM, the package should be "re-enabled" by deleting the gno.mod file.

Apologies for the long reply, but I do hope it's exhaustive. :)

@thehowl
Copy link
Member

thehowl commented Jun 5, 2023

One more note we were discussing; if we add AST analysis for import dependencies, as @moul was suggesting, this should (either in this or a future PR) be also public through either a gno mod why command or a gno list - similarly to their counterparts in Go, which can display info about dependency graphs or the whole dependency tree.

Hopefully this also helps in the usecase @ilgooz would have for having a gno.mod in each package directory?

@thehowl
Copy link
Member

thehowl commented Jun 6, 2023

I've had a call with Hariom to try to gather insight on each other's point of view. I think there are two main approaches we could go with and I'm slightly leaning towards Hariom's idea now:

(1) 🚀 one single gno.mod in examples/gno.land/gno.mod

Pros:

  • Simplified dependencies while examples/ is in development and not finalized: no need to specify module dependencies of other gno.land packages in gno.mod
  • examples/gno.land directory becomes less "special" than it is now (although, see below)

Cons:

  • Ideally, publishing a module would "block" an entire namespace (if I publish gno.land/p/foo, no-one else can publish module gno.land/p/foo/bar, because that would disallow me from creating sub-pkg bar). So at the current state of things, the behaviour of the examples/gno.land directory would still be "exceptional" as it is published module name-space where other people are expected to publish modules.
  • For this reason, the approach would inevitably have to be changed before mainnet and in no way permanent

(2) 👀 one gno.mod for each (future) module

Pros:

  • possibility of restructuring examples per-module, rather than having to organise directories in a tree with their import path (GOPATH-style)
  • and also of seamlessly "spinning off" packages and realms as their own repositories (without the "namespace blocking" issue above)
  • examples/ becomes just a collection of modules which "just so happen" to be published at gno.land startup, ie. basically not special at all

Cons:

  • very annoying and redundant module dependency specification in gno.mod (solvable with gno mod tidy)
  • bureaucracy for specifying import path when it's already in the directory structure (but could be used to our advantage if we want to restructure examples/)
  • for all packages and realms in monorepo, we'd actually just want to always rely on the latest version, even though this is something highly undesirable in normal contexts (IIRC gomod doesn't even have a concept of using the "latest" version). But this might be a secondary problem we'd want to discuss at the point where we actually need to introduce versioning.

I think one of the significant drawbacks of (1) is that it is still a temporary and transitory measure -- which we can understand and recognise as transitory even now, while (2) in many ways is already looking more realistically at how we'll want to be working with gno code and gno modules -- or at least, our current idea and understanding of it.

Notwithstanding, I think that (2) has mostly some usability concerns which I mentioned in the cons, so I think that if we do go with (2), after merging this PR we should give priority to go mod tidy, as it simplifies contributor workflow and makes management of go.mod not a bureaucratic but an automatic task.

Secondly, I think it might be nice to restructure examples in a way that improves exploration by repository visitors. This is highly subjective and one might argue that the current structure is already doing a great job; however I think it has some directories of indirection, and one improved organization could be of having direct "subprojects" under examples (say examples/groups and examples/groups/realm, for an example reorganization of p/demo/groups and r/demo/groups).

Further comments and takes on the topic are welcome; please also cast a vote with a 🚀 or 👀 reaction on this comment to see if we're strongly leaning towards one of the two options :)

@ilgooz
Copy link
Contributor

ilgooz commented Jun 6, 2023

For the IDE, we like the 2nd approach better because it helps us to define realms and packages, visualize and manage them.

The 1st approach seems to be same as with how go mod works for Go. It gives you the flexibility to only create a go.mod when needed (usually when want to version and release them separately). While I agree that this approach suits well Go, it may not necessarily be the best option for Gno.

Packages and realms in Gno is "deployable" and they will always have independent versions, they are slightly different than how standard Go packages are organized.

In Go, packages are only reusable libraries and they do not need to have a separate go.mod unless you want to version them separately and release (git-tag) them independently. But in Gno, you always need to version and deploy every package by default. Same applies to realms too.

I believe that everything that is deployable should have its own gno.mod file. Your realm and package can have some sub directories for code organization, and those dirs don’t need a gno.mod (because they are not deployable alone).

@moul
Copy link
Member

moul commented Jun 8, 2023

TL;DR:

Each realm and package (r/ & p/) should have its own gno.mod file for autonomy. Libraries can be developed separately in p/ with their own gno.mod file or as subfolders within realms (recommended?). gno.mod files are for local development, not chains. Later, simple packages may skip gno.mod in favor of import path comments. Starting with mandatory gno.mod files for realms and packages is recommended.


Detailed:

  1. Requiring clear identification from source: It is essential for each package and realm to have a clear way of identifying the package. This starts with the requirement of a gno.mod file, which provides additional options for specifying dependencies and configurations.

  2. Import path in comment: In the future, a more concise alternative to gno.mod for simpler projects could be using a comment line on the right side of the package declaration (package foobar // import "gno.land/r/foobar") to indicate the import path. Encouraging the use of a doc.gno file for high-level documentation and configuring comments is also suggested. This approach facilitates sharing code snippets in a tweet, gist, or slide-friendly manner.

  3. Removing -pkgpath: The -pkgpath option should be removed as it becomes unnecessary with the requirement of gno.mod or import paths for packages and realms.

  4. Failing addpkg without gno.mod or import path: To ensure proper package identification, the addpkg command should fail if a package or realm is added without a corresponding gno.mod file or import path. This prevents uploading packages without clear identification.

  5. gno.mod off-chain: gno.mod files are primarily intended for local development and are not required on-chain. They serve to manage dependencies and provide configuration options. Once uploaded to the chain, the import paths are known, rendering the gno.mod file unnecessary.

  6. Subfolders for packages: Encouraging the use of subfolders within realms, particularly for utility packages or libraries with states, helps organize the codebase and improves code readability.

These suggestions aim to provide clarity in package and realm identification, enhance the developer experience, and maintain consistency within the gno.land ecosystem.

@harry-hov
Copy link
Contributor Author

harry-hov commented Jun 8, 2023

  1. Import path in comment: In the future, a more concise alternative to gno.mod for simpler projects could be using a comment line on the right side of the package declaration (package foobar // import "gno.land/r/foobar") to indicate the import path. Encouraging the use of a doc.gno file for high-level documentation and configuring comments is also suggested. This approach facilitates sharing code snippets in a tweet, gist, or slide-friendly manner.

I have 2 questions here:

  • what do mean by "simpler projects"?
  • Why "Import path in comment"? Why not just use the module name from gno.mod file as an import path for every package?
  1. gno.mod off-chain: gno.mod files are primarily intended for local development and are not required on-chain. They serve to manage dependencies and provide configuration options. Once uploaded to the chain, the import paths are known, rendering the gno.mod file unnecessary.

Did you mean: Just publish *.gno files on chain and ignore gno.mod file? If yes,

I believe it would be beneficial to have a gno.mod file on the chain. Gno developers would appreciate being able to view the dependencies and their respective versions before utilizing or importing any on-chain packages. Typically, developers would look for a gno.mod file for this information.

Another concern I have is how we will manage versioning without a gno.mod file on the chain?

@moul
Copy link
Member

moul commented Jun 8, 2023

what do mean by "simpler projects"?

A project not using the replace directive.

Why "Import path in comment"? Why not just use the module name from gno.mod file as an import path for every package?

The "Import path in comment" approach is suitable for simpler packages where the use of a gno.mod file may not be necessary. It allows for the proposal of one-file contracts that can be easily reviewed, shared, tweeted, or used in presentations. The idea is to have a simplified import path that is self-contained within the code file, without the need for a separate gno.mod file.

The use of gno.mod is more beneficial for complex repositories that consist of multiple independent modules, such as multiple realms or packages. In such cases, the replace directive becomes necessary. However, gno.mod files are local-first and should be skipped from the chain to avoid publishing outdated or incorrect information.

While I suggest implementing the import comment as an alternative to gno.mod in the future, for now, it is recommended to make gno.mod files mandatory.

I believe it would be beneficial to have a gno.mod file on the chain. Gno developers would appreciate being able to view the dependencies and their respective versions before utilizing or importing any on-chain packages. Typically, developers would look for a gno.mod file for this information.

Having a gno.mod file on the chain may not be practical or meaningful once the code is deployed on-chain. The replace directive within gno.mod loses its significance in an on-chain context. Furthermore, if the import path can change due to versioning, it would require patching the .gno files after uploading to ensure deterministic import paths.

By default, a published gno.mod file is considered false, and over time it becomes even less accurate. Therefore, it is recommended to skip gno.mod during addpkg, and we should continue to use it primarily for local development and related purposes.

Edit: I will provide guidance on managing versioning in a safe manner. It's important to note that the concept of "latest" is only applicable during local development. Once code is published, if a "latest" alias was used, it is recommended to either replace it with specific linked versions or annotate the .gno files (patch) to ensure precise version references. This ensures accurate and reliable dependency management.

@thehowl
Copy link
Member

thehowl commented Jun 12, 2023

@ilgooz But in Gno, you always need to version and deploy every package by default.

Doesn't have to be this way -- addpkg, as far as I know, potentially supports having sub-directories. And I think it makes sense for an entire module to be published on chain at once; including its subpackages.

(because they are not deployable alone)

I think in some cases they could be deployable alone, this is not the "true" distinction I would argue. It's more about what makes sense, from an author's perspective, to be published together in a single module and to have all of the dependencies grouped together; as well as have the dependants update them simultaneously.

@moul gno.mod files are for local development, not chains.

Didn't we agree that -- maybe eventually -- but for dependency management (ie. version locking), it's important for chains to consider the gno.mod dependencies?

It allows for the proposal of one-file contracts that can be easily reviewed, shared, tweeted, or used in presentations. The idea is to have a simplified import path that is self-contained within the code file, without the need for a separate gno.mod file.

I know this is thinking ahead to an issue we'll need to tackle further down the line; does this imply, however, that these "simpler" packages have all of their dependencies always at the latest version? Or maybe "latest-compatible"?


Coming back to the PR at hand; I think that if we agree to have several gno.mods in examples/, the only code change to have in this case would be to make sure that the package loader can properly support loading sub-packages of the modules in the correct order.

@moul
Copy link
Member

moul commented Jun 12, 2023

Doesn't have to be this way -- addpkg, as far as I know, potentially supports having sub-directories. And I think it makes sense for an entire module to be published on chain at once; including its subpackages.
I think in some cases they could be deployable alone, this is not the "true" distinction I would argue. It's more about what makes sense, from an author's perspective, to be published together in a single module and to have all of the dependencies grouped together; as well as have the dependants update them simultaneously.

Yep, having one gno.mod for a single module makes sense. The module can be small, big, or have subfolders. However, we shouldn't have a gno.mod per subfolder.

For two independent realms, whether they are small or big, they should always have independent addpkg commands. When it comes to pure packages (p/), it's a good practice to keep them small and focused, with their own dedicated gno.mod.

As we develop better indexers and package explorers that showcase realms as applications in an app store and pure packages like npm, the developer perspective will adapt. Subfolders won't be something people generally discover, but rather something they will come across while exploring a specific module.

Didn't we agree that -- maybe eventually -- but for dependency management (ie. version locking), it's important for chains to consider the gno.mod dependencies?

I suggest implementing the patching of .gno files at the addpkg stage to specify the unspecified versions.

The reason behind my push for this approach is that, once a module is addpkged, the gno.mod file can either be incorrect or differ between the local and remote development environments. This inconsistency goes against the principles of simplicity and obviousness. I believe in the importance of having self-explanatory, copy-pastable code. Also, what about documentation if a gno.mod has a different usage and sense when onchain or locally. I think that skipping gno.mod files during addpkg would make things simpler. Please feel free to share your thoughts, insights, and examples.

I know this is thinking ahead to an issue we'll need to tackle further down the line; does this imply, however, that these "simpler" packages have all of their dependencies always at the latest version? Or maybe "latest-compatible"?

How about using "latest" as the default version when unspecified (in repositories, snippets, discussions)? Then, users can manually fix the version in .gno imports or gno.mod locally. When on-chain, contracts are always explicitly specified, which can be achieved thanks to auto-patching.

Coming back to the PR at hand; I think that if we agree to have several gno.mods in examples/, the only code change to have in this case would be to make sure that the package loader can properly support loading sub-packages of the modules in the correct order.

Yes 👍

@thehowl thehowl merged commit 24770d5 into gnolang:master Jun 19, 2023
Doozers pushed a commit to Doozers/gno that referenced this pull request Aug 31, 2023
* feat: add gno.mod to each package

* feat: add logic to load and sort gno pkgs

* feat(gno.land/cmd): auto load pkgs from examples

* fix failing tests

* Use more concise if statement

* Move pkg loading and sorting logic to gnomod

* Fix review comments

* Fix gno.mod files

* Extract visit() as named func

* Skip draft or depends on draft

* s/v0.0.0/v0.0.0-latest

* Fix `ListPkgs()`

* Test `ListPkgs()`

* oversight: remove print statement

* Export `Pkg` and add field `draft`

* Refactor and improvements

* make fmt

* ListPkgs(): don't skip nested pkgs

* Add gno.mod files to `tests/subtests`

* No need to subtests manually
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 ⛰️ gno.land Issues or PRs gno.land package related 📦 🤖 gnovm Issues or PRs gnovm related 🧾 package/realm Tag used for new Realms or Packages.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

7 participants