georust / proj

Rust bindings for the latest stable release of PROJ
https://docs.rs/proj
Apache License 2.0
146 stars 46 forks source link

Proj isnt compiling in docker container, but does locally #163

Closed gmmorris closed 1 year ago

gmmorris commented 1 year ago

I'm not sure if this is a Proj issue specifically, this might be an ffi issue, but I'm hoping you'll have a better hunch than I.

I have a project that compiles locally just fine, and runs fine on an ubuntu based image in CI. When I try to build a local docker image, I get the following error:

#16 236.3 error[E0308]: mismatched types
#16 236.3   --> /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/proj-0.27.0/src/network.rs:53:48
#16 236.3    |
#16 236.3 53 |             let _ = unsafe { CString::from_raw(header.as_ptr() as *mut i8) };
#16 236.3    |                              ----------------- ^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `*mut u8`, found `*mut i8`
#16 236.3    |                              |
#16 236.3    |                              arguments to this function are incorrect
#16 236.3    |
#16 236.3    = note: expected raw pointer `*mut u8`
#16 236.3               found raw pointer `*mut i8`
#16 236.3 note: associated function defined here
#16 236.3   --> /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/alloc/src/ffi/c_str.rs:397:19
#16 236.3
#16 236.3 For more information about this error, try `rustc --explain E0308`.
#16 236.3 error: could not compile `proj` (lib) due to previous error

It seems proj can't compile due to a type mismatch with CString in the container. I'm guessing this an API being called in proj-sys? But don't really know where to start debugging this. 😅

Any advice would be highly appreciated!

urschrei commented 1 year ago

Hmmmm. What Rust version is running in the Docker container?

gmmorris commented 1 year ago

I'm just using the official rust:1.70.0 image, which I believe is running latest stable 🤔 but I might be wrong - https://hub.docker.com/_/rust.

I can easily try other versions, if you think that might be the issue.

urschrei commented 1 year ago

That should be fine and that call to from_raw is also correct: it expects *mut i8, and it's been available since Rustc 1.4: https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw.

gmmorris commented 1 year ago

Weird, I've tried boiling down by dockerfile to the bare minimum needed for it to reproduce the issue.

Here it is: Dockerfile:

FROM rust:1.70

WORKDIR /app

# Install the required system dependencies based on what I'm seeing in the georust project: 
# See here: https://github.com/georust/docker-images/blob/main/rust-1.70/libproj-builder.Dockerfile
RUN apt-get update \
    && DEBIAN_FRONTEND="noninteractive" apt-get install -y \
    lld \
    clang \
    libcurl4-gnutls-dev \
    libsqlite3-dev \
    libtiff5-dev \
    cmake \
    pkg-config \
    sqlite3 \
    wget
# Copy all files from our working environment to our Docker image
COPY . .

RUN cargo build --release

ENTRYPOINT ["./target/release/nb-platform"]

And this gives me the above error:

#9 224.7 error[E0308]: mismatched types
#9 224.7   --> /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/proj-0.27.0/src/network.rs:53:48
#9 224.7    |
#9 224.7 53 |             let _ = unsafe { CString::from_raw(header.as_ptr() as *mut i8) };
#9 224.7    |                              ----------------- ^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `*mut u8`, found `*mut i8`
#9 224.7    |                              |
#9 224.7    |                              arguments to this function are incorrect
#9 224.7    |
#9 224.7    = note: expected raw pointer `*mut u8`
#9 224.7               found raw pointer `*mut i8`
#9 224.7 note: associated function defined here
#9 224.7   --> /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/alloc/src/ffi/c_str.rs:397:19

Is it possible that I'm missing some platform-specific dependency or flag? 🤔

urschrei commented 1 year ago

Is your Docker build running on aarch64 or another ARM platform? I've just pushed an experiment to my clone. If you update your proj dependency in Cargo.toml to:

proj = { git = "https://github.com/urschrei/rust-proj.git", features = ["network"] }

(Make sure you specify the same features as you are currently if you're using e.g. bundled-proj). Then run cargo update locally to make sure it works OK, and if it does, push the change to your Docker build.

gmmorris commented 1 year ago

Is your Docker build running on aarch64 or another ARM platform?

yes, and maybe 🤣 I've reproduced this locally (Macbook M1, so ARM) and on Github Actions which I'm not actually sure what architecture it is under the hood (I just specified ubuntu), but I'll do some googling to find out.

I've just pushed an experiment to my clone.

Thank you! I've out and about this afternoon, but I'll give this a try in a few hours when I'm back in front of my laptop. 🙏 Thank you so much for the quick engagement, I really appreciate it!

gmmorris commented 1 year ago

I just discovered that if I build from the latest Georust image (georust/geo-ci:proj-9.1.0-rust-1.70) it works. It fails on a different step, obviously, but passes compilation. 🤔 This isn't ideal as that image is quite big (and I'm not 100% sure what's in it), but perhaps this can help identify why compilation is failing in the container.

I'll give your fork a test shortly.

gmmorris commented 1 year ago

Is your Docker build running on aarch64 or another ARM platform? I've just pushed an experiment to my clone. If you update your proj dependency in Cargo.toml to:

proj = { git = "https://github.com/urschrei/rust-proj.git", features = ["network"] }

(Make sure you specify the same features as you are currently if you're using e.g. bundled-proj). Then run cargo update locally to make sure it works OK, and if it does, push the change to your Docker build.

This worked! Managed to build the container locally with this version instead of the crate on crates.io.

Thanks @urschrei !

Once this version is released I'll reenable the network feature (I've disabled it on CI again and the tests that fail without Network for the time being).

urschrei commented 1 year ago

Great, I'll see about integrating that as a fix and releasing a new crates.io version this week.

gmmorris commented 1 year ago

Thank you! I really appreciate the quick turnaround. Seriously, as an OSS maintainer of many years, I know how hard and thankless this can be - thank you 🙏 .

urschrei commented 1 year ago

@gmmorris This fix is now available on crates.io as 0.27.1

gmmorris commented 1 year ago

Thank you!