georust / proj

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

Fix CI #124

Closed michaelkirk closed 2 years ago

michaelkirk commented 2 years ago

Hmm CI is failing on macos proj-sys:

---- src/lib.rs - (line 142) stdout ----
thread 'main' panicked at 'assert_relative_eq!(result.x(), 158458.67, epsilon = 1e-2)
    left  = 158460.18744483535
    right = 158458.67

Looking into it...

nyurik commented 2 years ago

@michaelkirk would it make sense to do an interim release of the proj up to e39a671d9afd52c13e180516d6dffd9b79a89d2e - without the 9.0 update?

michaelkirk commented 2 years ago

@michaelkirk would it make sense to do an interim release of the proj up to https://github.com/georust/proj/commit/e39a671d9afd52c13e180516d6dffd9b79a89d2e - without the 9.0 update?

If you want to make a branch that passes CI from there I'd consider it.

nyurik commented 2 years ago

@michaelkirk would it make sense to do an interim release of the proj up to e39a671 - without the 9.0 update?

If you want to make a branch that passes CI from there I'd consider it.

@michaelkirk https://github.com/georust/proj/compare/v0.25?expand=1 (not for merging into main) -- the v0.25 branch

Sigh, seems like we have so much cross-repo dependencies, it is not possible to simply update a little thing - because it depends on the latest (?) release of geo, which has already been updated to 2021 edition. We ought to make it less tightly coupled, so that each project can be easily released without breaking the whole CI chain elsewhere

michaelkirk commented 2 years ago

Wild!

This is the last build that succeeded https://github.com/georust/proj/runs/5796562852?check_suite_focus=true#step:3:10

> ==> Downloading https://ghcr.io/v2/homebrew/core/proj/manifests/9.0.0_1
...
> ==> Pouring proj--9.0.0_1.big_sur.bottle.tar.gz
> 🍺  /usr/local/Cellar/proj/9.0.0_1: 73 files, 19MB

And this build failed: https://github.com/georust/proj/runs/6056168754?check_suite_focus=true#step:3:10

> ==> Downloading https://ghcr.io/v2/homebrew/core/proj/manifests/9.0.0_1-1
> ==> Pouring proj--9.0.0_1.big_sur.bottle.1.tar.gz
...
> 🍺  /usr/local/Cellar/proj/9.0.0_1: 436 files, 588MB

I wonder if the new (much larger) bottle contains some of the offline grid data or something, causing different results.

michaelkirk commented 2 years ago

Here are the two changes between the 9.0.0_1 vs 9.0.0_1-1 bottles:

seems probably innocuous:

statically link libraries:

https://github.com/Homebrew/homebrew-core/commit/89a28a5c9dba7b9f9994a55bb561083832432f31

🚨supports my theory about offline data changing:

Replace deprecated proj-datumgrid with proj-data

https://github.com/Homebrew/homebrew-core/commit/b3e7a635b35e77ece5e194a9520f061fd4afcaea

michaelkirk commented 2 years ago

In a ~hilarious~ (my bad) turn of events, the "proj-sys" CI for macos was actually only running tests for "proj", not "proj-sys".

I think they are failing because that job is using libproj from homebrew, rather than bundled proj. Homebrew recently started including some grid data in it's installation: https://github.com/Homebrew/homebrew-core/commit/b3e7a635b35e77ece5e194a9520f061fd4afcaea

Which gives different, presumably better, results. Maybe we should be doing that for our build too? That's another discussion though (I opened an issue: https://github.com/georust/proj/issues/125)

I think this can be merged!

(and then I think the release/0.26.0 should go out)

nyurik commented 2 years ago

well done!

michaelkirk commented 2 years ago

bors r+

bors[bot] commented 2 years ago

Build succeeded: