From 9651a1e2f87474b80ed0a3c7b480755c58daa65c Mon Sep 17 00:00:00 2001
From: Peter Ebden
Date: Sat, 17 Aug 2024 14:37:03 +0100
Subject: [PATCH 1/9] Start converting over
---
go.mod | 1 +
go.sum | 2 ++
src/core/build_target.go | 15 ++-------------
src/core/config.go | 1 +
src/parse/asp/targets.go | 24 +++++++++++++++++++++++-
third_party/go/BUILD | 7 +++++++
6 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/go.mod b/go.mod
index 50c5c30f7..1de07cc4c 100644
--- a/go.mod
+++ b/go.mod
@@ -66,6 +66,7 @@ require (
github.com/beorn7/perks v1.0.1 // indirect
github.com/chzyer/readline v1.5.1 // indirect
github.com/felixge/httpsnoop v1.0.4 // indirect
+ github.com/github/go-spdx/v2 v2.3.1 // indirect
github.com/go-logr/logr v1.4.1 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
github.com/go-ole/go-ole v1.3.0 // indirect
diff --git a/go.sum b/go.sum
index 4c9adffbf..2a9f552fe 100644
--- a/go.sum
+++ b/go.sum
@@ -61,6 +61,8 @@ github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2
github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U=
github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA=
github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM=
+github.com/github/go-spdx/v2 v2.3.1 h1:ffGuHTbHuHzWPt53n8f9o8clGutuLPObo3zB4JAjxU8=
+github.com/github/go-spdx/v2 v2.3.1/go.mod h1:2ZxKsOhvBp+OYBDlsGnUMcchLeo2mrpEBn2L1C+U3IQ=
github.com/go-kit/log v0.1.0/go.mod h1:zbhenjAZHb184qTLMA9ZjW7ThYL0H2mk7Q6pNt4vbaY=
github.com/go-logfmt/logfmt v0.5.0/go.mod h1:wCYkCAKZfumFQihp8CzCvQ3paCTfi41vtzG1KdI/P7A=
github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
diff --git a/src/core/build_target.go b/src/core/build_target.go
index 2b9ea38d3..82cbd1339 100644
--- a/src/core/build_target.go
+++ b/src/core/build_target.go
@@ -152,8 +152,8 @@ type BuildTarget struct {
// Acceptable hashes of the outputs of this rule. If the output doesn't match any of these
// it's an error at build time. Can be used to validate third-party deps.
Hashes []string
- // Licences that this target is subject to.
- Licences []string
+ // SPDX licence expression that this target is subject to.
+ Licence string
// Any secrets that this rule requires.
// Secrets are similar to sources but are always absolute system paths and affect the hash
// differently; they are not used to determine the hash for retrieving a file from cache, but
@@ -1764,17 +1764,6 @@ func (target *BuildTarget) insert(sl []string, s string) []string {
return append(sl, s)
}
-// AddLicence adds a licence to the target if it's not already there.
-func (target *BuildTarget) AddLicence(licence string) {
- licence = strings.TrimSpace(licence)
- for _, l := range target.Licences {
- if l == licence {
- return
- }
- }
- target.Licences = append(target.Licences, licence)
-}
-
// AddHash adds a new acceptable hash to the target.
func (target *BuildTarget) AddHash(hash string) {
target.Hashes = append(target.Hashes, hash)
diff --git a/src/core/config.go b/src/core/config.go
index 526f70388..864fb73a4 100644
--- a/src/core/config.go
+++ b/src/core/config.go
@@ -689,6 +689,7 @@ type Configuration struct {
buildEnvStored *storedBuildEnv
FeatureFlags struct {
+ SPDXLicencesOnly bool `help:"Licences on build targets can only be strings containing SPDX expressions, not lists."`
} `help:"Flags controlling preview features for the next release. Typically these config options gate breaking changes and only have a lifetime of one major release."`
Metrics struct {
PrometheusGatewayURL string `help:"The gateway URL to push prometheus updates to."`
diff --git a/src/parse/asp/targets.go b/src/parse/asp/targets.go
index d916a1a13..f881d20a9 100644
--- a/src/parse/asp/targets.go
+++ b/src/parse/asp/targets.go
@@ -273,8 +273,30 @@ func populateTarget(s *scope, t *core.BuildTarget, args []pyObject) {
addDependencies(s, "internal_deps", args[internalDepsBuildRuleArgIdx], t, false, true)
addStrings(s, "labels", args[labelsBuildRuleArgIdx], t.AddLabel)
addStrings(s, "hashes", args[hashesBuildRuleArgIdx], t.AddHash)
- addStrings(s, "licences", args[licencesBuildRuleArgIdx], t.AddLicence)
addStrings(s, "requires", args[requiresBuildRuleArgIdx], t.AddRequire)
+ if expr, ok := args[licencesBuildRuleArgIdx].(pyString); ok {
+ target.Licence = string(expr)
+ } else {
+ // TODO(v18): Remove all this once strings are the only option.
+ s.Assert(!s.state.config.FeatureFlags.SPDXLicencesOnly, "The licences argument must be a string")
+ l, ok := asList(args[licencesBuildRuleArgIdx])
+ s.Assert(ok, "The licences argument must be a string or list (was %s)", l.Type())
+ if len(l) == 1 {
+ // minor optimisation: avoid allocating a slice etc for the overwhelmingly common case of a single licence
+ str, ok := l[0].(pyString)
+ s.Assert(ok, "Items in the licences argument must be strings (was %s)", s.Type())
+ target.Licence = string(str)
+ } else {
+ l2 := make([]string, len(l))
+ for i, x := range l {
+ str, ok := x.(pyString)
+ s.Assert(ok, "Items in the licences argument must be strings (was %s)", s.Type())
+ l2[i] = string(str)
+ }
+ // Passing a list to Please is implicitly a series of alternative licences
+ target.Licence = strings.Join(l2, " OR ")
+ }
+ }
if vis, ok := asList(args[visibilityBuildRuleArgIdx]); ok && len(vis) != 0 {
if v, ok := vis[0].(pyString); ok && v == "PUBLIC" {
t.Visibility = core.WholeGraph
diff --git a/third_party/go/BUILD b/third_party/go/BUILD
index ce8633671..7493ef482 100644
--- a/third_party/go/BUILD
+++ b/third_party/go/BUILD
@@ -686,3 +686,10 @@ go_repo(
module = "github.com/grpc-ecosystem/go-grpc-prometheus",
version = "v1.2.0",
)
+
+go_repo(
+ licences = ["MIT"],
+ module = "github.com/github/go-spdx",
+ version = "v2.3.1",
+)
+
From f6c9e85bc14d69e997910a57429caeb3155dc009 Mon Sep 17 00:00:00 2001
From: Peter Ebden
Date: Sat, 17 Aug 2024 15:01:37 +0100
Subject: [PATCH 2/9] Make it all work
---
go.mod | 1 +
go.sum | 6 +++++
src/build/incrementality.go | 5 +---
src/build/incrementality_test.go | 2 +-
src/core/BUILD | 1 +
src/core/build_target.go | 26 +++++++--------------
src/core/build_target_test.go | 11 ++++++---
src/core/stamp.go | 11 ++++++---
src/core/stamp_test.go | 12 ++++++----
src/parse/asp/builtins.go | 7 ++++--
src/parse/asp/targets.go | 40 +++++++++++++++++---------------
third_party/go/BUILD | 9 +++++--
12 files changed, 75 insertions(+), 56 deletions(-)
diff --git a/go.mod b/go.mod
index 1de07cc4c..7f5f20583 100644
--- a/go.mod
+++ b/go.mod
@@ -86,6 +86,7 @@ require (
github.com/mostynb/zstdpool-syncpool v0.0.13 // indirect
github.com/opencontainers/go-digest v1.0.0 // indirect
github.com/pborman/uuid v1.2.1 // indirect
+ github.com/peterebden/go-spdx/v2 v2.4.3 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55 // indirect
diff --git a/go.sum b/go.sum
index 2a9f552fe..adca89360 100644
--- a/go.sum
+++ b/go.sum
@@ -189,6 +189,12 @@ github.com/peterebden/go-cli-init/v5 v5.2.1 h1:o+7EjS/PiYDvFUQRQVXJRjinmUzDjqcea
github.com/peterebden/go-cli-init/v5 v5.2.1/go.mod h1:0eBDoCJjj3BWyEtidFcP0TlD14cRtOtLCrTG/OVPB74=
github.com/peterebden/go-deferred-regex v1.1.0 h1:XNpUuRDU7iU59Toy+OJp9LUvsun5i3kEbe3c73oyCZg=
github.com/peterebden/go-deferred-regex v1.1.0/go.mod h1:EhIu4zsN+60671cx29rzxPSIEnDd2vOia4RFSOaYpRI=
+github.com/peterebden/go-spdx/v2 v2.4.1 h1:ah4SjMgKyFXqR5qkBGwKbtevTV3UK3+17OiPNw18GVM=
+github.com/peterebden/go-spdx/v2 v2.4.1/go.mod h1:6KrhEd9kpULcLYV6rGkV47Rolvt2IFcaRCyewNnLkLA=
+github.com/peterebden/go-spdx/v2 v2.4.2 h1:u+lPJXPPK1cJpCtDQBlUgu/cXgOg/dYUWKHUyHxakYI=
+github.com/peterebden/go-spdx/v2 v2.4.2/go.mod h1:6KrhEd9kpULcLYV6rGkV47Rolvt2IFcaRCyewNnLkLA=
+github.com/peterebden/go-spdx/v2 v2.4.3 h1:iiOEYy8+qvyTA3IBqVgLGpwTl5MAmU0Ej39w+8xr5gQ=
+github.com/peterebden/go-spdx/v2 v2.4.3/go.mod h1:6KrhEd9kpULcLYV6rGkV47Rolvt2IFcaRCyewNnLkLA=
github.com/peterebden/go-sri v1.1.1 h1:KK8yZ5/NX8YzWUY9QvhrP220QsvEKANLLAgvw35AkyU=
github.com/peterebden/go-sri v1.1.1/go.mod h1:KIRxtog35NfDWec5LV/iBqqfOEPcMpePZLc7EPE6goQ=
github.com/peterebden/tools v0.0.0-20190805132753-b2a0db951d2a h1:R4xz7BkSIQOS5CFmaadk2gwwOzy/u2Jvnimf1NHD2LY=
diff --git a/src/build/incrementality.go b/src/build/incrementality.go
index d9dc38dba..a88ac2c08 100644
--- a/src/build/incrementality.go
+++ b/src/build/incrementality.go
@@ -172,10 +172,7 @@ func ruleHash(state *core.BuildState, target *core.BuildTarget, runtime bool) []
h.Write([]byte(out))
}
}
- for _, licence := range target.Licences {
- h.Write([]byte(licence))
- }
-
+ h.Write([]byte(target.Licence))
for _, output := range target.OptionalOutputs {
h.Write([]byte(output))
}
diff --git a/src/build/incrementality_test.go b/src/build/incrementality_test.go
index dfb35d8ac..b9553b82e 100644
--- a/src/build/incrementality_test.go
+++ b/src/build/incrementality_test.go
@@ -41,7 +41,7 @@ var KnownFields = map[string]bool{
"PostBuildHash": true,
"outputs": true,
"namedOutputs": true,
- "Licences": true,
+ "Licence": true,
"Sandbox": true,
"Tools": true,
"namedTools": true,
diff --git a/src/core/BUILD b/src/core/BUILD
index 40a3bce6b..2e2487779 100644
--- a/src/core/BUILD
+++ b/src/core/BUILD
@@ -13,6 +13,7 @@ go_library(
"///third_party/go/github.com_coreos_go-semver//semver",
"///third_party/go/github.com_google_shlex//:shlex",
"///third_party/go/github.com_peterebden_go-deferred-regex//:go-deferred-regex",
+ "///third_party/go/github.com_peterebden_go-spdx_v2//spdxexp",
"///third_party/go/github.com_pkg_xattr//:xattr",
"///third_party/go/github.com_please-build_gcfg//:gcfg",
"///third_party/go/github.com_please-build_gcfg//types",
diff --git a/src/core/build_target.go b/src/core/build_target.go
index 82cbd1339..eda709bfe 100644
--- a/src/core/build_target.go
+++ b/src/core/build_target.go
@@ -11,6 +11,7 @@ import (
"sync/atomic"
"time"
+ "github.com/peterebden/go-spdx/v2/spdxexp"
"golang.org/x/sync/errgroup"
"github.com/thought-machine/please/src/fs"
@@ -1887,27 +1888,18 @@ func (target *BuildTarget) PackageDir() string {
}
// CheckLicences checks the target's licences against the accepted/rejected list.
-// It returns the licence that was accepted and an error if it did not match.
+// It returns the licence expression that was accepted and an error if it did not match.
func (target *BuildTarget) CheckLicences(config *Configuration) (string, error) {
- if len(target.Licences) == 0 {
+ if target.Licence == "" || len(config.Licences.Accept) == 0 {
return "", nil
}
- for _, licence := range target.Licences {
- for _, reject := range config.Licences.Reject {
- if strings.EqualFold(reject, licence) {
- return "", fmt.Errorf("Target %s is licensed %s, which is explicitly rejected for this repository", target.Label, licence)
- }
- }
- for _, accept := range config.Licences.Accept {
- if strings.EqualFold(accept, licence) {
- return licence, nil // Note licences are assumed to be an 'or', ie. any one of them can be accepted.
- }
- }
- }
- if len(config.Licences.Accept) > 0 {
- return "", fmt.Errorf("None of the licences for %s are accepted in this repository: %s", target.Label, strings.Join(target.Licences, ", "))
+ accepted, err := spdxexp.ExpressionSatisfies(target.Licence, config.Licences.Accept)
+ if err != nil {
+ return "", fmt.Errorf("Target %s has invalid licence %s: %s", target, target.Licence, err)
+ } else if accepted == "" {
+ return "", fmt.Errorf("The licences for %s are not accepted in this repository: %s", target, target.Licence)
}
- return "", nil
+ return accepted, nil
}
// BuildTargets makes a slice of build targets sortable by their labels.
diff --git a/src/core/build_target_test.go b/src/core/build_target_test.go
index d9a6cea38..97a672eb2 100644
--- a/src/core/build_target_test.go
+++ b/src/core/build_target_test.go
@@ -954,19 +954,24 @@ func TestIsTool(t *testing.T) {
func TestCheckLicences(t *testing.T) {
config := DefaultConfiguration()
- config.Licences.Accept = []string{"BSD"}
+ config.Licences.Accept = []string{"BSD", "Apache-2.0"}
config.Licences.Reject = []string{"GPL"}
target := makeTarget1("//src/core/test_data/project", "PUBLIC")
- target.Licences = []string{"BSD", "GPL"}
+ target.Licence = "BSD OR GPL"
accepted, err := target.CheckLicences(config)
assert.NoError(t, err)
assert.Equal(t, "BSD", accepted)
- target.Licences = []string{"MIT", "GPL"}
+ target.Licence = "MIT OR GPL"
accepted, err = target.CheckLicences(config)
assert.Error(t, err)
assert.Equal(t, "", accepted)
+
+ target.Licence = "BSD AND Apache-2.0 OR GPL"
+ accepted, err = target.CheckLicences(config)
+ assert.NoError(t, err)
+ assert.Equal(t, "BSD AND Apache-2.0", accepted)
}
func makeTarget1(label, visibility string, deps ...*BuildTarget) *BuildTarget {
diff --git a/src/core/stamp.go b/src/core/stamp.go
index e1e71e4b5..dd2ae667f 100644
--- a/src/core/stamp.go
+++ b/src/core/stamp.go
@@ -22,11 +22,15 @@ func StampFile(config *Configuration, target *BuildTarget) []byte {
func populateStampInfo(config *Configuration, target *BuildTarget, info *stampInfo) {
accepted, _ := target.CheckLicences(config)
- info.Targets[target.Label] = targetInfo{
- Licences: target.Licences,
+ ti := targetInfo{
+ Licence: target.Licence,
AcceptedLicence: accepted,
Labels: target.Labels,
}
+ if target.Licence != "" {
+ ti.Licences = []string{target.Licence}
+ }
+ info.Targets[target.Label] = ti
for _, dep := range target.Dependencies() {
if _, present := info.Targets[dep.Label]; !present {
populateStampInfo(config, dep, info)
@@ -40,6 +44,7 @@ type stampInfo struct {
type targetInfo struct {
Labels []string `json:"labels,omitempty"`
- Licences []string `json:"licences,omitempty"`
+ Licence string `json:"licence,omitempty"`
+ Licences []string `json:"licences,omitempty"` // Deprecated in favour of Licence
AcceptedLicence string `json:"accepted_licence,omitempty"`
}
diff --git a/src/core/stamp_test.go b/src/core/stamp_test.go
index 1747b9230..d3e8d71e7 100644
--- a/src/core/stamp_test.go
+++ b/src/core/stamp_test.go
@@ -14,14 +14,14 @@ func TestStampFile(t *testing.T) {
t3 := NewBuildTarget(ParseBuildLabel("//third_party/go:errors", ""))
t1.AddLabel("go")
t3.AddLabel("go_get:github.com/pkg/errors")
- t3.AddLicence("bsd-2-clause")
+ t3.Licence = "bsd-2-clause"
t1.AddDependency(t2.Label)
t1.resolveDependency(t2.Label, t2)
t1.AddDependency(t3.Label)
t1.resolveDependency(t3.Label, t3)
t2.AddDependency(t3.Label)
t2.resolveDependency(t3.Label, t3)
- expected := []byte(`{
+ const expected = `{
"targets": {
"//src/core:core": {
"labels": [
@@ -33,12 +33,14 @@ func TestStampFile(t *testing.T) {
"labels": [
"go_get:github.com/pkg/errors"
],
+ "licence": "bsd-2-clause",
"licences": [
"bsd-2-clause"
],
- "accepted_licence": "bsd-2-clause"
+ "accepted_licence": "BSD-2-Clause"
}
}
-}`)
- assert.Equal(t, expected, StampFile(config, t1))
+}`
+
+ assert.Equal(t, expected, string(StampFile(config, t1)))
}
diff --git a/src/parse/asp/builtins.go b/src/parse/asp/builtins.go
index 276957a39..71a3fde24 100644
--- a/src/parse/asp/builtins.go
+++ b/src/parse/asp/builtins.go
@@ -1315,13 +1315,16 @@ func getEntryPoints(s *scope, args []pyObject) pyObject {
// addLicence adds a licence to a target.
func addLicence(s *scope, args []pyObject) pyObject {
target := getTargetPost(s, string(args[0].(pyString)))
- target.AddLicence(string(args[1].(pyString)))
+ if target.Licence != "" {
+ target.Licence += " OR "
+ }
+ target.Licence += string(args[1].(pyString))
return None
}
// getLicences returns the licences for a single target.
func getLicences(s *scope, args []pyObject) pyObject {
- return fromStringList(getTargetPost(s, string(args[0].(pyString))).Licences)
+ return pyList{pyString(getTargetPost(s, string(args[0].(pyString))).Licence)}
}
// getCommand gets the command of a target, optionally for a configuration.
diff --git a/src/parse/asp/targets.go b/src/parse/asp/targets.go
index f881d20a9..6287364b0 100644
--- a/src/parse/asp/targets.go
+++ b/src/parse/asp/targets.go
@@ -274,27 +274,29 @@ func populateTarget(s *scope, t *core.BuildTarget, args []pyObject) {
addStrings(s, "labels", args[labelsBuildRuleArgIdx], t.AddLabel)
addStrings(s, "hashes", args[hashesBuildRuleArgIdx], t.AddHash)
addStrings(s, "requires", args[requiresBuildRuleArgIdx], t.AddRequire)
- if expr, ok := args[licencesBuildRuleArgIdx].(pyString); ok {
- target.Licence = string(expr)
- } else {
- // TODO(v18): Remove all this once strings are the only option.
- s.Assert(!s.state.config.FeatureFlags.SPDXLicencesOnly, "The licences argument must be a string")
- l, ok := asList(args[licencesBuildRuleArgIdx])
- s.Assert(ok, "The licences argument must be a string or list (was %s)", l.Type())
- if len(l) == 1 {
- // minor optimisation: avoid allocating a slice etc for the overwhelmingly common case of a single licence
- str, ok := l[0].(pyString)
- s.Assert(ok, "Items in the licences argument must be strings (was %s)", s.Type())
- target.Licence = string(str)
+ if arg := args[licencesBuildRuleArgIdx]; arg != None {
+ if expr, ok := arg.(pyString); ok {
+ t.Licence = string(expr)
} else {
- l2 := make([]string, len(l))
- for i, x := range l {
- str, ok := x.(pyString)
- s.Assert(ok, "Items in the licences argument must be strings (was %s)", s.Type())
- l2[i] = string(str)
+ // TODO(v18): Remove all this once strings are the only option.
+ s.Assert(!s.state.Config.FeatureFlags.SPDXLicencesOnly, "The licences argument must be a string")
+ l, ok := asList(arg)
+ s.Assert(ok, "The licences argument must be a string or list (was %s)", arg.Type())
+ if len(l) == 1 {
+ // minor optimisation: avoid allocating a slice etc for the overwhelmingly common case of a single licence
+ str, ok := l[0].(pyString)
+ s.Assert(ok, "Items in the licences argument must be strings (was %s)", str.Type())
+ t.Licence = string(str)
+ } else {
+ l2 := make([]string, len(l))
+ for i, x := range l {
+ str, ok := x.(pyString)
+ s.Assert(ok, "Items in the licences argument must be strings (was %s)", str.Type())
+ l2[i] = string(str)
+ }
+ // Passing a list to Please is implicitly a series of alternative licences
+ t.Licence = strings.Join(l2, " OR ")
}
- // Passing a list to Please is implicitly a series of alternative licences
- target.Licence = strings.Join(l2, " OR ")
}
}
if vis, ok := asList(args[visibilityBuildRuleArgIdx]); ok && len(vis) != 0 {
diff --git a/third_party/go/BUILD b/third_party/go/BUILD
index 7493ef482..9e63b34db 100644
--- a/third_party/go/BUILD
+++ b/third_party/go/BUILD
@@ -689,7 +689,12 @@ go_repo(
go_repo(
licences = ["MIT"],
- module = "github.com/github/go-spdx",
- version = "v2.3.1",
+ module = "github.com/peterebden/go-spdx/v2",
+ version = "v2.4.3",
)
+go_repo(
+ licences = ["MIT"],
+ module = "github.com/github/go-spdx/v2",
+ version = "v2.3.1",
+)
From 1d75ff462ad84f2b8005a20635d249c27c0fd4ca Mon Sep 17 00:00:00 2001
From: Peter Ebden
Date: Sat, 17 Aug 2024 15:44:22 +0100
Subject: [PATCH 3/9] Lots of string replacements
---
src/core/build_target.go | 4 ++--
src/parse/asp/builtins.go | 2 +-
src/parse/asp/targets.go | 7 ++++---
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/core/build_target.go b/src/core/build_target.go
index eda709bfe..4fb3106c2 100644
--- a/src/core/build_target.go
+++ b/src/core/build_target.go
@@ -1890,12 +1890,12 @@ func (target *BuildTarget) PackageDir() string {
// CheckLicences checks the target's licences against the accepted/rejected list.
// It returns the licence expression that was accepted and an error if it did not match.
func (target *BuildTarget) CheckLicences(config *Configuration) (string, error) {
- if target.Licence == "" || len(config.Licences.Accept) == 0 {
+ if target.Licence == "" || (len(config.Licences.Accept) == 0 && len(config.Licences.Reject) == 0) {
return "", nil
}
accepted, err := spdxexp.ExpressionSatisfies(target.Licence, config.Licences.Accept)
if err != nil {
- return "", fmt.Errorf("Target %s has invalid licence %s: %s", target, target.Licence, err)
+ return "", fmt.Errorf("Target %s has invalid licence '%s': %s", target, target.Licence, err)
} else if accepted == "" {
return "", fmt.Errorf("The licences for %s are not accepted in this repository: %s", target, target.Licence)
}
diff --git a/src/parse/asp/builtins.go b/src/parse/asp/builtins.go
index 71a3fde24..8c6a839a6 100644
--- a/src/parse/asp/builtins.go
+++ b/src/parse/asp/builtins.go
@@ -1318,7 +1318,7 @@ func addLicence(s *scope, args []pyObject) pyObject {
if target.Licence != "" {
target.Licence += " OR "
}
- target.Licence += string(args[1].(pyString))
+ target.Licence += strings.TrimSpace(strings.ReplaceAll(string(args[1].(pyString)), " ", "-"))
return None
}
diff --git a/src/parse/asp/targets.go b/src/parse/asp/targets.go
index 6287364b0..58fa39c2d 100644
--- a/src/parse/asp/targets.go
+++ b/src/parse/asp/targets.go
@@ -276,7 +276,8 @@ func populateTarget(s *scope, t *core.BuildTarget, args []pyObject) {
addStrings(s, "requires", args[requiresBuildRuleArgIdx], t.AddRequire)
if arg := args[licencesBuildRuleArgIdx]; arg != None {
if expr, ok := arg.(pyString); ok {
- t.Licence = string(expr)
+ // TODO(v18): Remove the replace here once all licences are expected to be SPDX expressions
+ t.Licence = strings.ReplaceAll(string(expr), " ", "-")
} else {
// TODO(v18): Remove all this once strings are the only option.
s.Assert(!s.state.Config.FeatureFlags.SPDXLicencesOnly, "The licences argument must be a string")
@@ -286,13 +287,13 @@ func populateTarget(s *scope, t *core.BuildTarget, args []pyObject) {
// minor optimisation: avoid allocating a slice etc for the overwhelmingly common case of a single licence
str, ok := l[0].(pyString)
s.Assert(ok, "Items in the licences argument must be strings (was %s)", str.Type())
- t.Licence = string(str)
+ t.Licence = strings.ReplaceAll(string(str), " ", "-")
} else {
l2 := make([]string, len(l))
for i, x := range l {
str, ok := x.(pyString)
s.Assert(ok, "Items in the licences argument must be strings (was %s)", str.Type())
- l2[i] = string(str)
+ l2[i] = strings.ReplaceAll(string(str), " ", "-")
}
// Passing a list to Please is implicitly a series of alternative licences
t.Licence = strings.Join(l2, " OR ")
From 905852376625ee161b46ef907e1c7c740a6d0af5 Mon Sep 17 00:00:00 2001
From: Peter Ebden
Date: Sat, 17 Aug 2024 15:53:00 +0100
Subject: [PATCH 4/9] Deal with invalid original licences
---
src/core/build_target.go | 2 +-
src/core/config.go | 31 +++++++++++++++++++++++++++----
2 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/src/core/build_target.go b/src/core/build_target.go
index 4fb3106c2..81f0a221e 100644
--- a/src/core/build_target.go
+++ b/src/core/build_target.go
@@ -1893,7 +1893,7 @@ func (target *BuildTarget) CheckLicences(config *Configuration) (string, error)
if target.Licence == "" || (len(config.Licences.Accept) == 0 && len(config.Licences.Reject) == 0) {
return "", nil
}
- accepted, err := spdxexp.ExpressionSatisfies(target.Licence, config.Licences.Accept)
+ accepted, err := spdxexp.ExpressionSatisfies(target.Licence, config.AcceptedLicences())
if err != nil {
return "", fmt.Errorf("Target %s has invalid licence '%s': %s", target, target.Licence, err)
} else if accepted == "" {
diff --git a/src/core/config.go b/src/core/config.go
index 864fb73a4..d961be7f9 100644
--- a/src/core/config.go
+++ b/src/core/config.go
@@ -11,6 +11,7 @@ import (
"path/filepath"
"reflect"
"runtime"
+ "slices"
"sort"
"strconv"
"strings"
@@ -19,6 +20,7 @@ import (
"github.com/coreos/go-semver/semver"
"github.com/google/shlex"
+ "github.com/peterebden/go-spdx/v2/spdxexp"
"github.com/please-build/gcfg"
gcfgtypes "github.com/please-build/gcfg/types"
"github.com/thought-machine/go-flags"
@@ -684,10 +686,6 @@ type Configuration struct {
Bazel struct {
Compatibility bool `help:"Activates limited Bazel compatibility mode. When this is active several rule arguments are available under different names (e.g. compiler_flags -> copts etc), the WORKSPACE file is interpreted, Makefile-style replacements like $< and $@ are made in genrule commands, etc.\nNote that Skylark is not generally supported and many aspects of compatibility are fairly superficial; it's unlikely this will work for complex setups of either tool." var:"BAZEL_COMPATIBILITY"`
} `help:"Bazel is an open-sourced version of Google's internal build tool. Please draws a lot of inspiration from the original tool although the two have now diverged in various ways.\nNonetheless, if you've used Bazel, you will likely find Please familiar."`
-
- // buildEnvStored is a cached form of BuildEnv.
- buildEnvStored *storedBuildEnv
-
FeatureFlags struct {
SPDXLicencesOnly bool `help:"Licences on build targets can only be strings containing SPDX expressions, not lists."`
} `help:"Flags controlling preview features for the next release. Typically these config options gate breaking changes and only have a lifetime of one major release."`
@@ -696,6 +694,13 @@ type Configuration struct {
Timeout cli.Duration `help:"timeout for pushing to the gateway. Defaults to 2 seconds." `
PushHostInfo bool `help:"Whether to push host info"`
} `help:"Settings for collecting metrics."`
+
+ // buildEnvStored is a cached form of BuildEnv.
+ buildEnvStored *storedBuildEnv
+
+ // acceptedLicences is a slightly processed form of Licences.Accept
+ acceptedLicences []string
+ acceptedLicencesOnce sync.Once
}
// An Alias represents aliases in the config.
@@ -845,6 +850,24 @@ func (config *Configuration) getBuildEnv(includePath bool, includeUnsafe bool) B
return env
}
+// AcceptedLicences returns the set of licences accepted in this repository
+func (config *Configuration) AcceptedLicences() []string {
+ config.acceptedLicencesOnce.Do(func() {
+ config.acceptedLicences = make([]string, len(config.Licences.Accept))
+ for i, licence := range config.Licences.Accept {
+ config.acceptedLicences[i] = strings.ReplaceAll(strings.ReplaceAll(licence, " ", "-"), ",", "-")
+ }
+ if ok, invalid := spdxexp.ValidateLicenses(config.acceptedLicences); !ok {
+ log.Warning("The following accepted licences are not valid licence names: %s", strings.Join(invalid, ", "))
+ // We now have to remove these, otherwise the SPDX parser gets upset later on
+ for _, licence := range invalid {
+ config.acceptedLicences = slices.DeleteFunc(config.acceptedLicences, func(l string) bool { return l == licence })
+ }
+ }
+ })
+ return config.acceptedLicences
+}
+
// TagsToFields returns a map of string represent the properties of CONFIG object to the config Structfield
func (config *Configuration) TagsToFields() map[string]reflect.StructField {
tags := make(map[string]reflect.StructField)
From 29be85f83aa5f7999a0cfbfd12b8c76e97d5e6f9 Mon Sep 17 00:00:00 2001
From: Peter Ebden
Date: Sat, 17 Aug 2024 15:58:35 +0100
Subject: [PATCH 5/9] DO better licence normalisation
---
src/parse/asp/BUILD | 1 +
src/parse/asp/builtins.go | 5 ++++-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/parse/asp/BUILD b/src/parse/asp/BUILD
index 486fa0fef..239273939 100644
--- a/src/parse/asp/BUILD
+++ b/src/parse/asp/BUILD
@@ -12,6 +12,7 @@ go_library(
"///third_party/go/github.com_Masterminds_semver_v3//:v3",
"///third_party/go/github.com_manifoldco_promptui//:promptui",
"///third_party/go/github.com_please-build_gcfg//types",
+ "///third_party/go/github.com_peterebden_go-deferred-regex//:go-deferred-regex",
"//src/cli",
"//src/cli/logging",
"//src/cmap",
diff --git a/src/parse/asp/builtins.go b/src/parse/asp/builtins.go
index 8c6a839a6..60b43a4ce 100644
--- a/src/parse/asp/builtins.go
+++ b/src/parse/asp/builtins.go
@@ -15,6 +15,7 @@ import (
"github.com/Masterminds/semver/v3"
"github.com/manifoldco/promptui"
+ "github.com/peterebden/go-deferred-regex"
"github.com/thought-machine/please/src/cli"
"github.com/thought-machine/please/src/core"
@@ -24,6 +25,8 @@ import (
// A nativeFunc is a function that implements a builtin function natively.
type nativeFunc func(*scope, []pyObject) pyObject
+var normaliseLicence = deferredregex.DeferredRegex{Re: `[ (),]`}
+
// registerBuiltins sets up the "special" builtins that map to native code.
func registerBuiltins(s *scope) {
const varargs = true
@@ -1318,7 +1321,7 @@ func addLicence(s *scope, args []pyObject) pyObject {
if target.Licence != "" {
target.Licence += " OR "
}
- target.Licence += strings.TrimSpace(strings.ReplaceAll(string(args[1].(pyString)), " ", "-"))
+ target.Licence += normaliseLicence.ReplaceAllString(strings.TrimSpace(string(args[1].(pyString))), "-")
return None
}
From 88ab21e701fa749dcdceee4d3a571547b6175dc7 Mon Sep 17 00:00:00 2001
From: Peter Ebden
Date: Sat, 17 Aug 2024 16:04:16 +0100
Subject: [PATCH 6/9] fix test
---
src/build/build_step_test.go | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/src/build/build_step_test.go b/src/build/build_step_test.go
index e414e89f7..77ed2b399 100644
--- a/src/build/build_step_test.go
+++ b/src/build/build_step_test.go
@@ -290,22 +290,18 @@ func TestLicenceEnforcement(t *testing.T) {
// A license (non case sensitive) that is not in the list of accepted licenses will panic.
assert.Panics(t, func() {
- target.Licences = append(target.Licences, "Bsd")
+ target.Licence = "Bsd"
checkLicences(state, target)
}, "A target with a non-accepted licence will panic")
- // Accepting bsd should resolve the panic
- state.Config.Licences.Accept = append(state.Config.Licences.Accept, "BSD")
- checkLicences(state, target)
-
// Now construct a new "bad" target.
state, target = newState("//pkg:bad")
state.Config.Licences.Reject = append(state.Config.Licences.Reject, "gpl")
state.Config.Licences.Accept = append(state.Config.Licences.Accept, "mit")
// Adding an explicitly rejected licence should panic no matter what.
- target.Licences = append(target.Licences, "GPL")
assert.Panics(t, func() {
+ target.Licence = "GPL"
checkLicences(state, target)
}, "Trying to add GPL should panic (case insensitive)")
}
From 24d7b41cbd960a474c9fa719095bc38a4bf7b6fa Mon Sep 17 00:00:00 2001
From: Peter Ebden
Date: Sat, 17 Aug 2024 16:08:07 +0100
Subject: [PATCH 7/9] Add get_licence and set_licence functions
---
docs/lexicon.html | 78 +++++++++++++++++++++++++++++++++++++++
rules/builtins.build_defs | 4 ++
src/parse/asp/builtins.go | 14 +++++++
3 files changed, 96 insertions(+)
diff --git a/docs/lexicon.html b/docs/lexicon.html
index 351d56517..a0d33864c 100644
--- a/docs/lexicon.html
+++ b/docs/lexicon.html
@@ -951,6 +951,9 @@
Adds a new licence to a target. The assumption (as usual) is that if
multiple are added, they are options, so any one can be accepted.
+
+ Deprecated in favour of set_licence
.
+
@@ -980,6 +983,45 @@
+
+
+ set_licence
+
+
+ set_licence(target, licence)
+
+
+ Sets the target's licence to the given SPDX expression. Note that no normalisation is done on it.
+
+
+
+
+
+
+ Argument |
+ Default |
+ Type |
+ |
+
+
+
+
+ target |
+ |
+ str |
+ Label of the target to add the licence to. |
+
+
+ licence |
+ |
+ str |
+ SPDX expression defining licensing of the target |
+
+
+
+
+
+
get_licences
@@ -990,6 +1032,9 @@
Returns all the licences that are currently known to apply to a target.
+
+ Deprecated in favour of get_licence.
+
@@ -1013,6 +1058,39 @@
+
+
+ get_licence
+
+
+ get_licence(target)
+
+
+ Returns the licence expression of the given target.
+
+
+
+
+
+
+ Argument |
+ Default |
+ Type |
+ |
+
+
+
+
+ target |
+ |
+ str |
+ Label of the target to get the licence expression for. |
+
+
+
+
+
+
package
diff --git a/rules/builtins.build_defs b/rules/builtins.build_defs
index cffabf11e..a79a228d2 100644
--- a/rules/builtins.build_defs
+++ b/rules/builtins.build_defs
@@ -266,8 +266,12 @@ def add_entry_point(target:str, name:str, out:str):
def get_entry_points(target:str) -> dict:
pass
def add_licence(target:str, licence:str):
+ """Deprecated in favour of set_licence"""
+def set_licence(target:str, licence: str):
pass
def get_licences(target:str):
+ """Deprecated in favour of get_licence"""
+def get_licence(target:str):
pass
def get_command(target:str, config:str=''):
pass
diff --git a/src/parse/asp/builtins.go b/src/parse/asp/builtins.go
index 60b43a4ce..ad2cc983b 100644
--- a/src/parse/asp/builtins.go
+++ b/src/parse/asp/builtins.go
@@ -73,7 +73,9 @@ func registerBuiltins(s *scope) {
setNativeCode(s, "add_entry_point", addEntryPoint)
setNativeCode(s, "get_entry_points", getEntryPoints)
setNativeCode(s, "add_licence", addLicence)
+ setNativeCode(s, "set_licence", setLicence)
setNativeCode(s, "get_licences", getLicences)
+ setNativeCode(s, "get_licence", getLicence)
setNativeCode(s, "get_command", getCommand)
setNativeCode(s, "set_command", setCommand)
setNativeCode(s, "json", valueAsJSON)
@@ -1325,11 +1327,23 @@ func addLicence(s *scope, args []pyObject) pyObject {
return None
}
+// setLicence sets the target's licence to the given string. Note that it must be a valid SPDX identifier.
+func setLicence(s *scope, args []pyObject) pyObject {
+ target := getTargetPost(s, string(args[0].(pyString)))
+ target.Licence += string(args[1].(pyString))
+ return None
+}
+
// getLicences returns the licences for a single target.
func getLicences(s *scope, args []pyObject) pyObject {
return pyList{pyString(getTargetPost(s, string(args[0].(pyString))).Licence)}
}
+// getLicence returns the licence for a single target.
+func getLicences(s *scope, args []pyObject) pyObject {
+ return pyString(getTargetPost(s, string(args[0].(pyString))).Licence)
+}
+
// getCommand gets the command of a target, optionally for a configuration.
func getCommand(s *scope, args []pyObject) pyObject {
target := getTargetPost(s, string(args[0].(pyString)))
From 8c8fe822b590119a27b764f62dc732138f3187fe Mon Sep 17 00:00:00 2001
From: Peter Ebden
Date: Sat, 17 Aug 2024 16:12:45 +0100
Subject: [PATCH 8/9] add assertions
---
src/parse/asp/builtins.go | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/parse/asp/builtins.go b/src/parse/asp/builtins.go
index ad2cc983b..a9cd2d480 100644
--- a/src/parse/asp/builtins.go
+++ b/src/parse/asp/builtins.go
@@ -1319,6 +1319,7 @@ func getEntryPoints(s *scope, args []pyObject) pyObject {
// addLicence adds a licence to a target.
func addLicence(s *scope, args []pyObject) pyObject {
+ s.Assert(!s.state.Config.FeatureFlags.SPDXLicencesOnly, "The add_licence builtin has been replaced with set_licence")
target := getTargetPost(s, string(args[0].(pyString)))
if target.Licence != "" {
target.Licence += " OR "
@@ -1336,11 +1337,12 @@ func setLicence(s *scope, args []pyObject) pyObject {
// getLicences returns the licences for a single target.
func getLicences(s *scope, args []pyObject) pyObject {
+ s.Assert(!s.state.Config.FeatureFlags.SPDXLicencesOnly, "The get_licences builtin has been replaced with get_licence")
return pyList{pyString(getTargetPost(s, string(args[0].(pyString))).Licence)}
}
// getLicence returns the licence for a single target.
-func getLicences(s *scope, args []pyObject) pyObject {
+func getLicence(s *scope, args []pyObject) pyObject {
return pyString(getTargetPost(s, string(args[0].(pyString))).Licence)
}
From 8dce30da4d40174efe5c7f9c83235543950828d5 Mon Sep 17 00:00:00 2001
From: Peter Ebden
Date: Sat, 17 Aug 2024 16:40:08 +0100
Subject: [PATCH 9/9] Appease linters
---
docs/BUILD | 4 ++--
src/core/config.go | 26 +++++++++++++++++---------
2 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/docs/BUILD b/docs/BUILD
index ea493fbab..d941af922 100644
--- a/docs/BUILD
+++ b/docs/BUILD
@@ -58,8 +58,8 @@ genrule(
# Plugin versions to pull the docs from
plugins = {
"python": "v1.7.0",
- "java": "v0.4.0",
- "go": "v1.20.1",
+ "java": "v0.4.1",
+ "go": "v1.21.1",
"cc": "v0.4.0",
"shell": "v0.2.0",
"go-proto": "v0.3.0",
diff --git a/src/core/config.go b/src/core/config.go
index d961be7f9..5e6ad380c 100644
--- a/src/core/config.go
+++ b/src/core/config.go
@@ -370,7 +370,10 @@ func defaultPathIfExists(conf *string, dir, file string) {
// DefaultConfiguration returns the default configuration object with no overrides.
// N.B. Slice fields are not populated by this (since it interferes with reading them)
func DefaultConfiguration() *Configuration {
- config := Configuration{buildEnvStored: &storedBuildEnv{}}
+ config := Configuration{
+ buildEnvStored: &storedBuildEnv{},
+ acceptedLicences: &storedAcceptedLicences{},
+ }
config.Please.SelfUpdate = true
config.Please.Autoclean = true
config.Please.DownloadLocation = "https://get.please.build"
@@ -699,8 +702,7 @@ type Configuration struct {
buildEnvStored *storedBuildEnv
// acceptedLicences is a slightly processed form of Licences.Accept
- acceptedLicences []string
- acceptedLicencesOnce sync.Once
+ acceptedLicences *storedAcceptedLicences
}
// An Alias represents aliases in the config.
@@ -748,6 +750,11 @@ type storedBuildEnv struct {
Once sync.Once
}
+type storedAcceptedLicences struct {
+ Licences []string
+ Once sync.Once
+}
+
// Hash returns a hash of the parts of this configuration that affect building targets in general.
// Most parts are considered not to (e.g. cache settings) or affect specific targets (e.g. changing
// tool paths which get accounted for on the targets that use them).
@@ -852,20 +859,21 @@ func (config *Configuration) getBuildEnv(includePath bool, includeUnsafe bool) B
// AcceptedLicences returns the set of licences accepted in this repository
func (config *Configuration) AcceptedLicences() []string {
- config.acceptedLicencesOnce.Do(func() {
- config.acceptedLicences = make([]string, len(config.Licences.Accept))
+ config.acceptedLicences.Once.Do(func() {
+ l := make([]string, len(config.Licences.Accept))
for i, licence := range config.Licences.Accept {
- config.acceptedLicences[i] = strings.ReplaceAll(strings.ReplaceAll(licence, " ", "-"), ",", "-")
+ l[i] = strings.ReplaceAll(strings.ReplaceAll(licence, " ", "-"), ",", "-")
}
- if ok, invalid := spdxexp.ValidateLicenses(config.acceptedLicences); !ok {
+ if ok, invalid := spdxexp.ValidateLicenses(l); !ok {
log.Warning("The following accepted licences are not valid licence names: %s", strings.Join(invalid, ", "))
// We now have to remove these, otherwise the SPDX parser gets upset later on
for _, licence := range invalid {
- config.acceptedLicences = slices.DeleteFunc(config.acceptedLicences, func(l string) bool { return l == licence })
+ l = slices.DeleteFunc(l, func(l string) bool { return l == licence })
}
}
+ config.acceptedLicences.Licences = l
})
- return config.acceptedLicences
+ return config.acceptedLicences.Licences
}
// TagsToFields returns a map of string represent the properties of CONFIG object to the config Structfield