rs-ipfs / rust-ipfs

The InterPlanetary File System (IPFS), implemented in Rust.
Apache License 2.0
1.27k stars 165 forks source link

Release ipfs 0.3.0, ipfs-unixfs 0.3.0 #458

Open koivunej opened 3 years ago

koivunej commented 3 years ago

It's been a while since the last release given all of the dependency churn in the past. While bug/feature-wise there hasn't been much progress, I think a 0.3 could be cut next week.

Tails commented 3 years ago

Would this include bumped dependencies? Because I cannot compile rust-ipfs together with rocket-0.5 for example:

# cargo.toml
[package]
name = "test_rocket_ipfs_deps"
version = "0.1.0"
edition = "2018"

[dependencies]
# this we want
rocket = "0.5.0-rc.1"

# ... but this is all we can have
# rocket = "0.4.1"

# err: failed to select a version for `if-addrs-sys`
ipfs = { git = "https://github.com/rs-ipfs/rust-ipfs" }

# err: mismatched types (futures impl)
# ipfs = "0.2.1" 
# ./src/main.rs
fn main() {
    println!("Hello, world!");
}
koivunej commented 3 years ago

Hi and thanks for the report @Tails! I'll try to find some time this week to see if the deps could be bumped easily.

Tails commented 3 years ago

By the way great job on this IPFS lib! Very stable, pleasant code style and easy api. Hope you'll get the/a next funding round!

koivunej commented 3 years ago

@Tails I posted a WIP #463. The "multiaddr don't contain peerid" assumption, while nicely documented, has been changed in a libp2p which will take a bit more time to tackle. Other than that... I don't think there's a huge amount of work here. If you want to continue exploration, using that branch directly would be a way forward.

Please be aware of our status and the lack of features, the lack of dht publishing #418 being the most prominent. Also while content can be discovered, but downloading it will be slow.

Tails commented 3 years ago

Thanks for the heads-up!

Tails commented 3 years ago

Unfortunately I'm still getting the same mismatch, even after cleaning:

error[E0308]: mismatched types
    | 
   ::: cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.7.0/src/fs/read_dir.rs:16:50
    |
