Conversation
The server downloaded, hash checked, unpacked and chmodded every SDK itself, and hardcoded each SDK type's install layout and invocation. The metadata for the same SDKs was duplicated in Sdks.kt, and many downloads had no hash at all. nix/sdks.nix now defines every SDK, both its metadata and its package. Each SDK is an attrset with a `meta` part (name, aliases, release, warnings, lombok support) and a `runtime` part describing how to invoke it: the compiler command, an alternative command for lombok, the javap command and the environment. How an SDK is packaged is now confined to its own definition, so for instance a kotlin maven jar and a kotlin zip distribution no longer differ anywhere else. gradle evaluates `nix eval .#sdkMetadata` to generate the metadata for the shared module, which does not build any SDK. The server reads the manifest built by `nix build .#sdks -o sdk`, which also lists the store paths to make readable in the sandbox, and it has a single generic RunnableSdk instead of per-type code. meta.compiler is the option dialect (javac, ecj, kotlinc, scalac), the only SDK distinction the client and the compiler configuration actually need, so Sdk.KotlinJar and Sdk.KotlinDistribution collapse into Sdk.Kotlin. The generated metadata is identical to the previous hardcoded values. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
yawkat
force-pushed
the
worktree-nix-sdks
branch
from
September 19, 2026 08:47
3d6ff63 to
f280dd1
Compare
flake.nix now builds the whole server (packages.default/server), not just the SDKs: same gradle_9 pin, offline yarn/gradle dependency caches (nix/deps.json, moved from a downstream nixos-config repo that had reimplemented this), and a stub `nix` binary so shared/build.gradle.kts's `nix eval .#sdkMetadata` call works inside the sandboxed build without needing network or a real nix. `nix run .#update-deps` refreshes nix/deps.json after a dependency bump, and a `devShells.default` provides a matching JDK for running ./gradlew directly. CI now builds the package via nix (`nix build .#default`), which fails if nix/deps.json or the yarn hash fall out of sync with the actual dependency graph -- the previous setup-java/gradle-based check had no such guard. The test suite itself still runs via `nix develop` + ./gradlew, since a couple of its tests (BubblewrapTest, SdkIntegrationTest) need real FHS paths and the built SDKs, neither of which the sandboxed nix build sees. Also caches /nix in Actions so SDKs aren't re-downloaded on every run. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016jPcxMSz4fFh3F81wPPFUB
flake.nix was injecting a chunk of Kotlin into build.gradle.kts via
substituteInPlace (matching on the literal string 'allprojects {') to point
the kotlin-js plugins at nix-provided node/yarn, plus registering the
dependency-prefetch task the same way. That's fragile to review and to edit,
and breaks silently if the anchor text ever changes.
Both are now real, reviewable code in the repo instead: build.gradle.kts
reads NIX_NODEJS_BIN/NIX_YARN_BIN (set by flake.nix, unset in every other
build) to decide whether to redirect node/yarn, and the download-deps task
lives directly in server/build.gradle.kts as an always-registered (but
otherwise unused) task, like `check` or `installShadowDist`.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016jPcxMSz4fFh3F81wPPFUB
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The default Nix package excludes the required SDK manifest despite being documented as the complete package.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Moves SDK metadata, packaging, and runtime invocation into Nix while simplifying Kotlin SDK handling.
Changes:
- Generates shared SDK metadata from
nix/sdks.nix. - Replaces server-side downloading with a generic Nix manifest-backed provider.
- Adds reproducible Nix builds and CI integration.
| File | Description |
|---|---|
.github/workflows/test.yml |
Builds and tests through Nix. |
.gitignore |
Ignores generated SDK outputs. |
README.md |
Documents Nix workflows. |
build.gradle.kts |
Configures offline Node/Yarn tools. |
client/.../CompilerConfigUi.kt |
Uses unified Kotlin SDK type. |
flake.lock |
Pins nixpkgs. |
flake.nix |
Defines packages and development shell. |
nix/deps.json |
Pins Gradle dependencies. |
nix/sdks.nix |
Defines SDK metadata and runtimes. |
server/build.gradle.kts |
Adds dependency-cache preparation task. |
server/.../JavapApplication.kt |
Loads configured SDK manifest. |
server/.../JavapConfiguration.kt |
Adds manifest configuration. |
server/.../LocalProcessor.kt |
Uses manifest commands and environment. |
server/.../SdkProvider.kt |
Introduces generic runnable SDKs. |
server/.../SdkProviderImpl.kt |
Loads Nix-generated runtime manifests. |
server/.../CollectHelp.kt |
Collects help through runtime metadata. |
server/.../FindLatestReleases.kt |
Emits Nix-compatible hashes. |
server/.../SdkIntegrationTest.kt |
Tests manifest-backed SDKs. |
server/.../SystemSdkProvider.kt |
Adapts system JDK test provider. |
shared/build.gradle.kts |
Generates Kotlin SDK metadata. |
shared/.../Sdks.kt |
Replaces hardcoded metadata with generated data. |
shared/.../CompilerConfiguration.kt |
Supports unified SDK metadata. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # SDK manifest used by the server at runtime: `nix build .#sdks -o sdk` | ||
| sdks = sdks.manifest; | ||
| server = javap; | ||
| default = javap; |
Owner
Author
There was a problem hiding this comment.
Fixed in 17a8224 — clarified in the README that nix build .#default only builds the server, and that SDKs are a deliberately separate output (large/rarely-changing vs. rebuilt-every-commit) wired together via sdkManifest in the deployment config rather than the relative-path default.
Owner
Author
There was a problem hiding this comment.
Actually reconsidered this -- correcting the docs was a weaker fix than it should've been, based on a wrong premise (that bundling would force SDK redownloads). Nix caches by content hash regardless of derivation grouping, so bundling costs nothing extra. In 89dc71b, default is now symlinkJoined from the server + a symlink to the SDK manifest at sdk/sdks.json, matching the default relative sdkManifest config value -- so nix build .#default; result/bin/javap-server config.json works out of the box, while server/sdks stay available unbundled for deployments managing the SDK closure's lifecycle separately.
packages.default only builds the server (its install phase just copies the jar); it never depended on or installed sdks.manifest. The README claimed 'nix build .#default' builds SDKs too, which isn't true and would mislead someone into deploying a server that can't find sdk/sdks.json. Clarified that the two outputs are separate on purpose (SDKs are large and rarely change, the server rebuilds on every commit) and that a deployment wires sdkManifest to the sdks output explicitly rather than relying on the relative default. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016jPcxMSz4fFh3F81wPPFUB
The previous fix for the Copilot finding (default omits the SDK manifest the server needs) just corrected the README to admit default doesn't include SDKs. That traded a real fix for a doc change based on a bad premise -- nix caches build outputs by content hash regardless of which derivation references them, so folding sdks.manifest into default costs nothing extra: unrelated commits still hit cache for the SDK closure. default is now `symlinkJoin`ed from the server derivation plus a symlink to the SDK manifest at sdk/sdks.json, matching the default relative sdkManifest config value, so `nix build; result/bin/javap-server config.json` works from a fresh checkout without wiring anything up manually. server and sdks remain available unbundled for deployments that want to manage the SDK closure's lifecycle independently (this repo's own downstream nixos config is exactly such a case: it rebuilds the server on every commit but wants the SDK closure to stay put). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016jPcxMSz4fFh3F81wPPFUB
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces the SDK download/unpack code with nix, and makes
nix/sdks.nixthe single source of SDK metadata.Previously the server downloaded, hash checked, unpacked and chmodded every SDK itself, and hardcoded each SDK type's install layout and invocation. The metadata for those same SDKs was duplicated in
Sdks.kt, and the ECJ, Kotlin, Scala, Lombok and coroutines downloads had no hash at all.How it works
Every SDK in
nix/sdks.nixis an attrset with two parts:meta— name, aliases, release, supported warnings, lombok support. Consumed by the client and the compiler configuration.runtime— how to invoke it: the compiler command, an alternative command used when lombok is enabled, the javap command and the environment.How an SDK is packaged stays inside its own definition, so a kotlin maven jar and a kotlin zip distribution no longer differ anywhere outside it.
meta.compileris the option dialect (javac,ecj,kotlinc,scalac), which is the only SDK distinction the rest of the code needs — soSdk.KotlinJarandSdk.KotlinDistributioncollapse intoSdk.Kotlin.nix eval .#sdkMetadatareturns the metadata without building any SDK; gradle generates the shared kotlin source from it.nix build .#sdks -o sdkbuilds everything intosdk/sdks.json. Each entry also lists the store paths to bind readable in the sandbox, taken from the closure of that SDK's runtime definition.SdkProviderImplloses all download, hashing, extraction and per-type code and builds one genericRunnableSdkper SDK.All artifacts now have hashes.
Verification
./gradlew checkpasses 57 of 58 tests. The OpenJDK 10 failure is pre-existing and unrelated to this change: extracting the same tarball with plaintarcrashes identically (NoClassDefFoundErrorwith an empty name fromUnsafe.defineAnonymousClass), so those binaries do not run on that host with or without nix.One caveat for CI: substituting the JDK tarballs from
cache.nixos.orgfailed with TLS errors locally, and nix does not fall back to the upstream URLs on its own. If the workflow hits the same, add--fallbackto the build step.🤖 Generated with Claude Code