-
Notifications
You must be signed in to change notification settings - Fork 42
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
Add migration for IPNS record move #72
Conversation
you comitted a binary |
gotta knock it off with that |
i'd say its probably worth comitting the gx vendored deps in this case, just to make this easily go-gettable |
b04b0e8
to
324ecc5
Compare
I can, it's just a lot of code as I currently depend on the go-ipfs repo. I can look into removing unused packages again, we'll see where that goes. |
324ecc5
to
56c7e28
Compare
So, I was able to do a selective vendor (only used packages): > go list -json -f '{{.Deps}}' | jq -r '.Deps[] | select(startswith("gx/"))' | while read pkg; do mkdir -p "vendor/$pkg" && cp $GOPATH/src/$pkg/*.{s,go}(N) "vendor/$pkg"; done |
8e1fceb
to
5eea3d9
Compare
Unless you're adding a bunch of new files (e.g., |
@whyrusleeping all vendored and passing tests. Should I unvendor iptb and fetch from dists? I vendored the code as it uses gx but even after stripping unnecessary packages, it's still large. |
|
5251eb6
to
fa4e5d7
Compare
fa4e5d7
to
26a332b
Compare
f0f1702
to
d9c6752
Compare
This needs to happen *after* publishing a release as we need to use that release in the test.
d9c6752
to
2bc282a
Compare
Note, I am going to want to also migrate the repo to check for blocks with an insecure has in the repo and also remove any instanced of blocks with an id hash as they should not be stored before a new go-ipfs version is released. Ref:ipfs/kubo#4766 |
"git gui" is really good for this. That is what I tend to use, you can then easily hand pick the correct files. |
I assume this will be a separate migration, right?
I can do that on the command line as well (faster than opening a program, usually). I just get lazy. |
I'm not sure yet. That would seam to be the easiest think to do. I am not sure (but have not looked closely yet) if it is possible to do two migrations at once in a single version bump. |
@kevina we'd have two separate repo versions but they'd be run at the same time (the repo version really just counts the number of migrations we've applied). I'd rather avoid adding anything to this current set of PRs as it's blocking everything and already contains a ton of changes (it'll also bring in some new datastore changes, logging changes, several libp2p fixes, etc.). |
@Stebalien okay them, I like the idea of two separate migrations to avoid complications, I just don't want the user to have to migrate twice when upgrading to a new release version. It was @whyrusleeping idea to possible combine them. |
return sk, nil | ||
} | ||
|
||
func applyForKey(dstore ds.Datastore, k ci.PrivKey) error { |
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.
it does feel a bit odd to be using private keys here instead of just public keys, but its not a huge deal i guess
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.
Yeah... but that's what the Keystore/Identity gives me.
Given that I've already built the dist (and linked to it from go-ipfs), I'll leave this as-is unless I have to make other changes.
return nil | ||
} | ||
|
||
func (m Migration) Apply(opts migrate.Options) error { |
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.
so this is only migrating the ipns entries, and not any of the other things the DHT writes, correct?
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.
Yep.
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.
migration seems pretty simple, only one weird bit around using private keys where we technically only need public keys, but its not actually that big of a deal.
see: ipfs/kubo#5007
@whyrusleeping am I doing this right?
distributions
. May need to do something to ensure we have gx available (as I've introduced it as a dep).