Skip to content

Optimize Nix release workflow#4927

Merged
automergerpr-permission-manager[bot] merged 2 commits into
developfrom
workflow-nix-optim
Jun 4, 2026
Merged

Optimize Nix release workflow#4927
automergerpr-permission-manager[bot] merged 2 commits into
developfrom
workflow-nix-optim

Conversation

@ehildenb
Copy link
Copy Markdown
Member

@ehildenb ehildenb commented Jun 3, 2026

The k-framework-binary (private, via kup) and k-framework (public, via cachix) publishes built the same three derivations on separate runners, recompiling K four times (2 jobs x 2 OS) for work that needs two builds. Fold them into one matrix job: the first publish populates the local Nix store so the second's nix build is an instant store hit. The two publishes stay independent (continue-on-error + if: always()) so a flaky upload to one cache neither blocks nor masks the other, and a final guard re-fails the job if either push failed, preserving the release gate the two separate jobs provided. Drop the removed cachix-release-dependencies from the release job's needs.

…shes into one job, building the shared derivations once

The k-framework-binary (private, via kup) and k-framework (public, via cachix)
publishes built the same three derivations on separate runners, recompiling K
four times (2 jobs x 2 OS) for work that needs two builds. Fold them into one
matrix job: the first publish populates the local Nix store so the second's
nix build is an instant store hit. The two publishes stay independent
(continue-on-error + if: always()) so a flaky upload to one cache neither blocks
nor masks the other, and a final guard re-fails the job if either push failed,
preserving the release gate the two separate jobs provided. Drop the removed
cachix-release-dependencies from the release job's needs.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@deosa-arch deosa-arch changed the base branch from master to develop June 3, 2026 16:31
@ehildenb ehildenb requested a review from juliankuners June 3, 2026 16:32
@ehildenb ehildenb self-assigned this Jun 3, 2026
@ehildenb ehildenb marked this pull request as ready for review June 3, 2026 16:32
@ehildenb ehildenb requested a review from a team as a code owner June 3, 2026 16:32
Copy link
Copy Markdown
Contributor

@juliankuners juliankuners left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments and suggestions.

Comment thread .github/workflows/release.yml Outdated
- name: 'Build and cache K and K dependencies'
- name: 'Build and cache K and K dependencies (public)'
id: dependencies
if: always()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if: always() is unnecessary, since the previous step already declared continue-on-error

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread .github/workflows/release.yml Outdated
nix-store --query --requisites --include-outputs ${DRV_K_OPENSSL_PROCPS_SECP256K1} | cachix push k-framework
- name: 'Fail if either cachix publish failed'
if: always()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same unnecessary if: always(), see above comment.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread .github/workflows/release.yml Outdated
Comment on lines +82 to +95
# The binary publish and the dependency-closure publish build the same three
# derivations. Running them as sequential steps on one runner means the build
# happens once: the first step populates the local Nix store, so the second
# step's `nix build` is an instant store hit rather than a full recompile.
#
# The two publishes target different caches with different tokens (private
# `k-framework-binary` via kup vs. public `k-framework` via cachix), so they are
# kept independent: `continue-on-error` plus `if: always()` ensures a flaky
# upload to one cache neither blocks nor masks the other, and the final guard
# re-fails the job if either push failed — preserving the release gate that the
# two separate jobs used to provide.
- name: 'Publish K to k-framework-binary cache (private)'
id: binary
continue-on-error: true
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bloated AI comment that reasons why the old pattern of two workflows was merged into one. Once we've adopted this pattern everywhere, this comment will be confusing since it's not clear that the workflows was split in two in the first place.

If you wish to add a clarifying comment, keep it short and concise, simply just describing what the workflow does.

Though I agree that the differentian between the caches k-framework and k-framework-binary was never properly documented. Though I doubt that the proper place is a long comment in a workflow in one of our dependency projects. Rather, there should be proper documentation in possibly https://github.com/runtimeverification/rv-nix-tools/tree/master/docs.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread .github/workflows/release.yml Outdated
cachix-release:
name: 'k-framework-binary cachix release'
name: 'Cachix release (binary cache + dependency closure)'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
name: 'Cachix release (binary cache + dependency closure)'
name: 'Cachix release (binary cache `k-framework-binary` + dependency cache `k-framework`)'

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Shorten the cachix-release comment to describe what the step does, drop the
redundant if: always() on steps following a continue-on-error step, and name
the caches explicitly in the job name.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@automergerpr-permission-manager automergerpr-permission-manager Bot merged commit 195ce2d into develop Jun 4, 2026
19 checks passed
@automergerpr-permission-manager automergerpr-permission-manager Bot deleted the workflow-nix-optim branch June 4, 2026 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants