gnolang / gno

Gno: An interpreted, stack-based Go virtual machine to build succinct and composable apps + gno.land: a blockchain for timeless code and fair open-source.
https://gno.land/
Other
901 stars 378 forks source link

rfc: the future of `gno.mod` #2904

Open thehowl opened 1 month ago

thehowl commented 1 month ago

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

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:

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:


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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

@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 commented 1 month ago

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 commented 1 month ago

Checkout this related PR: https://github.com/gnolang/gno/pull/2911

ajnavarro commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

@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 commented 1 month ago

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 commented 1 month ago

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

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 commented 1 month ago

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

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 commented 1 month ago

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

thehowl commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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

thehowl commented 4 weeks ago

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 commented 4 weeks ago

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 ^