Skip to content

Remove proto-lens SRP#1185

Merged
carbolymer merged 1 commit intomasterfrom
mgalazyn/chore/remove-prot-lens-srp
Apr 17, 2026
Merged

Remove proto-lens SRP#1185
carbolymer merged 1 commit intomasterfrom
mgalazyn/chore/remove-prot-lens-srp

Conversation

@carbolymer
Copy link
Copy Markdown
Contributor

@carbolymer carbolymer commented Apr 17, 2026

Context

Remove proto-lens SRP. Fix buf version drift.

Fix: #1184

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated. See Running tests for more details
  • Self-reviewed the diff
  • Changelog fragment added in .changes/

Copilot AI review requested due to automatic review settings April 17, 2026 10:03
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Removes the proto-lens source-repository-package pin from cabal.project so builds can rely on the package index instead of a git checkout, alongside dependency/index-state updates to support that shift.

Changes:

  • Update cabal.project index-states and remove the conditional proto-lens SRP stanza.
  • Bump cardano-rpc’s proto-lens lower bound to >= 0.7.1.7.
  • Refresh Nix flake inputs in flake.lock (CHaP + hackage.nix).

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.

File Description
flake.lock Updates locked revisions/hashes for CHaP and hackage.nix inputs.
cardano-rpc/cardano-rpc.cabal Raises proto-lens lower bound to align with dropping the SRP pin.
cabal.project Advances index-state and removes the proto-lens SRP stanza.

Comment thread cabal.project
Comment thread cabal.project
@carbolymer carbolymer force-pushed the mgalazyn/chore/remove-prot-lens-srp branch 3 times, most recently from 1b0a556 to 87a2920 Compare April 17, 2026 10:58
Copy link
Copy Markdown
Contributor

@angerman angerman left a comment

Choose a reason for hiding this comment

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

Hooray!

@carbolymer carbolymer force-pushed the mgalazyn/chore/remove-prot-lens-srp branch from 87a2920 to 73da39d Compare April 17, 2026 11:38
@carbolymer carbolymer requested a review from Copilot April 17, 2026 11:39
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.

Comment thread flake.nix
Comment thread .github/workflows/haskell.yml Outdated
project: cardano-rpc
pr: 1185
kind:
- maintenance
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

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

This fragment is marked as maintenance, but it changes the public build constraints of cardano-rpc (bumps the proto-lens lower bound). Similar dependency-bound changes in this repo have been categorized as compatible (e.g. .changes/20260409_cardano_rpc_proto_lens_lower_bound.yml). Consider using compatible here so release notes reflect that downstream build plans may need updating.

Suggested change
- maintenance
- compatible

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

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

Two small things beyond what Copilot flagged.

cardano-api/cardano-api.cabal:339 (not in the diff, flagging here): the main library bound was bumped to cardano-ledger-core >=1.20, but the test-suite's cardano-ledger-core:{cardano-ledger-core, testlib} >=1.17 is now stale. Cabal resolves to >=1.20 via the library constraint so it builds, but the bound here is misleading. Worth bumping to >=1.20 for consistency.

Comment thread flake.nix
# buf version must match `.github/workflows/haskell.yml` (buf is
# not backwards-compatible across minor versions for
# `buf generate` output).
unstable.buf
Copy link
Copy Markdown
Contributor

@Jimbo4350 Jimbo4350 Apr 17, 2026

Choose a reason for hiding this comment

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

unstable.buf doesn't actually pin a version. inputs.unstable.url = "nixpkgs/nixos-unstable" is locked by flake.lock today, so it's deterministic for this checkout but the next nix flake update can silently move buf off v1.66.1 and reintroduce the exact drift this PR is trying to fix. The comment above asserts alignment with CI but nothing enforces it. Could we fetch the pinned binary the same way CI does (or otherwise derive both versions from a single source of truth)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the next nix flake update can silently move buf off v1.66.1 and reintroduce the exact drift this PR is trying to fix.

This is fine in my opinion, as flake update is a thing we can control. I'll do the opposite and implement the check using buf from nixpkgs.

Copy link
Copy Markdown
Contributor Author

@carbolymer carbolymer Apr 17, 2026

Choose a reason for hiding this comment

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

I've split buf generate into separate job - that should speed up GHA since it'll run in parallel. It'll also use the same buf as nix development shell, so it'll be pinned to a version from nixpkgs.

@carbolymer carbolymer force-pushed the mgalazyn/chore/remove-prot-lens-srp branch 3 times, most recently from 3ebf81f to 05cd63b Compare April 17, 2026 14:21
@carbolymer carbolymer force-pushed the mgalazyn/chore/remove-prot-lens-srp branch from 05cd63b to 7b1aaf7 Compare April 17, 2026 14:29
@carbolymer carbolymer requested a review from Copilot April 17, 2026 14:30
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated no new comments.

@carbolymer carbolymer merged commit a9e0c9c into master Apr 17, 2026
36 of 37 checks passed
@carbolymer carbolymer deleted the mgalazyn/chore/remove-prot-lens-srp branch April 17, 2026 18:34
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.

CI: buf version unpinned, causing generated proto files to drift

4 participants