Upgrade npins

[qmonnet]

We had the following error for dependency updates:

$ bash scripts/bump.sh
++ dirname scripts/bump.sh
+ pushd scripts/..
/var/home/qmo/dev/dataplane /var/home/qmo/dev/dataplane
+ npins update
Error: Failed to deserialize sources.json

Caused by:
    Version 7 is too old, you need to run upgrade

To address it, we simply ran the following command and committed the changes:

$ npins upgrade

@daniel-noland Can you please double-check this is OK?

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6459d96a-12e9-4398-9c83-7c511089690a

📥 Commits

Reviewing files that changed from the base of the PR and between aa6ecc2 and 6e7d774.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (1)

• Cargo.toml

🚧 Files skipped from review as they are similar to previous changes (1)

• Cargo.toml

---

📝 Walkthrough

Walkthrough

Adds a dedicated URL source handler (mkUrlSource) for Url/MutableUrl, adds optional arch passthrough to container pulls, updates the sources.json format version to 8, refreshes several pinned sources, and bumps the workspace uuid dependency.

Changes

npins Source Handling and Version Updates

Layer / File(s)	Summary
URL source type handler npins/default.nix	New mkUrlSource handles Url and MutableUrl, selecting fetchTarball when unpack is true and fetchurl otherwise; dispatch comment removed.
Container image architecture support npins/default.nix	mkContainerSource captures @args and conditionally merges an arch attribute into dockerTools.pullImage when args.arch is set.
Format version gate, pins, and workspace dependency npins/default.nix, npins/sources.json, Cargo.toml	Top-level sources.json format version check updated from 7 to 8. Pinned entries for frr, nixpkgs (nixexprs artifact), and rust-overlay updated. Added a parsing-path comment. Workspace uuid bumped from 1.23.2 to 1.23.3 in Cargo.toml.

Estimated code review effort

3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: running npins upgrade to address a version compatibility issue in sources.json.
Description check	✅ Passed	The description clearly explains the error encountered, the command run to fix it, and includes a request for review, all directly related to the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

---

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

[Copilot]

Pull request overview

This PR updates the repository’s npins metadata to the newer sources.json format so npins update no longer fails with “Version 7 is too old”, and regenerates the corresponding Nix loader to match the new schema.

Changes:

• Upgrade npins/sources.json from format version 7 to 8.
• Regenerate npins/default.nix to match npins v8 schema changes (notably handling Url/MutableUrl and updated container fetching args).
• Update several pinned inputs (e.g., frr, nixpkgs, rust-overlay) as part of the regenerated sources.json.

Reviewed changes

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

File	Description
npins/sources.json	Bumps format version to v8 and updates multiple pin revisions/URLs/hashes.
npins/default.nix	Regenerated npins loader to support v8 sources schema and updated fetcher dispatch.

[daniel-noland]

@qmonnet

yup, this is correct. That said, hold off on the merge for a few so I can bump the cargo packages with it. If we are going to take cache churn we may as well concentrate it to minimize CI load. Should only take a few minutes anyway. I'll just target a bump run against this branch.

[daniel-noland]

Yup, LGTM

[qmonnet]

Ugh, looks like the cbindgen update breaks things

[qmonnet]

cargo update (commit 5d94383a829a) bumped cbindgen 0.29.3 → 0.29.4. The
0.29.4 release added "support for constant enums and arrays," which changes
the C bindings it generates — even though write_to_file itself is
unchanged.

The key is how cbindgen decides whether to write. Its write_to_file is:

let mut new_file_contents = Vec::new();
self.write(&mut new_file_contents);

let mut old_file_contents = Vec::new();
{
    let mut old_file = File::open(&path).unwrap();   // file already exists -> OK
    old_file.read_to_end(&mut old_file_contents).unwrap();
}

if old_file_contents != new_file_contents {
    let mut new_file = File::create(&path).unwrap();  // <-- line 263: PermissionDenied
    new_file.write_all(&new_file_contents).unwrap();
    ...
}

So in dplane-rpc's build.rs, cbindgen generates a header into the crate's
own source tree (a header that's committed to the dplane-rpc repo). The
flow:

- With 0.29.3: generated output matched the committed header byte-for-byte
  → old == new → it skips the write entirely → build succeeds, even though
  the source tree is read-only.
- With 0.29.4: the new const-enum/array codegen makes the generated output
  differ from the committed header → old != new → it tries
  File::create(&path) to overwrite it → in the nix sandbox the source tree
  is read-only, so you get Os { code: 13, PermissionDenied } → panic at
  bindings.rs:263.

It's not really a nix problem — it's that dplane-rpc writes a generated
artifact back into its source dir at build time, which is inherently
fragile, and any cbindgen output change trips it. The nix read-only sandbox
just turns the silent overwrite into a hard failure.

Proper fix (upstream in dplane-rpc): regenerate and commit the header with
cbindgen 0.29.4 on the pr/daniel-noland/bumps branch so the on-disk file
matches again — or better, have its build.rs emit into OUT_DIR instead of
the source tree so output churn never collides with a read-only build.

(Claude)

So basically there's a header we need to re-generate.

[qmonnet]

@daniel-noland I opened githedgehog/dplane-rpc#33 in an attempt to solve this. This is probably not the right long-term solution though, but should unblock this particular PR (I think?).

[daniel-noland]

@daniel-noland I opened githedgehog/dplane-rpc#33 in an attempt to solve this. This is probably not the right long-term solution though, but should unblock this particular PR (I think?).

Yeah, we need to reorg the way that works because this is a problem waiting to happen. I'll approve for now but we should either abosorb that repo into dataplane / nix infra or at least stop committing the generated code because that is a pain.

[qmonnet]

Rebased without changes to the commits in the PR, I expect CI to pick the new version of the dplane-rpc branch and to pass

[qmonnet]

In fact, I needed to re-run cargo update to catch the change in the Cargo.lock.