From 9cb066d4ff987aea909a5a8a1b6d3031e421eece Mon Sep 17 00:00:00 2001 From: callum Date: Fri, 27 Feb 2026 22:28:52 +0000 Subject: [PATCH 1/2] Add ShortenAbsoluteLabelsToRelative configuration option --- src/core/config.go | 3 +++ src/format/BUILD | 1 + src/format/fmt.go | 27 ++++++++++++++----- src/format/fmt_test.go | 16 +++++++++-- .../test_data/shorten_labels.after.build | 7 +++++ .../test_data/shorten_labels.before.build | 14 ++++++++++ .../shorten_labels_no_config.after.build | 10 +++++++ .../shorten_labels_no_config.before.build | 14 ++++++++++ 8 files changed, 83 insertions(+), 9 deletions(-) create mode 100644 src/format/test_data/shorten_labels.after.build create mode 100644 src/format/test_data/shorten_labels.before.build create mode 100644 src/format/test_data/shorten_labels_no_config.after.build create mode 100644 src/format/test_data/shorten_labels_no_config.before.build diff --git a/src/core/config.go b/src/core/config.go index b6fc6e2602..0e035a53fb 100644 --- a/src/core/config.go +++ b/src/core/config.go @@ -546,6 +546,9 @@ type Configuration struct { StoreCommand string `help:"Use a custom command to store cache entries."` RetrieveCommand string `help:"Use a custom command to retrieve cache entries."` } `help:"Please has several built-in caches that can be configured in its config file.\n\nThe simplest one is the directory cache which by default is written into the .plz-cache directory. This allows for fast retrieval of code that has been built before (for example, when swapping Git branches).\n\nThere is also a remote RPC cache which allows using a centralised server to store artifacts. A typical pattern here is to have your CI system write artifacts into it and give developers read-only access so they can reuse its work.\n\nFinally there's a HTTP cache which is very similar, but a little obsolete now since the RPC cache outperforms it and has some extra features. Otherwise the two have similar semantics and share quite a bit of implementation.\n\nPlease has server implementations for both the RPC and HTTP caches."` + Format struct { + ShortenAbsoluteLabelsToRelative bool `help:"Shortens absolute labels to relative if the label is in the same package as the associated target."` + } `help:"A config section describing settings related to the 'format' sub command."` Test struct { Timeout cli.Duration `help:"Default timeout applied to all tests. Can be overridden on a per-rule basis."` DisableCoverage []string `help:"Disables coverage for tests that have any of these labels spcified."` diff --git a/src/format/BUILD b/src/format/BUILD index 1a6fbed019..919ff2b030 100644 --- a/src/format/BUILD +++ b/src/format/BUILD @@ -5,6 +5,7 @@ go_library( visibility = ["//src/..."], deps = [ "///third_party/go/github.com_please-build_buildtools//build", + "///third_party/go/github.com_please-build_buildtools//tables", "///third_party/go/golang.org_x_sync//errgroup", "//src/cli/logging", "//src/core", diff --git a/src/format/fmt.go b/src/format/fmt.go index 3f4d438565..8ea466bb45 100644 --- a/src/format/fmt.go +++ b/src/format/fmt.go @@ -11,6 +11,7 @@ import ( "sync/atomic" "github.com/please-build/buildtools/build" + "github.com/please-build/buildtools/tables" "golang.org/x/sync/errgroup" "github.com/thought-machine/please/src/cli/logging" @@ -27,7 +28,7 @@ var log = logging.Log // The returned bool is true if any changes were needed. func Format(config *core.Configuration, filenames []string, rewrite, quiet bool) (bool, error) { if len(filenames) == 0 { - return formatAll(plz.FindAllBuildFiles(config, core.RepoRoot, ""), config.Please.NumThreads, rewrite, quiet) + return formatAll(config, plz.FindAllBuildFiles(config, core.RepoRoot, ""), rewrite, quiet) } ch := make(chan string) go func() { @@ -36,17 +37,17 @@ func Format(config *core.Configuration, filenames []string, rewrite, quiet bool) } close(ch) }() - return formatAll(ch, config.Please.NumThreads, rewrite, quiet) + return formatAll(config, ch, rewrite, quiet) } -func formatAll(filenames <-chan string, parallelism int, rewrite, quiet bool) (bool, error) { +func formatAll(config *core.Configuration, filenames <-chan string, rewrite, quiet bool) (bool, error) { var changed int64 var g errgroup.Group - g.SetLimit(parallelism) + g.SetLimit(config.Please.NumThreads) for filename := range filenames { filename := filename g.Go(func() error { - c, err := format(filename, rewrite, quiet) + c, err := format(config, filename, rewrite, quiet) if c { atomic.AddInt64(&changed, 1) } @@ -57,7 +58,7 @@ func formatAll(filenames <-chan string, parallelism int, rewrite, quiet bool) (b return changed > 0, err } -func format(filename string, rewrite, quiet bool) (bool, error) { +func format(config *core.Configuration, filename string, rewrite, quiet bool) (bool, error) { before, err := os.ReadFile(filename) if err != nil { return true, err @@ -67,7 +68,19 @@ func format(filename string, rewrite, quiet bool) (bool, error) { return true, err } simplify(f) - after := build.Format(f) + after := build.FormatWithRewriter( + &build.Rewriter{ + IsLabelArg: tables.IsLabelArg, + LabelDenyList: tables.LabelDenylist, + IsSortableListArg: tables.IsSortableListArg, + SortableDenylist: tables.SortableDenylist, + SortableAllowlist: tables.SortableAllowlist, + NamePriority: tables.NamePriority, + StripLabelLeadingSlashes: tables.StripLabelLeadingSlashes, + ShortenAbsoluteLabelsToRelative: config.Format.ShortenAbsoluteLabelsToRelative, + }, + f, + ) if bytes.Equal(before, after) { log.Debug("%s is already in canonical format", filename) return false, nil diff --git a/src/format/fmt_test.go b/src/format/fmt_test.go index dc120b40d2..c327cbcfc3 100644 --- a/src/format/fmt_test.go +++ b/src/format/fmt_test.go @@ -17,18 +17,30 @@ const testDir = "src/format/test_data" func TestFormat(t *testing.T) { files, err := os.ReadDir(testDir) assert.NoError(t, err) + + patchConfigByFile := map[string]func(*core.Configuration) { + "shorten_labels": func(c *core.Configuration) { + c.Format.ShortenAbsoluteLabelsToRelative = true + }, + } + for _, file := range files { if test, isBefore := strings.CutSuffix(file.Name(), ".before.build"); isBefore { t.Run(test, func(t *testing.T) { before := filepath.Join(testDir, test+".before.build") after := filepath.Join(testDir, test+".after.build") - changed, err := Format(core.DefaultConfiguration(), []string{before}, false, true) + config := core.DefaultConfiguration() + if patchConfig := patchConfigByFile[test]; patchConfig != nil { + patchConfig(config) + } + + changed, err := Format(config, []string{before}, false, true) assert.NoError(t, err) assert.True(t, changed) // N.B. this rewrites the file; be careful if you're adding more tests here. - changed, err = Format(core.DefaultConfiguration(), []string{before}, true, false) + changed, err = Format(config, []string{before}, true, false) assert.NoError(t, err) assert.True(t, changed) diff --git a/src/format/test_data/shorten_labels.after.build b/src/format/test_data/shorten_labels.after.build new file mode 100644 index 0000000000..e1895664d1 --- /dev/null +++ b/src/format/test_data/shorten_labels.after.build @@ -0,0 +1,7 @@ +go_library( + name = "name", + srcs = [ + "src.go", + ], + deps = [":test_data"], +) diff --git a/src/format/test_data/shorten_labels.before.build b/src/format/test_data/shorten_labels.before.build new file mode 100644 index 0000000000..8ad091dfd6 --- /dev/null +++ b/src/format/test_data/shorten_labels.before.build @@ -0,0 +1,14 @@ +go_library( + name = "name", + srcs = [ + "src.go", + ], + deps = [ + "//src/format/test_data:test_data", + "//src/format/test_data:test_data", + "//src/format/test_data", + "//src/format/test_data", + ":test_data", + ":test_data", + ], +) diff --git a/src/format/test_data/shorten_labels_no_config.after.build b/src/format/test_data/shorten_labels_no_config.after.build new file mode 100644 index 0000000000..c0c218cb90 --- /dev/null +++ b/src/format/test_data/shorten_labels_no_config.after.build @@ -0,0 +1,10 @@ +go_library( + name = "name", + srcs = [ + "src.go", + ], + deps = [ + ":test_data", + "//src/format/test_data", + ], +) diff --git a/src/format/test_data/shorten_labels_no_config.before.build b/src/format/test_data/shorten_labels_no_config.before.build new file mode 100644 index 0000000000..8ad091dfd6 --- /dev/null +++ b/src/format/test_data/shorten_labels_no_config.before.build @@ -0,0 +1,14 @@ +go_library( + name = "name", + srcs = [ + "src.go", + ], + deps = [ + "//src/format/test_data:test_data", + "//src/format/test_data:test_data", + "//src/format/test_data", + "//src/format/test_data", + ":test_data", + ":test_data", + ], +) From 736e69501e885d9d012111b5c480ca7995e8540e Mon Sep 17 00:00:00 2001 From: callum Date: Sat, 28 Feb 2026 11:27:16 +0000 Subject: [PATCH 2/2] add documentation for new config option --- docs/config.html | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/docs/config.html b/docs/config.html index 021d0798f4..d1d86f38eb 100644 --- a/docs/config.html +++ b/docs/config.html @@ -1211,6 +1211,24 @@

TimeoutName

+
+

[Format]

+ +
    +
  • +
    +

    + ShortenAbsoluteLabelsToRelative (bool) +

    + +

    + Shortens absolute labels to relative when running plz format if the label is in the same package as the associated target. +

    +
    +
    • +
+
+

[Test]