bazel: difficult to get nogo to ignore all third-party code
#124154
Labels
A-build-system
C-enhancement
Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
T-dev-inf
Comments
rickystewart
added
C-enhancement
rickystewart
added a commit
to rickystewart/cockroach
that referenced
this issue
May 14, 2024
As of cockroachdb#79929, we have been using `only_files` using the pattern `cockroach/pkg` in order to only run lints against first-party code. While this has worked locally, this doesn't work in remote execution where the structure of the staging directory for the code differs. Instead of trying to allow-list first-party code, we instead deny-list third-party code using the name "external". This works for most third-party dependencies except for the cgo code, which need some special handling. Separately, it is unfortunate that it's difficult to match exclusively against first-party code (see cockroachdb#124154). I will look into whether we can submit a change upstream to address this. To avoid future packages accidentally skipping lints due to this change, I added a separate lint check to make sure no Go package names end in the string `external`. Epic: CRDB-8308 Release note: None
craig bot
pushed a commit
that referenced
this issue
May 14, 2024
124155: nogo: adjust include/exclude rules r=rail a=rickystewart As of #79929, we have been using `only_files` using the pattern `cockroach/pkg` in order to only run lints against first-party code. While this has worked locally, this doesn't work in remote execution where the structure of the staging directory for the code differs. Instead of trying to allow-list first-party code, we instead deny-list third-party code using the name "external". This works for most third-party dependencies except for the cgo code, which need some special handling. Separately, it is unfortunate that it's difficult to match exclusively against first-party code (see #124154). I will look into whether we can submit a change upstream to address this. To avoid future packages accidentally skipping lints due to this change, I added a separate lint check to make sure no Go package names end in the string `external`. Epic: CRDB-8308 Release note: None Co-authored-by: Ricky Stewart <ricky@cockroachlabs.com>
blathers-crl bot
pushed a commit
that referenced
this issue
May 14, 2024
As of #79929, we have been using `only_files` using the pattern `cockroach/pkg` in order to only run lints against first-party code. While this has worked locally, this doesn't work in remote execution where the structure of the staging directory for the code differs. Instead of trying to allow-list first-party code, we instead deny-list third-party code using the name "external". This works for most third-party dependencies except for the cgo code, which need some special handling. Separately, it is unfortunate that it's difficult to match exclusively against first-party code (see #124154). I will look into whether we can submit a change upstream to address this. To avoid future packages accidentally skipping lints due to this change, I added a separate lint check to make sure no Go package names end in the string `external`. Epic: CRDB-8308 Release note: None
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
To ignore certain files in
nogo, we need to pass in a regex to match the filename as described here. However, there is no one regex that allows us to reliably recognize all third-party code and match no first-party code. Ideally, this should be a "meta" thing wherenogocan simply check what is or is not inexternaland allow configuring that directly.Epic: CRDB-17171
Jira issue: CRDB-38733
The text was updated successfully, but these errors were encountered: