feat(ci): list the paths changed between two commits for push filters

commit: f17c3d99b990677ab7e399695e5114444810595a
author: brice · 9d ago · 2026-10-12 17:36 UTC
parent: 9cf140e
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

4 files · +134 -5

A internal/adapters/gitcli/changed.go +60 -0

@@ -0,0 +1,60 @@
	1	+	// SPDX-License-Identifier: AGPL-3.0-or-later
	2	+
	3	+	package gitcli
	4	+
	5	+	import (
	6	+	"bufio"
	7	+	"context"
	8	+	"fmt"
	9	+
	10	+	"bnema.dev/ermite/internal/domain"
	11	+	"bnema.dev/ermite/internal/ports"
	12	+	)
	13	+
	14	+	var _ ports.CIGit = (*Git)(nil)
	15	+
	16	+	// emptyTree is the id of the empty tree of a SHA-1 repository, what a diff
	17	+	// from "nothing" starts at.
	18	+	const emptyTree = "4b825dc642cb6eb9a060e54bf8d69288fbee4904"
	19	+
	20	+	// ChangedPaths runs `git diff-tree -r --name-only -z` from base to head, with
	21	+	// renames not detected (a rename lists both paths, as GitHub's push filters
	22	+	// see it), and stops reading after max paths: truncated tells there were more.
	23	+	// A zero base diffs against the empty tree.
	24	+	//
	25	+	//hexcheck:ignore suspicious-business-logic-in-adapter reads the path list git prints; the bound comes from the port
	26	+	func (g *Git) ChangedPaths(ctx context.Context, repo domain.RepoName, base, head domain.ObjectID, limit int) ([]string, bool, error) {
	27	+	if limit < 1 \|\| head.IsZero() {
	28	+	return nil, false, fmt.Errorf("changed paths: %w: limit %d", domain.ErrInvalid, limit)
	29	+	}
	30	+	dir, err := g.repoDir(repo)
	31	+	if err != nil {
	32	+	return nil, false, err
	33	+	}
	34	+	from := emptyTree
	35	+	if !base.IsZero() {
	36	+	from = base.String()
	37	+	}
	38	+	args := readArgs(dir, "diff-tree", "-r", "--name-only", "-z", "--no-renames", "--no-commit-id", "--end-of-options", from, head.String())
	39	+	var (
	40	+	paths []string
	41	+	truncated bool
	42	+	)
	43	+	err = g.pipe(ctx, args, func(r *bufio.Reader) error {
	44	+	for {
	45	+	name, err := r.ReadString(0)
	46	+	if err != nil {
	47	+	return nil // the end of the output; a failing exit status is reported by pipe
	48	+	}
	49	+	if len(paths) == limit {
	50	+	truncated = true
	51	+	return errStop
	52	+	}
	53	+	paths = append(paths, name[:len(name)-1])
	54	+	}
	55	+	})
	56	+	if err != nil {
	57	+	return nil, false, fmt.Errorf("changed paths %s..%s in %q: %w", from, head, repo, err)
	58	+	}
	59	+	return paths, truncated, nil
	60	+	}

