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 deps #4094

Merged
merged 3 commits into from
Jul 29, 2017
Merged

gx: update deps #4094

merged 3 commits into from
Jul 29, 2017

Conversation

Stebalien
Copy link
Member

@Stebalien Stebalien commented Jul 27, 2017 •
edited
Loading

This updates a lot of libp2p deps. Do not merge yet.

Waiting on libp2p/go-libp2p-kad-dht#74

@Stebalien Stebalien added status/blocked Unable to be worked further until needs are met status/in-progress In progress labels Jul 27, 2017
@Stebalien
Copy link
Member Author

Really: DO NOT MERGE. This definitely introduces bugs that we'll have to fix.

@whyrusleeping, @vyzo.

@vyzo vyzo mentioned this pull request Jul 27, 2017
Merged
@vyzo
Copy link
Contributor

vyzo commented Jul 27, 2017 •
edited
Loading

I will rebase #4091 once this gets moving.

Stebalien
Stebalien commented Jul 27, 2017
View reviewed changes
exchange/bitswap/wantmanager.go
peerReqs chan chan []peer.ID // channel to request connected peers on
incoming chan *wantSet
connectEvent chan peerStatus // notification channel for peers connecting/disconnecting
peerReqs chan chan []peer.ID // channel to request connected peers on
Copy link
Member Author

Choose a reason for hiding this comment

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

Technically, this bug existed before this PR. However, the whole point of the peerstream update was to fix bugs like this so I figured I might as well fix this now.

exchange/bitswap/wantmanager.go
case <-pm.ctx.Done():
}
}

// TODO: use goprocess here once i trust it
func (pm *WantManager) Run() {
tock := time.NewTicker(rebroadcastDelay.Get())
defer tock.Stop()
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed this ticker as it doesn't appear to be used.

@Stebalien Stebalien removed the status/blocked Unable to be worked further until needs are met label Jul 27, 2017
@Stebalien
gx: update deps
28be1d4 
License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
@Stebalien
bitswap: serialize connect/disconnect notifications over one channel.
4ef73ee 
Otherwise, we could end up receiving a disconnect notification before a connect
notification (and think we have a connection that we don't have).

License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
@Stebalien Stebalien added the status/blocked Unable to be worked further until needs are met label Jul 28, 2017
@Stebalien
Copy link
Member Author

Stebalien commented Jul 28, 2017 •
edited
Loading

Waiting on libp2p/go-libp2p-pubsub#29.

@Stebalien
Copy link
Member Author

Stebalien commented Jul 28, 2017 •
edited
Loading

Waiting on libp2p/go-mplex#5 and whyrusleeping/go-smux-multiplex#1.

@Stebalien Stebalien force-pushed the gx/update-libp2p-stuff branch 2 times, most recently from d1c7878 to 90e7531 Compare July 28, 2017 22:30
@Stebalien
Copy link
Member Author

Ok. This should be unblocked as I've simply removed the broken test cases (and will be removing go-multiplexer support in the near future as it's very buggy).

Kubuxu
Kubuxu reviewed Jul 28, 2017
View reviewed changes
test/sharness/t0125-twonode.sh Outdated
# test multiplex muxer
export LIBP2P_MUX_PREFS="/mplex/6.7.0"
run_advanced_test "--enable-mplex-experiment"
unset LIBP2P_MUX_PREFS
Copy link
Member

Choose a reason for hiding this comment

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

Why remove this?

Copy link
Member Author

Choose a reason for hiding this comment

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

go-multiplexer is very broken, has been failing travis for a while, and @whyrusleeping told me to simply remove it. I've removed the test cases in this PR and will remove the code in another (so this one can move forward).

Copy link
Member Author

Choose a reason for hiding this comment

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

(e.g., go-multiplexer is responsible for this bug #4038)

As for why I made this change in this PR... I was having trouble getting CI green.

Copy link
Member Author

Choose a reason for hiding this comment

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

So..., @diasdavid says this is the only multiplexer that works in both go and JavaScript so we shouldn't remove it.

@magik6k magik6k mentioned this pull request Jul 28, 2017
Closed
@Stebalien Stebalien changed the title [WIP] gx: update deps gx: update deps Jul 28, 2017
@Stebalien Stebalien removed the status/blocked Unable to be worked further until needs are met label Jul 28, 2017
@Stebalien
sharness: fix #4003 test case
0aca2a3 
1. Use printf to reliably expand escape sequences by default.
2. grep needs to exit after the first match as we're using HTTP/1.1 (the remote
side will not close the connection after sending the response).

License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
whyrusleeping
whyrusleeping approved these changes Jul 29, 2017
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.

Changes so far LGTM. Any specific PRs i should review that are included as deps?

@Stebalien
Copy link
Member Author

Nope. I've removed the multiplexer changes so this should be good to go.

@whyrusleeping whyrusleeping merged commit a8c5c04 into master Jul 29, 2017
@whyrusleeping whyrusleeping deleted the gx/update-libp2p-stuff branch July 29, 2017 01:28
hacdias pushed a commit to ipfs/boxo that referenced this pull request Jan 27, 2023
@whyrusleeping
Merge pull request ipfs/kubo#4094 from ipfs/gx/update-libp2p-stuff
0ee8965 
gx: update deps

This commit was moved from ipfs/kubo@a8c5c04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kubuxu Kubuxu Kubuxu left review comments

@whyrusleeping whyrusleeping whyrusleeping approved these changes

Assignees
No one assigned
Labels
status/in-progress In progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Stebalien @vyzo @whyrusleeping @Kubuxu