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!: build: separate miner and node version strings #12035

Merged
merged 1 commit into from
May 30, 2024

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented May 23, 2024

Proposal asking for feedback.

This removes build.BuildVersion and build.UserVersion() and replaces them with build.NodeBuildVersion, build.MinerBuildVersion and build.NodeUserVersion(), build.MinerUserVersion() so they can be incremented separately.

Why? Because as per #12010 we would like to be able to release them separately. This has two problems:

  1. Once we fix the version number in here without a -dev and tag it, you can build both miner and node with that version string and it looks official. But, we may be prepared to release one and not the other (node vs miner) and may consider one of them not release-ready so not deserving of an official release.
  2. We would like to automate the build and release process such that it only produces the binaries we want, either node or miner, not both, so we can release them separately and clearly signal such.

I'm thinking that we could copy the ipdx version.json pattern implemented by https://github.com/ipdxco/unified-github-workflows/blob/main/.github/workflows/release-check.yml but with some minor tweaks.

Instead of doing the version.json check:

version="$(gh api -X GET "repos/$HEAD_FULL_NAME/contents/version.json" -f ref="$HEAD_SHA" --jq '.content' | base64 -d | jq -r '.version')"

We could run two checks on PRs (probably just on the releases branch?):

node_version="$(gh api -X GET "repos/$HEAD_FULL_NAME/contents/build/version.go" -f ref="$HEAD_SHA" --jq '.content' | base64 -d | grep '^const NodeBuildVersion = ' | cut -d\" -f2)"
miner_version="$(gh api -X GET "repos/$HEAD_FULL_NAME/contents/build/version.go" -f ref="$HEAD_SHA" --jq '.content' | base64 -d | grep '^const MinerBuildVersion = ' | cut -d\" -f2)"

Then, for both:

  1. If it ends in -dev, no more need for further checks.
  2. If there isn't a tag for that version and proceed with roughly the same things it does now, including making a draft release that can be further edited (could even build in some smarts to copypasta the versioned section of the changelog, but that'd be a stretch-goal).
  3. It needs to remember whether it's Node or Miner and then either run make deps lotus or make deps lotus-miner lotus-worker and also probably do something different on the release draft.

I'm not sure yet what it should do if they both pass that, probably that just shouldn't be allowed and it has an error condition with a "nope" comment in the PR.

Copy link

github-actions bot commented May 23, 2024

All checks have passed

@galargh
Copy link
Contributor

galargh commented May 24, 2024

The split in build/version.go is definitely needed given the plans to support separate releases. In my opinion, we do need a little bit more separation between the tags associated with the main lotus releases and the ones for miner.

One idea I proposed here (#12010 (comment)) is to use prefixes to make the distinction. As in, the main lotus releases would happen on v1.29.0, v1.29.1-dev, v1.29.2-rc3, etc. while the miner would be on a separate cycle miner-v1.29.0, miner-v1.29.1-dev, miner-v1.29.2-rc3.

I'm afraid otherwise it might be confusing that there exists a release tag, but there is no lotus or lotus-miner published for it.

As for the automation part, once we solve the above and agree on the source of truth (version.json, CHANGELOG.md, etc.), it won't be a problem. We'll adjust the workflow specifically to lotus' needs.

I'm not sure yet what it should do if they both pass that, probably that just shouldn't be allowed and it has an error condition with a "nope" comment in the PR.

It's either not allowed or create both releases - we could easily accomplish that with using a matrix configuration for executing the release job for example. I'd lean towards do both myself.

@rvagg rvagg marked this pull request as ready for review May 29, 2024 00:41
@rjan90
Copy link
Contributor

rjan90 commented May 29, 2024

Did a quick test on a devnet, setting a different version for miner and the lotus node in version.go, which worked well:

./lotus-miner -v
lotus-miner version 1.27.0+2k+git.67c3a6fd0.dirty
./lotus -v
lotus version 1.27.4-rc1+2k+git.67c3a6fd0.dirty
./lotus-worker -v
lotus-worker version 1.27.0+2k+git.67c3a6fd0.dirty

Also ran make docsgen-cli, and the documentation output correctly had seperate versions for the daemon and the miner.

@rvagg
Copy link
Member Author

rvagg commented May 30, 2024

k, well let's make it so

@rvagg rvagg merged commit 0a51a0a into master May 30, 2024
94 checks passed
@rvagg rvagg deleted the rvagg/separate-version branch May 30, 2024 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants