Add a label to the build rule if a go repo target has a replace direc…

…tive in go.mod (#114)

We're thinking it would be useful to have a label on the build rule if a go module is pinned with a replace directive.
This way we can see which versions are pinned when checking through which need updating and such

edit/edit.go

@@ -1,6 +1,7 @@
 package edit
 
 import (
+	"errors"
 	"strings"
 
 	"github.com/please-build/buildtools/build"
@@ -117,7 +118,7 @@ func BoolAttr(rule *build.Rule, attrName string) bool {
 	return ident.Name == "True"
 }
 
-func NewGoRepoRule(mod, version, download string, licences []string) *build.CallExpr {
+func NewGoRepoRule(mod, version, download string, licences []string, labels []string) *build.CallExpr {
 	rule := build.NewRule(&build.CallExpr{
 		X:    &build.Ident{Name: "go_repo"},
 		List: []build.Expr{},
@@ -132,6 +133,9 @@ func NewGoRepoRule(mod, version, download string, licences []string) *build.Call
 	if len(licences) != 0 {
 		rule.SetAttr("licences", NewStringList(licences))
 	}
+	if len(labels) != 0 {
+		rule.SetAttr("labels", NewStringList(labels))
+	}
 	return rule.Call
 }
 
@@ -145,3 +149,33 @@ func NewModDownloadRule(mod, version string, licences []string) (*build.CallExpr
 	}
 	return rule.Call, rule.Name()
 }
+
+// AddLabel adds a specified string label to a build Rule's labels, unless it already exists
+func AddLabel(rule *build.Rule, label string) error {
+	// Fetch the labels attribute, or initialise it
+	ruleLabels := rule.Attr("labels")
+	if ruleLabels == nil {
+		ruleLabels = &build.ListExpr{}
+	}
+	// Check it's a list of expressions
+	ruleLabelsList, ok := ruleLabels.(*build.ListExpr)
+	if !ok {
+		return errors.New("rule already has a `labels` attribute, and it is not a list")
+	}
+	// Check for already-matching label
+	for _, labelExpr := range ruleLabelsList.List {
+		// Ignore any non-string labels
+		labelStringExpr, ok := labelExpr.(*build.StringExpr)
+		if !ok {
+			continue
+		}
+		// If a matching label already exists, no need to do anything
+		if labelStringExpr.Value == label {
+			return nil
+		}
+	}
+	// Add the new label
+	ruleLabelsList.List = append(ruleLabelsList.List, NewStringExpr(label))
+	rule.SetAttr("labels", ruleLabelsList)
+	return nil
+}

generate/generate.go

@@ -236,7 +236,7 @@ func (u *updater) addNewModules(conf *config.Config) error {
 		if err != nil {
 			return fmt.Errorf("failed to get license for mod %v: %v", mod.Module, err)
 		}
-		file.Stmt = append(file.Stmt, edit.NewGoRepoRule(mod.Module, mod.Version, "", ls))
+		file.Stmt = append(file.Stmt, edit.NewGoRepoRule(mod.Module, mod.Version, "", ls, []string{}))
 	}
 	return nil
 }

logging/BUILD

@@ -6,6 +6,7 @@ go_library(
         "//cmd/puku:all",
         "//generate:all",
         "//graph:all",
+        "//sync:all",
         "//watch:all",
     ],
     deps = [

sync/BUILD

@@ -6,6 +6,7 @@ go_library(
         "//edit",
         "//graph",
         "//licences",
+        "//logging",
         "//please",
         "//proxy",
         "///third_party/go/github.com_please-build_buildtools//build",

sync/integration/syncmod/BUILD

@@ -12,5 +12,6 @@ go_test(
         "//please",
         "//third_party/go:testify",
         "//sync",
+        "///third_party/go/github.com_please-build_buildtools//build",
     ],
 )

sync/integration/syncmod/sync_mod_test.go

@@ -5,6 +5,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/please-build/buildtools/build"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/mod/modfile"
@@ -20,6 +21,7 @@ func TestModSync(t *testing.T) {
 		panic(err)
 	}
 
+	// Setup puku and please config
 	conf, err := config.ReadConfig(".")
 	require.NoError(t, err)
 
@@ -28,13 +30,16 @@ func TestModSync(t *testing.T) {
 	plzConf, err := please.QueryConfig(conf.GetPlzPath())
 	require.NoError(t, err)
 
+	// Parse the puku graph of test repo build files
 	g := graph.New(plzConf.BuildFileNames())
 	err = sync.SyncToStdout("text", plzConf, g)
 	require.NoError(t, err)
 
+	// Fetch the generated third_party/go build file
 	thirdPartyBuildFile, err := g.LoadFile(conf.GetThirdPartyDir())
 	require.NoError(t, err)
 
+	// Read version info from the go.mod file
 	expectedVers := readModFileVersions()
 
 	// We expect to generate the following for the replace in the go.mod:
@@ -52,12 +57,24 @@ func TestModSync(t *testing.T) {
 	for _, repoRule := range thirdPartyBuildFile.Rules("go_repo") {
 		t.Run(repoRule.AttrString("module"), func(t *testing.T) {
 			// Check that we've replaced build tools
-			if repoRule.AttrString("version") == "" {
-				assert.Equal(t, "github.com/bazelbuild/buildtools", repoRule.AttrString("module"))
+			if repoRule.AttrString("module") == "github.com/bazelbuild/buildtools" {
+				// Assert there's no version set
+				assert.Equal(t, "", repoRule.AttrString("version"))
+				// Ensure the download attribute is set
 				assert.Equal(t, ":github.com_peterebden_buildtools_dl", repoRule.AttrString("download"))
+				// Check that a label has been added
+				labels := listLabels(repoRule)
+				assert.Contains(t, labels, "go_replace_directive")
 				return
 			}
-			// All rules start off at v0.0.1 and should be updated to v1.0.0 as per the go.mod
+
+			// Check that testify is labelled for a replace directive
+			if repoRule.AttrString("module") == "github.com/stretchr/testify" {
+				labels := listLabels(repoRule)
+				assert.Contains(t, labels, "go_replace_directive")
+			}
+
+			// All rules start off at v0.0.1 and should be updated to their version listed in the go.mod
 			assert.Equal(t, expectedVers[repoRule.AttrString("module")], repoRule.AttrString("version"))
 		})
 	}
@@ -71,6 +88,15 @@ func TestModSync(t *testing.T) {
 	assert.Equal(t, expectedVers[dlRule.AttrString("module")], dlRule.AttrString("version"))
 }
 
+func listLabels(rule *build.Rule) []string {
+	labelsExpr := rule.Attr("labels")
+	labels := make([]string, 0, len(labelsExpr.(*build.ListExpr).List))
+	for _, labelExpr := range labelsExpr.(*build.ListExpr).List {
+		labels = append(labels, labelExpr.(*build.StringExpr).Value)
+	}
+	return labels
+}
+
 func readModFileVersions() map[string]string {
 	f, err := os.ReadFile("go.mod")
 	if err != nil {

sync/integration/syncmod/test_repo/go.mod

@@ -12,3 +12,5 @@ require (
 )
 
 replace github.com/bazelbuild/buildtools => github.com/peterebden/buildtools v1.6.0
+
+replace github.com/stretchr/testify => github.com/stretchr/testify v1.3.0

sync/sync.go

@@ -12,16 +12,21 @@ import (
 	"github.com/please-build/puku/edit"
 	"github.com/please-build/puku/graph"
 	"github.com/please-build/puku/licences"
+	"github.com/please-build/puku/logging"
 	"github.com/please-build/puku/please"
 	"github.com/please-build/puku/proxy"
 )
 
+var log = logging.GetLogger()
+
 type syncer struct {
 	plzConf  *please.Config
 	graph    *graph.Graph
 	licences *licences.Licenses
 }
 
+const ReplaceLabel = "go_replace_directive"
+
 func newSyncer(plzConf *please.Config, g *graph.Graph) *syncer {
 	p := proxy.New(proxy.DefaultURL)
 	l := licences.New(p, g)
@@ -78,7 +83,7 @@ func (s *syncer) sync() error {
 	return nil
 }
 
-func (s *syncer) syncModFile(conf *config.Config, file *build.File, exitingRules map[string]*build.Rule) error {
+func (s *syncer) syncModFile(conf *config.Config, file *build.File, existingRules map[string]*build.Rule) error {
 	outs, err := please.Build(conf.GetPlzPath(), s.plzConf.ModFile())
 	if err != nil {
 		return err
@@ -100,27 +105,32 @@ func (s *syncer) syncModFile(conf *config.Config, file *build.File, exitingRules
 
 	for _, req := range f.Require {
 		reqVersion := req.Mod.Version
-		var replace *modfile.Replace
+		var matchingReplace *modfile.Replace
 		for _, r := range f.Replace {
 			if r.Old.Path == req.Mod.Path {
+				matchingReplace = r
 				reqVersion = r.New.Version
-				if r.New.Path == req.Mod.Path { // we are just replacing version so don't need a replace
-					continue
-				}
-				replace = r
 			}
 		}
 
 		// Existing rule will point to the go_mod_download with the version on it so we should use the original path
-		r, ok := exitingRules[req.Mod.Path]
+		rule, ok := existingRules[req.Mod.Path]
 		if ok {
-			if replace != nil && r.Kind() == "go_repo" {
-				// Looks like we've added in a replace for this module so we need to delete the old go_repo rule
-				// and regen with a go_mod_download and a go_repo.
-				edit.RemoveTarget(file, r)
+			if matchingReplace != nil && matchingReplace.New.Path != req.Mod.Path && rule.Kind() == "go_repo" {
+				// Looks like we've added in a replace directive for this module which changes the path, so we need to
+				// delete the old go_repo rule and regenerate it with a go_mod_download and a go_repo.
+				edit.RemoveTarget(file, rule)
 			} else {
 				// Make sure the version is up-to-date
-				r.SetAttr("version", edit.NewStringExpr(reqVersion))
+				rule.SetAttr("version", edit.NewStringExpr(reqVersion))
+				// Add label for the replace directive
+				if matchingReplace != nil {
+					err := edit.AddLabel(rule, ReplaceLabel)
+					if err != nil {
+						log.Warningf("Failed to add replace label to %v: %v", req.Mod.Path, err)
+					}
+				}
+				// No other changes needed
 				continue
 			}
 		}
@@ -130,14 +140,15 @@ func (s *syncer) syncModFile(conf *config.Config, file *build.File, exitingRules
 			return fmt.Errorf("failed to get licences for %v: %v", req.Mod.Path, err)
 		}
 
-		if replace == nil {
-			file.Stmt = append(file.Stmt, edit.NewGoRepoRule(req.Mod.Path, reqVersion, "", ls))
+		// If no replace directive, or replace directive is just replacing the version, add a simple rule
+		if matchingReplace == nil || matchingReplace.New.Path == req.Mod.Path {
+			file.Stmt = append(file.Stmt, edit.NewGoRepoRule(req.Mod.Path, reqVersion, "", ls, []string{ReplaceLabel}))
 			continue
 		}
 
-		dl, dlName := edit.NewModDownloadRule(replace.New.Path, replace.New.Version, ls)
+		dl, dlName := edit.NewModDownloadRule(matchingReplace.New.Path, matchingReplace.New.Version, ls)
 		file.Stmt = append(file.Stmt, dl)
-		file.Stmt = append(file.Stmt, edit.NewGoRepoRule(req.Mod.Path, "", dlName, nil))
+		file.Stmt = append(file.Stmt, edit.NewGoRepoRule(req.Mod.Path, "", dlName, nil, []string{ReplaceLabel}))
 	}
 
 	return nil