Skip to content

Stage icu for the macOS osx_arm64 test path too#141

Open
estebanzimanyi wants to merge 2 commits into
fix/macos-int64-bigint-to-set-forwarderfrom
fix/stage-icu-macos-platform
Open

Stage icu for the macOS osx_arm64 test path too#141
estebanzimanyi wants to merge 2 commits into
fix/macos-int64-bigint-to-set-forwarderfrom
fix/stage-icu-macos-platform

Conversation

@estebanzimanyi
Copy link
Copy Markdown
Member

The stage_icu helper mapped only the Linux uname values, so on the macOS arm64 test runner uname -m returned arm64 and the locally built icu extension was copied to .duckdb/extensions/v1.4.4/arm64 instead of .../osx_arm64 where DuckDB autoload looks, and the hub fallback is not reliably resolvable there, so the osx_arm64 Test step failed to load the extension. This maps the OS and architecture to the DuckDB platform string so icu is staged where autoload expects on every tested platform, with the Linux mapping unchanged. It is logically an iteration of the #136 icu pre-stage (build/CI wiring); stacked on #140 so the leaf goes fully green, and the committer may prefer to fold it into #136 at merge. Stacked on fix/macos-int64-bigint-to-set-forwarder (#140); its commit shows in this diff until it merges.

The stage_icu helper mapped only the Linux uname values, so on the
macOS arm64 test runner uname -m returned "arm64" and the icu
extension was copied to .duckdb/extensions/v1.4.4/arm64 instead of
.../osx_arm64, where DuckDB's autoload looks. The hub fallback is not
reliably resolvable on that runner, so the osx_arm64 Test step failed
to load the extension. Map the OS and architecture to the DuckDB
platform string (linux_amd64, linux_arm64, osx_amd64, osx_arm64) so
the locally built icu is staged at the path autoload expects on every
tested platform; the Linux mapping is unchanged.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@estebanzimanyi
Copy link
Copy Markdown
Member Author

Reviewer's quickstart — ~2 minutes

What this PR does: Stage icu for the macOS osx_arm64 test path too.

Files to read: `` (plus minimal surrounding context).

Risk: narrow scope; the diff is small and self-contained. Stacked on its base PR — once the stack ahead of it lands, this rebases trivially.

Cross-link: Linux arm64 CI needs #161 for the MeosType build error (orthogonal to this PR's content).

`meosType` (lower-case) is the **pre-consolidation** MEOS type name;
`MeosType` (upper-case) is the **post-consolidation** target that the
upstream rename sweep has not yet reached.  The current vcpkg pin
(`vcpkg_ports/meos/portfile.cmake` REF f11b7443ee98…) is still
pre-consolidation: `meos/include/temporal/meos_catalog.h` line 121
declares the typedef as `} meosType;` and every MEOS API uses the
lower-case spelling.  MobilityDuck's source code consistently uses
`meosType` to match — `grep -rn '\bMeosType\b' src/` finds the name
only on the alias line and its comment, nowhere else.

c8cad6d added `using meosType = MeosType;` as a forward-looking
bridge for the eventual consolidation bump.  That bridge points at
`MeosType`, which the current pin does NOT yet expose, so it
breaks every PR's Linux arm64 build with:

  /duckdb_build_dir/src/include/tydef.hpp:18:18:
    error: ‘MeosType’ does not name a type; did you mean ‘meosType’?

The fix is to drop the premature alias and replace the misleading
comment with one that documents the pre/post-consolidation distinction
and the resume path for the next pin bump — at that point a reviewer
can either restore the bridge (this time it'll be valid because
`MeosType` will exist) or sweep the MobilityDuck source from
`meosType` to `MeosType` in a single PR.

Unblocks every in-flight PR's Linux arm64 build: #126, #130, #149,
#158, #159, #160, plus the entire `feat/*_port_core` extended-type
stack (#148/#150/#151/#153/#155/#156).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant