Skip to content

fix(query): compile libpg_query from the submodule instead of a copy in OUT_DIR - #1106

Merged
psteinroe merged 1 commit into
mainfrom
fix/pgls-query-build-from-submodule
Nov 2, 2026
Merged

psteinroe merged 1 commit into
mainfrom
fix/pgls-query-build-from-submodule

Conversation

@psteinroe

@psteinroe psteinroe commented Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator

build.rs copied the vendored libpg_query tree into OUT_DIR and compiled every *.c it found there. The copy overwrites files but never removes any, so an OUT_DIR that a build of another libpg_query version left behind still holds that version's files, and the glob compiles them too. That's what broke Lint, Codegen and Test on #1072: CI restored the target/ that main saved after #1055, and the branch, still on libpg_query 17, compiled the 18.1.0 pg_query_is_utility_stmt.c from it (unknown type name 'PgQueryIsUtilityResult'). Switching branches locally does the same.

What changed

  • The vendored layout now points at the submodule itself, so the C sources, headers, pg_query.proto and the include paths all come from vendor/libpg_query. Nothing is copied anymore. OUT_DIR only holds the objects, libpg_query.a and bindings.rs. The copy wasn't needed: nothing writes into the tree, and the system layout already reads its prefix in place. fs_extra is no longer a build dependency.
  • The build script now prints rerun-if-changed for the submodule's headers, src, protobuf and vendor, plus build.rs and LIBPG_QUERY_PATH. Before, cc's rerun-if-env-changed lines turned off Cargo's default "rerun when any file of the package changes", so a change to the submodule alone (e.g. git submodule update after switching branches) didn't rebuild the C library at all.

The CI caches stay as they are. moonrepo/setup-rust restores by the prefix setup-rustcargo-v1-<os>, so a key that includes the submodule commit would still restore main's latest cache, and the tree-sitter cache only holds the generated grammar. A branch that is still on libpg_query 17 needs main merged in (it then gets #1055 and this fix), because CI runs the branch's own build.rs.

Tests

  • Reproduced the CI failure: built pgls_query on main before this change, then checked out the commit before chore(query): upgrade libpg_query to 18.1.0 #1055 (same Cargo.lock, same OUT_DIR) and built again. It failed with the same pg_query_is_utility_stmt.c errors as fix(suppressions): end lines at \n only, with the shared line index #1072.
  • With this change: filled the OUT_DIR with the 18.1.0 copies plus a .c file that only contains #error, then built the commit before chore(query): upgrade libpg_query to 18.1.0 #1055 with this build.rs and switched back to this branch. Both builds pass, and the archive built from the 17 sources has no pg_query_is_utility_stmt.
  • Touching a file under vendor/libpg_query/src now reruns the build script; before, the crate stayed Fresh. Deleting one reruns it too.
  • cargo test -p pgls_query, cargo clippy -p pgls_query --all-targets -- -D warnings.

@psteinroe
psteinroe merged commit 2ae5000 into main Nov 2, 2026
12 checks passed
@psteinroe
psteinroe deleted the fix/pgls-query-build-from-submodule branch November 2, 2026 18:27
psteinroe added a commit that referenced this pull request Nov 4, 2026
`pgls_query` lists `which` as a build dependency, but `build.rs` never
uses it, and nothing in its history did. This removes it, which also
drops `which`, `winsafe` and an old `rustix`/`linux-raw-sys` from
`Cargo.lock`.

I built and tested `pgls_query` both ways: with the vendored libpg_query
(the default), and with `LIBPG_QUERY_PATH` pointing at a libpg_query
installed by its own `make install`. `cargo clippy --workspace
--all-targets` passes.

This touches the same build-dependency list as #1106, which removes
`fs_extra` a few lines above; the two changes merge without conflicts.
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.

2 participants