Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a label to the build rule if a go repo target has a replace directive in go.mod #114

Merged
Merged
36 changes: 35 additions & 1 deletion edit/edit.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package edit

import (
"errors"
"strings"

"github.com/please-build/buildtools/build"
Expand Down Expand Up @@ -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{},
Expand All @@ -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
}

Expand All @@ -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
}
2 changes: 1 addition & 1 deletion generate/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
1 change: 1 addition & 0 deletions sync/integration/syncmod/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@ go_test(
"//please",
"//third_party/go:testify",
"//sync",
"///third_party/go/github.com_please-build_buildtools//build",
],
)
32 changes: 29 additions & 3 deletions sync/integration/syncmod/sync_mod_test.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package syncmod

import (
"github.com/please-build/buildtools/build"

Check failure on line 4 in sync/integration/syncmod/sync_mod_test.go

View workflow job for this annotation

GitHub Actions / lint

File is not `gci`-ed with --skip-generated -s standard -s default -s prefix(github.com/please-build/puku) (gci)
"os"
"path/filepath"
"testing"

Check failure on line 8 in sync/integration/syncmod/sync_mod_test.go

View workflow job for this annotation

GitHub Actions / lint

File is not `gci`-ed with --skip-generated -s standard -s default -s prefix(github.com/please-build/puku) (gci)
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.org/x/mod/modfile"
Expand All @@ -20,6 +21,7 @@
panic(err)
}

// Setup puku and please config
conf, err := config.ReadConfig(".")
require.NoError(t, err)

Expand All @@ -28,13 +30,16 @@
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:
Expand All @@ -52,12 +57,24 @@
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"))
})
}
Expand All @@ -71,6 +88,15 @@
assert.Equal(t, expectedVers[dlRule.AttrString("module")], dlRule.AttrString("version"))
}

func listLabels(rule *build.Rule) []string {
labelsExpr := rule.Attr("labels")
var labels []string

Check failure on line 93 in sync/integration/syncmod/sync_mod_test.go

View workflow job for this annotation

GitHub Actions / lint

Consider pre-allocating `labels` (prealloc)
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 {
Expand Down
2 changes: 2 additions & 0 deletions sync/integration/syncmod/test_repo/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
40 changes: 24 additions & 16 deletions sync/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
licences *licences.Licenses
}

const REPLACE_LABEL = "go_replace_directive"

Check warning on line 25 in sync/sync.go

View workflow job for this annotation

GitHub Actions / lint

var-naming: don't use ALL_CAPS in Go names; use CamelCase (revive)

func newSyncer(plzConf *please.Config, g *graph.Graph) *syncer {
p := proxy.New(proxy.DefaultURL)
l := licences.New(p, g)
Expand Down Expand Up @@ -78,7 +80,7 @@
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
Expand All @@ -100,27 +102,32 @@

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, REPLACE_LABEL)
if err != nil {
return fmt.Errorf("failed to add replace label to %v: %v", req.Mod.Path, err)
SpangleLabs marked this conversation as resolved.
Show resolved Hide resolved
}
}
// No other changes needed
continue
}
}
Expand All @@ -130,14 +137,15 @@
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{REPLACE_LABEL}))
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{REPLACE_LABEL}))
}

return nil
Expand Down
Loading