@@ -0,0 +1,60 @@
	1	+	// SPDX-License-Identifier: AGPL-3.0-or-later
	2	+
	3	+	package gitcli
	4	+
	5	+	import (
	6	+	"bufio"
	7	+	"context"
	8	+	"fmt"
	9	+
	10	+	"bnema.dev/ermite/internal/domain"
	11	+	"bnema.dev/ermite/internal/ports"
	12	+	)
	13	+
	14	+	var _ ports.CIGit = (*Git)(nil)
	15	+
	16	+	// emptyTree is the id of the empty tree of a SHA-1 repository, what a diff
	17	+	// from "nothing" starts at.
	18	+	const emptyTree = "4b825dc642cb6eb9a060e54bf8d69288fbee4904"
	19	+
	20	+	// ChangedPaths runs `git diff-tree -r --name-only -z` from base to head, with
	21	+	// renames not detected (a rename lists both paths, as GitHub's push filters
	22	+	// see it), and stops reading after max paths: truncated tells there were more.
	23	+	// A zero base diffs against the empty tree.
	24	+	//
	25	+	//hexcheck:ignore suspicious-business-logic-in-adapter reads the path list git prints; the bound comes from the port
	26	+	func (g *Git) ChangedPaths(ctx context.Context, repo domain.RepoName, base, head domain.ObjectID, limit int) ([]string, bool, error) {
	27	+	if limit < 1 \|\| head.IsZero() {
	28	+	return nil, false, fmt.Errorf("changed paths: %w: limit %d", domain.ErrInvalid, limit)
	29	+	}
	30	+	dir, err := g.repoDir(repo)
	31	+	if err != nil {
	32	+	return nil, false, err
	33	+	}
	34	+	from := emptyTree
	35	+	if !base.IsZero() {
	36	+	from = base.String()
	37	+	}
	38	+	args := readArgs(dir, "diff-tree", "-r", "--name-only", "-z", "--no-renames", "--no-commit-id", "--end-of-options", from, head.String())
	39	+	var (
	40	+	paths []string
	41	+	truncated bool
	42	+	)
	43	+	err = g.pipe(ctx, args, func(r *bufio.Reader) error {
	44	+	for {
	45	+	name, err := r.ReadString(0)
	46	+	if err != nil {
	47	+	return nil // the end of the output; a failing exit status is reported by pipe
	48	+	}
	49	+	if len(paths) == limit {
	50	+	truncated = true
	51	+	return errStop
	52	+	}
	53	+	paths = append(paths, name[:len(name)-1])
	54	+	}
	55	+	})
	56	+	if err != nil {
	57	+	return nil, false, fmt.Errorf("changed paths %s..%s in %q: %w", from, head, repo, err)
	58	+	}
	59	+	return paths, truncated, nil
	60	+	}

A internal/adapters/gitcli/changed_test.go +69 -0

@@ -0,0 +1,69 @@
	1	+	// SPDX-License-Identifier: AGPL-3.0-or-later
	2	+
	3	+	package gitcli_test
	4	+
	5	+	import (
	6	+	"context"
	7	+	"testing"
	8	+
	9	+	"github.com/stretchr/testify/assert"
	10	+	"github.com/stretchr/testify/require"
	11	+
	12	+	"bnema.dev/ermite/internal/domain"
	13	+	)
	14	+
	15	+	func TestChangedPaths(t *testing.T) {
	16	+	g, data := newGit(t)
	17	+	ctx := context.Background()
	18	+	m := newMergeFixture(t, data)
	19	+	t.Cleanup(func() { _ = g.Close(ctx) })
	20	+
	21	+	// b..d changed a.txt and added c.txt.
	22	+	got, truncated, err := g.ChangedPaths(ctx, m.f.Name, m.b, m.d, 300)
	23	+	require.NoError(t, err)
	24	+	assert.False(t, truncated)
	25	+	assert.ElementsMatch(t, []string{"a.txt", "c.txt"}, got)
	26	+
	27	+	// A zero base lists every file of the commit, as for a new repository.
	28	+	got, _, err = g.ChangedPaths(ctx, m.f.Name, domain.ObjectID{}, m.b, 300)
	29	+	require.NoError(t, err)
	30	+	assert.ElementsMatch(t, []string{"a.txt", "b.txt"}, got)
	31	+
	32	+	// No change is an empty list, not an error.
	33	+	got, truncated, err = g.ChangedPaths(ctx, m.f.Name, m.d, m.d, 300)
	34	+	require.NoError(t, err)
	35	+	assert.Empty(t, got)
	36	+	assert.False(t, truncated)
	37	+
	38	+	// A bound cuts the list and says so.
	39	+	got, truncated, err = g.ChangedPaths(ctx, m.f.Name, domain.ObjectID{}, m.d, 2)
	40	+	require.NoError(t, err)
	41	+	assert.Len(t, got, 2)
	42	+	assert.True(t, truncated)
	43	+	got, truncated, err = g.ChangedPaths(ctx, m.f.Name, domain.ObjectID{}, m.d, 3)
	44	+	require.NoError(t, err)
	45	+	assert.Len(t, got, 3)
	46	+	assert.False(t, truncated, "a list of exactly the bound is complete")
	47	+
	48	+	_, _, err = g.ChangedPaths(ctx, m.f.Name, m.b, m.d, 0)
	49	+	assert.ErrorIs(t, err, domain.ErrInvalid)
	50	+	_, _, err = g.ChangedPaths(ctx, name(t, "missing"), m.b, m.d, 5)
	51	+	assert.ErrorIs(t, err, domain.ErrNotFound)
	52	+	_, _, err = g.ChangedPaths(ctx, m.f.Name, m.b, oidOf(t, "1111111111111111111111111111111111111111"), 5)
	53	+	assert.Error(t, err, "an unknown commit is an error")
	54	+	}
	55	+
	56	+	func TestChangedPathsKeepsAWholeRenameAndOddNames(t *testing.T) {
	57	+	g, data := newGit(t)
	58	+	ctx := context.Background()
	59	+	m := newMergeFixture(t, data)
	60	+	t.Cleanup(func() { _ = g.Close(ctx) })
	61	+
	62	+	m.f.Git("checkout", "-q", "feat")
	63	+	m.f.Write("dir with space/é.txt", "see\n")
	64	+	m.f.Git("rm", "-q", "c.txt")
	65	+	d2 := oidOf(t, m.f.Commit("rename"))
	66	+	got, _, err := g.ChangedPaths(ctx, m.f.Name, m.featTip, d2, 10)
	67	+	require.NoError(t, err)
	68	+	assert.ElementsMatch(t, []string{"c.txt", "dir with space/é.txt"}, got, "both sides of a rename, verbatim")
	69	+	}

