[RFC][WIP] feat: implement versioning #1631

harry-hov · 2024-02-05T15:45:25Z

This is rough and partial implementation of the proposal that I mentioned here

(NOTE: Marked this as "ready for review" for discussion and early feedback)

Changes/Status

gno.mod is mandatory to publish pkg/realm(s)
gno.mod's module directive takes version now
e.g:
```
module gno.land/p/{handle}/foo v1.2.3
```
pkg/realm(s) are versioned (IOW, multiple version of pkgs/realms can exist with same import path)
gnokey maketx addpkg command takes pkg path and version from gno.mod file
gnokey maketx call command takes -pkgPath in the format gno.land/p/{handle}/[email protected]
disable previous version of realm when someone publishes new version
realm state migration

Testing

$ cd gno.land/cmd/gnoland
$ go test -v . -run TestTestdata/versioning

Contributors' checklist...

Added new tests, or not needed, or not feasible
Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
Updated the official documentation or not needed
No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
Added references to related issues and PRs
Provided any useful hints for running manual tests
Added new benchmarks to generated graphs, if any. More info here.

gno.land/cmd/gnoland/testdata/versioning.txtar

gno.land/pkg/gnoclient/client_txs.go

zivkovicmilos · 2024-02-12T10:52:03Z

@harry-hov

Can you please resolve the merge conflicts in this PR? 🙏

ajnavarro

If we want to keep following semver, we have to change the first module release to v0.1.0:

https://semver.org/spec/v2.0.0.html#how-should-i-deal-with-revisions-in-the-0yz-initial-development-phase

gnovm/cmd/gno/mod.go

gnovm/pkg/gnomod/gnomod_test.go

gno.land/pkg/keyscli/run.go

gnovm/cmd/gno/mod.go

moul · 2024-03-11T16:26:34Z

My main concern is visualizing its daily use. @leohhhn, can you collaborate with @harry-hov on creating a high-level tutorial draft (quick and high-level is probably enough) to clarify its functionality? Including one or two compelling examples would be great.

codecov · 2024-03-19T20:35:53Z

Codecov Report

Attention: Patch coverage is 0% with 12 lines in your changes are missing coverage. Please review.

Project coverage is 44.35%. Comparing base (5c5d9ef) to head (7279190).
Report is 201 commits behind head on master.

Files	Patch %	Lines
tm2/pkg/std/memfile.go	0.00%	11 Missing ⚠️
tm2/pkg/amino/binary_decode.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1631      +/-   ##
==========================================
- Coverage   47.49%   44.35%   -3.14%     
==========================================
  Files         388      433      +45     
  Lines       61311    65287    +3976     
==========================================
- Hits        29117    28956     -161     
- Misses      29756    33931    +4175     
+ Partials     2438     2400      -38

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

harry-hov · 2024-03-26T06:52:23Z

If we want to keep following semver, we have to change the first module release to v0.1.0:

https://semver.org/spec/v2.0.0.html#how-should-i-deal-with-revisions-in-the-0yz-initial-development-phase

I agree. changed it to v0.1.0 🙌

Thanks @ajnavarro

zivkovicmilos

I like the idea behind this PR, but I don't think it has been executed correctly, yet 🙁

I am really not a fan of interacting with Realms (calling their methods...) by specifying their version number, alongside the path.
The path is clear, and the version should be resolved from the path when calling Realm methods (latest).

There's a lot to unpack here, I think we need to collectively decide the API for versioning before we continue

cc @moul @thehowl

zivkovicmilos · 2024-03-26T08:00:11Z

gno.land/cmd/gnoland/testdata/versioning.txtar

+## add `hello`(v1) package located in $WORK/helloworld/v1
+gnokey maketx addpkg -pkgdir $WORK/helloworld/v1 -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
+## add `hello`(v2) package located in $WORK/helloworld/v2
+gnokey maketx addpkg -pkgdir $WORK/helloworld/v2 -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
+
+## add `foo` realm; imports pkg `hello` v1
+gnokey maketx addpkg -pkgdir $WORK/foo -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
+
+## add `bar` realm; imports pkg `hello` v2
+gnokey maketx addpkg -pkgdir $WORK/bar -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
+


