[S2Geography] New recipe by asinghvi17 · Pull Request #9450 · JuliaPackaging/Yggdrasil

asinghvi17 · 2024-09-18T13:03:05Z

https://github.com/paleolimbot/S2Geography is a wrapper around S2Geometry that provides a GEOS-like C++ API around the S2 objects which often don't act like geospatial objects.

This PR is now ready to go.

Co-authored-by: Mosè Giordano <765740+giordano@users.noreply.github.com>

fingolfin · 2025-11-24T21:10:14Z

@asinghvi17 hi there, are you still interested in completing this?

asinghvi17 · 2025-11-24T21:51:51Z

Hey @fingolfin, thanks for the ping! I marked this as ready for review and am re-running CI now.

fingolfin · 2025-11-24T21:53:00Z

@asinghvi17 that won't work, you'll have to rebase this on latest master, or merge latest master into it

asinghvi17 · 2025-11-24T21:55:09Z

Somehow it doesn't let me merge online, so I will do that locally tomorrow

fingolfin · 2025-11-24T22:13:57Z

I merged master for you so CI can run

Co-authored-by: Max Horn <max@quendi.de>

- Bump source to tag 0.3.0 (geoarrow now built into main library) - Remove libs2geography_geoarrow product (no longer a separate target) - Add nanoarrow_jll dependency (new requirement in v0.3.0) - Update msvc_to_win32_target.patch for v0.3.0 CMakeLists.txt Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…chContent - Add CMAKE_INSTALL_PREFIX and CMAKE_TOOLCHAIN_FILE so the library installs to the correct destdir and cross-compiles properly - Set FETCHCONTENT_FULLY_DISCONNECTED=ON to use system nanoarrow JLL instead of downloading a separate copy Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Inject find_package(nanoarrow) and create an alias target so the upstream CMakeLists.txt skips its FetchContent download and uses the nanoarrow_jll instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

CMake cannot create an ALIAS of an ALIAS target. Use the real nanoarrow::nanoarrow_shared target instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The /J flag (make char unsigned) is MSVC-specific. When cross-compiling for Windows with MinGW/GCC, this flag is not recognized. Keep _USE_MATH_DEFINES and NOMINMAX for all WIN32, but guard /J behind an MSVC check. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- Bump preferred_gcc_version to v10 to fix MinGW std::filesystem bugs in GCC 8's headers - Filter out all freebsd (OpenSSL sysroot mismatch) and riscv64 (s2geometry not found) platforms Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- Merge MSVC/WIN32 patch and nanoarrow sed hack into a single cmake_fixes.patch - Remove stale TODO comment and duplicate platforms declaration - Delete old msvc_to_win32_target.patch Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Instead of patching the nanoarrow FetchContent block out of CMakeLists.txt, use CMAKE_PROJECT_s2geography_INCLUDE to inject find_package(nanoarrow) after project(). This creates the nanoarrow target before the FetchContent block runs, so it's skipped naturally. The patch now only contains the WIN32/MSVC fix. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The CMAKE_PROJECT_INCLUDE approach found nanoarrow from the wrong sysroot on some platforms (static instead of shared). The combined patch is more reliable since it runs at the right point in cmake processing with the correct find paths. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

asinghvi17 · 2026-02-26T01:57:56Z

According to Claude:

The bundled cmake_fixes.patch makes two changes to upstream's CMakeLists.txt:

Nanoarrow: Replaces the FetchContent download with find_package(nanoarrow) so it uses the system nanoarrow from nanoarrow_jll. An alias target is needed because upstream checks if(NOT TARGET nanoarrow) (plain name) but find_package only creates nanoarrow::nanoarrow_shared.
WIN32/MSVC: Widens if(MSVC) to if(WIN32) so _USE_MATH_DEFINES and NOMINMAX are set when cross-compiling with MinGW, but keeps the MSVC-only /J flag guarded behind if(MSVC).

Match paleolimbot/s2geography#70: try find_package(nanoarrow) first, fall back to FetchContent if not found. This keeps the FetchContent block intact as a fallback rather than deleting it. Once the upstream PR is merged, the nanoarrow hunk of this patch can be dropped. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

asinghvi17 · 2026-02-26T02:24:27Z

Updated to s2geography v0.3.0 (from v0.1.2). Summary of changes:

