Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions cmd/root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2484,6 +2484,35 @@ func TestConcurrentInvocation(t *testing.T) {
as.NoError(eg.Wait())
}

func TestMaxBatchSize(t *testing.T) {
tempDir := test.TempExamples(t)
configPath := filepath.Join(tempDir, "/treefmt.toml")

test.ChangeWorkDir(t, tempDir)

maxBatchSize := 1
cfg := &config.Config{
FormatterConfigs: map[string]*config.Formatter{
"echo": {
Command: "test-fmt-only-one-file-at-a-time",
Includes: []string{"*"},
MaxBatchSize: &maxBatchSize,
},
},
}

treefmt(t,
withConfig(configPath, cfg),
withNoError(t),
withStats(t, map[stats.Type]int{
stats.Traversed: 33,
stats.Matched: 33,
stats.Formatted: 33,
stats.Changed: 33,
}),
)
}

type options struct {
args []string
env map[string]string
Expand Down
2 changes: 2 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ type Formatter struct {
Excludes []string `mapstructure:"excludes,omitempty" toml:"excludes,omitempty"`
// Indicates the order of precedence when executing this Formatter in a sequence of Formatters.
Priority int `mapstructure:"priority,omitempty" toml:"priority,omitempty"`
// The maximum number of files we should pass to this Formatter at once.
MaxBatchSize *int `mapstructure:"max-batch-size" toml:"max-batch-size"`
}

// SetFlags appends our flags to the provided flag set.
Expand Down
26 changes: 26 additions & 0 deletions format/formatter.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ func (f *Formatter) Name() string {
return f.name
}

func (f *Formatter) MaxBatchSize() int {
if f.config.MaxBatchSize == nil {
return 1024 //<<<
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have 2 other BatchSize constants in treefmt already;

cmd/format/format.go
26:	BatchSize = 1024

walk/walk.go
30:	BatchSize = 1024

Not sure if we really want a 3rd, or if we should instead import one of those here.

} else {
return *f.config.MaxBatchSize
}
}

func (f *Formatter) Priority() int {
return f.config.Priority
}
Expand Down Expand Up @@ -78,6 +86,24 @@ func (f *Formatter) Hash(h hash.Hash) error {
}

func (f *Formatter) Apply(ctx context.Context, files []*walk.File) error {
for i := 0; i < len(files); i += f.MaxBatchSize() {
end := min(i+f.MaxBatchSize(), len(files))
if err := f.apply(ctx, files[i:end]); err != nil {
return err
}
}

return nil
}

func (f *Formatter) apply(ctx context.Context, files []*walk.File) error {
if len(files) > f.MaxBatchSize() {
//<<< learn me some go >>>
//<<< this error gets swallowed by scheduler.schedule and turned into a generic "Error: failed to finalise formatting: formatting failures detected" >>>
//<<< should we update that code to print more information? or should this be a panic instead? >>>
return fmt.Errorf("formatter cannot format %d files at once (max batch size: %d)", len(files), f.MaxBatchSize())
}

start := time.Now()

// construct args, starting with config
Expand Down
3 changes: 3 additions & 0 deletions format/scheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"context"
"crypto/md5" //nolint:gosec
"fmt"
"os"
"runtime"
"slices"
"strings"
Expand Down Expand Up @@ -127,6 +128,7 @@ func (s *scheduler) submit(
s.batches[key] = append(s.batches[key], file)

// schedule the batch for processing if it's full
// <<< TODO: impedance mismatch? instead compute LCM (up to some max) of max-batch-size of all matched formatters? >>>
if len(s.batches[key]) == s.batchSize {
s.schedule(ctx, key, s.batches[key])
// reset the batch
Expand All @@ -146,6 +148,7 @@ func (s *scheduler) schedule(ctx context.Context, key batchKey, batch []*walk.Fi
formatter := s.formatters[name]

if err := formatter.Apply(ctx, batch); err != nil {
fmt.Fprintf(os.Stderr, "formatter failed with error: %s\n", err) //<<<
formatErrors = append(formatErrors, err)
}
}
Expand Down
11 changes: 11 additions & 0 deletions nix/packages/treefmt/formatters.nix
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,15 @@ with pkgs; [
test-fmt-append "$@"
'';
})
(pkgs.writeShellApplication {
name = "test-fmt-only-one-file-at-a-time";
text = ''
if [ $# -ne 1 ]; then
echo "I only support formatting exactly 1 file at a time"
exit 1
fi

test-fmt-append "suffix" "$1"
'';
})
]