You can use the loadpkg txtar directive instead

zivkovicmilos · 2024-03-26T08:01:04Z

gno.land/cmd/gnoland/testdata/realm-state.txtar

+stdout 'GAS USED:   [0-9]+'
+
+## execute Increment
+gnokey maketx call -pkgpath gno.land/r/temp/[email protected] -func Increment -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1


Will the version always be a part of the Realm path?

zivkovicmilos · 2024-03-26T08:06:13Z

gno.land/pkg/gnoclient/client_txs.go

-	Args     []string // Function arguments
-	Send     string   // Send amount
+	PkgPath    string   // Package path
+	PkgVersion string   // Package version


This is an insanely breaking change. Please detail it in the PR description.

Why would it be necessary to provide the package version if the realm path already exists as part of the message?

zivkovicmilos · 2024-03-26T08:06:57Z

gno.land/pkg/gnoclient/client_txs.go

@@ -144,7 +147,12 @@ func (c *Client) Run(cfg BaseTxCfg, msgs ...MsgRun) (*ctypes.ResultBroadcastTxCo
 		}

 		msg.Package.Name = "main"
-		msg.Package.Path = ""
+		if msg.Package.ModFile == nil {


When is this situation possible, if we require gno.mods?

zivkovicmilos · 2024-03-26T08:15:53Z

gno.land/pkg/keyscli/addpkg.go

 	if memPkg.IsEmpty() {
-		panic(fmt.Sprintf("found an empty package %q", cfg.PkgPath))
+		panic(fmt.Sprintf("found an empty package ar dir %q", cfg.PkgDir))


Why not return an error here, instead of panicking? The function supports an error return

zivkovicmilos · 2024-03-26T13:04:25Z

gnovm/pkg/gnolang/utils.go

+		// TODO(hariom): return error instead
+		panic(fmt.Sprintf("error parsing gno.mod at: %q", dir))
+	}
+	// TODO(hariom): make sure requires are accurate in gno.mod


zivkovicmilos · 2024-03-26T13:05:08Z

gnovm/pkg/gnolang/utils.go

+
+// TODO(hariom): move to better place
+func ParseMemMod(dir string) *std.MemMod {
+	var memMod std.MemMod


Why declare here, when you're actually setting the object at the end of the method?

zivkovicmilos · 2024-03-26T13:05:36Z

gnovm/pkg/gnolang/utils.go

+		panic(fmt.Sprintf("error parsing gno.mod at: %q", dir))
+	}
+	// TODO(hariom): make sure requires are accurate in gno.mod
+	var requires []*std.Requirements


You know exactly what the size of this slice is, make should be used

zivkovicmilos · 2024-03-26T13:07:06Z

gnovm/pkg/gnolang/uverse.go

 const uversePkgPath = ".uverse"
+const uversePkgVersion = "v0.1.0"



Nitpick: you can combine this in a common const

zivkovicmilos · 2024-03-26T13:08:54Z

gnovm/pkg/gnolang/machine.go

@@ -55,18 +56,20 @@ type Machine struct {
 //
 // Like for [NewMachineWithOptions], Machines initialized through this
 // constructor must be finalized with [Machine.Release].
-func NewMachine(pkgPath string, store Store) *Machine {
+func NewMachine(pkgPath, pkgVersion string, store Store) *Machine {


What do you think about making NewMachine take in the Store, and have the other params as options?
As far as I can see, there is only one instance where pkgVersion != """

harry-hov · 2024-04-08T08:57:16Z

Initially, I envisioned this PR to serve as a proof of concept for versioning. I marked it as ready for review to draw attention to this topic and to receive feedback on the idea in general.

As directed by @zivkovicmilos, I am converting this to a draft until I complete the implementation.

piux2 · 2024-04-09T22:32:35Z

gnovm/pkg/gnolang/frame.go

@@ -39,7 +39,7 @@ func (fr Frame) String() string {
 			fr.NumExprs,
 			fr.NumStmts,
 			fr.NumBlocks,
-			fr.LastPackage.PkgPath,
+			fr.LastPackage.ModFile.Path,


Let's give it a second thought on implementing mod and package versioning at the VM level. Shouldn't the module be a development tool that manages package versioning outside of VM execution?

piux2 · 2024-04-09T22:33:29Z

gnovm/pkg/gnolang/op_types.go

@@ -142,7 +142,7 @@ func (m *Machine) doOpInterfaceType() {
 	}
 	// push interface type
 	it := &InterfaceType{
-		PkgPath: m.Package.PkgPath,
+		PkgPath: m.Package.ModFile.Path,


Let's give it a second thought on implementing mod and package versioning at the VM level. Shouldn't the module be a development tool that manages package versioning outside of VM execution?

Same as many other places in this implementation

Kouteki · 2024-04-11T11:42:41Z

@harry-hov is this a breaking change? Trying to determine whether we can do this feature post test4 launch.

harry-hov · 2024-04-16T13:21:20Z

@harry-hov is this a breaking change? Trying to determine whether we can do this feature post test4 launch.

@Kouteki Yes it's a breaking change. It changes how you publish and interact with gno packages.

I have no problem with moving this post launch. Regardless, it's on my priority list.

thehowl · 2024-06-18T12:36:22Z

Closing this PR; improved proposals are welcome.

github-actions bot assigned harry-hov Feb 5, 2024

github-actions bot added 🧾 package/realm Tag used for new Realms or Packages. 📦 🤖 gnovm Issues or PRs gnovm related 📦 🌐 tendermint v2 Issues or PRs tm2 related 📦 ⛰️ gno.land Issues or PRs gno.land package related labels Feb 5, 2024

moul reviewed Feb 5, 2024

View reviewed changes

gno.land/cmd/gnoland/testdata/versioning.txtar Outdated Show resolved Hide resolved

moul reviewed Feb 5, 2024

View reviewed changes

gno.land/pkg/gnoclient/client_txs.go Outdated Show resolved Hide resolved

harry-hov requested a review from gfanton February 8, 2024 18:08

harry-hov marked this pull request as ready for review February 8, 2024 18:12

harry-hov requested review from thehowl, jaekwon, piux2, a team and zivkovicmilos as code owners February 8, 2024 18:12

harry-hov force-pushed the hariom/versioning branch from 0d14e77 to 181308e Compare February 9, 2024 09:47

zivkovicmilos mentioned this pull request Feb 12, 2024

Minutes: Core Staff Weekly Syncs [every Monday] gnolang/meetings#36

Open

michelleellen mentioned this pull request Feb 12, 2024

Minutes: Public Dev Calls [every Wednesday] gnolang/meetings#37

Open

harry-hov force-pushed the hariom/versioning branch from 181308e to 82d13ce Compare February 26, 2024 13:37

harry-hov force-pushed the hariom/versioning branch from ce8456c to ee1d69f Compare March 4, 2024 19:11

ajnavarro requested changes Mar 5, 2024

View reviewed changes

gnovm/cmd/gno/mod.go Show resolved Hide resolved

gnovm/pkg/gnomod/gnomod_test.go Show resolved Hide resolved

gno.land/pkg/keyscli/run.go Show resolved Hide resolved

gnovm/cmd/gno/mod.go Show resolved Hide resolved

harry-hov force-pushed the hariom/versioning branch from 09cecd6 to ff270fa Compare March 6, 2024 12:38

kristovatlas requested a review from a team March 6, 2024 17:12

harry-hov and others added 5 commits March 19, 2024 22:35

feat: implement versioning

f3beccf

fix: query handlers now handle version in pkgPath

b65762d

fix docker integ test

6ec2806

fix TestTestdata/pr-1112

c785555

realm.String() now includes version as well

cb370ec

harry-hov added 2 commits March 19, 2024 22:35

fixups

1009f3f

s/v0.0.1/v0.1.0

7279190

harry-hov force-pushed the hariom/versioning branch from 1286a4a to 7279190 Compare March 19, 2024 20:30

zivkovicmilos added this to the 🏗4️⃣ test4.gno.land milestone Mar 25, 2024

dongwon8247 mentioned this pull request Mar 25, 2024

Onbloc x Gno Core Workshop gnolang/hackerspace#61

Open

zivkovicmilos mentioned this pull request Mar 25, 2024

[chain] State Migration between Realm versions #1835

Open

zivkovicmilos added the breaking change Functionality that contains breaking changes label Mar 26, 2024

zivkovicmilos requested changes Mar 26, 2024

View reviewed changes

harry-hov marked this pull request as draft April 8, 2024 08:47

harry-hov mentioned this pull request Apr 8, 2024

feat: addpkg command respects gno.mod file #922

Draft

7 tasks

piux2 reviewed Apr 9, 2024

View reviewed changes

Kouteki modified the milestones: 🏗4️⃣ test4.gno.land, 🏗4️⃣ test4.gno.land [POST LAUNCH] Apr 11, 2024

Kouteki added this to the 🏗4️⃣ test4.gno.land [POST LAUNCH] milestone Apr 26, 2024

Kouteki added the don't merge Please don't merge this functionality temporarily label May 31, 2024

Kouteki unassigned harry-hov Jun 7, 2024

thehowl closed this Jun 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFC][WIP] feat: implement versioning #1631

[RFC][WIP] feat: implement versioning #1631

harry-hov commented Feb 5, 2024 •

edited

Loading

zivkovicmilos commented Feb 12, 2024

ajnavarro left a comment •

edited

Loading

moul commented Mar 11, 2024

codecov bot commented Mar 19, 2024 •

edited

Loading

harry-hov commented Mar 26, 2024

zivkovicmilos left a comment

zivkovicmilos Mar 26, 2024

zivkovicmilos Mar 26, 2024

zivkovicmilos Mar 26, 2024

zivkovicmilos Mar 26, 2024

zivkovicmilos Mar 26, 2024

zivkovicmilos Mar 26, 2024

zivkovicmilos Mar 26, 2024

zivkovicmilos Mar 26, 2024

zivkovicmilos Mar 26, 2024

zivkovicmilos Mar 26, 2024

harry-hov commented Apr 8, 2024

piux2 Apr 9, 2024 •

edited

Loading

piux2 Apr 9, 2024 •

edited

Loading

Kouteki commented Apr 11, 2024

harry-hov commented Apr 16, 2024

thehowl commented Jun 18, 2024 •

edited

Loading

		const uversePkgPath = ".uverse"
		const uversePkgVersion = "v0.1.0"

[RFC][WIP] feat: implement versioning #1631

[RFC][WIP] feat: implement versioning #1631

Conversation

harry-hov commented Feb 5, 2024 • edited Loading

(NOTE: Marked this as "ready for review" for discussion and early feedback)

Changes/Status

Testing

zivkovicmilos commented Feb 12, 2024

ajnavarro left a comment • edited Loading

Choose a reason for hiding this comment

moul commented Mar 11, 2024

codecov bot commented Mar 19, 2024 • edited Loading

Codecov Report

harry-hov commented Mar 26, 2024

zivkovicmilos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

harry-hov commented Apr 8, 2024

piux2 Apr 9, 2024 • edited Loading

Choose a reason for hiding this comment

piux2 Apr 9, 2024 • edited Loading

Choose a reason for hiding this comment

Kouteki commented Apr 11, 2024

harry-hov commented Apr 16, 2024

thehowl commented Jun 18, 2024 • edited Loading

harry-hov commented Feb 5, 2024 •

edited

Loading

ajnavarro left a comment •

edited

Loading

codecov bot commented Mar 19, 2024 •

edited

Loading

piux2 Apr 9, 2024 •

edited

Loading

piux2 Apr 9, 2024 •

edited

Loading

thehowl commented Jun 18, 2024 •

edited

Loading