-
Notifications
You must be signed in to change notification settings - Fork 60
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
sharness: add t0040-install-many.sh #12
Conversation
4218885
to
7bb7cd7
Compare
seems like it failed. cc @whyrusleeping |
Yeah, it fails with:
It also fails locally on my machine. |
7bb7cd7
to
6363fc7
Compare
I just added revert tests that also fail. |
@whyrusleeping it looks to me that the following code in migrations.go should be improved:
When v0.3.7 is installed the version file is not created. It is only created when "ipfs init" is run. |
6363fc7
to
d833d83
Compare
I fixed a few things in migrations.go, main.go and revert.go, so that the tests now pass. |
@@ -14,13 +12,11 @@ import ( | |||
|
|||
func CheckMigration() error { | |||
stump.Log("checking if repo migration is needed...") | |||
p := util.IpfsDir() | |||
oldverB, err := ioutil.ReadFile(filepath.Join(p, "version")) | |||
oldver, err := GetCurrentVersion() |
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.
I don't know if this will work right. The version returned here is the ipfs binary version, and we're looking for the version of the repo.
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.
Erroring out when the "version" file is not there was wrong anyway, because the "version" file is created by some versions of IPFS only when ipfs init
is run.
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.
agreed. we should have some function that maps ipfs version to a repo version for the versions we can't call ipfs version --repo
on.
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 maybe that is a separate topic and you can have a different PR do that.
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.
actually, now that i think about it. if theres no version file, theres no repo to migrate. So we can just short circuit out of this check in that case.
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.
can change it to something like:
vfilePath := filepath.Join(p, "version")
if _, err := os.Stat(vfilePath); err == os.ErrNotExist {
VLog(" - no prexisting repo to migrate")
return nil
}
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, I just pushed a new version of this patch series that does something like that.
@chriscool i'll push a fix to this branch for you for the repo version thing. |
4f8db6a
to
e610af0
Compare
e610af0
to
d24bd36
Compare
I had to push another version of the series, because as PR #15 had been merged, tests didn't pass anymore. |
this LGTM |
sharness: add t0040-install-many.sh
This is to test installing different ipfs versions.
If the added tests pass we can close issue #7 (Timeouts with v0.3.8 and v0.3.7).