deps: explain why Dependabot PRs land red, and add the one command that fixes them - #140
Merged
Merged
Conversation
…R needs, and script it #139 left a decision open: the Android head is in Dependabot's `directories` but Dependabot cannot refresh its packages.lock.json, so the choice was between documenting a manual refresh and dropping the head the way the iOS head is dropped. This takes the first option, because investigating it turned up a bigger problem that decides the question. Four of the five app lock files reach their packages through a ProjectReference to Daqifi.Avalonia and record its full transitive closure. Dependabot rewrites the lock file only in the directory whose manifest it edited. So a one-line bump in the shared library invalidates five lock files and refreshes one, and locked mode fails the other four with NU1004. That is 23 of the 28 packages managed here, and it is the current state of the repo: #96 and #130 are both red on it now, on the Android and iOS heads alike. That refutes the alternative. Dropping /Daqifi.Avalonia.Android would fix nothing — its NU1004 comes from the ProjectReference closure, not from its own two pins, and the iOS head proves it by already being out of `directories` and failing identically. The guard offered as cover cannot help either: avalonia-graph `needs: [desktop, android, ios]`, so it is SKIPPED on exactly the PRs in question. Dropping the head would only cost the one automated watch on Avalonia.Android. Reproduced on the pinned SDK 10.0.302 by doing what Dependabot does — bump Sentry 6.8.0 -> 6.9.0 in the shared library, refresh only its own lock file, then restore each head in locked mode. All four fail NU1004; the new script makes all four pass. - refresh_lock_files.sh regenerates all six lock files. It refuses to do a partial job: wrong SDK or a missing android/ios workload exits 2 without writing, because a half-refresh looks finished and leaves the skipped head failing NU1004 anyway. $DOTNET is authoritative rather than one candidate among several. It iterates projects rather than the slnx, which does not contain AvaloniaCapture, and never passes -r, which would prune the lock file to one RID. - test_refresh_lock_files.py drives it with a stub dotnet across four refusal cases and asserts nothing is written. Picked up by the existing `scripts` job, which globs test_*.py. Verified to fail when the refusal is removed. - The README section that deferred this decision now records it, and the dependabot.yml comment says which PRs are affected before someone tries to merge one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PR Summary by QodoDocument and automate Dependabot lock-file refreshes
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Qodo review on #140, finding 2. The preflight refuses to start without the pinned SDK and both workloads, but once restores begin they run one project at a time — so a failure on the fourth left the first three already rewritten. That is the same partial refresh the preflight exists to prevent, arriving by a different door, and it looks like an ordinary refresh diff on the way out. The script now snapshots every lock file before the first restore and rolls all of them back on failure, exiting 1. The snapshot is of the WORKING TREE, not HEAD: on a Dependabot branch the shared library's lock file is already legitimately modified, and rolling back to HEAD would discard Dependabot's own work along with ours. Exit codes are now 0 refreshed / 1 a restore failed, rolled back / 2 could not run, which lines up with the contract the other scripts here follow. The self-test grows a rollback group: a stub dotnet that mutates each lock file it restores and fails on the third. It carries a CONTROL case whose stub never fails, because a rollback test whose stub never wrote anything would pass without proving anything. Verified to fail when the rollback call is removed — three lock files left modified. Also records what the review's other finding got wrong. --force-evaluate IS what overrides RestoreLockedMode, so CI=true does not block the refresh. NuGet's own NU1004 text says so, and it is verified: same state, obj cleared, CI=true, RestoreLockedMode evaluating true — a plain restore fails NU1004 and the same restore with --force-evaluate regenerates the lock file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
What
Dependabot PRs in this repo arrive with a broken build, and there was nothing telling anyone why or what to do about it. This adds that explanation, plus a script that does the fix in one command.
It also settles a question #139 deliberately left open — whether the Android head should stay in Dependabot's
directoriesor be dropped like the iOS head. It stays. The reasoning is in the README.No configuration changes. The only edit to
dependabot.ymlis a comment.Why
The app is one shared library (
Daqifi.Avalonia) plus four thin heads that reference it — Desktop, Android, iOS, and the capture harness. Each of those five projects has apackages.lock.json, and a lock file lists every package the project ends up using, including everything inherited from the library. SoSentryis written into all five files, not just the library's.Dependabot only updates the lock file in the folder it edited. Bump one package in the shared library and it updates one lock file, leaving the other four describing a world that no longer exists. CI checks lock files strictly, so those four heads fail with:
This isn't a prediction — it's happening now. #96 and #130 are both red on exactly this. It affects 23 of the 28 packages we manage, which is essentially every dependency that isn't an Avalonia platform package.
Why not just drop the Android head from
directories? That was the alternative, and testing killed it:Avalonia.Androidfor updates.How
Verified, not assumed. I reproduced the whole thing locally on the pinned SDK: bumped
Sentryin the shared library, updated only its lock file the way Dependabot does, then built each head. All four failed. Ran the new script. All four passed.refresh_lock_files.shregenerates all six lock files. The important part is what it won't do — if the SDK is wrong or a required workload is missing, it stops and tells you, without writing anything. A half-finished refresh is worse than none: it looks done and leaves the skipped head broken. Two details that cost real debugging time in the past are baked in — it walks projects individually because the solution file is missing the capture harness, and it never uses-r, which quietly rewrites lock files to a single platform.test_refresh_lock_files.pyfeeds it a fakedotnetand checks it refuses in four different situations without touching any files. It's picked up automatically by the existing self-test job. I confirmed it actually fails when the refusal is removed, so it isn't a test that only ever passes.Docs go where the related history already lives — the section in
.github/dependency-updates/README.mdthat deferred this decision now answers it, anddependabot.ymlwarns before someone tries to merge one of these PRs.What this doesn't do
It doesn't automate the fix. A workflow could run this script on Dependabot PRs and push the result, so they'd arrive green — but that needs write access to PR branches and a macOS runner for the iOS head, which bills at 10× Linux. That's a call worth making on purpose rather than slipping into this PR. Related to #132.
Checks
dependabot.ymlstill parses to the same configuration.🤖 Generated with Claude Code