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.

Sign up for GitHub

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

Jump to bottom

gx: update go-ds-flatfs #4872

Merged
merged 1 commit into from
Apr 26, 2018
Merged

gx: update go-ds-flatfs #4872

merged 1 commit into from
Apr 26, 2018

Conversation

djdv
Copy link
Contributor

@djdv djdv commented Mar 23, 2018 •
edited
Loading

Includes (at least): ipfs/go-ds-flatfs@ebb0eca
Which fixes: #4527
And should fix: #3971, and #4710
as well.

@djdv djdv requested a review from Kubuxu as a code owner March 23, 2018 23:02
@ghost ghost assigned djdv Mar 23, 2018
@ghost ghost added the status/in-progress In progress label Mar 23, 2018
@djdv
Copy link
Contributor Author

djdv commented Mar 23, 2018

hold up there's conflicts everywhere

kevina
kevina previously requested changes Mar 24, 2018
View reviewed changes
Copy link
Contributor

@kevina kevina left a comment

Choose a reason for hiding this comment

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

Blocking because I want to get ipfs/go-ds-flatfs#35 in first.

package.json Outdated
"name": "go-ds-flatfs",
"version": "1.1.7"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we please wait on updating this dependency. I would like to get ipfs/go-ds-flatfs#35 in first. Otherwise we will have an on-disk format change.

@djdv
Copy link
Contributor Author

djdv commented Mar 24, 2018

@kevina
Sounds good to me.

@kevina
Copy link
Contributor

kevina commented Mar 24, 2018

@kevina
Sounds good to me.

Thanks. Will unblock once that's in, hopefully in by early next week.

@Stebalien Stebalien added the status/blocked Unable to be worked further until needs are met label Mar 24, 2018
@Stebalien Stebalien self-requested a review March 24, 2018 17:37
@djdv djdv force-pushed the big-deps branch 4 times, most recently from e454798 to 2c7c132 Compare March 26, 2018 12:35
@djdv
Copy link
Contributor Author

djdv commented Mar 26, 2018

Just as an aside, I'm going to repeat the process here:
#4742 (comment)
after the desired flatfs commits are merged.
Moving 3.0.16 (origin) ->3.0.18 (@hsanjuan's special version) -> 3.0.19 (same version with datastore dep updated)
So ignore the current hash for go-libp2p-kad-dht until the next push.

@hsanjuan
Copy link
Contributor

Is it worth that I rebase and we can merge my original #4742 (which was LGMT'ed already) before this?

@djdv
Copy link
Contributor Author

djdv commented Mar 26, 2018

@hsanjuan
I'm in favour of that, especially since this is now blocked. I can just branch off again from master and apply the relevant dep updates.

@hsanjuan
Copy link
Contributor

@djdv it was merged now

@djdv djdv changed the title gx: update go-datastore and friends gx: update go-ds-flatfs and its deps Mar 28, 2018
@djdv djdv force-pushed the big-deps branch 2 times, most recently from 5029ab6 to ab99ea2 Compare April 3, 2018 20:28
@kevina kevina removed the status/blocked Unable to be worked further until needs are met label Apr 5, 2018
@djdv
Copy link
Contributor Author

djdv commented Apr 10, 2018

@kevina
Do you know anything about these test failures? The log is getting flooded by proto: duplicate proto type registered: record.pb.Record

@Stebalien
Copy link
Member

@djdv take a look at the output of gx deps dupes. That'll show you duplicate dependencies (usually the source of errors like that).

@djdv
Copy link
Contributor Author

djdv commented Apr 11, 2018

@Stebalien
Thanks.

go-libp2p-record is being imported twice, an old version is being pulled in by our special version of go-libp2p-kad-dht, updating it there results in build errors due to API changes.
Trying to use the latest version of libp2p-kad-dht requires changes in namesys which seem to be happening here: #4817

I'm not positive, but I don't think there's a way to avoid being blocked on these namesys changes. It may be best to close this PR in favour of #4817
just making sure your PR pulls in the latest flatfs with it.

@djdv
gx: update go-ds-flatfs
62e20ef 
License: MIT
Signed-off-by: Dominic Della Valle <ddvpublic@gmail.com>
@djdv
Copy link
Contributor Author

djdv commented Apr 11, 2018

Given that this solves a big issue on Windows, what if we just cherry picked the patch and published like this for now?
1.1.8 -> 1.1.9
gx diff QmPiYdqnkkiPEffrWq2M3Phb3NT8r49kqUXMev2hkkM7uV QmaCTqBCt1aKaGfHfSVzsprqWRXCjHthK8xXrPbUZYCWga

We can pull in changes from flatfs v1.2.X+ later.

@djdv djdv changed the title gx: update go-ds-flatfs and its deps gx: update go-ds-flatfs Apr 11, 2018
@djdv
Copy link
Contributor Author

djdv commented Apr 24, 2018

@Stebalien
I need a decision/review from you on this ^
I would very much like to pull this in for Windows users before the next release.

whyrusleeping
whyrusleeping approved these changes Apr 26, 2018
View reviewed changes
Copy link
Member

@whyrusleeping whyrusleeping left a comment

Choose a reason for hiding this comment

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

LGTM, this is just the flatfs with the windows fix patch, and not the diskusage changes.

@whyrusleeping whyrusleeping merged commit 8408904 into ipfs:master Apr 26, 2018
@ghost ghost removed the status/in-progress In progress label Apr 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@whyrusleeping whyrusleeping whyrusleeping approved these changes

@kevina kevina kevina left review comments

@Kubuxu Kubuxu Awaiting requested review from Kubuxu

@Stebalien Stebalien Awaiting requested review from Stebalien

Assignees

@djdv djdv

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Weird files created when I try to initialize IPFS. Error writing block to datastore: Access is denied.
5 participants
@djdv @kevina @hsanjuan @Stebalien @whyrusleeping