-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
btcutil: update modules to replace to top-level btcd repo #1788
Conversation
c9bee7e
to
fb2dfdc
Compare
go.sum
Outdated
github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f h1:bAs4lUbRJpnnkd9VhRV3jjAVU7DJVjMaK+IsvSeZvFo= | ||
github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f/go.mod h1:TdznJufoqS23FtqVCzL0ZqgP5MqXbb4fg/WgDys70nA= | ||
github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d/go.mod h1:+5NJ2+qvTyV9exUAL/rxXi3DcLg2Ts+ymUAY5y4NvMg= | ||
github.com/btcsuite/btcutil v1.0.3-0.20201208143702-a53e38424cce/go.mod h1:0DVlHczLPewLcPGEIeUEzfOJhqGPQ0mJJRDBtD307+o= |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why this is still in the mod sum...
I guess because we need to make a new tag after this set of changes?
Pull Request Test Coverage Report for Build 1684907091
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🎉
btcutil/go.mod
Outdated
@@ -4,8 +4,10 @@ go 1.16 | |||
|
|||
require ( | |||
github.com/aead/siphash v1.0.1 | |||
github.com/btcsuite/btcd v0.20.1-beta | |||
github.com/btcsuite/btcd v0.22.0-beta |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The v0.22.0-beta
version still pulls in btcsuite/btcutil
, that's why you end up with btcsuite/btcutil
still being in the top level go.sum
. If you change this to v0.22.0-beta.0.20220111032746-97732e52810c
, it goes away.
This is because replace
directives are ignored in transitive dependencies. So the replace
directive in this file is ignored when go mod is looking at the top level btcd
package. Perhaps we should try to get rid of this circular replace directive soon to avoid issues like this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC, once we tag a new version of btcd
after that, the dep hash goes away?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, ok I see now, I can add that.
Perhaps we should try to get rid of this circular replace directive soon to avoid issues like this?
IMO the main matter is if we want to have btcutil
be a proper sub-module or not. The btcutil/psbt
module also exists. I think as long as we make more sub-modules we'll have this problem until each package is isolated as its own module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main packages that btcutil
uses are: wire
, chainhash
, btcec
, and txscript
(FWIW).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
Now that the new `btcutil/v1.0.0` tag has been pushed, we update the new internal modules to point to the top-level `btcd` repo via replace directives.
fb2dfdc
to
161863e
Compare
Pushed up @guggero's suggestion to clean up |
Now that the new
btcutil/v1.0.0
tag has been pushed, we update the newinternal modules to point to the top-level
btcd
repo via replacedirectives.