Run nogo in a separate validation action #3995

fmeum · 2024-07-26T13:57:43Z

What type of PR is this?

Feature

What does this PR do? Why is it needed?

nogo is run in a separate action that can be toggled with --run_validations. This avoids rerunning compilation if the nogo tool changes and also allows to collect nogo findings of targets that depend on other targets that themselves have nogo findings (with --keep_going).

Which issues(s) does this PR fix?

Fixes #3619
Fixes #3695
Fixes #3846
Closes #3707

Other notes for review

fmeum · 2024-07-27T17:45:40Z

@joeljeske Sorry for the long delay, but this should finally be ready (and is certainly heavily inspired by your PR!).

tests/core/nogo/custom/custom_test.go

fmeum · 2024-07-27T17:50:22Z

CC @tingilee as you raised the original issue

go/tools/builders/nogo_validation.go

go/private/actions/compilepkg.bzl

go/tools/builders/nogo.go

go/tools/builders/compilepkg.go

go/tools/builders/constants.go

go/private/actions/compilepkg.bzl

sluongng

Few nits, but the current approach lgtm

go/tools/builders/compilepkg.go

go/tools/builders/constants.go

Co-authored-by: Zhongpeng Lin <zplin@uber.com>

sluongng

Lgtm. Let's ship this while the pr is still small 😂

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [io_bazel_rules_go](https://redirect.github.com/bazelbuild/rules_go) | http_archive | minor | `v0.49.0` -> `v0.50.0` | --- ### Release Notes <details> <summary>bazelbuild/rules_go (io_bazel_rules_go)</summary> ### [`v0.50.0`](https://redirect.github.com/bazelbuild/rules_go/releases/tag/v0.50.0) [Compare Source](https://redirect.github.com/bazelbuild/rules_go/compare/v0.49.0...v0.50.0) #### `WORKSPACE` code load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( name = "io_bazel_rules_go", sha256 = "67b4d1f517ba73e0a92eb2f57d821f2ddc21f5bc2bd7a231573f11bd8758192e", urls = [ "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.50.0/rules_go-v0.50.0.zip", "https://github.com/bazelbuild/rules_go/releases/download/v0.50.0/rules_go-v0.50.0.zip", ], ) load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies") go_rules_dependencies() go_register_toolchains(version = "1.23.0") #### What's Changed - Break reliance on GOROOT_FINAL by [@JacobOaks](https://redirect.github.com/JacobOaks) in [bazelbuild/rules_go#3984 - Migrate to macos_arm64 by [@meteorcloudy](https://redirect.github.com/meteorcloudy) in [bazelbuild/rules_go#3990 - Support matching release candidate toolchain versions by [@JacobOaks](https://redirect.github.com/JacobOaks) in [bazelbuild/rules_go#3998 - rm crosstool by [@sluongng](https://redirect.github.com/sluongng) in [bazelbuild/rules_go#3986 - fix(timeout.go): remove redundant leaked go func in RegisterTimeoutHandler by [@Roytangrb](https://redirect.github.com/Roytangrb) in [bazelbuild/rules_go#4004 - Run nogo in a separate validation action by [@fmeum](https://redirect.github.com/fmeum) in [bazelbuild/rules_go#3995 #### New Contributors - [@JacobOaks](https://redirect.github.com/JacobOaks) made their first contribution in [bazelbuild/rules_go#3984 - [@Roytangrb](https://redirect.github.com/Roytangrb) made their first contribution in [bazelbuild/rules_go#4004 **Full Changelog**: bazelbuild/rules_go@release-0.49...release-0.50 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://redirect.github.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://redirect.github.com/renovatebot/renovate).  Co-authored-by: cgrindel-self-hosted-renovate[bot] <139595543+cgrindel-self-hosted-renovate[bot]@users.noreply.github.com>

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [io_bazel_rules_go](https://togithub.com/bazelbuild/rules_go) | http_archive | minor | `v0.49.0` -> `v0.50.0` | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>bazelbuild/rules_go (io_bazel_rules_go)</summary> ### [`v0.50.0`](https://togithub.com/bazelbuild/rules_go/releases/tag/v0.50.0) [Compare Source](https://togithub.com/bazelbuild/rules_go/compare/v0.49.0...v0.50.0) #### `WORKSPACE` code load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( name = "io_bazel_rules_go", sha256 = "67b4d1f517ba73e0a92eb2f57d821f2ddc21f5bc2bd7a231573f11bd8758192e", urls = [ "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.50.0/rules_go-v0.50.0.zip", "https://github.com/bazelbuild/rules_go/releases/download/v0.50.0/rules_go-v0.50.0.zip", ], ) load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies") go_rules_dependencies() go_register_toolchains(version = "1.23.0") #### What's Changed - Break reliance on GOROOT_FINAL by [@JacobOaks](https://togithub.com/JacobOaks) in [bazelbuild/rules_go#3984 - Migrate to macos_arm64 by [@meteorcloudy](https://togithub.com/meteorcloudy) in [bazelbuild/rules_go#3990 - Support matching release candidate toolchain versions by [@JacobOaks](https://togithub.com/JacobOaks) in [bazelbuild/rules_go#3998 - rm crosstool by [@sluongng](https://togithub.com/sluongng) in [bazelbuild/rules_go#3986 - fix(timeout.go): remove redundant leaked go func in RegisterTimeoutHandler by [@Roytangrb](https://togithub.com/Roytangrb) in [bazelbuild/rules_go#4004 - Run nogo in a separate validation action by [@fmeum](https://togithub.com/fmeum) in [bazelbuild/rules_go#3995 #### New Contributors - [@JacobOaks](https://togithub.com/JacobOaks) made their first contribution in [bazelbuild/rules_go#3984 - [@Roytangrb](https://togithub.com/Roytangrb) made their first contribution in [bazelbuild/rules_go#4004 **Full Changelog**: bazelbuild/rules_go@release-0.49...release-0.50 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/kreempuff/rules_unreal_engine).

fmeum force-pushed the extract-nogo branch 2 times, most recently from 80ba35c to 296eddd Compare July 26, 2024 14:34

fmeum marked this pull request as ready for review July 27, 2024 17:45

fmeum requested review from tyler-french and linzhp July 27, 2024 17:45

fmeum commented Jul 27, 2024

View reviewed changes

tests/core/nogo/custom/custom_test.go Show resolved Hide resolved

linzhp approved these changes Jul 27, 2024

View reviewed changes

fmeum requested a review from linzhp July 27, 2024 22:04

linzhp approved these changes Jul 28, 2024

View reviewed changes

go/tools/builders/constants.go Outdated Show resolved Hide resolved

fmeum requested a review from linzhp July 29, 2024 09:03

sluongng reviewed Jul 31, 2024

View reviewed changes

go/private/actions/compilepkg.bzl Outdated Show resolved Hide resolved

sluongng reviewed Jul 31, 2024

View reviewed changes

go/tools/builders/compilepkg.go Show resolved Hide resolved

go/tools/builders/compilepkg.go Show resolved Hide resolved

go/tools/builders/constants.go Show resolved Hide resolved

fmeum force-pushed the extract-nogo branch from b82e58d to be76a5e Compare July 31, 2024 17:53

fmeum mentioned this pull request Aug 2, 2024

Nogo: provide a way to ignore external cgo source #3619

Closed

fmeum force-pushed the extract-nogo branch from 58afe68 to 5731503 Compare August 2, 2024 16:43

fmeum requested a review from sluongng August 5, 2024 13:24

fmeum mentioned this pull request Aug 6, 2024

compilepkg for cgo targets fails with nogo and --collect_code_coverage enabled #3846

Closed

fmeum added 11 commits August 6, 2024 17:53

Register generated cgo Go sources as outputs

d4dbece

Extract nogo

954c098

Remove old nogo logic

0dc29a9

Always produce facts

76eaa03

Refactor out file definitions

47d0190

Fix buildifier

9ca56a1

Add recompile_internal_deps

eb1dbbc

Do not print output twice

0aa2e17

Wire up cover_mode and cgo srcs

e9b484e

Wire up cover_archive

483afa1

Use a dedicated exit code for findings

4ea34a2

fmeum and others added 21 commits August 6, 2024 17:53

Update custom_test.go

3563e27

Update custom_test.go

3063bfe

Run cgo twice with coverage

9c1c01c

Add docs and register output for binary rules

62c42d4

Use a Go tool for validation

b180314

Update go/tools/builders/nogo.go

55804d6

Co-authored-by: Zhongpeng Lin <zplin@uber.com>

Update go/tools/builders/nogo.go

e858e2c

Co-authored-by: Zhongpeng Lin <zplin@uber.com>

Address more comments

3cf2ee0

Use exit code

9ec89eb

Update constants.go

e2ea091

Update nogo.go

d2c0599

Update nogo_main.go

4460988

Update nogo_validation.go

ca62338

Update nogo_validation.go

8f311d6

Rename constants

fc8bbfb

Add comments

6436295

Fix output of nogovalidation

62ef390

Fix output of nogovalidation

89b22b5

Fix out of date info on nogo configuration

cfbf986

Optionally let nogo itself fail on findings

7bb2d8a

Also fix 3846

ddb181b

fmeum force-pushed the extract-nogo branch from f66c6d6 to ddb181b Compare August 6, 2024 15:53

sluongng approved these changes Aug 6, 2024

View reviewed changes

fmeum merged commit 29d4e5d into master Aug 6, 2024
5 checks passed

fmeum deleted the extract-nogo branch August 6, 2024 20:12

TvdW mentioned this pull request Aug 23, 2024

nogo incorrectly flags strict dependency error #4062

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run nogo in a separate validation action #3995

Run nogo in a separate validation action #3995

fmeum commented Jul 26, 2024 •

edited

Loading

fmeum commented Jul 27, 2024

fmeum commented Jul 27, 2024 •

edited

Loading

sluongng left a comment

sluongng left a comment

Run nogo in a separate validation action #3995

Run nogo in a separate validation action #3995

Conversation

fmeum commented Jul 26, 2024 • edited Loading

fmeum commented Jul 27, 2024

fmeum commented Jul 27, 2024 • edited Loading

sluongng left a comment

Choose a reason for hiding this comment

sluongng left a comment

Choose a reason for hiding this comment

fmeum commented Jul 26, 2024 •

edited

Loading

fmeum commented Jul 27, 2024 •

edited

Loading