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

fix: compilation error when used as a dependency #470

Merged
merged 8 commits into from
Aug 4, 2021

Conversation

koivunej
Copy link
Collaborator

@koivunej koivunej commented Aug 2, 2021

Fixes the issue reported in comment of #458 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.

@koivunej
Copy link
Collaborator Author

koivunej commented Aug 2, 2021

Pasted the error in a gist: https://gist.github.com/koivunej/a57c843758b01bd82e9883d9c997e857

@koivunej koivunej force-pushed the add_dependent_crate branch 2 times, most recently from afa0e47 to 159678a Compare August 2, 2021 17:28
@koivunej koivunej changed the title ci: add dependent crate fix: compilation error when used as a dependency Aug 2, 2021
@koivunej koivunej marked this pull request as ready for review August 2, 2021 17:33
src/repo/fs/blocks.rs Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@koivunej koivunej force-pushed the add_dependent_crate branch 2 times, most recently from 20480ff to bfe3626 Compare August 3, 2021 09:15
@koivunej
Copy link
Collaborator Author

koivunej commented Aug 3, 2021

Thought of keeping the back and forth of "ci: add dependent crate" to remind what went down, but will probably just clean it up if it now builds ok.

@koivunej
Copy link
Collaborator Author

koivunej commented Aug 3, 2021

Finally the expansions between async-trait versions: https://gist.github.com/koivunej/cbcaae52b7242a73419ef62e4c606bd7

@CHr15F0x
Copy link
Contributor

CHr15F0x commented Aug 3, 2021

lgtm

src/repo/fs/blocks.rs Outdated Show resolved Hide resolved
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.
either required the use of the empty stream, which probably was an
additional extra type which is now worked away with.
this was kept around for ipfs-http but it lead to problems using the
ipfs crate as a dependency.
hopefully this will catch if we go out of sync with some upstream
crates.
this was not set originally because probably mpart-async sets it, but
the code in `ipfs` assumed `std` feature was enabled and thus the from
conversion was accessible.
@koivunej
Copy link
Collaborator Author

koivunej commented Aug 4, 2021

Cancelling, letting bors handle.

bors r+

bors bot added a commit that referenced this pull request 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 <[email protected]>
@koivunej
Copy link
Collaborator Author

koivunej commented Aug 4, 2021

Hmm.... Wonder what's happening. master is not yet fast-forwarded to staging even thought he build succeeded.

bors ping

@bors
Copy link
Contributor

bors bot commented Aug 4, 2021

pong

@koivunej
Copy link
Collaborator Author

koivunej commented Aug 4, 2021

Cannot find any related bug reports...

bors r+

@koivunej
Copy link
Collaborator Author

koivunej commented Aug 4, 2021

Aah found it for the earlier merge from the logs, it was a crash:

batch | {:timeout,  {GenServer, :call,   [     BorsNG.GitHub,     {:get_file, {{:installation, 10059986}, 170600558},      {"staging", ".github/bors.toml"}},     10000   ]}}
-- | --

@bors
Copy link
Contributor

bors bot commented Aug 4, 2021

Build succeeded:

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

Successfully merging this pull request may close these issues.

None yet

4 participants