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

[release-v0.22.1] multi: Switch to chaincfg/chainhash module. #1851

Merged
merged 3 commits into from
Apr 28, 2022

Conversation

davecgh
Copy link
Member

@davecgh davecgh commented Apr 28, 2022

Currently, there are some issues for consumers importing the main github.com/btcsuite/btcd@v0.22.0-beta module:

  • Anyone attempting to import anything github.com/btcsuite/btcd end up with an old and broken version v0.18.0 since all of the later tags are pre-release versions and the go tooling prefers release tags over pre-release tags
  • Any consumers that import github.com/btcsuite/btcd@v0.22.0-beta along with the new github.com/btcsuite/btcd/btcec/v2 or github.com/btcsuite/btcd/chaincfg/chainhash modules will have conflicts due to the existence of both the package and the module at the same path
  • The documentation shows an ancient version and improperly claims it is bchd

These all boil down to a couple of things:

  • As can be seen in the module release workflow docs, versions without pre-release components are preferred over those with them. In other words, v0.22.0-beta is seen as before v0.18.1, so currently anyone just importing or doing a plain go get github.com/btcsuite/btcd is going to attempt to get v0.18.1 which is a broken version.
  • The latest pre-release tag v0.22.0-beta has chaincfg/chainhash as a package, but there is also now a separate module with the same path.

This consists of the following changes which will resolve all of the aforementioned issues:

  • Branches from the v0.22.0-beta tag so that no new code that is not ready to be released is introduced
  • Removes the chaincfg/chainhash package
    • NOTE: The package is removed in this side branch only, not the master branch which still has the module code as expected
  • Updates the go.mod to use the chaincfg/chainhash module instead
  • Bumps the version of btcd and btcctl to v0.22.1
  • Updates the CHANGES for the release

Once this is merged, all that will be needed is to create a v0.22.1 tag at the Update CHANGES file for 0.22.1 release. commit.

Closes #1839.

This removes the chaincfg/chainhash package in favor of using the module
to resolve issues with the go dependency tooling due to a conflict
between the package and module of the same name.
@coveralls
Copy link

Pull Request Test Coverage Report for Build 2237026905

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.07%) to 52.822%

Files with Coverage Reduction New Missed Lines %
peer/peer.go 2 76.18%
Totals Coverage Status
Change from base Build 2236974951: -0.07%
Covered Lines: 20945
Relevant Lines: 39652

💛 - Coveralls

@Roasbeef
Copy link
Member

+1 for branching off of 0.22, there're quite a lot of core changes in master due to the big taproot push we went through earlier this year.

In the PR body:

Bumps the version of dcrd and dcrctl to v0.22.1

Should be btcd + btcctl ;)

@davecgh
Copy link
Member Author

davecgh commented Apr 28, 2022

In the PR body:

Bumps the version of dcrd and dcrctl to v0.22.1

Should be btcd + btcctl ;)

Oops. Finger memory... Updated.

Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🥨

chaincfg/chainhash/hash.go Show resolved Hide resolved

// appPreRelease MUST only contain characters from semanticAlphabet
// per the semantic versioning spec.
appPreRelease = "beta"
appPreRelease = ""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So btcd is now officially out of beta? 🤩

Copy link
Member Author

@davecgh davecgh Apr 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something I highly suggest is to separate the product version (aka the release versions of btcd) from the go module versions. I know it might be a little annoying, but Go hijacked semver for its modules and, as things like this show, module versions really need to be independent.

The model I use for dcrd that has worked out really well is for each release to create a release-vX.Y branch and on that branch update the version info to remove the pre-release details, remove any overrides from go.mod, etc and tag the release-vX.Y.Z commit(s) on that release branch accordingly. Then, bump to the next pre-release version on the master branch where development continues as normal.

That way all releases have their own branch that you can back port to if it is necessary and it is decoupled from the go module version(s).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something I highly suggest is to separate the product version (aka the release versions of btcd) from the go module versions.

Yeah I'm slowly coming around to this realization myself. Takes a bit more work, but worth it not to cause headaches for consumers of the modules.

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