btcd
btcd copied to clipboard
go-modularize packages (part I)
Motivation
There are two main motivations of this issue:
-
Primary: be able to introduce breaking changes to packages without doing a
MAJOR
semver bump to btcd. -
Long term: make btcutil package not rely on the main
btcd
module, and instead import only the necessary ones.
Candidates
I have narrowed down the scope of this PR to the following packages:
These packages are the ones that are often imported in third-party packages and are isolated enough for a good first version. For the long-term objective of using modules in btcutil, that'd need navigating some circular dependency mess.
Action points
Once this is merged, we need to create some Git tags to bootstrap the initial versions (I chose v0.1.0
). If Concept ACK, can someone with commit access create the following tags? (cc: @jcvernaleo or @jakesyl)
- [ ]
btcec/v0.1.0
- [ ]
chaincfg/v0.1.0
- [ ]
chaincfg/chainhash/v0.1.0
- [ ]
wire/v0.1.0
@Roasbeef would love to get your input on this- I know lnd has some similar properties and would be curious if you'd tried this over there
Pull Request Test Coverage Report for Build 301814101
- 0 of 0 changed or added relevant lines in 0 files are covered.
- 8 unchanged lines in 1 file lost coverage.
- Overall coverage decreased (-8.2%) to 45.296%
Files with Coverage Reduction | New Missed Lines | % |
---|---|---|
peer/peer.go | 8 | 75.71% |
<!-- | Total: | 8 |
Totals | |
---|---|
Change from base Build 289529546: | -8.2% |
Covered Lines: | 14590 |
Relevant Lines: | 32210 |
💛 - Coveralls
Leaving unmerged for another day or two in case anyone else wants to review.
Once merged, someone remind me to make tags please!
I believe @jalavosus had looked into doing something similar @xplorfin so would love to get his thoughts.
Also, I've had limited success with this kind of project setup so I wanted to run a test. I setup a repo here with a replace github.com/btcsuite/btcd v0.21.0-beta => github.com/onyb/btcd v0.20.1-beta.0.20201012085148-740a56acae3f
and was unable to get it to run.
This could be something on my end/my misunderstanding of how gomodules work. My main concern here relates to golang/go#30354.
This is working as designed. replace directives have no effect outside of your own module, so if you invoke go get outside of your own module, they have no effect. - @bcmills
I had an issue earlier this week and have a concise explanation of the effects of replace
downstream here. I'm not sure if the same thing applies here (and I know some great work has been done around tooling w/ gopls
) but I would like to figure out the issue w/ the btcd-test repo to make sure these kind of downstream effects won't happen with local replace.
@jakesyl Regarding the error, maybe you need to create a tag called btcec/v0.1.0
for it to work? 🤔
@onyb on btcd? Possibly - this is kind of my concern. In my experience the effects of replace
on downstream workflows are just a huge pain. So as an example, I used the base of your branch in the replace in jakesyl/btcd-test#1 and had no issues. Just want to make sure we're not breaking downstream dependencies here
Also, would that mean we'd have to tag every subfolder on release? Btw - if you have an example of another go library that implements this would love to take a look. I've looked at a few different tutorials around this. Most of the ones I've looked at either aren't libraries or have no interdependencies between modules because of the versioning issue.
@ckeyes88 probably articulated this best:
If there's a way to do it, I'm super for it since btcutil pulling in btcd isn't ideal.
One more thing I'll add, it does look like lnd has tried this approach (see: lightningnetwork/lnd#2602) in a few different places which definitely makes me more comfortable. There's only one case of them doing this with a replace