From 0167fde4103bd8b15775e19816345d5c54d9fb0e Mon Sep 17 00:00:00 2001
From: Michael Matloob <matloob@golang.org>
Date: Thu, 9 Aug 2018 13:48:11 -0400
Subject: [PATCH] go/packages: make corrections to fallback golist processing

This change addresses adonovan's comments in golang.org/cl/125939.
Make sure to include CXXFiles, MFiles, HFiles, FFiles, SwigFiles,
SwigCXXFiles, and SysoFiles from the go list output into the Package
struct's OtherFiles field.
When computing packages from the output of fallback golist,
make sure to create the test variant of a package whenever creating
an x_test for a package because the x_test always depends on
the test variant even when there are no non-x_test files.
Sort dependency packages from the fallback golist to ensure
output is deterministic.

Change-Id: I3a942898c7edbe0ad62dbdf3d521775ffd9b9594
Reviewed-on: https://go-review.googlesource.com/128838
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
---
 go/packages/golist/golist.go          | 13 ++++++++-
 go/packages/golist/golist_fallback.go | 42 ++++++++++++++-------------
 2 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/go/packages/golist/golist.go b/go/packages/golist/golist.go
index a41c049d..f0bbeb4b 100644
--- a/go/packages/golist/golist.go
+++ b/go/packages/golist/golist.go
@@ -129,7 +129,14 @@ type jsonPackage struct {
 	CompiledGoFiles []string
 	CFiles          []string
 	CgoFiles        []string
+	CXXFiles        []string
+	MFiles          []string
+	HFiles          []string
+	FFiles          []string
 	SFiles          []string
+	SwigFiles       []string
+	SwigCXXFiles    []string
+	SysoFiles       []string
 	Imports         []string
 	ImportMap       map[string]string
 	Deps            []string
@@ -141,6 +148,10 @@ type jsonPackage struct {
 	DepOnly         bool
 }
 
+func otherFiles(p *jsonPackage) [][]string {
+	return [][]string{p.CFiles, p.CXXFiles, p.MFiles, p.HFiles, p.FFiles, p.SFiles, p.SwigFiles, p.SwigCXXFiles, p.SysoFiles}
+}
+
 // golistPackages uses the "go list" command to expand the
 // pattern words and return metadata for the specified packages.
 // dir may be "" and env may be nil, as per os/exec.Command.
@@ -249,7 +260,7 @@ func golistPackages(ctx context.Context, cfg *raw.Config, words ...string) ([]st
 			Name:            p.Name,
 			GoFiles:         absJoin(p.Dir, p.GoFiles, p.CgoFiles),
 			CompiledGoFiles: absJoin(p.Dir, p.CompiledGoFiles),
-			OtherFiles:      absJoin(p.Dir, p.SFiles, p.CFiles),
+			OtherFiles:      absJoin(p.Dir, otherFiles(p)...),
 			PkgPath:         pkgpath,
 			Imports:         imports,
 			Export:          export,
diff --git a/go/packages/golist/golist_fallback.go b/go/packages/golist/golist_fallback.go
index 38b943a3..0b59f36d 100644
--- a/go/packages/golist/golist_fallback.go
+++ b/go/packages/golist/golist_fallback.go
@@ -16,6 +16,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"sort"
 	"strings"
 )
 
@@ -103,7 +104,7 @@ func golistPackagesFallback(ctx context.Context, cfg *raw.Config, words ...strin
 			Name:            p.Name,
 			GoFiles:         absJoin(p.Dir, p.GoFiles, p.CgoFiles),
 			CompiledGoFiles: compiledGoFiles,
-			OtherFiles:      absJoin(p.Dir, p.SFiles, p.CFiles),
+			OtherFiles:      absJoin(p.Dir, otherFiles(p)...),
 			PkgPath:         pkgpath,
 			Imports:         importMap(p.Imports),
 			// TODO(matloob): set errors on the Package to cgoErrors
@@ -119,31 +120,31 @@ func golistPackagesFallback(ctx context.Context, cfg *raw.Config, words ...strin
 					Name:            p.Name,
 					GoFiles:         absJoin(p.Dir, p.GoFiles, p.CgoFiles, p.TestGoFiles),
 					CompiledGoFiles: append(compiledGoFiles, absJoin(p.Dir, p.TestGoFiles)...),
-					OtherFiles:      absJoin(p.Dir, p.SFiles, p.CFiles),
+					OtherFiles:      absJoin(p.Dir, otherFiles(p)...),
 					PkgPath:         pkgpath,
 					Imports:         importMap(append(p.Imports, p.TestImports...)),
 					// TODO(matloob): set errors on the Package to cgoErrors
 				})
-			}
-			if len(p.XTestGoFiles) > 0 {
-				xtestID := fmt.Sprintf("%s_test [%s.test]", id, id)
-				if isRoot {
-					roots = append(roots, xtestID)
-				}
-				for i, imp := range p.XTestImports {
-					if imp == p.ImportPath {
-						p.XTestImports[i] = testID
-						break
+				if len(p.XTestGoFiles) > 0 {
+					xtestID := fmt.Sprintf("%s_test [%s.test]", id, id)
+					if isRoot {
+						roots = append(roots, xtestID)
 					}
+					for i, imp := range p.XTestImports {
+						if imp == p.ImportPath {
+							p.XTestImports[i] = testID
+							break
+						}
+					}
+					result = append(result, &raw.Package{
+						ID:              xtestID,
+						Name:            p.Name + "_test",
+						GoFiles:         absJoin(p.Dir, p.XTestGoFiles),
+						CompiledGoFiles: absJoin(p.Dir, p.XTestGoFiles),
+						PkgPath:         pkgpath,
+						Imports:         importMap(p.XTestImports),
+					})
 				}
-				result = append(result, &raw.Package{
-					ID:              xtestID,
-					Name:            p.Name + "_test",
-					GoFiles:         absJoin(p.Dir, p.XTestGoFiles),
-					CompiledGoFiles: absJoin(p.Dir, p.XTestGoFiles),
-					PkgPath:         pkgpath,
-					Imports:         importMap(p.XTestImports),
-				})
 			}
 		}
 	}
@@ -233,6 +234,7 @@ func getDeps(ctx context.Context, cfg *raw.Config, words ...string) (originalSet
 	for dep := range depsSet {
 		deps = append(deps, dep)
 	}
+	sort.Strings(deps) // ensure output is deterministic
 	return originalSet, deps, nil
 }