Generate correct subrepo info in pkg.json

build_defs/go.build_defs

@@ -221,7 +221,7 @@ def go_library(name:str, srcs:list, resources:list=[], asm_srcs:list=None, hdrs:
                _needs_transitive_deps=False, _all_srcs=False, cover:bool=True,
                filter_srcs:bool=True, _link_private:bool=False, _link_extra:bool=True, _abi:str=None,
                _generate_import_config:bool=True, _generate_pkg_info:bool=CONFIG.GO.PKG_INFO,
-               import_path:str='', labels:list=[], package:str=None, pgo_file:str=None, _module:str=''):
+               import_path:str='', labels:list=[], package:str=None, pgo_file:str=None, _module:str='', _subrepo:str=''):
     """Generates a Go library which can be reused by other rules.
 
     Args:
@@ -386,6 +386,7 @@ def go_library(name:str, srcs:list, resources:list=[], asm_srcs:list=None, hdrs:
             _needs_transitive_deps = _needs_transitive_deps,
             _all_srcs = _all_srcs,
             _generate_import_config=False,
+            _subrepo = _subrepo,
             labels=labels + ["foo"],
             import_path=package_path,
         )
@@ -451,6 +452,7 @@ def go_library(name:str, srcs:list, resources:list=[], asm_srcs:list=None, hdrs:
             name = name,
             srcs = srcs + resources,
             import_path = import_path,
+            subrepo = _subrepo,
             package = package,
             test_only = test_only,
             visibility = visibility,
@@ -1221,6 +1223,7 @@ def go_repo(module: str, version:str='', download:str=None, name:str=None, insta
     labels = ["go_module_path:" + module]
     if version:
         labels += [f"go_module:{module}@{version}"]
+    pkg_name = package_name()
     requirements = " ".join(requirements)
 
     build_tag_args = '--build_tag ' + ' --build_tag '.join(build_tags) if build_tags else ''
@@ -1231,7 +1234,7 @@ def go_repo(module: str, version:str='', download:str=None, name:str=None, insta
         name = name,
         tag = "repo" if install else None,
         srcs =  srcs,
-        cmd  = f"rm -rf $SRCS_DOWNLOAD/.plzconfig && find $SRCS_DOWNLOAD -name BUILD -delete && mkdir -p $(dirname {pkgRoot}) && mv $SRCS_DOWNLOAD {pkgRoot} && $TOOL generate {modFileArg} --module {module} --version '{version}' {build_tag_args} --src_root={pkgRoot} --third_part_folder='{third_party_path}' {install_args} {requirements} && mv {pkgRoot} $OUT",
+        cmd  = f"rm -rf $SRCS_DOWNLOAD/.plzconfig && find $SRCS_DOWNLOAD -name BUILD -delete && mkdir -p $(dirname {pkgRoot}) && mv $SRCS_DOWNLOAD {pkgRoot} && $TOOL generate {modFileArg} --module {module} --version '{version}' {build_tag_args} --src_root={pkgRoot} --third_part_folder='{third_party_path}' --subrepo '{pkg_name}/{subrepo_name}' {install_args} {requirements} && mv {pkgRoot} $OUT",
         outs = [subrepo_name],
         tools = [CONFIG.GO.PLEASE_GO_TOOL],
         env= {
@@ -1251,7 +1254,6 @@ def go_repo(module: str, version:str='', download:str=None, name:str=None, insta
     )
 
     if install:
-        pkg_name = package_name()
         return filegroup(
             name = name,
             visibility = visibility,
@@ -1523,11 +1525,8 @@ def _go_modinfo(name:str, test_only:bool=False, deps:list):
 
 
 def _go_pkg_info(name:str, srcs:list, visibility:list, importconfig:str=None, import_path:str="", package:str="",
-                 module:str="", test_only:bool=False, install:list=[]):
+                 module:str="", subrepo:str="", test_only:bool=False, install:list=[]):
     """Internal-only function for generating the pkg_info files"""
-
-
-
     install_flags = ""
     for i in install:
         install_flags += f" -p {i}" if i else " -p ."
@@ -1544,6 +1543,8 @@ def _go_pkg_info(name:str, srcs:list, visibility:list, importconfig:str=None, im
         else:
             import_path = _get_import_path(package, import_path)
         cmd = f'$TOOL p -i "{import_path}" -m "{import_path}:$PKG_DIR/{package}.a"'
+        if subrepo:
+            cmd = f"{cmd} -s {subrepo}"
     return build_rule(
         name = name,
         tag = "pkg_info",

tools/driver/ChangeLog

@@ -1,3 +1,7 @@
+Version 0.4.0
+-------------
+    * Handle subrepos correctly for go_repo (#191)
+
 Version 0.3.1
 -------------
     * Handle some additional cases for third-party packages

tools/driver/VERSION

@@ -1 +1 @@
-0.3.1
+0.4.0

tools/driver/packages/packages.go

@@ -94,7 +94,7 @@ func Load(req *DriverRequest, files []string) (*DriverResponse, error) {
 	for _, file := range relFiles {
 		dirs[filepath.Dir(file)] = struct{}{}
 	}
-	pkgs, err := loadPackageInfo(relFiles)
+	pkgs, err := loadPackageInfo(relFiles, req.Mode)
 	if err != nil {
 		return nil, err
 	}
@@ -161,10 +161,12 @@ func packagesToResponse(rootpath string, pkgs []*packages.Package, dirs map[stri
 		for i, file := range pkg.GoFiles {
 			// This is pretty awkward; we need to try to figure out where these files exist now,
 			// which isn't particularly clear to the build actions that generated them.
-			if _, err := os.Lstat(file); err == nil { // file exists
-				pkg.GoFiles[i] = filepath.Join(rootpath, file)
+			if path := filepath.Join(rootpath, "plz-out/subrepos", file); pathExists(path) {
+				pkg.GoFiles[i] = path
+			} else if path := filepath.Join(rootpath, "plz-out/gen", file); pathExists(path) {
+				pkg.GoFiles[i] = path
 			} else {
-				pkg.GoFiles[i] = filepath.Join(rootpath, "plz-out/gen", file)
+				pkg.GoFiles[i] = filepath.Join(rootpath, file)
 			}
 		}
 		pkg.CompiledGoFiles = pkg.GoFiles
@@ -195,7 +197,7 @@ func packagesToResponse(rootpath string, pkgs []*packages.Package, dirs map[stri
 // loadPackageInfo loads all the package information by executing Please.
 // A cooler way of handling this in future would be to do this in-process; for that we'd
 // need to define the SDK we keep talking about as a supported programmatic interface.
-func loadPackageInfo(files []string) ([]*packages.Package, error) {
+func loadPackageInfo(files []string, mode packages.LoadMode) ([]*packages.Package, error) {
 	isTerminal := term.IsTerminal(int(os.Stderr.Fd()))
 	plz := func(args ...string) *exec.Cmd {
 		cmd := exec.Command("plz", args...)
@@ -218,7 +220,11 @@ func loadPackageInfo(files []string) ([]*packages.Package, error) {
 	// N.B. deliberate not to close these here, they happen exactly when needed.
 	whatinputs := plz(append([]string{"query", "whatinputs"}, files...)...)
 	whatinputs.Stdout = w1
-	deps := plz("query", "deps", "-", "--hidden", "--include", "go_pkg_info", "--include", "go_src")
+	args := []string{"query", "deps", "-", "--hidden", "-i", "go_pkg_info", "-i", "go_src"}
+	if (mode & packages.NeedExportFile) != 0 {
+		args = append(args, "-i", "go")
+	}
+	deps := plz(args...)
 	deps.Stdin = r1
 	deps.Stdout = w2
 	build := plz("build", "-")
@@ -263,6 +269,7 @@ func loadPackageInfoFiles(paths []string) ([]*packages.Package, error) {
 		if !strings.HasSuffix(file, ".json") {
 			continue // Ignore all the various Go sources etc.
 		}
+		log.Debug("Package file: %s", file)
 		g.Go(func() error {
 			f, err := os.Open(file)
 			if err != nil {
@@ -313,7 +320,7 @@ func loadStdlibPackages() ([]*packages.Package, error) {
 		} else if err != nil {
 			return nil, err
 		}
-		pkgs = append(pkgs, packageinfo.FromBuildPackage(pkg))
+		pkgs = append(pkgs, packageinfo.FromBuildPackage(pkg, "", ""))
 	}
 	return pkgs, nil
 }
@@ -358,3 +365,8 @@ func allGoFilesInDir(dirname string, includeTests bool) []string {
 	}
 	return files
 }
+
+func pathExists(filename string) bool {
+	_, err := os.Lstat(filename)
+	return err == nil
+}

tools/please_go/ChangeLog

@@ -1,3 +1,7 @@
+Version 1.9.0
+-------------
+    * Generate correct subrepo info for package driver (#191)
+
 Version 1.8.5
 --------------
     * in go_repo, add c source to the generated cgo_library (#213)

tools/please_go/VERSION

@@ -1 +1 @@
-1.8.5
+1.9.0

tools/please_go/generate/generate.go

@@ -20,6 +20,7 @@ type Generate struct {
 	moduleName         string
 	moduleArg          string
 	srcRoot            string
+	subrepo            string
 	buildContext       build.Context
 	modFile            string
 	buildFileNames     []string
@@ -31,7 +32,7 @@ type Generate struct {
 	install            []string
 }
 
-func New(srcRoot, thirdPartyFolder, modFile, module, version string, buildFileNames, moduleDeps, install []string, buildTags []string) *Generate {
+func New(srcRoot, thirdPartyFolder, modFile, module, version, subrepo string, buildFileNames, moduleDeps, install []string, buildTags []string) *Generate {
 	moduleArg := module
 	if version != "" {
 		moduleArg += "@" + version
@@ -51,6 +52,7 @@ func New(srcRoot, thirdPartyFolder, modFile, module, version string, buildFileNa
 		install:            install,
 		moduleName:         module,
 		moduleArg:          moduleArg,
+		subrepo:            subrepo,
 	}
 }
 
@@ -443,6 +445,7 @@ func (g *Generate) ruleForPackage(pkg *build.Package, dir string) *Rule {
 		kind:          packageKind(pkg),
 		srcs:          pkg.GoFiles,
 		module:        g.moduleArg,
+		subrepo:       g.subrepo,
 		cgoSrcs:       pkg.CgoFiles,
 		cSrcs:         pkg.CFiles,
 		compilerFlags: pkg.CgoCFLAGS,

tools/please_go/generate/rules.go

@@ -6,6 +6,7 @@ type Rule struct {
 	name          string
 	kind          string
 	module        string
+	subrepo       string
 	srcs          []string
 	cgoSrcs       []string
 	cSrcs         []string
@@ -57,5 +58,6 @@ func populateRule(r *build.Rule, targetState *Rule) {
 	}
 	if !targetState.isCMD {
 		r.SetAttr("_module", NewStringExpr(targetState.module))
+		r.SetAttr("_subrepo", NewStringExpr(targetState.subrepo))
 	}
 }

tools/please_go/packageinfo/packageinfo.go

@@ -10,14 +10,15 @@ import (
 	"io/fs"
 	"os"
 	"path/filepath"
+	"runtime"
 	"sort"
 	"strings"
 
 	"golang.org/x/tools/go/packages"
 )
 
 // WritePackageInfo writes a series of package info files to the given file.
-func WritePackageInfo(importPath string, srcRoot, importconfig string, imports map[string]string, installPkgs []string, w io.Writer) error {
+func WritePackageInfo(importPath string, srcRoot, importconfig string, imports map[string]string, installPkgs []string, subrepo string, w io.Writer) error {
 	// Discover all Go files in the module
 	goFiles := map[string][]string{}
 
@@ -59,13 +60,21 @@ func WritePackageInfo(importPath string, srcRoot, importconfig string, imports m
 	pkgs := make([]*packages.Package, 0, len(goFiles))
 	for dir := range goFiles {
 		pkgDir := strings.TrimPrefix(strings.TrimPrefix(dir, srcRoot), "/")
-		pkg, err := createPackage(filepath.Join(importPath, pkgDir), dir)
+		pkg, err := createPackage(filepath.Join(importPath, pkgDir), dir, subrepo, importPath)
 		if _, ok := err.(*build.NoGoError); ok {
 			continue // Don't really care, this happens sometimes for modules
 		} else if err != nil {
 			return fmt.Errorf("failed to import directory %s: %w", dir, err)
 		}
-		pkg.ExportFile = imports[pkg.PkgPath]
+		if subrepo != "" {
+			_, pkgPath, ok := strings.Cut(imports[pkg.PkgPath], pkg.PkgPath)
+			if !ok {
+				return fmt.Errorf("Cannot determine export file path for package %s from %s", pkg.PkgPath, imports[pkg.PkgPath])
+			}
+			pkg.ExportFile = filepath.Join(subrepo, pkgPath)
+		} else {
+			pkg.ExportFile = imports[pkg.PkgPath]
+		}
 		pkgs = append(pkgs, pkg)
 	}
 	// Ensure output is deterministic
@@ -75,7 +84,7 @@ func WritePackageInfo(importPath string, srcRoot, importconfig string, imports m
 	return serialise(pkgs, w)
 }
 
-func createPackage(pkgPath, pkgDir string) (*packages.Package, error) {
+func createPackage(pkgPath, pkgDir, subrepo, importPath string) (*packages.Package, error) {
 	if pkgDir == "" || pkgDir == "." {
 		// This happens when we're in the repo root, ImportDir refuses to read it for some reason.
 		path, err := filepath.Abs(pkgDir)
@@ -89,7 +98,7 @@ func createPackage(pkgPath, pkgDir string) (*packages.Package, error) {
 		return nil, err
 	}
 	bpkg.ImportPath = pkgPath
-	return FromBuildPackage(bpkg), nil
+	return FromBuildPackage(bpkg, subrepo, importPath), nil
 }
 
 func serialise(pkgs []*packages.Package, w io.Writer) error {
@@ -99,7 +108,7 @@ func serialise(pkgs []*packages.Package, w io.Writer) error {
 }
 
 // FromBuildPackage creates a packages Package from a build Package.
-func FromBuildPackage(pkg *build.Package) *packages.Package {
+func FromBuildPackage(pkg *build.Package, subrepo, importPath string) *packages.Package {
 	p := &packages.Package{
 		ID:            pkg.ImportPath,
 		Name:          pkg.Name,
@@ -110,7 +119,14 @@ func FromBuildPackage(pkg *build.Package) *packages.Package {
 		Imports:       make(map[string]*packages.Package, len(pkg.Imports)),
 	}
 	for i, file := range pkg.GoFiles {
-		p.GoFiles[i] = filepath.Join(pkg.Dir, file)
+		if subrepo != "" {
+			// this is fairly nasty... there must be a better way of getting it without the pkg/ prefix
+			dir := strings.TrimPrefix(pkg.Dir, "pkg/"+runtime.GOOS+"_"+runtime.GOARCH)
+			dir = strings.TrimPrefix(strings.TrimPrefix(dir, "/"), importPath)
+			p.GoFiles[i] = filepath.Join(subrepo, dir, file)
+		} else {
+			p.GoFiles[i] = filepath.Join(pkg.Dir, file)
+		}
 	}
 	p.CompiledGoFiles = p.GoFiles // This seems to be important to e.g. gosec
 	for _, imp := range pkg.Imports {

tools/please_go/please_go.go

@@ -83,6 +83,7 @@ var opts = struct {
 		ImportPath string            `short:"i" long:"import_path" description:"Go import path (e.g. github.com/please-build/go-rules)"`
 		Pkg        string            `long:"pkg" env:"PKG_DIR" description:"Package that we're in within the repo"`
 		ImportMap  map[string]string `short:"m" long:"import_map" description:"Existing map of imports"`
+		Subrepo    string            `short:"s" long:"subrepo" description:"Subrepo root that this package is within"`
 	} `command:"package_info" alias:"p" description:"Creates an info file about a Go package"`
 	ModuleInfo struct {
 		ModulePath   string   `short:"m" long:"module_path" required:"true" description:"Import path of the module in question"`
@@ -99,6 +100,7 @@ var opts = struct {
 		Version          string   `long:"version" description:"The version of the current module"`
 		Install          []string `long:"install" description:"The packages to add to the :install alias"`
 		BuildTags        []string `long:"build_tag" description:"Any build tags to apply to the build"`
+		Subrepo          string   `long:"subrepo" description:"The subrepo root to output into"`
 		Args             struct {
 			Requirements []string `positional-arg-name:"requirements" description:"Any module requirements not included in the go.mod"`
 		} `positional-args:"true"`
@@ -180,7 +182,8 @@ var subCommands = map[string]func() int{
 		return 0
 	},
 	"generate": func() int {
-		g := generate.New(opts.Generate.SrcRoot, opts.Generate.ThirdPartyFolder, opts.Generate.ModFile, opts.Generate.Module, opts.Generate.Version, []string{"BUILD", "BUILD.plz"}, opts.Generate.Args.Requirements, opts.Generate.Install, opts.Generate.BuildTags)
+		gen := opts.Generate
+		g := generate.New(gen.SrcRoot, gen.ThirdPartyFolder, gen.ModFile, gen.Module, gen.Version, gen.Subrepo, []string{"BUILD", "BUILD.plz"}, gen.Args.Requirements, gen.Install, gen.BuildTags)
 		if err := g.Generate(); err != nil {
 			log.Fatalf("failed to generate go rules: %v", err)
 		}
@@ -200,14 +203,14 @@ var subCommands = map[string]func() int{
 	},
 	"package_info": func() int {
 		pi := opts.PackageInfo
-		if err := packageinfo.WritePackageInfo(pi.ImportPath, pi.Pkg, "", pi.ImportMap, nil, os.Stdout); err != nil {
+		if err := packageinfo.WritePackageInfo(pi.ImportPath, pi.Pkg, "", pi.ImportMap, nil, pi.Subrepo, os.Stdout); err != nil {
 			log.Fatalf("failed to write package info: %s", err)
 		}
 		return 0
 	},
 	"module_info": func() int {
 		mi := opts.ModuleInfo
-		if err := packageinfo.WritePackageInfo(mi.ModulePath, mi.Srcs, mi.ImportConfig, nil, mi.Packages, os.Stdout); err != nil {
+		if err := packageinfo.WritePackageInfo(mi.ModulePath, mi.Srcs, mi.ImportConfig, nil, mi.Packages, "", os.Stdout); err != nil {
 			log.Fatalf("failed to write module info: %s", err)
 		}
 		return 0