Skip to content

Conversation

r-hang
Copy link
Contributor

@r-hang r-hang commented May 12, 2025

go/tools/gopackagesdriver: Filter cgo sources and add cgo processed files

The gopackagesdriver today fails to load Go programs
with cgo dependencies.

go-code/bazel-pkgdrv/external/com_github_mattn_go_sqlite3/backup.go:16:8: could not import C (no metadata for C)

These errors come from two sources:

  • First, the gopackagesdriver includes cgo sources files in the
    CompiledGoFiles response.
  • Second, the gopackagesdriver response and CompiledGoFiles do not
    contain cgo processed Go sources necessary to compile the program.

This change addresses both error points by

  • Exposing the location of cgo generated artifacts in the rules_go archive object.
  • Filtering out cgo source files from the CompiledGoFiles response, by
    checking if the files import "C".
  • Adding cgo processed files to the CompiledGoFiles response by exploiting
    knowledge of how rules_go compiles cgo files.

I am not sure if there is a better solution to leaking build system
details to the gopackagesdriver as even 'go list' leaks where
the go build cache stores cgo processed files. Though in the case of 'go list'
the conventions for cgo processed files are a bit different, they don't
seem to have a file extension.

For example,

$ go list -json=ImportPath,CgoFiles,GoFiles,CompiledGoFiles -compiled -find github.com/mattn/go-sqlite3
 "CompiledGoFiles": [
                "convert.go",
                "doc.go",
                "sqlite3_func_crypt.go",
                "sqlite3_go18.go",
                "sqlite3_opt_preupdate.go",
                "sqlite3_opt_preupdate_omit.go",
                "/Users/rhang/Library/Caches/go-build/8f/8f8bfe0b75fcceeb093e990af3eaa4f05147147b13db050b2f706cb563280415-d",
                "/Users/rhang/Library/Caches/go-build/9c/9c0b42e0a548721eb17f0fe5cfc2b6f1c0f0f7fcfb0f0caf526229723b9d4927-d",
                "/Users/rhang/Library/Caches/go-build/f7/f7e25424abcdc8442dbe22670456cab79a881268995c4ed43715e6d56930bff6-d",
...

This PR resolves #4337 additional
details are provided in the issue.

Copy link
Member

@fmeum fmeum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks great! Could you run buildifier to fix CI?

@r-hang r-hang force-pushed the rhang/gopackagesdriver-cgo-support branch from b6eb225 to 3536f78 Compare May 13, 2025 03:03
@r-hang r-hang force-pushed the rhang/gopackagesdriver-cgo-support branch 9 times, most recently from d28387f to 80a1f02 Compare May 15, 2025 15:01
Copy link
Contributor

@JamyDev JamyDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also please add a test case for this functionality.

@r-hang r-hang force-pushed the rhang/gopackagesdriver-cgo-support branch from ff74244 to 46816cf Compare May 16, 2025 21:21
The gopackagesdriver today fails to load Go programs
with cgo dependencies.

These errors come from two sources:
- First, Cgo source files are included in the CompiledGoFiles field of
  the packages driver response.
- Second, the CompiledGoFiles field of the gopackages driver response
  does not contain the cgo processed Go sources necessary to compile
  programs.

This change fixes these issues by:
- Exposing the Cgo generated artifact directory in the GoArchive data.
- Creating a Bazel rule to specially process and write *.pkg.json
  files that exclude Cgo source files and include Cgo generated code.

Testing:

With these changes, I was able to load one of the largest projects
in my company's Bazel monorepo and use this data to generate
a callgraph.

With the sole package load error being caused by an external
package with a go build taged file that renders the file
inert for more recent versions of Go.
```
>>
packages loaded:  991
-: sources missing for package @com_github_aws_aws_sdk_go//internal/context:context
errors:  1
building ssa
building callgraph with RTA
callgraph nodes #: 209074
```

ref bazel-contrib#4337
@r-hang r-hang force-pushed the rhang/gopackagesdriver-cgo-support branch from 0278d30 to 86352a8 Compare May 16, 2025 22:10
@linzhp linzhp enabled auto-merge (squash) May 19, 2025 17:49
@linzhp linzhp merged commit 11c6145 into bazel-contrib:master May 19, 2025
1 check passed
@r-hang r-hang deleted the rhang/gopackagesdriver-cgo-support branch May 19, 2025 19:38
r-hang added a commit to r-hang/rules_go that referenced this pull request Jun 4, 2025
When testing the bazel driver, we ran into an error that flaged that
the argument list as too long.

```
ERROR: ...json failed: (Exit -1): pkgjson failed: error executing Action command (from target //...) bazel-out/k8-opt-exec-ST-a828a81199fe/bin/external/io_bazel_rules_go/go/tools/gopackagesdriver/pkgjson/reset_pkgjson/pkgjson --id @//src/... --pkg_path ... (remaining 15 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
Action failed to execute: java.io.IOException: Cannot run program "/home/user/.cache/bazel/_bazel_rhang/install/7e4ce7b0d69e79cb6bd84c7f9dfefe6b/process-wrapper" (in directory "/home/user/.cache/pkgdrv/0ddf1c72b811bee41d29991c732306ef72553747/sandbox/processwrapper-sandbox/29913/execroot/__main__"): error=7, Argument list too long
ERROR: Build did NOT complete successfully
```

Digging into this issue, the cause is that the pkgjson command takes
in all of the fields of package archive data as arguments.

To work around this, we should preserve the original approach of writing
a pkg json, before  bazel-contrib#4338,
which used Skylark builtins to write the package content directly to disk.

The pkgjson command is updated to parse ths json file directly and
write out a transformed pkg json with the cgo related corrections in
order to avoid limits regarding argument size.
r-hang added a commit to r-hang/rules_go that referenced this pull request Jun 6, 2025
When testing the bazel driver, we ran into an error that flaged that
the argument list as too long.

```
ERROR: ...json failed: (Exit -1): pkgjson failed: error executing Action command (from target //...) bazel-out/k8-opt-exec-ST-a828a81199fe/bin/external/io_bazel_rules_go/go/tools/gopackagesdriver/pkgjson/reset_pkgjson/pkgjson --id @//src/... --pkg_path ... (remaining 15 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
Action failed to execute: java.io.IOException: Cannot run program "/home/user/.cache/bazel/_bazel_rhang/install/7e4ce7b0d69e79cb6bd84c7f9dfefe6b/process-wrapper" (in directory "/home/user/.cache/pkgdrv/0ddf1c72b811bee41d29991c732306ef72553747/sandbox/processwrapper-sandbox/29913/execroot/__main__"): error=7, Argument list too long
ERROR: Build did NOT complete successfully
```

Digging into this issue, the cause is that the pkgjson command takes
in all of the fields of package archive data as arguments.

To work around this, we should preserve the original approach of writing
a pkg json, before  bazel-contrib#4338,
which used Skylark builtins to write the package content directly to disk.

The pkgjson command is updated to parse ths json file directly and
write out a transformed pkg json with the cgo related corrections in
order to avoid limits regarding argument size.

Note:

This diff also makes changes to undo a breaking change (i.e. changing
the signature of the make_pkg_json function) that was made in
bazel-contrib#4338.

I revert changes to the functionalty of make_pkg_json and add a new
replacement function make_pkg_json_with_archive.
r-hang added a commit to r-hang/rules_go that referenced this pull request Jun 6, 2025
When testing the bazel driver, we ran into an error that flaged that
the argument list as too long.

```
ERROR: ...json failed: (Exit -1): pkgjson failed: error executing Action command (from target //...) bazel-out/k8-opt-exec-ST-a828a81199fe/bin/external/io_bazel_rules_go/go/tools/gopackagesdriver/pkgjson/reset_pkgjson/pkgjson --id @//src/... --pkg_path ... (remaining 15 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
Action failed to execute: java.io.IOException: Cannot run program "/home/user/.cache/bazel/_bazel_rhang/install/7e4ce7b0d69e79cb6bd84c7f9dfefe6b/process-wrapper" (in directory "/home/user/.cache/pkgdrv/0ddf1c72b811bee41d29991c732306ef72553747/sandbox/processwrapper-sandbox/29913/execroot/__main__"): error=7, Argument list too long
ERROR: Build did NOT complete successfully
```

Digging into this issue, the cause is that the pkgjson command takes
in all of the fields of package archive data as arguments.

To work around this, we should preserve the original approach of writing
a pkg json, before  bazel-contrib#4338,
which used Skylark builtins to write the package content directly to disk.

The pkgjson command is updated to parse ths json file directly and
write out a transformed pkg json with the cgo related corrections in
order to avoid limits regarding argument size.

Note:

This diff also makes changes to undo a breaking change (i.e. changing
the signature of the make_pkg_json function) that was made in
bazel-contrib#4338.

I revert changes to the functionalty of make_pkg_json and add a new
replacement function make_pkg_json_with_archive.
fmeum pushed a commit that referenced this pull request Jun 10, 2025
…#4371)

When testing the bazel driver, we ran into an error that flaged that the
argument list as too long.

```
ERROR: ...json failed: (Exit -1): pkgjson failed: error executing Action command (from target //...) bazel-out/k8-opt-exec-ST-a828a81199fe/bin/external/io_bazel_rules_go/go/tools/gopackagesdriver/pkgjson/reset_pkgjson/pkgjson --id @//src/... --pkg_path ... (remaining 15 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
Action failed to execute: java.io.IOException: Cannot run program "/home/user/.cache/bazel/_bazel_rhang/install/7e4ce7b0d69e79cb6bd84c7f9dfefe6b/process-wrapper" (in directory "/home/user/.cache/pkgdrv/0ddf1c72b811bee41d29991c732306ef72553747/sandbox/processwrapper-sandbox/29913/execroot/__main__"): error=7, Argument list too long
ERROR: Build did NOT complete successfully
```

Digging into this issue, the cause is that the pkgjson command takes in
all of the fields of package archive data as arguments.

To work around this, we should preserve the original approach of writing
a pkg json, before #4338,
which used Skylark builtins to write the package content directly to
disk.

The pkgjson command is updated to parse ths json file directly and write
out a transformed pkg json with the cgo related corrections in order to
avoid limits regarding argument size.

Note:
    
This diff also makes changes to undo a breaking change (i.e. changing
the signature of the make_pkg_json function) that was made in
#4338.

I revert changes to the functionalty of make_pkg_json and add a new
replacement function make_pkg_json_with_archive.
linzhp pushed a commit that referenced this pull request Jun 10, 2025
…#4371)

When testing the bazel driver, we ran into an error that flaged that the
argument list as too long.

```
ERROR: ...json failed: (Exit -1): pkgjson failed: error executing Action command (from target //...) bazel-out/k8-opt-exec-ST-a828a81199fe/bin/external/io_bazel_rules_go/go/tools/gopackagesdriver/pkgjson/reset_pkgjson/pkgjson --id @//src/... --pkg_path ... (remaining 15 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
Action failed to execute: java.io.IOException: Cannot run program "/home/user/.cache/bazel/_bazel_rhang/install/7e4ce7b0d69e79cb6bd84c7f9dfefe6b/process-wrapper" (in directory "/home/user/.cache/pkgdrv/0ddf1c72b811bee41d29991c732306ef72553747/sandbox/processwrapper-sandbox/29913/execroot/__main__"): error=7, Argument list too long
ERROR: Build did NOT complete successfully
```

Digging into this issue, the cause is that the pkgjson command takes in
all of the fields of package archive data as arguments.

To work around this, we should preserve the original approach of writing
a pkg json, before #4338,
which used Skylark builtins to write the package content directly to
disk.

The pkgjson command is updated to parse ths json file directly and write
out a transformed pkg json with the cgo related corrections in order to
avoid limits regarding argument size.

Note:
    
This diff also makes changes to undo a breaking change (i.e. changing
the signature of the make_pkg_json function) that was made in
#4338.

I revert changes to the functionalty of make_pkg_json and add a new
replacement function make_pkg_json_with_archive.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

go/tools/packagesdriver: Missing Cgo support
4 participants