rules_go icon indicating copy to clipboard operation
rules_go copied to clipboard

Nogo: provide a way to ignore external cgo source

Open sluongng opened this issue 2 years ago • 2 comments

What version of rules_go are you using?

v0.40.1

What version of gazelle are you using?

Irrelevant

What version of Bazel are you using?

6.2.1

Does this issue reproduce with the latest releases of all the above?

Yes

What operating system and processor architecture are you using?

Darwin / ARM64

Any other potentially useful information about your toolchain?

N/A

What did you do?

We have a dependency over github.com/shirou/gopsutil/v3/process package.

When enable copylocks analyzer by including "@org_golang_x_tools//go/analysis/passes/copylock:go_default_library" in nogo config, the code fail to compile.

We have already set nogo_config.json with

    "copylocks": {
        "exclude_files": {
            "external[\\\\,\\/]": "third_party"
        }
    },

However, the cgo source path does not include external and therefore, escaped the current regex.

We also want to avoid specific regex like this

    "copylocks": {
        "exclude_files": {
            "cgo[\\\\,\\/]github.com[\\\\,\\/]shirou[\\\\,\\/]gopsutil[\\\\,\\/]": "third_party cgo",
            "external[\\\\,\\/]": "third_party"
        }
    },

Even though it works, it might be harder to maintain.

What did you expect to see?

If possible, external cgo sources should be included into nogo with external inside the source path. This would allow us to ignore those files using existing regex.

What did you see instead?

ERROR: /private/var/tmp/_bazel_sluongng/06e573a93bc2d6a9cad4ad41f00b4310/external/com_github_shirou_gopsutil_v3/process/BUILD.bazel:3:11: GoCompilePkg external/com_github_shirou_gopsutil_v3/process/process.a failed: (Exit 1): builder failed: error executing command (from target @com_github_shirou_gopsutil_v3//process:process) bazel-out/darwin_arm64-opt-exec-2B5CBBC6/bin/external/go_sdk_darwin_arm64/builder_reset/builder compilepkg -sdk external/go_sdk_darwin_arm64 -installsuffix darwin_arm64 -src ... (remaining 95 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
compilepkg: nogo: errors found by nogo during build-time code analysis:
/var/folders/ly/gjsqx9wd0cv1ck7xdslrt6t00000gn/T/rules_go_work-1366086039/cgo/github.com/shirou/gopsutil/v3/process/process.go:144:9: String passes lock by value: github.com/shirou/gopsutil/v3/process.Process contains sync.RWMutex (copylocks)
/var/folders/ly/gjsqx9wd0cv1ck7xdslrt6t00000gn/T/rules_go_work-1366086039/cgo/github.com/shirou/gopsutil/v3/process/process.go:145:23: call of json.Marshal copies lock value: github.com/shirou/gopsutil/v3/process.Process contains sync.RWMutex (copylocks)

Note how rules_go_work-1366086039/cgo/github.com/shirou/gopsutil/v3/process/process.go does not contain external inside.

sluongng avatar Jul 07 '23 09:07 sluongng

@emmaxy do you know if the change in https://github.com/bazelbuild/rules_go/pull/3770 will exclude out cgo sources like rules_go_work-1366086039/cgo/github.com/shirou/gopsutil/v3/process/process.go from being fed into nogo?

If yes, we should be able to close this issue.

sluongng avatar Jan 08 '24 15:01 sluongng

@emmaxy do you know if the change in #3770 will exclude out cgo sources like rules_go_work-1366086039/cgo/github.com/shirou/gopsutil/v3/process/process.go from being fed into nogo?

If yes, we should be able to close this issue.

#3770 does not exclude any cgo sources from being fed into nogo, I don't think it would fix this issue.

emmaxy avatar Jan 08 '24 19:01 emmaxy

With #3995, the output becomes:

nogo: errors found by nogo during build-time code analysis:
external/gazelle~~go_deps~com_github_shirou_gopsutil_v3/process/process.go:144:9: String passes lock by value: github.com/shirou/gopsutil/v3/process.Process contains sync.RWMutex (copylocks)
external/gazelle~~go_deps~com_github_shirou_gopsutil_v3/process/process.go:145:23: call of json.Marshal copies lock value: github.com/shirou/gopsutil/v3/process.Process contains sync.RWMutex (copylocks)

Based on that, I would assume that the PR fixes this issue. @emmaxy Could you confirm that?

fmeum avatar Aug 02 '24 16:08 fmeum