Skip to content

feat(cli)!: Extract CLI binary into separate prqlc-cli crate#5749

Draft
snth wants to merge 12 commits intomainfrom
feat-cli-crate
Draft

feat(cli)!: Extract CLI binary into separate prqlc-cli crate#5749
snth wants to merge 12 commits intomainfrom
feat-cli-crate

Conversation

@snth
Copy link
Copy Markdown
Member

@snth snth commented Mar 27, 2026

Split the prqlc crate into a pure library (prqlc) and a CLI binary (prqlc-cli). This fixes the structural MSRV problem where CLI-only dependencies (clap, clio, notify, etc.) forced prqlc's MSRV higher than needed, blocking Dependabot PRs.

Key changes:

  • prqlc: Remove cli feature, add display feature gating ariadne+anstream as optional deps. Library is now pure with no binary target.
  • prqlc-cli: New binary crate with all CLI code moved here. Binary name remains prqlc via [[bin]] config. Depends on prqlc with display and serde_yaml features.
  • error_message.rs: Split composed() — location always computed via new offset_to_line_col() helper (no ariadne needed), display rendering behind #[cfg(feature = "display")].
  • semantic/reporting.rs: Remove dead label_references()/Labeler code.
  • utils/mod.rs: Gate color functions behind display feature with no-op fallback.
  • CI: Update build-prqlc action to build prqlc-cli package, upgrade cargo-msrv, update release features.
  • Install via: cargo install prqlc-cli

@snth snth marked this pull request as draft March 27, 2026 11:24
@snth snth marked this pull request as draft March 27, 2026 11:24
@snth snth changed the title Extract CLI binary into separate prqlc-cli crate feat(cli)!: Extract CLI binary into separate prqlc-cli crate Mar 27, 2026
@snth snth force-pushed the feat-cli-crate branch 2 times, most recently from 60bb554 to 1a70ff7 Compare March 27, 2026 12:56
@max-sixty
Copy link
Copy Markdown
Member

max-sixty commented Mar 27, 2026

we went back & forth on this a bunch in the past. I can see the benefits of both, but I def don't think we should change back now; which requires a large number of external-facing updates. what are the specific MSRV issues?

@snth
Copy link
Copy Markdown
Member Author

snth commented Mar 27, 2026

I hear that but I've tried to organise it so that library consumers face no updates. It's only the prql-cli crate that changes, i.e. the install command changes to cargo install prqlc-cli but the binary is still called prqlc. Library consumers like Clickhouse and Databend shouldn't have to change anything. I'll still check that. Trying to get it to pass tests atm.

snth and others added 2 commits March 27, 2026 17:37
@snth
chore: update insta snapshot format
504885d 
Update inline snapshot string delimiters (@r" -> @", r### -> r#)
and remove deprecated snapshot_kind: text metadata from snap files.
All changes are format-only with no semantic differences.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth
chore: update stale snapshot metadata and content
c971aff 
Non-format changes from insta snapshot regeneration:

- compile/fmt append_select_nulls.snap: expression field corrected
  to match current .prql file (stale '# duckdb:skip' / '# postgres:skip'
  comments were removed from the file in a prior commit but the snap
  metadata was never regenerated)

- compileall snap files: leading blank lines added to snapshot content.
  Each blank line represents a dialect whose output matches the generic
  dialect (empty diff). The previous insta version trimmed these; the
  new version preserves them. The actual diff content is unchanged.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth
Copy link
Copy Markdown
Member Author

snth commented Mar 27, 2026

MSVR is currently at 1.75. There are 3 pending updates that would move this up to 1.85. They are all in packages only used in the CLI though. Hence with this split the prqlc library could stay at 1.75 while the CLI can follow whatever its dependencies require.

Overall I think this is much cleaner as well. Compiler library consumers shouldn't need to depend on the cli-only dependencies. I know the 'default_features = false' flag addresses that somewhat, but my understanding is that this doesn't work for the MSVR.

@max-sixty
Copy link
Copy Markdown
Member

max-sixty commented Mar 27, 2026

can we just bump the MSRV though?

otherwise the proposed change would means that anyone with the CLI installed would need to change their installation (for a binary with currently slow updates! the only update is breaking :) )

I also think that it adds complication to have different names for the binary and the cargo install command...

@snth
Copy link
Copy Markdown
Member Author

snth commented Mar 27, 2026

Yeah, we could totally just bump the MSVR. I thought that would be more breaking but we can tackle this in separate parts. I'll try and get an agent to do some research on what limits are by downstream consumers. I don't quite trust the answer I got from Copilot earlier.

snth and others added 9 commits March 27, 2026 19:01
@snth
ci: add Docker Hub login to workflows pulling Docker images
0bad64c 
Authenticate with Docker Hub before pulling images to avoid rate limits.

