Add option get to regex that match the go cmd
#4911
Merged
Conversation
This file contains 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
There's been several annoying bugs that turned out to be caused by error messages that used to be emitted as `go: ABC` now emitted as `go get: ABC`: * dependabot#4868 * dependabot#4910 Given this, we should proactively handle this in the other regexes that look like they might be susceptible to this. There are two ways to do this: 1. Add an optional non-capturing group: `(?: get)?` 2. Completely remove the `go: ` prefix from the regex. I went with the first option, because I was concerned the second might match individual lines within a _legitimate_ multi-line error message. IE, I've seen go error messages that list a mult-line dep tree, with only the last line listing the actual error text. But the first line listed the `go: ` prefix. And by keeping the regex wide, it reported the entire error in the Dependabot status page, which was very useful for tracking down what was throwing the error. Whereas if we go with option two, then since that will match an individual line, the "stack trace" of the error will be dropped. But I'm open to feedback if there's something I'm overlooking here.
jeffwidman
commented
Mar 26, 2022
| # (Private) module could not be found | ||
| /cannot find module providing package/.freeze, | ||
| # Package in module was likely renamed or removed | ||
| /module .* found \(.*\), but does not contain package/m.freeze, | ||
| # Package pseudo-version does not match the version-control metadata | ||
| # https://golang.google.cn/doc/go1.13#version-validation | ||
| /go: .*: invalid pseudo-version/m.freeze, | ||
| /go(?: get)?: .*: invalid pseudo-version/m.freeze, | ||
| # Package does not exist, has been pulled or cannot be reached due to | ||
| # auth problems with either git or the go proxy | ||
| /go: .*: unknown revision/m.freeze, |
There was a problem hiding this comment.
This one is already handled in #4910
jurre
approved these changes
Mar 28, 2022
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.
There's been several annoying bugs that turned out to be caused by error
messages that used to be emitted as
go: ABCnow emitted asgo get: ABC:go: downloadingis emitted before an unknown revision error #4910Given this, we should proactively handle this in the other regexes that
look like they might be susceptible to this.
There are two ways to do this:
(?: get)?go:prefix from the regex.I went with the first option, because I was concerned the second might
match individual lines within a legitimate multi-line error message.
IE, I've seen go error messages that list a multi-line dep tree, with
only the last line listing the actual error text. But the first line
listed the
go:prefix. And by keeping the regex wide, it reported theentire error in the Dependabot status page, which was very useful for
tracking down what was throwing the error.
Whereas if we go with option two, then since that will match an
individual line, the "stack trace" of the error will be dropped.
But I'm open to feedback if there's something I'm overlooking here.