[runtime] near-vm-errors 3.0.0 => 3.1.0

Sticks out like a sore thumb. I'll try to merge the crate into primitives to remove one thing we need to think about here.

I'll try to merge the crate into primitives to remove one thing we need to think about here.

I've reconsidered that. The list above misses near-vm-logic, but that is the crate we need to publish some version of, as the near-sdk-rs team needs it. cc @austinabell. The publishing is very much on the "just last one more time" basis though, we don't plant to support near-vm-logic to be usable outside of nearcore, current usages should migrate to sandbox instead.

near-vm-logic is also past 1.0, so solving the problem just for vm-errors doesn't help with keeping error numbers unified. While removing vm-errors crate and merging it into primitives makes sense by itself, I don't want to do that until we publish near-vm-logic version for the sdk team to use, to avoid shaking crate structure sdk uses today too much.

So, I suggest to do the following, to keep the versioning conssitent: publish all crates, including near-vm-errors and near-vm-logic, with version 0.10.0. After sdk no longer uses past-1.0 versioned crates, we yank all those versions.

It's very weird to have 0.10 which is newer than "3.0.0", but this is the weirdness only the nearcore team is exposed to (users shouldn't directly use any version of runtime crates), and it, especially after yanking, it is less weird than having some crates at 0.92 and some crates at 92.0 for obscure historical reason.

Bumping the versions across the entire workspace by one minor tick. Which then opens a PR with all the changes to be reviewed, approved and merged.

What value is brought in by this process? My gut instinct is that it's pure overhead.

To take a step back, do we need "The versions of published crates on crates.io should always match their versions on GitHub." if we are also striving for "All crates in the workspace should share the same version."? A simpler solution is to avoid any meaningful versions in Cargo.tomls at all, and instead just fill them in at the point of publishing. I think that's what rustc and rust-analyzer are doing.

An alternative, used by Cargo, is to release the crates only after the binary release is cut (so, every six weeks), and have 0.xx.0 as a version in source Cargo.tomls for the 1.xx.0 release of the binary.

Between the two, I lean towards the first option -- publish arbitrary commit every week, fill-in versions on publish.

publish all crates, including near-vm-errors and near-vm-logic, with version 0.10.0. After sdk no longer uses past-1.0 versioned crates, we yank all those versions.

Love this idea. Thank you so much for your contributions, I wasn't sure if we could safely down-version those runtime crates already published past 1.0.0 but seeing as the major thing is near-vm-logic and that it's only used in near-sdk (please confirm), here's what we can do:

I'll version the [runtime] group to 0.5.0, yank the >=1.0.0 versions from crates.io, I'll then dissolve that group into the rest of the workspace. Thereafter, I'll re-version everything to 0.0.0 (stay with me).

A simpler solution is to avoid any meaningful versions in Cargo.tomls at all, and instead just fill them in at the point of publishing

On the tooling side, I'll introduce a version field to [workspace.metadata.workspaces] that predefines a version for the entire workspace and set this to 0.10.0 as suggested.

So, on every push to master, Hermes (CI) would apply that version across the entire workspace, and if there's a version change, all non-private crates in the workspace gets published to crates.io.

do we need "The versions of published crates on crates.io should always match their versions on GitHub."

Since we're dropping individual crate versions from the repo, Hermes (CI) can employ tagging instead. Any commit on master that changes the version field would be tagged. Something like lib-v0.10.0 so you can, at least, easily find the git revision from which a non-private crate's version was published from. We could discuss this, because you could also just git blame on the version field, step through changes until you find the version, enter the tree. But tagging is easier, of course.

This would ensure we have consistent versioning across the workspace without needing Chronos (CI) to automate bumping versions across the workspace, nor do we need the system creating PRs with those changes. As, it would be as easy as bumping the version field in the workspace's manifest.

What value is brought in by this process? My gut instinct is that it's pure overhead.

With the process describes in this comment, we won't need that anymore.

Yeah, really like the updated proposal! Just one small thing, I wouldn't do "yank the >=1.0.0 versions from crates.io" right away. For that, I'd wait until the new version of the SDK is released, so that contract developers don't have yanked crates in the lockfiles.

1 to tags. Implementation wise, you want to first cargo publish, and then git push --tags. As a general tidbit of Rust knowledge, cargo publish now actually includes current commit into the published crate automatically:

λ cat ~/.cargo/registry/src/github.com-1ecc6299db9ec823/once_cell-1.8.0/.cargo_vcs_info.json 
{
  "git": {
    "sha1": "8671ffc69cb78e345b7647802e5a5fd0c53f8c07"
  }
}

So, tagging isn's strictly necessary, but would be nice.

and if there's a version change

For my project, I use a stateless implementation of the idea -- new version is published if there isn't a git tag for the version currently specified in the manifest. This allows for workflows where you upgrade the version, publishing fails for some reason, you push a fix to publishing, and the publishing of the same version is retried.

https://github.com/matklad/xshell/blob/4e5090e9f79baeed1037bc0925a6a84174566eb1/examples/ci.rs#L42-L90

