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

rfc: the future of gno.mod #2904

Open
thehowl opened this issue Oct 4, 2024 · 18 comments
Open

rfc: the future of gno.mod #2904

thehowl opened this issue Oct 4, 2024 · 18 comments
Labels
gno-proposal Gnolang change proposals

Comments

@thehowl
Copy link
Member

thehowl commented Oct 4, 2024

There's been some misalignments in the vision for what we want to do with gno.mod. I think it makes sense here to attempt to align on what vision we see on it, especially for the short, practical term; ie. ahead of the mainnet launch. I think there's some outdated and redundant functionality that can be removed; some critical that should be added. But in general I think we need to agree on where want to go with it in a pre-versioning world (note: I've closed #694 in light of many discussions within the core team; we decided to delay any considerations on built-in upgrade mechanisms for realm to after a point where we've acquired some experience with upgrading the chain and its contracts.)

What should be cut

  • require. This is a redundant feature; it's not computationally complex to compute the Gno dependencies of gno files with static analysis, and it creates discrepancies between a package's actual dependencies (through import statements - the "source of truth) and what exists in gno.mod, often just generated with gno mod tidy. We don't have versioning, and won't for another while, so the version field saying v0.0.0-latest is absolutely redundant.
  • as a consequence, gno mod why is not useful - or at least not as part of the gno mod suite.
  • Similarly, gno mod tidy is no longer necessary.

What should be added

In essence, gno.mod can be a good tool right now to be developed in conjunction with a better "importer" module: #2201.

The purpose of the importer PR is to create a centralized place to resolve import paths to specific files. At the moment, how this is done is scattered all throughout the codebase, and there are different implementations for how this works, for instance in gno doc and gno run. Once we unify this behaviour, we can make the importer understand gno.mod and become useful to us, enabling some crucial workflows:

  • Supporting working outside the monorepo.
  • Supporting working on multiple testnets.

The first workflow is already partially supported. Outside of the monorepo, we can use gno.mod to automatically set the import paths for gnodev. Plus, I think it may be partially supported for gno test - though this matters little. The problem is that any imports downloaded with gno mod download are still not resolved (except by gno doc, lol).

Hence why, after we unify this, here are other things I see as features we should have for gno mod, especially in conjunction with correct resolution into the packages downloaded in ~/.config/gno/pkg/mod:

  • gno mod download v2:
    • Support -u to update packages - this makes sure we can support the portal loop.
    • Support downloading from different testnets; for instance, gno mod download test4.gno.land/p/demo/avl will download the test4-specific version of p/demo/avl.
    • Note: importer should understand test4.gno.land/ as an import path - considerations to be done here on how this should be handled on-chain.
  • replace
    • See-also [gno.mod] Add replace #2601
    • Essentially, this should support both replaces in local directories, like replace gno.land/p/demo/boards => ./boards
    • But also imports on testnets: replace gno.land/p/demo/avl => test4.gno.land/p/demo/avl.
    • Possibly on entire sets of paths: replace gno.land/... => test4.gno.land/....
  • Make gno mod not required. Make gnomod optional everywhere #1039
    • possibly support the syntax // import "gno.land/r/demo/myrealm" as a way to short-hand set an import for the current directory; an implicit "gno.mod".
  • Support sub-packages within the directory in the import as well: if directory morgan has a gno.mod with module gno.land/r/morgan, then sub-directory morgan/home should have import path gno.land/r/morgan/home. (Circles back to Gnopher hole: Module VS Pkg VS Path VS Dir VS SubPkg VS SubModule VS SubDir VS SubPath ... #1024 )

This is a request-for-comment; I want to gather whether we agree that this is a good way forward for now, so we can point to this plan.

@thehowl
Copy link
Member Author

thehowl commented Oct 4, 2024

I asked @n0izn0iz to consider working on the importer package, given that he's already proven effective at implementing something similar for a "port" of gopls to gno.

I really think unifying it can be good for both our internal tools and for all external tools, aside from development in general. After that, we can work on better support gno.mod for the most critical workflows we face today and that we currently have to "bodge" with scripts to get working properly.

Let me know what you think.

@moul
Copy link
Member

moul commented Oct 4, 2024

Thank you very much; addressing this clarifying issue was on my to-do list. 🙏

Overall, it looks good.

For the replace directive, I recommend using only local paths. I anticipate that this directive will be used for temporary local work, which gnokey addpkg completely ignores. This means that when you publish on the mainnet, either your local dependency is missing, causing an error, or it is already present (your local version may include some printf debugging). Allowing different testnets could create ambiguousness issues, such as a successful addpkg that behaves differently on another network.

I recommend that we do not support downloading packages prefixed with gno.land from test4.gno.land. We should only allow downloading packages prefixed with test4.gno.land. We need to start using testnets with the full prefix of test4.gno.land. While gno.land packages can exist on a testnet, they should be imported during gnoland start and, in the future, through IBC. However, you cannot use addpkg gno.land/xxx on test4; you can only use addpkg test4.gno.land/xxx. Packages from test4.gno.land can import any available package, even from a different domain.

I think we should primarily allow replace to provide a local alternative to a package. If you’re not replacing it with something local, there should be no way to have a different representation of the same package recognized by its name.

Everything else sounds great to me. Thank you again.

@thehowl
Copy link
Member Author

thehowl commented Oct 5, 2024

@moul if the suggested behaviour is to use different domains on-chain for different testnets, I think we should provide some tool to replace all imports from an import path to another. Or else, how do you think we should handle developing outside of the repo, and wanting to target both portal loop and test4, for example?

@moul
Copy link
Member

moul commented Oct 5, 2024

When we look to the future with IBC-powered transfers of libraries, imports will remain unchanged from portal loop and test4. If you want "gno.land/p/avl," it can be made available everywhere with this import, not just on portal loop.

Ultimately, the only change will be the current addpkg path, as you may want to upload the same contract on two different chains; today this information is only available in ... gno.mod :).

The tools you're suggesting will exist. I believe it’s wise not to rush them from the core team's perspective; perhaps we should just propose some specifications.

My main point is to emphasize that "an import path IS a contract," and this should apply universally around the world. We may encounter some limits, but for now, I support making this a universal rule as much as possible.

I also want to avoid any scripts that could produce the same "source code" leading to "different contracts." However, that will happen. I just want to discourage it a bit.

Testnets are definitely a special case (local gnodev too) to consider, which is why I'm thinking about implementing strong exceptions for them. However, I want to avoid situations where you can have "something that works without knowing why." I would prefer to have more "things that fail without knowing why" than the opposite.

@moul moul moved this from 📥 Inbox to 🎯 Current Topics in 😎 Manfred's Board Oct 5, 2024
@moul moul moved this from 🎯 Current Topics to 👀 Watching in 😎 Manfred's Board Oct 5, 2024
@moul
Copy link
Member

moul commented Oct 6, 2024

Checkout this related PR: #2911

@ajnavarro ajnavarro added the gno-proposal Gnolang change proposals label Oct 7, 2024
@ajnavarro
Copy link
Contributor

The rfc looks great, just a couple of requests for clarification:

Support -u to update packages - this makes sure we can support the portal loop.

Can you elaborate on this? I'm not sure what you meant here.

possibly support the syntax // import "gno.land/r/demo/myrealm" as a way to short-hand set an import for the current directory; an implicit "gno.mod".

With that you mean adding the comment // import "gno.land/r/demo/myrealm" on top of all package expressions?

@leohhhn
Copy link
Contributor

leohhhn commented Oct 8, 2024

Similarly, gno mod tidy is no longer necessary.

💯 its very annoying to have to do it every time for gnodev :)