16  |   pub async fn read_dir(path: impl AsRef<Path>) -> io::Result<ReadDir> {
    |                                                    -------------------
    |                                                    |
    |                                                    one of the expected opaque types
    |                                                    one of the found opaque types
...
267 |       pub async fn file_type(&self) -> io::Result<FileType> {
    |                                        --------------------
    |                                        |
    |                                        checked the `Output` of this `async fn`, one of the expected opaque types
    |                                        checked the `Output` of this `async fn`, one of the found opaque types
    | 
   ::: .rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/future/mod.rs:61:43
    |
61  |   pub const fn from_generator<T>(gen: T) -> impl Future<Output = T::Return>
    |                                             -------------------------------
    |                                             |
    |                                             one of the expected opaque types
    |                                             one of the found opaque types
   --> .cargo/git/checkouts/rust-ipfs-ef9abfeeaf1ad0aa/b0e56b5/src/repo/fs/blocks.rs:357:53
    |
357 |       async fn list(&self) -> Result<Vec<Cid>, Error> {
    |  _____________________________________________________^
358 | |         use futures::future::{ready, Either};
359 | |         use futures::stream::{empty, TryStreamExt};
360 | |         use tokio_stream::wrappers::ReadDirStream;
...   |
397 | |         .await
398 | |     }
    | |_____^ one type is more general than the other
    |
    = note: while checking the return type of the `async fn`
    = note: while checking the return type of the `async fn`
    = note: expected opaque type `impl futures::Future`
               found opaque type `impl futures::Future`
[package]
name = "test_rocket_ipfs_deps"
version = "0.1.0"
edition = "2018"

[dependencies]
ipfs = { git = "https://github.com/eqlabs/rust-ipfs", branch = "update_deps" }
#ipfs = "0.2.1"
rocket = "0.5.0-rc.1"

[workspace]
fn main() {
    println!("Hello, world!");
}

There is some kind of mismatch going on in the dependencies of a futures or futures-dependent library.

koivunej commented 3 years ago

Thanks for letting me know, this is interesting. What nightly are you using? rustc -V.

There's definitely an issue with still using older tokio, but that shouldn't be a compilation error worthy. With rustc 1.55.0-nightly (149f4836d 2021-06-17) at least the cargo check passes. It's also okay with the latest I can update to with rustup: rustc 1.55.0-nightly (885399992 2021-07-06).

Hmm interestingly your repro does fail with stable cargo check as well. Interesting... I'd be interested to know of the nightly version you used either way! Also, removing the rocket = ... dependency doesn't change the failure, so this happens whenever an external crate is using the library, so this is definitely a bug on our end. Strange that it doesn't happen with http!

Tails commented 3 years ago

I had rustc 1.54.0-nightly (4e3e6db01 2021-05-18). Getting the same with rustc 1.55.0-nightly (885399992 2021-07-06) :/

Christian7573 commented 3 years ago

I'm getting the same futures::Future mismatch from the list function, but on 0.2.1 fresh from cargo. Should I discuss this here or create a new issue? It's also strange that a conflict like this occurs when cargo tree doesn't appear to contain different futures versions, only futures 0.3.15 and futures-core 0.3.15 throughout.

Tails commented 3 years ago

I would prefer discussing it here; it seems to me to be the core problem. An update to futures was done that is not recognized as an incompatible change, where something in opaqueness differs. So perhaps we have to track down all the crates in rs-ipfs that use a futures lib and see if we can get them to be consistent, preferably with the highest version possible, and not leave that to cargo.

koivunej commented 3 years ago

I do remember wondering if there should be "an external crate" test against cases like this. I suspect the lockfile in the repo (which is used while developing) avoids this issue, but there's some combination which forces the issue when using the crate just as a dependency (without existing lockfile). The lockfile exists for the http crate mostly, so this is mostly a self-inflicted issue.

I'm thinking I'll add a new kind of test crate inside the repo, which isn't part of the workspace to get this into the build to avoid creating these issues in the future. Thanks for the info on "0.2.1"! It makes me think this was most likely created in my previous dependency upgrade or the sled additions.

If you have interest in bisecting this against post 0.2.1 merges on the master, this information would get us closer to understanding what's the cause.

Christian7573 commented 3 years ago

If you have interest in bisecting this against post 0.2.1 merges on the master, this information would get us closer to understanding what's the cause.

Sure! I'll see what I can do.

Christian7573 commented 3 years ago

The issue persists even at release 0.2.0, so perhaps this isn't a self-inflicted problem but a very unfortunate combination of dot-release dependencies. I did some experimentation with the function at fault. Adding a .boxed() adds a little more to the message:

...
   --> /home/christian7573/Documents/ipfs_rs_testing/rust-ipfs/src/repo/fs/blocks.rs:397:10
    |
369 |                   .and_then(|d| async move {
    |  __________________________________________-
370 | |                     // map over the shard directories
371 | |                     Ok(if d.file_type().await?.is_dir() {
372 | |                         Either::Left(ReadDirStream::new(fs::read_dir(d.path()).await?))
...   |
375 | |                     })
376 | |                 })
    | |                 -
    | |                 |
    | |_________________the expected `async` block
    |                   the found `async` block
...
397 |           .boxed()
    |            ^^^^^ one type is more general than the other
    |
    = note: while checking the return type of the `async fn`
    = note: while checking the return type of the `async fn`
    = note: expected opaque type `impl futures::Future`
               found opaque type `impl futures::Future`

which gave me the thought that there might be some lifetime troubles between all of the combinators and the generated async blocks, so I rewrote it using for and while let loops and the error has gone. I can do some further experimentation if you'd like me to try and narrow down which specific combinator is the cause of the problem.

Tails commented 3 years ago

Amazing, your branch compiles! Lifesaver.

ghost commented 3 years ago

Thank you so much, @Christian7573! It works!

koivunej commented 3 years ago

I do remember wondering if there should be "an external crate" test against cases like this.

In #470 it would appear that it does reproduce this issue.

Joera commented 3 years ago

Christian, you may have just saved my week!

koivunej commented 3 years ago

Please note that the above issue was fixed in #470 which is now merged.

Joera commented 3 years ago

Many thanks to you too, Koivunej!

hannotauber commented 2 years ago

@koivunej Is there any plan for new release here?

koivunej commented 2 years ago

Yes, the plan is to release it. Current blockers include the #480, and of course libp2p needs to be updated once more.

Perhaps the git dependency works in the meanwhile.

koivunej commented 2 years ago

unixfs 0.3.0 since there's the #[non_exhaustive] which at least requires a later rustc.