Add patchcheck, check vendor patch#2211
Merged
dagood merged 4 commits intomicrosoft/mainfrom Mar 25, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new _util “patchcheck” facility to validate repository patch files (especially the vendor patch) deterministically, without applying patches or relying on submodule state. This fits into the eng/_util tool suite and CI/selftest workflow by providing a repeatable check over patches/*.patch.
Changes:
- Introduces an internal
patchcheckpackage that inspects patch modifications viagit apply --numstatand enforces vendor-patch path constraints. - Adds a
patchcheckcommand undereng/_util/cmdto run the checks and report issues. - Adds new tests: one to fail the build if patch issues are found, and one to enforce minimal dependency rules for
_utilcommands.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| eng/_util/internal/patchcheck/vendoronly.go | Implements vendor-patch-only path rules and vendor patch identification. |
| eng/_util/internal/patchcheck/patchcheck.go | Core patch scanning logic (walk patches, parse numstat, report issues). |
| eng/_util/internal/patchcheck/patchcheck_test.go | Runs patchcheck in tests and fails on any issues. |
| eng/_util/internal/dependency_test.go | Adds enforcement that _util command packages keep dependencies minimal. |
| eng/_util/cmd/patchcheck/patchcheck.go | Adds CLI entry point for running patch checks from eng/run.ps1. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: dagood <12819531+dagood@users.noreply.github.com> Agent-Logs-Url: https://github.com/microsoft/go/sessions/d35ddbc7-6b8f-49a5-9db7-2b6fd461e791
qmuntal
approved these changes
Mar 25, 2026
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
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.
Add
patchcheckutil command. Wire it up withselftestso CI automatically runs it.patchcheckruns deterministic tests on our patch files without involving submodule state. Currently just includes a rule for the vendoring patch based on what it looks like now. (It can always be easily adjusted later. We can also add more rules.)Copilot review doesn't want to touch the vendoring patch, presumably because it's huge. But what we ask it to do there seems like it can be tooled, so instead, Copilot can write this tool.
Ran into a Git oddity similar to this: https://stackoverflow.com/q/42386491. Copilot floundered (reflecting a lack of docs), even just asking it questions.
Moved
testutil/dependency_test.go tointernal/dependency_test.gobecause it doesn't make sense to put tests intotestutiljust because they're tests. Tests generally belong in the package they're testing. So, along with putting thepatchchecktests in thepatchcheckpackage, remove the potentially misleadingtestutilpackage.