The only consideration point I have is: how will gnodev know where to deploy a package?
// import "gno.land/r/demo/myrealm" seems like a workaround / people will forget to do it. Then, the only way so specify the correct path is as a flag to addpkg.

So, if we do introduce other features, such as replace, I think keeping module gno.land/r/xyz is useful.

@thehowl
Copy link
Member Author

thehowl commented Oct 8, 2024

@ajnavarro: Can you elaborate on this? I'm not sure what you meant here.

IIRC, the current implementation of download assumes packages are immutable, and as such never updates them if they already exist in a local cache.

But, on the portal loop, packages are indeed "mutable", because p/demo/avl can change if we change its source here on github.

So we need to support "forcing" an update to all dependencies.

With that you mean adding the comment // import "gno.land/r/demo/myrealm" on top of all package expressions?

No, just one. We can reject having multiple of them. This extends the existing feature of Go.

@leohhhn:
The only consideration point I have is: how will gnodev know where to deploy a package?
// import "gno.land/r/demo/myrealm" seems like a workaround / people will forget to do it. Then, the only way so specify the correct path is as a flag to addpkg.

Most users will still use gno.mod with the module declaration, as we are currently using. But we want to support a "simplified" use-case, where you can write a single-file realm, for instance, which can be shared without having to share an associated gno.mod.

I don't know what behaviour gnodev currently has when a package doesn't have a gnodev. Imo, if gnodev has a path which has no gno.mod or // import comment, then it can reject the package.

And yeah, the module declaration is not going away, don't worry ;)

@leohhhn
Copy link
Contributor

leohhhn commented Oct 8, 2024

I don't know what behaviour gnodev currently has when a package doesn't have a gnodev. Imo, if gnodev has a path which > has no gno.mod or // import comment, then it can reject the package.

