Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a workaround to allow Go overlay analysis to work with any build mode, bypassing the normal restriction that requires build-mode: none for traced languages. The change introduces a hard-coded exception for Go since it doesn't currently declare Build Mode None (BMN) support but has equivalent autobuild capabilities.
Key changes:
- Added Go-specific exception in overlay database mode validation
- Modified the traced language check to exclude Go from the build mode restriction
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/config-utils.ts | Added Go exception in getOverlayDatabaseMode function to bypass build mode validation |
| lib/init-action.js | Generated JavaScript equivalent of the TypeScript changes |
| l !== KnownLanguage.go && // Workaround to allow overlay analysis for Go with any build | ||
| // mode, since it does not support BMN. | ||
| (await codeql.isTracedLanguage(l)), |
There was a problem hiding this comment.
The hard-coded Go exception creates a maintenance burden and breaks the logical flow. Consider extracting this logic into a helper function like shouldCheckBuildModeForLanguage(language) to improve readability and make future language exceptions easier to manage.
henrymercer
left a comment
There was a problem hiding this comment.
You mention:
even though it has a similar autobuild mode that will work for overlay analysis
but this PR enables overlay for workflows using manual build steps too. Will those setups work too with overlay?
We have a check that a traced language can only run overlay analysis with build-mode: none, but Go does not currently declare support for BMN, even though it has a similar autobuild mode that will work for overlay analysis. This commit adds a hard-coded exception to that check, allowing any build mode for Go. This is intended as a short-term solution until Go declares BMN support. It should be safe, since we can choose not to enable the feature flag for Go repos using traced builds.
d2af337 to
7892cb2
Compare
|
I've updated the PR description and code comment to mention that the Go extractor/autobuilder will aim to ensure that we fall back to full analysis for build setups that won't work with overlays. |
henrymercer
left a comment
There was a problem hiding this comment.
Thanks for the extra context!
We have a check that a traced language can only run overlay analysis with
build-mode: none, but Go does not currently declare support for BMN, even though it has a similar autobuild mode that will work for overlay analysis.This commit adds a hard-coded exception to that check, allowing any build mode for Go. This is intended as a short-term solution until Go declares BMN support.
For now, it should be safe to enable for all build modes:
Risk assessment
For internal use only. Please select the risk level of this change:
Which use cases does this change impact?
analysis-kinds: code-scanning).analysis-kinds: code-quality).How did/will you validate this change?
If something goes wrong after this change is released, what are the mitigation and rollback strategies?
How will you know if something goes wrong after this change is released?
Merge / deployment checklist