- test-rust.yaml: login before docker compose up (postgres, mysql, clickhouse)
- build-devcontainer.yaml: login before image builds as defensive measure

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth
ci: pass secrets to reusable workflows for Docker Hub login
b19df46 
The Docker Hub login step in test-rust and build-devcontainer workflows
needs access to DOCKERHUB_USERNAME and DOCKERHUB_TOKEN secrets. These
are only available via 'secrets: inherit' on workflow_call invocations.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth
feat(cli)!: Extract CLI binary into separate prqlc-cli crate
46e945a 
Split the prqlc crate into a pure library (prqlc) and a CLI binary
(prqlc-cli). This fixes the structural MSRV problem where CLI-only
dependencies (clap, clio, notify, etc.) forced prqlc's MSRV higher
than needed, blocking Dependabot PRs.

Key changes:
- prqlc: Remove `cli` feature, add `display` feature gating
  ariadne+anstream as optional deps. Library is now pure with no
  binary target.
- prqlc-cli: New binary crate with all CLI code moved here. Binary
  name remains `prqlc` via [[bin]] config. Depends on prqlc with
  `display` and `serde_yaml` features.
- error_message.rs: Split composed() — location always computed via
  new offset_to_line_col() helper (no ariadne needed), display
  rendering behind #[cfg(feature = "display")].
- semantic/reporting.rs: Remove dead label_references()/Labeler code.
- utils/mod.rs: Gate color functions behind `display` feature with
  no-op fallback.
- CI: Update build-prqlc action to build prqlc-cli package, upgrade
  cargo-msrv, update release features.
- Install via: cargo install prqlc-cli

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth
fix(ci): split build-prqlc action and fix post-crate-split CI failures
36652b6 
- Rename build-prqlc action → build-prqlc-cli (it builds the CLI binary)
- Remove test-dbs feature from CLI release matrix (belongs to prqlc library)
- Add anyhow as dev-dependency of prqlc (used by integration test runner)
- Make 'display' feature default for prqlc (error formatting is core)
- Fix JS test: remove erroneous 'new' on get_targets() function call
- Update all release.yaml job references from build-prqlc → build-prqlc-cli

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth
fix(ci): Fix CI failures — formatting, binary lookup, unused deps
1882c00 
- Fix rustfmt issues in error_message.rs and lib.rs
- Replace get_cargo_bin("prqlc") with robust fallback that finds the
  binary relative to the test executable (fixes nextest/unit test compat)
- Remove unused dev-deps: insta-cmd from prqlc, glob from prqlc-cli
- Fix clippy unnecessary_map_or lint in prqlc-cli

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth
fix(ci): Fix display feature gating, color stripping, and bench target
bb43283 
- Add unconditional strip_colors() for DisplayOptions::Plain — restores
  the pre-PR behavior where compile() always stripped ANSI codes, fixing
  36 integration test failures in test-deps-min-versions and book tests
- Remove default-features = false from bindings and mdbook-prql — the
  old cli feature that motivated disabling defaults no longer exists;
  default features now just include display, which all consumers need
- Add bench = false to prqlc-cli [[bin]] — prevents cargo bench from
  trying to pass criterion flags (--warm-up-time) to the CLI binary

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth
fix: resolve CI failures for dead code and binary discovery
1329f93 
- Remove unreachable maybe_strip_colors #[cfg(not(feature = "display"))]
  variant that caused dead_code error on wasm32 builds
- Add shared test_utils module with prqlc_command() that auto-builds
  the prqlc binary on demand using std::sync::Once, fixing test failures
  when cargo test doesn't build the binary (e.g. minimal-versions check)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth
fix(ci): pass --target and --target-dir in on-demand prqlc build
0810cac 
When CI runs tests with --target=<triple>, cargo places artifacts in
target/<triple>/debug/ instead of target/debug/. The on-demand build in
test_utils::prqlc_bin_path() was running `cargo build --bin prqlc` without
--target, so the binary landed in the wrong directory and insta-cmd tests
could not find it.

Fix by:
- Emitting PRQLC_BUILD_TARGET from build.rs (the TARGET env var is only
  available in build scripts)
- Detecting the target triple from the test binary path and passing
  --target to the on-demand build when needed
- Also passing --target-dir to handle custom target directories like
  those used by cargo-llvm-cov

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth
docs: update installation instructions to reference prqlc-cli
7cf16a3 
The CLI was extracted into the separate prqlc-cli crate. Update:
- prqlc/prqlc/README.md: cargo install now references prqlc-cli, intro
  and CLI section clarify that the CLI is a separate crate
- web/website/content/_index.md: crates.io link and cargo install
  command updated to prqlc-cli

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth
fix(ci): resolve codecov/patch coverage failure
979653b 
- Remove dead code in offset_to_line_col() — the trailing None was
  unreachable since the guard at the top ensures offset <= source.len()
  and the loop handles all positions < source.len()
- Add test_utils.rs and build.rs to codecov ignore — test_utils.rs is
  #[cfg(test)] infrastructure with branches that depend on how cargo was
  invoked (impossible to fully cover in a single CI run), and build.rs
  runs at compile time so is never instrumented by cargo-llvm-cov

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@snth @max-sixty