Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compilation error #34

Closed
lh3 opened this issue Aug 7, 2024 · 15 comments
Closed

Compilation error #34

lh3 opened this issue Aug 7, 2024 · 15 comments

Comments

@lh3
Copy link

lh3 commented Aug 7, 2024

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

rob-p commented Aug 7, 2024

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

jermp commented Aug 7, 2024

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

Thanks,
-Giulio

@lh3
Copy link
Author

lh3 commented Aug 7, 2024

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

jermp commented Aug 7, 2024

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

@lh3
Copy link
Author

lh3 commented Aug 7, 2024

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

lh3 commented Aug 7, 2024

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 lh3 closed this as completed Aug 7, 2024
@lh3
Copy link
Author

lh3 commented Aug 7, 2024

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

heuermh commented Aug 23, 2024

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

lh3 commented Aug 23, 2024

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

lh3 commented Aug 23, 2024

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

jermp commented Aug 23, 2024

Thank you @lh3 for the answer!

@rob-p
Copy link
Collaborator

rob-p commented Aug 23, 2024

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

mr-eyes commented Aug 23, 2024

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

lh3 commented Aug 23, 2024

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

rob-p commented Aug 23, 2024

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.

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

No branches or pull requests

5 participants