jermp / fulgor

Fulgor is a fast and space-efficient colored de Bruijn graph index.
MIT License
44 stars 9 forks source link

Compilation error #34

Closed lh3 closed 3 months ago

lh3 commented 3 months ago

I could not compile fulgor due to an error in one of its dependencies:

   Compiling parallel-processor v0.1.13 (.../fulgor/external/ggcat/libs-crates/parallel-processor-rs)
error[E0562]: `impl Trait` only allowed in function and inherent method return types, not in trait method return types
  --> libs-crates/parallel-processor-rs/src/execution_manager/executor.rs:42:10
   |
42 |     ) -> impl Future<Output = ()> + Send + 'a;
   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: see issue #91611 <https://github.com/rust-lang/rust/issues/91611> for more information

For more information about this error, try `rustc --explain E0562`.
error: could not compile `parallel-processor` (lib) due to previous error

The rustc version I am using is:

rustc 1.72.1 (d5c2e9c34 2023-09-13) (Red Hat 1.72.1-2.el7)
rob-p commented 3 months ago

Hi @lh3,

This is a ggcat compilation error (which fulgor uses for dBG construction). Your rustc version is too old and the feature being used wasn’t stabilized by 1.72. Please try upgrading your rustc with rustup.

Best, Rob

jermp commented 3 months ago

Yes, on my Mac, the rust version I'm using is 1.76.

Thanks, -Giulio

lh3 commented 3 months ago

Thanks for the prompt response! Now I am using

rustc 1.79.0-nightly (c9f8f3438 2024-03-27)

but I got another error:

   Compiling ggcat-cpp-bindings v0.1.0 (.../fulgor/external/ggcat/crates/capi)
error[E0635]: unknown feature `stdsimd`
  --> /homes/hli/.cargo/registry/src/index.crates.io-6f17d22bba15001f/ahash-0.8.6/src/lib.rs:99:42
   |
99 | #![cfg_attr(feature = "stdsimd", feature(stdsimd))]
   |                                          ^^^^^^^

For more information about this error, try `rustc --explain E0635`.
error: could not compile `ahash` (lib) due to previous error

Google pointed me to tkaitchuck/aHash#200. I have nightly installed to compile mapquik a while ago. I will try a non-nightly version of rustc later.

jermp commented 3 months ago

Ok, with a non-nightly version, it should compile just fine. Please, let us know!

lh3 commented 3 months ago

Now I am on the latest stable

rustc 1.80.0 (051478957 2024-07-21)

and I see a new error:

   Compiling time v0.3.30
(... some warnings and then)
error[E0282]: type annotations needed for `Box<_>`
  --> /homes/hli/.cargo/registry/src/index.crates.io-6f17d22bba15001f/time-0.3.30/src/format_description/parse/mod.rs:83:9
   |
83 |     let items = format_items
   |         ^^^^^
...
86 |     Ok(items.into())
   |              ---- type must be known at this point
   |
help: consider giving `items` an explicit type, where the placeholders `_` are specified
   |
83 |     let items: Box<_> = format_items
   |              ++++++++

For more information about this error, try `rustc --explain E0282`.
error: could not compile `time` (lib) due to 1 previous error

I will try v1.76.0 next.

PS: some google results:

lh3 commented 3 months ago

I fixed the compiling issue by manually updating "time" to the latest version in external/ggcat. Perhaps my previous failed builds were locking "time" to an old version.

Anyway, I can run the executable now. Thanks for the help!

lh3 commented 3 months ago

Actually, the Cargo.lock file in the version of ggcat included in fulgor locks "time" to 0.3.30. See

https://github.com/algbio/ggcat/blob/b6a2c2bf409a6a58540b8d23504c4fb7f8c655ef/Cargo.lock#L2859-L2863

heuermh commented 2 months ago

Sorry for coming late on this issue -- what is the actual fix?

$ make -j
...
   Compiling termcolor v1.1.3
   Compiling cxxbridge-flags v1.0.110
error[E0282]: type annotations needed for `Box<_>`
  --> /Users/foo/.cargo/registry/src/index.crates.io-6f17d22bba15001f/time-0.3.30/src/format_description/parse/mod.rs:83:9
   |
83 |     let items = format_items
   |         ^^^^^
...
86 |     Ok(items.into())
   |              ---- type must be known at this point
   |
help: consider giving `items` an explicit type, where the placeholders `_` are specified
   |
83 |     let items: Box<_> = format_items
   |              ++++++++

   Compiling disjoint-sets v0.4.2
   Compiling cxx v1.0.110
   Compiling codespan-reporting v0.11.1
For more information about this error, try `rustc --explain E0282`.
error: could not compile `time` (lib) due to 1 previous error
warning: build failed, waiting for other jobs to finish...
make[3]: *** [lib/libggcat_cpp_bindings.a] Error 101
make[2]: *** [CMakeFiles/ggcat_cpp_api] Error 2
make[1]: *** [CMakeFiles/ggcat_cpp_api.dir/all] Error 2
make: *** [all] Error 2

$ rustc --version
rustc 1.80.1 (3f5fd8dd4 2024-08-06) (Homebrew)
lh3 commented 2 months ago

Find the Cargo.lock file in build/.../ggcat. Open it in a text editor. Search for the time package and manually bump the version from 0.3.30 to 0.3.36. There is probably a better way.

lh3 commented 2 months ago

I am not sure if this is a breaking change in the rust compiler or the time package was using experimental features, but this is concerning.

jermp commented 2 months ago

Thank you @lh3 for the answer!

rob-p commented 2 months ago

@lh3 : this looks like it was a subtle (and unanticipated?) backward incompatibility in 1.80 (which explains why we weren't seeing it with 1.78 / 1.79 etc.). The incompatibility is due to changes in type inference. In fact, the time crate seems to have been the most popular crate affected. Updating the crate version is the recommended fix, but the bigger question is why crater didn't catch this issue earlier.

mr-eyes commented 2 months ago

Note to people still visiting this for a quick time fix (at least worked for me):

cd external/ggcat
rm Cargo.lock
cargo update

Then, proceed with building.

# in fulgor root dir
cmake -Bbuild
cmake --build build
lh3 commented 2 months ago

@rob-p Thanks for the link. The rust compiler is breaking the backward compatibility here. I agree with the commenter from last week that it is a matter of trust – if the rust devs break a popular package now, they can break more in future. Hope they could issue a fix (though my sense is that is probably not happening).

rob-p commented 2 months ago

Hi @lh3. Indeed. In the first link, there is a description from the relevant Rust team why they do not view this as a Rust-lang issue (i.e. essentially and under-specification of the type in the library code itself). However, that feels a little slippery to me. The code compiled before, and now it doesn't. While there may be valid technical reasons, the intuitive back-compat promise is that code won't break within edition boundaries unless it is to fix a soundness issue or critical safety bug.

Alternatively, I'm used to such types of breakages happening in C++ as compilers get changed (newer compilers reject code, even with the same C++ version) that older compilers accepted. However, in that case, since the language has a formal specification (though most folks aren't actually reading it), the compiler implementer can at least point to the spec to say "the previous behavior wasn't compliant and the new behavior is", or "the behavior here is implementation-defined". Though, ultimately, that explanation is no more satisfying than under-specified type constraints.