For all packages it loads, the imports must match that package's requires. gnodev screams with red otherwise, while it doesn't/shouldn't have to be the case. cc @gfanton

I guess I was mainly worried about making gno.mod optional; in that case, no gno.mod => // import is required

@n0izn0iz
Copy link
Contributor

n0izn0iz commented Oct 11, 2024

I have a dilemma in my implementation of import #2932:

  • Follow go logic: don't allow to run commands outside of a gno module (a path that contains a gno.mod in it or in one of it's parent). This will break current usage in gno monorepo and also ignore "sub-modules" (aka sub-directories with a gno.mod)
  • Follow current usage in gno monorepo: allow to run commands like gno test ./... at a monorepo root without a root gno.mod

Following current usage would make some things weird, for example, running gno test ./... correctly uses local packages as dependency but running inside a module like gno test ./r/someorg/somepkg would try to download all dependencies from portal-loop instead

I think we should follow go logic and then either put a single gno.mod at packages root (usual go monorepo pattern) or support workspace definition.

wdyt? hope this is clear ^^

@thehowl
Copy link
Member Author

thehowl commented Oct 21, 2024

Mhh. What do you think should happen if I want to enable a workflow like a repo having:

  • package dir with module gno.land/p/demo/blog
  • realm dir with moddule gno.land/r/demo/blog and a replace on gno.land/p/demo/blog => ../package

Shouldn't I be able to do gno test -v ./...? How do you propose we support this workflow? (Or similar alternative compatible with Go's "module-boundary" behaviour).

@n0izn0iz
Copy link
Contributor

in this case I think we should support workspaces via gno.work file

@thehowl
Copy link
Member Author

thehowl commented Oct 21, 2024

gno.work seems even more work to try to maintain.

I tend towards ./... also includes other modules, but each package is tested and executed depending on the context laid out by its relative gno.mod - so gno test can actually test in different "contexts" if the gno.mods are different.

Of course, this is with the exception of require -> for instance if I require a package in a local dir from my realm, it is imported in the context of the package I'm testing.

@n0izn0iz
Copy link
Contributor

n0izn0iz commented Oct 21, 2024

if we do that, we must assume the Cwd is the "workspace" root as well as resolving modules implicitly and I think this is a bad idea.

Using your previous example:

If we cd in the realm dir, we then resolve imports differently than golang will.

If in the realm package we import something in the project that is in package, it will try to resolve gno.land/p/demo/blog from the remote and not use the local version because there is no way to know where the local gno.land/r/demo/blog source is unless including all packages from the filesystem root or having some kind of flag/env to specify "workspaces" roots in scope

Also if you have some gno modules in your tree that should not be part of the current project, like some gno module embedded in a node module, ./... will include it too

This differs heavily from golang behavior and will probably confuse people

@thehowl
Copy link
Member Author

thehowl commented Oct 22, 2024

It's not based on cwd. It just that when we do ./... on the example I mentioned, realm is executed taking its gno.mod into consideration, and package is considered taking its gno.mod into consideration.

@n0izn0iz
Copy link
Contributor

n0izn0iz commented Oct 23, 2024

can you write the content of both go.mod from your example the way you want it pls?

@thehowl
Copy link
Member Author

thehowl commented Oct 30, 2024

Take the gnochess repository

I'd like for there to be just two gnochess files:

realm/gno.mod

module gno.land/r/demo/chess

replace gno.land/p/demo/chess => ../package

package/gno.mod

module gno.land/p/demo/chess

If I run, from the root of the project, gno test ./..., the following packages are tested:

package           # gno.land/p/demo/chess         | package/gno.mod
package/zobrist   # gno.land/p/demo/chess/zobrist | package/gno.mod
package/glicko2   # gno.land/p/demo/chess/glicko2 | package/gno.mod
realm             # gno.land/r/demo/chess         | realm/gno.mod
realm/raffle      # gno.land/r/demo/chess/raffle  | realm/gno.mod

After the # the pkgpath we're testing; after the | the gno.mod that is used to resolve dependencies.

@thehowl
Copy link
Member Author

thehowl commented Oct 30, 2024

From Manfred's comment on the call:

1. am I in the context of a gno.mod, if yes, is there an explicit replace directive? else
2. am I in the context of a gno.mod, if yes, is the import path available relatively (several packages/ in a monorepo)
3. download & cache from chain

`-u` or `clean -modcache` to refresh deps

As a general flow for the package loader.

When you specify paths on the CLI, we should parse and understand them as per the above comment ^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gno-proposal Gnolang change proposals
Projects
Status: 👀 Watching
Status: Backlog
Development

No branches or pull requests

5 participants