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

Build libproj 9.6.0 images #41

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Build libproj 9.6.0 images #41

wants to merge 1 commit into from

Conversation

urschrei
Copy link
Member

  • I agree to follow the project's code of conduct.
  • I added an entry to the project's change log file if knowledge of this change could be valuable to users.
    • Usually called CHANGES.md or CHANGELOG.md
    • Prefix changelog entries for breaking changes with "BREAKING: "

@urschrei urschrei force-pushed the push-tqqszkmokmuq branch 2 times, most recently from 4b34f2b to e2d676d Compare March 18, 2025 20:58
@urschrei
Copy link
Member Author

We have two failing builds on 1.81, related to libsqlite-sys. This is confusing because it's a Rust error, and the images for proj 0.29.0 (same MSRV) built correctly.

@urschrei
Copy link
Member Author

I say "built correctly", but that's only technically accurate: I bypassed the tests so the images would build and become available to ecosystem consumers. The tests pass on the proj side:

https://github.com/georust/proj/actions/runs/13901038496

 
Fail fast on main build step, continue otherwise
@urschrei urschrei force-pushed the push-tqqszkmokmuq branch from e2d676d to 0230287 Compare March 18, 2025 23:47
@urschrei
Copy link
Member Author

urschrei commented Mar 19, 2025

OK, can repro locally: in proj-sys:

cargo +1.81 build

fails with the rust-lang/rust#123743 issue.

@urschrei
Copy link
Member Author

I see the problem. Because we didn't have to update our bindings for the 0.29.0 release, we didn't hit it: libsqlite3-sys 0.32.0 depends on bindgen ^0.71. That combination will not build on Rust 1.81, but we didn't notice because crate consumers don't use the buildtime_bindgen feature: we bundle the bindings so they don't have to. In order for it to build we would have to bump our MSRV (and thus geo's) to 1.82. That's outside our stated MSRV of six months until the 17th of April. What I'm trying to figure out is whether it matters: when I downgrade bindgen to 0.69.5 and libsqlite-sys to 0.31 I can generate the bindings and save them. If I then subsequently bump the deps up again, libsqlite3-sys 0.32 has no problem with the generated bindings.

This mostly matters because I want libproj 9.6.0 in the next geo release because it's hot off the presses, so I want to merge georust/proj#223 and then release it so we can incorporate it into geo. I assume we'll be cutting a new release before the 17th of April…

@urschrei
Copy link
Member Author

@michaelkirk @weiznich tagging you to get some thoughts.

@michaelkirk
Copy link
Member

Good sleuthing!

Checking in bindings built using a different version of rust seems totally fine.

consumers don't use the buildtime_bindgen feature

Just for my own understanding, isn't it possible for consumers to use this feature with a feature flag?

I imagine it's a pretty small group of people wanting to use that feature though - and then the subset of that small group who are also on our oldest rust would seem very small indeed. I'd wager that any downside here is much outweighed by being able to update libproj. 👍

@weiznich
Copy link

libsqlite3-sys 0.32.0 depends on bindgen ^0.71. That combination will not build on Rust 1.81, but we didn't notice because crate consumers don't use the buildtime_bindgen feature: we bundle the bindings so they don't have to. In order for it to build we would have to bump our MSRV (and thus geo's) to 1.82.

proj-sys currently allows any libsqlite3-sys version between 0.28 and including 0.32. If you build with resolver=2 that will automatically resolve to the newest version if no lock file exists, so 0.32, which might be not compatible with your rust version. If you build with the new resolver=3 setting it should select the latest version that is compatible with your rust version (assuming libsqlite3-sys declares these correctly). You can also manually downgrade the libsqlite3-sys, version locally via cargo update -p [email protected] --precise=0.28.0 to get an older version with resolver=2. Any older version won't use bindgen 0.71, but an older version that is compatible with older rust versions. So yes, in the end users that care about older rust version would need to either have a existing Cargo.lock file and carefully update only single dependencies or manually tweak their dependencies via cargo update. Depending on how strict you interprete your MSRV policy that might be inside (== builds with specific dependency versions) or outside (== builds with all dependency versions). If you follow the former MSRV interpretation you likely can fix the tests by replacing the cargo test call with cargo minimal-versions test like I recently did for the proj-rs CI setup.

I personally do not care much about support for older rust versions, as I mostly use the latest stable rust for projects that depend on proj-sys.

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

Successfully merging this pull request may close these issues.

3 participants