yamux 0.10.1, set_split_send_size

[folex]

• Update yamux to 0.10.1
• Add set_split_send_size to YamuxConfig

As I understand, it affects #2461.

P.S. I'm kinda surprised you don't commit Cargo.lock, is there a reason for that? From my PoV, it makes CI builds unpredictable and generates hard-to-investigate dependency bugs.

[mxinden]

P.S. I'm kinda surprised you don't commit Cargo.lock, is there a reason for that? From my PoV, it makes CI builds unpredictable and generates hard-to-investigate dependency bugs.

This is a best practice as a library. Which dependency bug did you run into?

[mxinden]

Out of curiosity, why do you need this flag?

[ValeryAntopol]

We had a hard time debugging issue when we encountered UnexpectedEOF errors. Payload data was N * split_send_size, so we thought that it can affect the problem, and wondered why it is impossible to change this value. The reason was that we forgot to close stream after write and it got dropped immediately after write.

Regarding this issue, i thought that it can be useful to add panic, assert or at least warning when garbage collecting stream in "Open" state here. I have seen that you claim that poll_close must be called before dropping the stream here, so it may help users catch the same issue before it leads to serious consequences.

[ValeryAntopol]

Also you can add a comment in the examples mentioning the importance of the closing of the socked, like here. I can make a PR or issue if you agree

[mxinden]

Also you can add a comment in the examples mentioning the importance of the closing of the socked, like here. I can make a PR or issue if you agree

Sure. That sounds good to me @ValeryAntopol.

[folex]

This is a best practice as a library. Which dependency bug did you run into?

We're running build on nightly, so sometimes cargo update brings compilation errors. Without Cargo.lock commited, every build on CI is basically preceded by cargo update.

It's the usual practice to commit .lock files in JS and Haskell. But in Rust, Cargo book suggests against it for library authors. And it makes little sense to me, cuz Cargo.lock only affects CI/local builds, it doesn't affect publishing to crates.io.

So I'm curious on your reasons to do that :)

[folex]

@mxinden anything else I can do to merge this PR?

[mxinden]

It's the usual practice to commit .lock files in JS and Haskell. But in Rust, Cargo book suggests against it for library authors. And it makes little sense to me, cuz Cargo.lock only affects CI/local builds, it doesn't affect publishing to crates.io.

So I'm curious on your reasons to do that :)

No particular reasoning on my end other than following the best practice. I guess it does reduce the maintenance work a bit as one does not need to keep it up to date, e.g. via Dependabot.

[mxinden]

@mxinden anything else I can do to merge this PR?

I still don't understand why you need to configure this flag. Can you expand on that once more? I am reluctant to add a configuration flag just for the sake of debugging.

[folex]

@mxinden anything else I can do to merge this PR?

I still don't understand why you need to configure this flag. Can you expand on that once more? I am reluctant to add a configuration flag just for the sake of debugging.

I needed it to overcome the issue as I described. I've no other use-case. In general, I feel that it should be possible to configure underlying mechanisms without going through rather hard process of forking the whole libp2p.

[thomaseizinger]

This is a best practice as a library. Which dependency bug did you run into?

We're running build on nightly, so sometimes cargo update brings compilation errors. Without Cargo.lock commited, every build on CI is basically preceded by cargo update.

It's the usual practice to commit .lock files in JS and Haskell. But in Rust, Cargo book suggests against it for library authors. And it makes little sense to me, cuz Cargo.lock only affects CI/local builds, it doesn't affect publishing to crates.io.

So I'm curious on your reasons to do that :)

As library authors, we should strive for having our library work with as many version combinations as possible. Locking down dependencies to specific patch versions is not helpful in that regard.

By default, cargo uses the caret modifier for dependency versions, meaning it will only build against that exact version or newer patch versions.

Within semver, there should never be breaking changes within patch versions. I think it is useful to recommend against tracking Cargo.lock in Git for library authors because it has the ecosystem-wide effect that breaking API changes in patch versions are extremely rare and when they happen, they are detected very early. Libraries tend to have less dependencies so it is also easier for library authors to work out, which dependency is the problem instead of putting this burden onto the application projects.

[mergify]

This pull request has merge conflicts. Could you please resolve them @folex? 🙏