@@ -0,0 +1,69 @@
	1	+	// SPDX-License-Identifier: AGPL-3.0-or-later
	2	+
	3	+	package gitcli_test
	4	+
	5	+	import (
	6	+	"context"
	7	+	"testing"
	8	+
	9	+	"github.com/stretchr/testify/assert"
	10	+	"github.com/stretchr/testify/require"
	11	+
	12	+	"bnema.dev/ermite/internal/domain"
	13	+	)
	14	+
	15	+	func TestChangedPaths(t *testing.T) {
	16	+	g, data := newGit(t)
	17	+	ctx := context.Background()
	18	+	m := newMergeFixture(t, data)
	19	+	t.Cleanup(func() { _ = g.Close(ctx) })
	20	+
	21	+	// b..d changed a.txt and added c.txt.
	22	+	got, truncated, err := g.ChangedPaths(ctx, m.f.Name, m.b, m.d, 300)
	23	+	require.NoError(t, err)
	24	+	assert.False(t, truncated)
	25	+	assert.ElementsMatch(t, []string{"a.txt", "c.txt"}, got)
	26	+
	27	+	// A zero base lists every file of the commit, as for a new repository.
	28	+	got, _, err = g.ChangedPaths(ctx, m.f.Name, domain.ObjectID{}, m.b, 300)
	29	+	require.NoError(t, err)
	30	+	assert.ElementsMatch(t, []string{"a.txt", "b.txt"}, got)
	31	+
	32	+	// No change is an empty list, not an error.
	33	+	got, truncated, err = g.ChangedPaths(ctx, m.f.Name, m.d, m.d, 300)
	34	+	require.NoError(t, err)
	35	+	assert.Empty(t, got)
	36	+	assert.False(t, truncated)
	37	+
	38	+	// A bound cuts the list and says so.
	39	+	got, truncated, err = g.ChangedPaths(ctx, m.f.Name, domain.ObjectID{}, m.d, 2)
	40	+	require.NoError(t, err)
	41	+	assert.Len(t, got, 2)
	42	+	assert.True(t, truncated)
	43	+	got, truncated, err = g.ChangedPaths(ctx, m.f.Name, domain.ObjectID{}, m.d, 3)
	44	+	require.NoError(t, err)
	45	+	assert.Len(t, got, 3)
	46	+	assert.False(t, truncated, "a list of exactly the bound is complete")
	47	+
	48	+	_, _, err = g.ChangedPaths(ctx, m.f.Name, m.b, m.d, 0)
	49	+	assert.ErrorIs(t, err, domain.ErrInvalid)
	50	+	_, _, err = g.ChangedPaths(ctx, name(t, "missing"), m.b, m.d, 5)
	51	+	assert.ErrorIs(t, err, domain.ErrNotFound)
	52	+	_, _, err = g.ChangedPaths(ctx, m.f.Name, m.b, oidOf(t, "1111111111111111111111111111111111111111"), 5)
	53	+	assert.Error(t, err, "an unknown commit is an error")
	54	+	}
	55	+
	56	+	func TestChangedPathsKeepsAWholeRenameAndOddNames(t *testing.T) {
	57	+	g, data := newGit(t)
	58	+	ctx := context.Background()
	59	+	m := newMergeFixture(t, data)
	60	+	t.Cleanup(func() { _ = g.Close(ctx) })
	61	+
	62	+	m.f.Git("checkout", "-q", "feat")
	63	+	m.f.Write("dir with space/é.txt", "see\n")
	64	+	m.f.Git("rm", "-q", "c.txt")
	65	+	d2 := oidOf(t, m.f.Commit("rename"))
	66	+	got, _, err := g.ChangedPaths(ctx, m.f.Name, m.featTip, d2, 10)
	67	+	require.NoError(t, err)
	68	+	assert.ElementsMatch(t, []string{"c.txt", "dir with space/é.txt"}, got, "both sides of a rename, verbatim")
	69	+	}