Source: bumped to tag 0.3.0 (c4d4e5f)
Products: removed libs2geography_geoarrow (geoarrow is now compiled into the main libs2geography)
Dependencies: added nanoarrow_jll (new dep in v0.3.0)
GCC: bumped to v10 (GCC 8's MinGW std::filesystem headers are broken)
Platforms: filtered out freebsd (OpenSSL sysroot mismatch) and riscv64 (s2geometry not found)

The nanoarrow portion of cmake_fixes.patch matches an upstream PR (paleolimbot/s2geography#70) — once merged, that hunk can be dropped.

Test whether MinGW builds work without the MSVC->WIN32 change. MinGW typically defines M_PI without _USE_MATH_DEFINES and may not need NOMINMAX. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

MinGW does not define M_PI without _USE_MATH_DEFINES, confirmed by CI failure. The WIN32 patch is necessary. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Cleaner approach: try find_package inside the existing guard, close it, then let the FetchContent block run under its own guard. This way the find_package only runs if nanoarrow isn't already available. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Update s2geography to 04a31a89 which includes the CMake fixes from paleolimbot/s2geography#70 and JuliaPackaging#71, so the patch is no longer needed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

imciner2 · 2026-03-02T13:20:27Z

@fingolfin any last comments? The recipe looks fine to me now.

asinghvi17 added 5 commits September 18, 2024 06:02

Create S/S2Geography/build_tarballs.jl

f530f5a

Use Pkg in build_tarballs.jl

34eb96c

Apply the MSVC -> Win32 patch

c70ba5f

Fix cd command

8ff5e6b

capitalization?

f89da08

giordano reviewed Sep 18, 2024

View reviewed changes

asinghvi17 and others added 2 commits September 18, 2024 06:53

Apply suggestions from code review

1853d73

Co-authored-by: Mosè Giordano <765740+giordano@users.noreply.github.com>

Revert capitalization

98e2c3f

fingolfin added the Status: Waiting for Author label Nov 24, 2025

asinghvi17 marked this pull request as ready for review November 24, 2025 21:51

asinghvi17 closed this Nov 24, 2025

asinghvi17 reopened this Nov 24, 2025

Merge branch 'master' into as/s2geography

dd213a8

move patch

9cfe02f

fingolfin reviewed Nov 24, 2025

View reviewed changes

Comment thread S/S2Geography/build_tarballs.jl Outdated

fingolfin changed the title ~~[WIP] New recipe: S2Geography~~ [S2Geography] New recipe Nov 24, 2025

fingolfin added the new recipe label Nov 25, 2025

fingolfin removed the Status: Waiting for Author label Jan 6, 2026

fingolfin reviewed Jan 6, 2026

View reviewed changes

Comment thread S/S2Geography/build_tarballs.jl Outdated

jmr mentioned this pull request Jan 6, 2026

Check for WIN32 rather than MSVC when building google/s2geometry#379

Closed

asinghvi17 and others added 5 commits February 25, 2026 17:40

Prefer GCC 8

2983e95

Co-authored-by: Max Horn <max@quendi.de>

Merge remote-tracking branch 'origin/master' into as/s2geography

6584b94

Use system nanoarrow instead of FetchContent in S2Geography build

c4e3678

Inject find_package(nanoarrow) and create an alias target so the upstream CMakeLists.txt skips its FetchContent download and uses the nanoarrow_jll instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

asinghvi17 and others added 4 commits February 25, 2026 19:33

Fix nanoarrow alias: use nanoarrow_shared (not alias-of-alias)

2dbff72

CMake cannot create an ALIAS of an ALIAS target. Use the real nanoarrow::nanoarrow_shared target instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

asinghvi17 force-pushed the as/s2geography branch from e767a9b to 921ba86 Compare February 26, 2026 01:14

asinghvi17 and others added 2 commits February 25, 2026 20:36

asinghvi17 and others added 4 commits February 25, 2026 21:26

Drop WIN32/MSVC patch, keep only nanoarrow find_package

9b4c355

Test whether MinGW builds work without the MSVC->WIN32 change. MinGW typically defines M_PI without _USE_MATH_DEFINES and may not need NOMINMAX. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Restore WIN32/MSVC patch — MinGW needs _USE_MATH_DEFINES

d6c954b

MinGW does not define M_PI without _USE_MATH_DEFINES, confirmed by CI failure. The WIN32 patch is necessary. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Note that cmake_fixes.patch can be dropped after upstream PRs merge

ced966a

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

asinghvi17 requested review from fingolfin and giordano February 26, 2026 11:27

Drop cmake_fixes.patch — now upstreamed in s2geography

0371825

Update s2geography to 04a31a89 which includes the CMake fixes from paleolimbot/s2geography#70 and JuliaPackaging#71, so the patch is no longer needed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[S2Geography] New recipe#9450

[S2Geography] New recipe#9450
asinghvi17 wants to merge 26 commits intoJuliaPackaging:masterfrom
asinghvi17:as/s2geography

asinghvi17 commented Sep 18, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fingolfin commented Nov 24, 2025

Uh oh!

asinghvi17 commented Nov 24, 2025

Uh oh!

fingolfin commented Nov 24, 2025

Uh oh!

asinghvi17 commented Nov 24, 2025 •

edited

Loading

Uh oh!

fingolfin commented Nov 24, 2025

Uh oh!

Uh oh!

Uh oh!

asinghvi17 commented Feb 26, 2026 •

edited

Loading

Uh oh!

asinghvi17 commented Feb 26, 2026

Uh oh!

imciner2 commented Mar 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

asinghvi17 commented Sep 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fingolfin commented Nov 24, 2025

Uh oh!

asinghvi17 commented Nov 24, 2025

Uh oh!

fingolfin commented Nov 24, 2025

Uh oh!

asinghvi17 commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fingolfin commented Nov 24, 2025

Uh oh!

Uh oh!

Uh oh!

asinghvi17 commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asinghvi17 commented Feb 26, 2026

Uh oh!

imciner2 commented Mar 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

asinghvi17 commented Sep 18, 2024 •

edited

Loading

asinghvi17 commented Nov 24, 2025 •

edited

Loading

asinghvi17 commented Feb 26, 2026 •

edited

Loading