georust / geo

Geospatial primitives and algorithms for Rust
https://crates.io/crates/geo
Other
1.51k stars 195 forks source link

Compiling with cross set to target `armv7-unknown-linux-gnueabihf` causes issues with `try_map_coords_inplace`. #1010

Closed AlterionX closed 1 year ago

AlterionX commented 1 year ago

Repro repo: https://github.com/AlterionX/bug-repro/tree/cross-geo-compilation

Running cross build --release fails to compile with the following:

error[E0275]: overflow evaluating the requirement `[closure@/home/benxu/.cargo/registry/src/index.crates.io-6f17d22bba15001f/geo-0.24.1/src/algorithm/map_coords.rs:855:69: 855:72]: Fn<(geo_types::Coord<T>,)>`
  |
  = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`geo`)
  = note: required for `&[closure@/home/benxu/.cargo/registry/src/index.crates.io-6f17d22bba15001f/geo-0.24.1/src/algorithm/map_coords.rs:855:69: 855:72]` to implement `Fn<(geo_types::Coord<T>,)>`
  = note: 128 redundant requirements hidden
  = note: required for `&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&[closure@/home/benxu/.cargo/registry/src/index.crates.io-6f17d22bba15001f/geo-0.24.1/src/algorithm/map_coords.rs:855:69: 855:72]` to implement `Fn<(geo_types::Coord<T>,)>`

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

cross build, cargo build, and cargo build --release all work.

cross is targeting armv7-unknown-linux-gnueabihf. cargo is targeting x86_64-unknown-linux-gnu. I have not tested with other architectures.

Bumping up the recusion limit as suggested simply increases the number of &s, suggesting an actual error?

rustc --version
rustc 1.71.0-nightly (7908a1d65 2023-04-17)
AlterionX commented 1 year ago

Fork with problematic compilation portion panic-ing instead: https://github.com/AlterionX/geo/tree/monkeypatch

michaelkirk commented 1 year ago

Thank you for providing the reproduction details, but I'm still having trouble reproducing this.

Note that I am running on macos.

I'm not very familiar with cross, but I installed it and everything seemed to just work.

$ rustc --version
rustc 1.69.0 (84c898d65 2023-04-16)
$ cross build --release
...
    Finished release [optimized] target(s) in 2.30s
$ rustc --version               
rustc 1.71.0-nightly (39c6804b9 2023-04-19)
$ cross +nightly build --release
...
    Finished release [optimized] target(s) in 1.97s

Do things work for you on rust stable?

One thing I'll note is that error is originating inside of some deprecated code that I think we could probably delete at this point. Do you want to try building from this branch which removes the deprecated code?

https://github.com/georust/geo/tree/mkirk/remove-deprecated-map-coords

AlterionX commented 1 year ago

I'm running on WSL Ubuntu, architecture is x86_64. cross is accessing Docker on windows.

If you're on Mac, perhaps it's not causing a problem due to the mac's architecture being ARM?

I would expect that changing the branch to the version listed to resolve the issues.

AlterionX commented 1 year ago

Ah, rustc fixed something on their side. Closing.

michaelkirk commented 1 year ago

Another user is hitting this issue @AlterionX - can you clarify, were you able to get this working on an updated nightly?

(And which version was it that worked for you?)

AlterionX commented 1 year ago

Yep, I was able to get it working on nightly @michaelkirk . Not sure which version, but I assume 2023-04-20

I ended up resetting my PC so might take a bit to check the most recent nightly though.

lnicola commented 1 year ago

This might be a bug on our side, according to https://github.com/rust-lang/rust/issues/110475.

worace commented 1 year ago

Started hitting this as well installing a crate that depends on geo. Here are some versions and logs in case it helps anyone.

$ rustup -V
rustup 1.26.0 (5af9b9484 2023-04-05)
info: This is the version for the rustup toolchain manager, not the rustc compiler.
info: The currently active `rustc` version is `rustc 1.71.0-nightly (18bfe5d8a 2023-05-14)`

$ cargo install geoq
# ...truncated
error[E0275]: overflow evaluating the requirement `[closure@/Users/horace/.cargo/registry/src/index.crates.io-6f17d22bba15001f/geo-0.23.1/src/algorithm/map_coords.rs:855:69: 855:72]: Fn<(geo_types::Coord<T>,)>`
  |
  = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`geo`)
  = note: required for `&[closure@/Users/horace/.cargo/registry/src/index.crates.io-6f17d22bba15001f/geo-0.23.1/src/algorithm/map_coords.rs:855:69: 855:72]` to implement `Fn<(geo_types::Coord<T>,)>`
  = note: 128 redundant requirements hidden
  = note: required for `&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&...` to implement `Fn<(geo_types::Coord<T>,)>`
  = note: the full type name has been written to '/var/folders/75/9336gq4s19s_h_cd11cfx8q80000gn/T/cargo-installxywk3g/release/deps/geo-f93e1a6d8732e9b2.long-type-369038258378914190.txt'

For more information about this error, try `rustc --explain E0275`.
error: could not compile `geo` (lib) due to previous error
warning: build failed, waiting for other jobs to finish...
error: failed to compile `geoq v0.0.25`, intermediate artifacts can be found at `/var/folders/75/9336gq4s19s_h_cd11cfx8q80000gn/T/cargo-installxywk3g`

I'll be honest I had not run into the recursion_limit before. Does it have to be set by the crate publisher? Is it possible for me to increase the limit for my own build when I depend on the crate?

lnicola commented 1 year ago

@worace the recursion limit is a red herring. There might have been a bug in some geo code that tried to keep some deprecated methods working. We removed the code, but there's no release yet.

Unfortunately, the easiest fix is probably pinning an older nightly (or using cargo install --locked as suggested in the original issue).

worace commented 1 year ago

ok good to know. I'll try the next release of geo whenever it comes out

michaelkirk commented 1 year ago

This has been published just now with https://crates.io/crates/geo/0.25.0