M internal/adapters/kvstore/ci.go +3 -4


@@ -487,11 +487,10 @@ func (c CI) stageRecords(ctx context.Context, b kv.Batch, w ports.CIWrite) (gu
487	487		guarded[j.Repo] = true
488	488		}
489	489		}
	490	+	// Checks are derived from the jobs, written in the same batch and under
	491	+	// the lock of their run: they are upserted without a precondition.
490	492		for _, ck := range w.Checks {
491		-	stage(b, checkKey(ck.Repo, ck.Sha, ck.JobID), encodeCheck(ck), ck.Version)
492		-	if ck.Version == 0 {
493		-	guarded[ck.Repo] = true
494		-	}
	493	+	b.Put(checkKey(ck.Repo, ck.Sha, ck.JobID), encodeCheck(ck))
495	494		}
496	495		return guarded, nil
497	496		}

 			guarded[j.Repo] = true
 		}
 	}
+	// Checks are derived from the jobs, written in the same batch and under
+	// the lock of their run: they are upserted without a precondition.
 	for _, ck := range w.Checks {
-		stage(b, checkKey(ck.Repo, ck.Sha, ck.JobID), encodeCheck(ck), ck.Version)
+		b.Put(checkKey(ck.Repo, ck.Sha, ck.JobID), encodeCheck(ck))
-		if ck.Version == 0 {
-			guarded[ck.Repo] = true
-		}
 	}
 	return guarded, nil
 }

M internal/ports/ci.go +2 -1


@@ -19,8 +19,9 @@ type CIRunDraft struct {
19	19		Checks []domain.CICheck
20	20		}
21	21
22		-	// CIWrite is a set of records written in one atomic batch. A record with
	22	+	// CIWrite is a set of records written in one atomic batch. A run or a job with
23	23		// Version 0 must not exist yet; any other must be unchanged since it was read.
	24	+	// Checks are derived from the jobs and upserted without a precondition.
24	25		// Apply returns domain.ErrConflict otherwise and domain.ErrNotFound when the
25	26		// repository no longer exists.
26	27		type CIWrite struct {

 	Checks []domain.CICheck
 }
-// CIWrite is a set of records written in one atomic batch. A record with
+// CIWrite is a set of records written in one atomic batch. A run or a job with
 // Version 0 must not exist yet; any other must be unchanged since it was read.
+// Checks are derived from the jobs and upserted without a precondition.
 // Apply returns domain.ErrConflict otherwise and domain.ErrNotFound when the
 // repository no longer exists.
 type CIWrite struct {