We should consider phasing out unit testing. Blockchains like Ethereum don't have that so it does not seem necessary. Also, it will create a maintenance overhead in the future given that we are building RS testing framework. Without unit tests we might also be able to make Rust SDK simpler. For now we can separate tests into a separate published crate for which we stop doing updates and generate a warning when used, which should unblock the Contract Runtime team. When RS testing framework is released we can replace all unit tests in examples with RS framework tests. Eventually it would be great if unit tests crate will be evolving independently from Rust SDK and RS testing framework, which could even be something that is done by the community. WDYT?

I'm not sure I completely agree with this. Unit tests can be a great way to quickly test and avoid the pattern of compile to wasm -> start up sandbox (if not using existing network) -> deploy contract -> send transactions through RPC when it can be done all in a Rust test and potentially include many other architectures we currently don't support.

As for not having unit tests in solidity, there are some pseudo solutions like https://remix-ide.readthedocs.io/en/latest/unittesting.html, but I think it's important to compare against other Rust/wasm based VM in the ecosystem, and the ones I know about all support unit tests (edit: actually Elrond is one that doesn't, but more popular ones do).

Maybe we can consider moving these test utils to a separate crate to that of the SDK, but it currently has no overhead for actual compilation because none are enabled for wasm32-unknown-unknown

Who is going to be responsible to increasing the version number?

Anyone really, but since it goes through our familiar PR review process, we can ensure to only justifiably bump the version.

Who will publish those creates?

Once this PR is merged, CI automation kicks in to auto-publish crates once it detects a change to the version number on master. So, for every PR that updates the version, when pushed to master, all non-private crates would in turn, get published with that version number.

How often will they be publishes?

This is variable, there's no requirement to constantly turn out new versions, so we only need to publish when we actually need to. I suppose reasonable times are when we have protocol changes, the resolution of a critical bug involving the published crates or a security flaw is addressed.

Is that going tto imply that we will have a requirement that packages api will not to change between versions?

Could you elaborate? I'm not sure I understood the question.

Is that going tto imply that we will have a requirement that packages api will not to change between versions?

Could you elaborate? I'm not sure I understood the question.

There is a general guideline in create.io that packages shouldn't break api if there is a minor version change. We are going to be publishing multiple crates at the same time. Strictly applying this rule to us, would hinder the development process. My assumption is that we will not.

Thanks for clarifying, well because we have a unified version across the workspace, we can't maintain that rule on a per-crate basis. We're keeping things on v0.x indicating that we don't necessarily have a stable API. So, we arbitrarily publish with minor version bumps. Regardless of whether we have breaking changes. Anything that should have proper versioning can either opt-in to having an "independent" version from the workspace or be moved outside the workspace and managed independently.

What is the review process of bumping version going to be. Are we going to require that for simple change such as bumping the version number up, we will require everyone from the company to approve?

Currently, with no codeowner for the workspace manifest, GitHub, I believe should only require approval from at least, one maintainer.

Maybe we should give someone ownership to all code to make version changes easier?

Given the process, one would only need to own the workspace's Cargo.toml, but seeing as that spec dictates the versions of a number of non-private crates owned by sets of people, notifying those owners might be in order, but not as necessary since versioning and publishing is rather trivial.

Proposal B - give someone permissions to be code owners of everything or make a GitHub bot for it

if a bot's job is only to detect and auto-approve changes to the version, it's no different from the usual approval from a maintainer. But if we stick to the manual approval, at least we get eyes on it.

Proposal C - Don't bump up the version, but the script that published the change, bumps up the version temporarily, without committing, and it does cargo publish

I think this defeats the purpose of publishing automation, though.

script that goes through all files and bumps up versions

We have this already: a custom fork of cargo-workspaces pksunkara/cargo-workspaces@master...miraclx:grouping-versioning-and-exclusion, but this only needs to happen on CI because it's only relevant to the publishing step.

In the repo, we keep all crates on v0.0.0. Delegating the versions to all be governed by the what's specified in the workspace manifest.

It's equivalent to what rustc and rust-analyzer both do.

@pmnoxx Thanks for your inputs! I feel we are on the same page, and as @miraclx pointed out, the current implementation almost follows your proposal D (the order of version bump and PR is flitted to simplify the flow (it is easier to edit a single line in the project root Cargo.toml rather than needing to have a script / cargo-workspaces on every developer machine willing to bump the version)

What is the review process of bumping version going to be. Are we going to require that for simple change such as bumping the version number up, to require everyone from the company to approve?

I proposed this strategy of publishing crates through a regular PR process CI automation, and I believe we indeed don't need a heavy review process here, but at the same time, it allows the process to be controlled and fairly straightforward, i.e. one person proposes the version bump based on some need (via PR), and at least one other person approves it, thus there are at least two people in the loop.

@pmnoxx I think this PR is fine as is. We don't always need to split a PR for the sake of making it easier to approve. Especially when it is about some renaming or version updates, I actually think it is easier to review it this way.

Happy to announce: the automation succeeded! We have everything published, and we have it tagged as a reference point!

https://github.com/near/nearcore/releases/tag/crates-0.10.0

@miraclx thank you so much for persevering here!