Skip to content
This repository has been archived by the owner on Oct 23, 2022. It is now read-only.

Release ipfs 0.3.0, ipfs-unixfs 0.3.0 #458

Open
koivunej opened this issue Feb 6, 2021 · 22 comments
Open

Release ipfs 0.3.0, ipfs-unixfs 0.3.0 #458

koivunej opened this issue Feb 6, 2021 · 22 comments

Comments

@koivunej
Copy link
Collaborator

koivunej commented Feb 6, 2021

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.

@koivunej koivunej changed the title Release ipfs 0.3, ipfs-unixfs 0.2.1 Release ipfs 0.3.0, ipfs-unixfs 0.2.1 Feb 6, 2021
@Tails
Copy link

Tails commented Jun 27, 2021

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
Copy link
Collaborator Author

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
Copy link

Tails commented Jun 29, 2021

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
Copy link
Collaborator Author

koivunej commented Jul 2, 2021

@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
Copy link

Tails commented Jul 4, 2021

Thanks for the heads-up!

@Tails
Copy link

Tails commented Jul 5, 2021

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
Copy link
Collaborator Author

koivunej commented Jul 7, 2021

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
Copy link

Tails commented Jul 8, 2021

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
Copy link

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
Copy link

Tails commented Jul 13, 2021

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
Copy link
Collaborator Author

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
Copy link

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
Copy link

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
Copy link

Tails commented Jul 15, 2021

Amazing, your branch compiles! Lifesaver.

@ghost
Copy link

ghost commented Jul 16, 2021

Thank you so much, @Christian7573! It works!

Christian7573 pushed a commit to Christian7573/rust-ipfs that referenced this issue Jul 16, 2021
Christian7573 pushed a commit to Christian7573/rust-ipfs that referenced this issue Jul 16, 2021
Christian7573 pushed a commit to Christian7573/rust-ipfs that referenced this issue Jul 16, 2021
Christian7573 pushed a commit to Christian7573/rust-ipfs that referenced this issue Jul 16, 2021
Christian7573 pushed a commit to Christian7573/rust-ipfs that referenced this issue Jul 16, 2021
koivunej added a commit to eqlabs/rust-ipfs that referenced this issue Aug 2, 2021
there is an issue reported in comments of rs-ipfs#458 that it happens on any
dependent crate, without any code. hoping that this will work as a test
case so that we wont introduce such again.
@koivunej
Copy link
Collaborator Author

koivunej commented Aug 2, 2021

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.

koivunej added a commit to eqlabs/rust-ipfs that referenced this issue Aug 2, 2021
there is an issue reported in comments of rs-ipfs#458 that it happens on any
dependent crate, without any code. hoping that this will work as a test
case so that we wont introduce such again.
koivunej added a commit to eqlabs/rust-ipfs that referenced this issue Aug 2, 2021
see comments in rs-ipfs#458. it is unknown why this is needed but it does work
away of the compiler error.
koivunej added a commit to eqlabs/rust-ipfs that referenced this issue Aug 2, 2021
see comments in rs-ipfs#458. it is unknown why this is needed but it does work
away of the compiler error.
koivunej added a commit to eqlabs/rust-ipfs that referenced this issue Aug 3, 2021
see comments in rs-ipfs#458. it is unknown why this is needed but it does work
away of the compiler error.

it was later found out by bisecting Cargo.lock that this is caused by
async-trait 0.1.42 => 0.1.43 upgrade, but it's very unclear why this
would be caused by the only PR in that release.
koivunej added a commit to eqlabs/rust-ipfs that referenced this issue Aug 3, 2021
see comments in rs-ipfs#458. it is unknown why this is needed but it does work
away of the compiler error.

it was later found out by bisecting Cargo.lock that this is caused by
async-trait 0.1.42 => 0.1.43 upgrade, but it's very unclear why this
would be caused by the only PR in that release.
koivunej added a commit to eqlabs/rust-ipfs that referenced this issue Aug 4, 2021
see comments in rs-ipfs#458. it is unknown why this is needed but it does work
away of the compiler error.

it was later found out by bisecting Cargo.lock that this is caused by
async-trait 0.1.42 => 0.1.43 upgrade, but it's very unclear why this
would be caused by the only PR in that release.
bors bot added a commit that referenced this issue Aug 4, 2021
470: fix: compilation error when used as a dependency r=koivunej a=koivunej

Fixes the issue reported in [comment of #458](#458 (comment)) that whenever dependent on, even without any code calling `ipfs`, there is a build failure. This PR:

- removes top-level Cargo.lock now that I've learned why it's not good to keep around
- adds weekly scheduled build
- moves the `FsBlockStore::list` method body over to `FsBlockStore::list0` outside the `#[async_trait] impl BlockStore for FsBlockStore { ... }` where it does get the correct lifetimes inferred for the returned boxed future

The original compilation failure was caused by `async-trait` 0.1.42 => 0.1.43 upgrade deduced by bisecting Cargo.lock changes. It's still unknown why does the moved version work, but there are probably a lot of subleties to the code generated by `async-trait`. I don't yet know if this warrants any upstream bug reports; most likely not, we were stuck on a quite old version and many crates now work with it.

Co-authored-by: Joonas Koivunen <joonas.koivunen@gmail.com>
bors bot added a commit that referenced this issue Aug 4, 2021
470: fix: compilation error when used as a dependency r=koivunej a=koivunej

Fixes the issue reported in [comment of #458](#458 (comment)) that whenever dependent on, even without any code calling `ipfs`, there is a build failure. This PR:

- removes top-level Cargo.lock now that I've learned why it's not good to keep around
- adds weekly scheduled build
- moves the `FsBlockStore::list` method body over to `FsBlockStore::list0` outside the `#[async_trait] impl BlockStore for FsBlockStore { ... }` where it does get the correct lifetimes inferred for the returned boxed future

The original compilation failure was caused by `async-trait` 0.1.42 => 0.1.43 upgrade deduced by bisecting Cargo.lock changes. It's still unknown why does the moved version work, but there are probably a lot of subleties to the code generated by `async-trait`. I don't yet know if this warrants any upstream bug reports; most likely not, we were stuck on a quite old version and many crates now work with it.

Co-authored-by: Joonas Koivunen <joonas.koivunen@gmail.com>
@Joera
Copy link

Joera commented Aug 13, 2021

Christian, you may have just saved my week!

@koivunej
Copy link
Collaborator Author

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

@Joera
Copy link

Joera commented Aug 16, 2021

Many thanks to you too, Koivunej!

Mirko-von-Leipzig pushed a commit to Mirko-von-Leipzig/rust-ipfs that referenced this issue Aug 16, 2021
see comments in rs-ipfs#458. it is unknown why this is needed but it does work
away of the compiler error.

it was later found out by bisecting Cargo.lock that this is caused by
async-trait 0.1.42 => 0.1.43 upgrade, but it's very unclear why this
would be caused by the only PR in that release.
@hannotauber
Copy link

@koivunej Is there any plan for new release here?

@koivunej
Copy link
Collaborator Author

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 koivunej changed the title Release ipfs 0.3.0, ipfs-unixfs 0.2.1 Release ipfs 0.3.0, ipfs-unixfs 0.3.0 Jan 31, 2022
@koivunej
Copy link
Collaborator Author

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants