Skip to content

LocalStore::addToStore(): Show hash mismatches in SRI format#423

Merged
edolstra merged 1 commit intomainfrom
hash-mismatch-sri
Apr 13, 2026
Merged

LocalStore::addToStore(): Show hash mismatches in SRI format#423
edolstra merged 1 commit intomainfrom
hash-mismatch-sri

Conversation

@edolstra
Copy link
Copy Markdown
Collaborator

@edolstra edolstra commented Apr 13, 2026

Motivation

This is more consistent with e.g. hash mismatches in fixed-output derivations and the output of nix path-info.

Context

Summary by CodeRabbit

  • Bug Fixes
    • Updated error message formatting for NAR content hash mismatches to display hashes in SRI format, improving readability and consistency. This change only affects the displayed error text; comparison logic and behavior remain unchanged.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4536769a-6431-46ca-9c8d-98d7af18d7cc

📥 Commits

Reviewing files that changed from the base of the PR and between 2898ddb and ac3c92f.

📒 Files selected for processing (1)
  • src/libstore/local-store.cc
✅ Files skipped from review due to trivial changes (1)
  • src/libstore/local-store.cc

📝 Walkthrough

Walkthrough

Error message text for NAR content hash mismatches was changed to present both expected and computed hashes using SRI formatting instead of Nix32; no other logic or branching was modified.

Changes

Cohort / File(s) Summary
Error message formatting
src/libstore/local-store.cc
Updated error string to format both expected (info.narHash) and computed (hashResult.hash) values using HashFormat::SRI instead of HashFormat::Nix32 for NAR mismatch reports.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 I hopped through bytes and found a tweak so spry,
Old Nix32 gave way — now SRI catches the eye,
When NARs disagree and errors chime,
I smile and nibble code, one hop at a time. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: converting hash mismatch error messages from Nix32 to SRI format in LocalStore::addToStore().

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch hash-mismatch-sri

Comment @coderabbitai help to get the list of available commands and usage tips.

@edolstra edolstra changed the title LocalStore::addToStore(): Show hash mismatching in SRI format LocalStore::addToStore(): Show hash mismatches in SRI format Apr 13, 2026
This is more consistent with e.g. hash mismatches in fixed-output
derivations and the output of `nix path-info`.
@edolstra edolstra force-pushed the hash-mismatch-sri branch from 2898ddb to ac3c92f Compare April 13, 2026 15:44
@edolstra edolstra enabled auto-merge April 13, 2026 15:44
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 13, 2026

@github-actions github-actions bot temporarily deployed to pull request April 13, 2026 15:49 Inactive
@edolstra edolstra added this pull request to the merge queue Apr 13, 2026
Merged via the queue into main with commit 7c99fe6 Apr 13, 2026
31 checks passed
@edolstra edolstra deleted the hash-mismatch-sri branch April 13, 2026 16:35
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.

2 participants