Add specific compile step, and require compile step to complete#13325
Add specific compile step, and require compile step to complete#13325Caleb-T-Owens wants to merge 1 commit intomasterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a dedicated Turborepo compile task intended to separate Rust/but-sdk generation from the existing package/build pipeline, and updates the @gitbutler/but-sdk scripts accordingly.
Changes:
- Introduces a new
compiletask inturbo.jsonand rewires task dependencies. - Updates
dev/check/testtask dependency lists to include the new compile phase. - Renames
@gitbutler/but-sdkscripts frombuild:*tocompile:*.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
turbo.json |
Adds compile task and adjusts dependsOn relationships across tasks |
packages/but-sdk/package.json |
Renames build scripts to compile scripts for NAPI + type generation |
Caleb-T-Owens
force-pushed
the
f-copilot/compile-step
branch
from
e11dd87 to
8780481
Compare
Caleb-T-Owens
force-pushed
the
f-copilot/compile-step
branch
from
8780481 to
e35f4c3
Compare
| }, | ||
| "dev": { | ||
| "dependsOn": ["package"], | ||
| "dependsOn": ["package", "compile"], |
There was a problem hiding this comment.
dev.dependsOn includes "compile" without the ^ prefix, so Turbo won’t run dependency compiles (notably @gitbutler/but-sdk) before starting dev servers. Since desktop/lite import types/runtime from @gitbutler/but-sdk, consider switching this to ^compile (and optionally compile if the package has its own compile step).
| "dependsOn": ["package", "compile"], | |
| "dependsOn": ["package", "^compile", "compile"], |
| "build": { | ||
| "dependsOn": ["package"], | ||
| "dependsOn": ["package", "compile"], | ||
| "passThroughEnv": ["SENTRY_AUTH_TOKEN", "GITHUB_TOKEN"], |
There was a problem hiding this comment.
build.dependsOn includes "compile" without the ^ prefix, so Turbo will only try to run compile in the same package. Apps like @gitbutler/desktop don’t have a compile script, and this won’t force @gitbutler/but-sdk to compile before the app build even though it’s a dependency. Use ^compile here (and keep compile too only if the package itself needs it).
Caleb-T-Owens
force-pushed
the
f-copilot/compile-step
branch
from
e35f4c3 to
6d4a319
Compare
`dev` only required `package`, but the but-sdk compile was considered a `build`. This adds a specific `compile` step for tasks that compiles rust, and fix the dependencies. I’ve also updated the dependencies to use the `^` notation, which AFAIK means that we actually wait for the dependency to complete before continuing (which seems like sane default behaviour?) so this probably makes sense.
Caleb-T-Owens
force-pushed
the
f-copilot/compile-step
branch
from
6d4a319 to
dcff13e
Compare
| "tasks": { | ||
| "compile": { | ||
| "outputs": ["dist/**", ".svelte-kit/**", "src/generated/**"] | ||
| }, |
There was a problem hiding this comment.
compile in @gitbutler/but-sdk depends on Rust sources outside the package directory (../../crates/but-napi, but-ts, and likely but-api schemas). With Turbo caching enabled and no explicit inputs/globalDependencies, changes in those crates may not invalidate the compile cache, leaving src/generated stale. Consider adding explicit inputs for the relevant crates/** paths (and Cargo.lock), or mark compile as non-cacheable if correctness is more important than caching.
| }, | |
| }, | |
| "@gitbutler/but-sdk#compile": { | |
| "inputs": [ | |
| "$TURBO_DEFAULT$", | |
| "$TURBO_ROOT$/Cargo.lock", | |
| "$TURBO_ROOT$/crates/but-napi/**", | |
| "$TURBO_ROOT$/crates/but-ts/**", | |
| "$TURBO_ROOT$/crates/but-api/**" | |
| ], | |
| "outputs": ["dist/**", ".svelte-kit/**", "src/generated/**"] | |
| }, |
devonly requiredpackage, but the but-sdk compile was considered abuild.This adds a specific
compilestep for tasks that compiles rust, and fix the dependencies.I’ve also updated the dependencies to use the
^notation, which AFAIK means that we actually wait for the dependency to complete before continuing (which seems like sane default behaviour?) so this probably makes sense.