[internal] Refactor computing metadata for first-party Go packages #13161
Conversation
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
| # Raise an exception if any unsupported source file keys are present in the metadata. | ||
| for key in ( | ||
| "CompiledGoFiles", | ||
| "FFiles", | ||
| "SwigFiles", | ||
| "SwigCXXFiles", | ||
| ): | ||
| files = metadata.get(key, []) | ||
| if files: | ||
| raise ValueError( | ||
| f"The go_package at address {address} contains the following unsupported source " | ||
| f"files that were detected under the key '{key}': {', '.join(files)}." | ||
| ) |
There was a problem hiding this comment.
NB that these errors should have never actually triggered. We run in a sandbox, and we ban the sources field from having those file endings. So this code path couldn't happen afaict.
There was a problem hiding this comment.
As explained in my other comment, it can happen for cgo since that is just a .go file that does import "C".
There was a problem hiding this comment.
Ah, specifically CGoFiles is the issue. Everything else has a different file extension. See the entry on GoFiles.
// Source files
GoFiles []string // .go source files (excluding CgoFiles, TestGoFiles, XTestGoFiles)
CgoFiles []string // .go source files that import "C"
CompiledGoFiles []string // .go files presented to compiler (when using -compiled)
IgnoredGoFiles []string // .go source files ignored due to build constraints
IgnoredOtherFiles []string // non-.go source files ignored due to build constraints
CFiles []string // .c source files
CXXFiles []string // .cc, .cxx and .cpp source files
MFiles []string // .m source files
HFiles []string // .h, .hh, .hpp and .hxx source files
FFiles []string // .f, .F, .for and .f90 Fortran source files
SFiles []string // .s source files
SwigFiles []string // .swig files
SwigCXXFiles []string // .swigcxx files
SysoFiles []string // .syso object files to add to archive
TestGoFiles []string // _test.go files in package
XTestGoFiles []string // _test.go files outside package
Will fix in a followup.
| working_dir=owning_go_mod.address.spec_path, | ||
| ), | ||
| ) | ||
| metadata = json.loads(result.stdout) |
There was a problem hiding this comment.
This method should error if any of the supported source code keys are set in the JSON. This will at least prevent Pants from acting incorrectly with respect to first-party code. For example, imagine the user writes a cgo file (which is triggered merely by including import "C" in a .go file). We want to alert the user that we will not do what they expect in that instance. Plus it raises our attention to what are users will need as they contact us for such support.
There was a problem hiding this comment.
I used to have code for that in an earlier version of the plugin but got rid of it when ResolvedGoPackage had all of the source-related fields added to it. (And then I never added back erroring on unsupported source files, e.g. cgo.) But we should fix up erroring now.
| # Raise an exception if any unsupported source file keys are present in the metadata. | ||
| for key in ( | ||
| "CompiledGoFiles", | ||
| "FFiles", | ||
| "SwigFiles", | ||
| "SwigCXXFiles", | ||
| ): | ||
| files = metadata.get(key, []) | ||
| if files: | ||
| raise ValueError( | ||
| f"The go_package at address {address} contains the following unsupported source " | ||
| f"files that were detected under the key '{key}': {', '.join(files)}." | ||
| ) |
There was a problem hiding this comment.
As explained in my other comment, it can happen for cgo since that is just a .go file that does import "C".
This renames the type to
FirstPartyPkgInfoand renames the file.It removes all unused fields so that it's the simplest implementation possible. We can add back fields for things like
.hfiles when we add support.[ci skip-rust]