Accept new #cgo directives introduced by go 1.24

[skirino]

(First of all thank you for the great build tool!)

Go 1.24 introduced new #cgo directives for performance improvements. Unfortunately the current pants go plugin does not understand the directives and results in the following error when building:

21:02:21.05 [ERROR] Failed to analyze github.com/pantsbuild/pants/testprojects/src/go/pants_test/cgo/directives for dependency inference:
invalid Go sources encountered
nocallback_noescape.go: cgo error: nocallback_noescape.go: invalid #cgo line: #cgo nocallback addNumbers

The error message was obtained by running the new test ./pants test testprojects/src/go/pants_test/cgo/directives: with the following diff (to recognize code under testprojects/src/go/).

diff --git a/pants.toml b/pants.toml
index fde22eaf8e..f3e4f56313 100644
--- a/pants.toml
+++ b/pants.toml
@@ -72,7 +72,7 @@ pants_ignore.add = [

 build_ignore.add = [
   # Disable Go targets by default so Pants developers do not need Go installed.
-  "testprojects/src/go/**",
+  # "testprojects/src/go/**",
 ]

 unmatched_build_file_globs = "error"

This PR fixes the issue by changing saveCgo function in go package analyzer. Specifically I updated saveCgo based on https://github.com/golang/go/blob/go1.26.4/src/go/build/build.go#L1710-L1713 (i.e. catch up with upstream) so that #cgo nocallback directive is not rejected.

I also added a simple cgo test case. Please let me know if you have a better idea how to test this feature.

[sureshjoshi]

Thanks for the PR!

@tdyas would this be something you'd be able to review? Seems simple, but I know approximately nothing about Go.

[tdyas]

This code ignores these #cgo comments instead of recording them in the comment before import "C" like other #cgo comments. That seems wrong since ignoring them here will result in go not seeing the directives at a later compilation stage.

Also, the call to strings.Fields duplicates the call on line 161 to strings.Fields.

[skirino]

@tdyas Thank you for reviewing this PR!

IIUC nocallback and noescape directives are somewhat different in the sense that they affect the contents of cgo-generated source files, whereas other #cgo directives affect how the generated sources are handled. There's nothing we need to explicitly tell pants about the directive; the directive in .go should suffice.

To back up the idea: the following code (intentionally violate "nocallback" by calling back into Go) results in a panic when pants run. Without the #cgo nocallback line it runs successfully. (I'm trying to turn this into a python test...)

package main

// #include <stdio.h>
// void goCallback(void);
// static inline void triggerViolation(void) {
//     printf("[C] Executing triggerViolation...\n");
//     goCallback(); 
//     printf("[C] This line will never execute.\n");
// }
// #cgo nocallback triggerViolation
import "C"
import "fmt"

//export goCallback
func goCallback() {
	fmt.Println("[Go] Inside goCallback!")
}

func main() {
	fmt.Println("[Go] Calling triggerViolation...")
	C.triggerViolation()
}

As for the duplicated calls of strings.Fields: I think they are not pretty, but saveCgo() function is taken from go compiler code and it's good to keep diffs from the upstream code minimal. What do you think?

[tdyas]

Source code under testprojects is not necessarily part of the Pants test suite by default. These would be better framed as part of a Python test file which sets up a Go project to be built. There should be existing CGo test files for this purpose.

[skirino]

OK, thank you for the comment. I'm going to turn the test into a python test like the ones in https://github.com/pantsbuild/pants/blob/main/src/python/pants/backend/go/util_rules/cgo_test.py, so that we can have more fine-grained setup/assertions.

[skirino]

In 24f5353 I turned the test under testprojects/ into a python test. Also changed the CGo source code to violate "nocallback", and confirm in test that pants run causes go runtime panic as expected. Please take a look.