detached 4d9eaac

#8 chore(hexcheck): bump to v0.4.0 and drop unused ignore directives

state
merged · 4d9eaac · fast-forward
branch
chore/hexcheck-v0.4.0 → main
opened
bnema ·
updated

Bump hexcheck to v0.4.0 and make hexcheck -fail-on warn pass. Comment-only changes in adapters.

  • Remove 89 ignore directives that v0.4.0 reports as unused.
  • Annotate the 16 new findings with accurate reasons.
  • Rewrite wrong reasons. 14 functions hold business rules marked 'pending move to domain/usecase' (revision resolution, commit budget, diff budget, workflow guard, commitsAhead, banner, mrPage, repoRecord, CreateRun, RotateOIDCKey, issuesOn, issueSubmittedPage); they move in follow-up MRs.

Ignore directives: 129 -> 57.

CommitSubjectAuthorDateSignatureStatus
4d9eaacfix(usecase): drop the unused repository read before the merge lockbriceGPG Verified
05fc636refactor: reuse the domain settings predicates and tighten the moved rules after reviewbriceGPG Verified
558568cfix(web,rpc): handle an API or web server built without merge requests or OIDC keysbriceGPG Verified
5344be5merge: hexmove/workflow into chore/hexcheck-v0.4.0briceGPG Verified
9f9eee5merge: hexmove/history into chore/hexcheck-v0.4.0briceGPG Verified
c902d2fmerge: hexmove/policies into chore/hexcheck-v0.4.0briceGPG Verified
6b4272dmerge: hexmove/budgets into chore/hexcheck-v0.4.0briceGPG Verified
fe7ab7dmerge: hexmove/revision into chore/hexcheck-v0.4.0briceGPG Verified
a1ec1afrefactor(domain): own the workflow-change rule for job tokens in WorkflowChangeOfbriceGPG Verified
6080cberefactor(usecase): own branch comparison, push summaries and unread commits in MergeRequestsbriceGPG Verified
c0f84a6refactor(usecase): own repo defaults, disabled workflows, OIDC rotation and issue visibility rulesbriceGPG Verified
0424a8arefactor(domain): own commit-object and diff read budgetsbriceGPG Verified
ac21956chore(hexcheck): bump to v0.4.0 and drop unused ignore directivesbriceGPG Verified
e188800refactor(domain): own the revision resolution order in Revision.CandidatesbriceGPG Verified

Checks 8

102 files · +2491 -1408
7/8
M docs/merge-requests.md +13 -0
@@ -91,3 +91,16 @@ A root note can carry a verdict: `--approve` or `--request-changes`. The text ma
9191 - `mr show` lists the latest verdict of each author (`verdicts` in `--json`).
9292 - An approval is stale once the branch head differs from the head it was given on. Changes requested stay current until their author gives a new verdict, whatever is pushed.
9393 - Verdicts are immutable: editing or deleting the note keeps its verdict. Only a new verdict of the same author replaces it.
94+
95+## Shared read position and history queries
96+
97+The read position is shared, not per principal. `MergeRequestDetail.HeadUnread`
98+is a fact: the nonzero head differs from that position. The web presentation
99+still decides who sees the mark-read button.
100+
101+Repository comparisons return at most `ports.MaxCompareCommits` (500) commits.
102+Push summaries count at most `ports.MaxPushSummaryCommits` (100) commits and
103+report whether more exist; ancestry, rather than the bounded list, detects a
104+rewrite. Merge-request details include at most `ports.MaxMergeRequestCommits`
105+(100) commits. These history queries remain on the merge-request service
106+because it owns the merge-git port.
@@ -91,3 +91,16 @@ A root note can carry a verdict: `--approve` or `--request-changes`. The text ma
91 - `mr show` lists the latest verdict of each author (`verdicts` in `--json`).91 - `mr show` lists the latest verdict of each author (`verdicts` in `--json`).
92 - An approval is stale once the branch head differs from the head it was given on. Changes requested stay current until their author gives a new verdict, whatever is pushed.92 - An approval is stale once the branch head differs from the head it was given on. Changes requested stay current until their author gives a new verdict, whatever is pushed.
93 - Verdicts are immutable: editing or deleting the note keeps its verdict. Only a new verdict of the same author replaces it.93 - Verdicts are immutable: editing or deleting the note keeps its verdict. Only a new verdict of the same author replaces it.
94+
95+## Shared read position and history queries
96+
97+The read position is shared, not per principal. `MergeRequestDetail.HeadUnread`
98+is a fact: the nonzero head differs from that position. The web presentation
99+still decides who sees the mark-read button.
100+
101+Repository comparisons return at most `ports.MaxCompareCommits` (500) commits.
102+Push summaries count at most `ports.MaxPushSummaryCommits` (100) commits and
103+report whether more exist; ancestry, rather than the bounded list, detects a
104+rewrite. Merge-request details include at most `ports.MaxMergeRequestCommits`
105+(100) commits. These history queries remain on the merge-request service
106+because it owns the merge-git port.
M go.mod +1 -1
@@ -64,7 +64,7 @@ require (
6464 github.com/bep/godartsass/v2 v2.5.0 // indirect
6565 github.com/bep/golibsass v1.2.0 // indirect
6666 github.com/bits-and-blooms/bitset v1.24.5 // indirect
67- github.com/bnema/hexcheck v0.3.1 // indirect
67+ github.com/bnema/hexcheck v0.4.0 // indirect
6868 github.com/brunoga/deep v1.3.1 // indirect
6969 github.com/bufbuild/buf v1.73.0 // indirect
7070 github.com/bufbuild/protocompile v0.14.2-0.20260910151042-7436f7c76201 // indirect
@@ -64,7 +64,7 @@ require (
64 github.com/bep/godartsass/v2 v2.5.0 // indirect64 github.com/bep/godartsass/v2 v2.5.0 // indirect
65 github.com/bep/golibsass v1.2.0 // indirect65 github.com/bep/golibsass v1.2.0 // indirect
66 github.com/bits-and-blooms/bitset v1.24.5 // indirect66 github.com/bits-and-blooms/bitset v1.24.5 // indirect
67- github.com/bnema/hexcheck v0.3.1 // indirect67+ github.com/bnema/hexcheck v0.4.0 // indirect
68 github.com/brunoga/deep v1.3.1 // indirect68 github.com/brunoga/deep v1.3.1 // indirect
69 github.com/bufbuild/buf v1.73.0 // indirect69 github.com/bufbuild/buf v1.73.0 // indirect
70 github.com/bufbuild/protocompile v0.14.2-0.20260910151042-7436f7c76201 // indirect70 github.com/bufbuild/protocompile v0.14.2-0.20260910151042-7436f7c76201 // indirect
M go.sum +2 -2 generated file: collapsed
M internal/adapters/cilogs/cilogs.go +2 -0
@@ -244,6 +244,8 @@ func (s *Store) Count(_ context.Context, ref ports.CILogRef) (int64, error) {
244244 }
245245
246246 // Read returns lines from line number from.
247+//
248+//hexcheck:ignore suspicious-business-logic-in-adapter reads a bounded snapshot of stored log lines using sparse offsets and maps missing or out-of-range reads to port errors
247249 func (s *Store) Read(_ context.Context, ref ports.CILogRef, from int64, maxLines, maxBytes int) ([]domain.CILogLine, error) {
248250 if !validRef(ref) || from < 0 {
249251 return nil, domain.ErrNotFound
@@ -244,6 +244,8 @@ func (s *Store) Count(_ context.Context, ref ports.CILogRef) (int64, error) {
244 }244 }
245 245
246 // Read returns lines from line number from.246 // Read returns lines from line number from.
247+//
248+//hexcheck:ignore suspicious-business-logic-in-adapter reads a bounded snapshot of stored log lines using sparse offsets and maps missing or out-of-range reads to port errors
247 func (s *Store) Read(_ context.Context, ref ports.CILogRef, from int64, maxLines, maxBytes int) ([]domain.CILogLine, error) {249 func (s *Store) Read(_ context.Context, ref ports.CILogRef, from int64, maxLines, maxBytes int) ([]domain.CILogLine, error) {
248 if !validRef(ref) || from < 0 {250 if !validRef(ref) || from < 0 {
249 return nil, domain.ErrNotFound251 return nil, domain.ErrNotFound
M internal/adapters/cisummaries/cisummaries.go +2 -0
@@ -56,6 +56,8 @@ func (s *Store) path(ref ports.CILogRef, step int32) string {
5656 }
5757
5858 // Put stores the summary of a step, or removes it when markdown is empty.
59+//
60+//hexcheck:ignore suspicious-business-logic-in-adapter validates storage coordinates and atomically writes a step summary, deleting its file when empty
5961 func (s *Store) Put(_ context.Context, ref ports.CILogRef, step int32, markdown string) error {
6062 if !validRef(ref) || step < 1 || step > domain.CIMaxSteps {
6163 return domain.ErrNotFound
@@ -56,6 +56,8 @@ func (s *Store) path(ref ports.CILogRef, step int32) string {
56 }56 }
57 57
58 // Put stores the summary of a step, or removes it when markdown is empty.58 // Put stores the summary of a step, or removes it when markdown is empty.
59+//
60+//hexcheck:ignore suspicious-business-logic-in-adapter validates storage coordinates and atomically writes a step summary, deleting its file when empty
59 func (s *Store) Put(_ context.Context, ref ports.CILogRef, step int32, markdown string) error {61 func (s *Store) Put(_ context.Context, ref ports.CILogRef, step int32, markdown string) error {
60 if !validRef(ref) || step < 1 || step > domain.CIMaxSteps {62 if !validRef(ref) || step < 1 || step > domain.CIMaxSteps {
61 return domain.ErrNotFound63 return domain.ErrNotFound
M internal/adapters/ermitegit/commitobjects.go +12 -11
@@ -22,10 +22,10 @@ var _ ports.CommitObjectReader = (*Adapter)(nil)
2222 // always serves it, and the wrapped reader only the repositories the library
2323 // cannot read.
2424 //
25-//hexcheck:ignore suspicious-business-logic-in-adapter applies the limits of the port and of the domain
25+//hexcheck:ignore suspicious-business-logic-in-adapter streams object headers and bytes; eligibility, deduplication and byte limits are decided by domain.CommitObjectBudget, request validation by domain.ValidateCommitObjects
2626 func (a *Adapter) ReadCommitObjects(ctx context.Context, repo domain.RepoName, ids []domain.ObjectID) (map[domain.ObjectID][]byte, error) {
27- if len(ids) > ports.MaxCommitLimit {
28- return nil, fmt.Errorf("%w: %d commit objects asked, at most %d", domain.ErrInvalid, len(ids), ports.MaxCommitLimit)
27+ if err := domain.ValidateCommitObjects(ids, ports.MaxCommitLimit); err != nil {
28+ return nil, err
2929 }
3030 cli := func() (map[domain.ObjectID][]byte, error) {
3131 r, ok := a.cli.(ports.CommitObjectReader)
@@ -36,12 +36,12 @@ func (a *Adapter) ReadCommitObjects(ctx context.Context, repo domain.RepoName, i
3636 }
3737 return serve(ctx, a, repo, func(r *git.Repo) (map[domain.ObjectID][]byte, error) {
3838 out := make(map[domain.ObjectID][]byte, len(ids))
39- var total int64
39+ var budget domain.CommitObjectBudget
4040 for _, id := range ids {
4141 if err := ctx.Err(); err != nil {
4242 return nil, err
4343 }
44- if _, done := out[id]; done {
44+ if budget.Seen(id) {
4545 continue
4646 }
4747 gid, err := git.ParseID(id.String())
@@ -55,12 +55,13 @@ func (a *Adapter) ReadCommitObjects(ctx context.Context, repo domain.RepoName, i
5555 if err != nil {
5656 return nil, err
5757 }
58- if typ != git.TypeCommit || size > domain.MaxCommitObjectBytes {
59- continue
60- }
61- if total+size > domain.MaxCommitObjectsBytes {
58+ take, stop := budget.Admit(id, typ == git.TypeCommit, size)
59+ if stop {
6260 break
6361 }
62+ if !take {
63+ continue
64+ }
6465 typ, data, err := r.ReadObject(gid, nil)
6566 if errors.Is(err, git.ErrNotFound) {
6667 continue // removed meanwhile
@@ -68,9 +69,9 @@ func (a *Adapter) ReadCommitObjects(ctx context.Context, repo domain.RepoName, i
6869 if err != nil {
6970 return nil, err
7071 }
71- if typ == git.TypeCommit && len(data) <= domain.MaxCommitObjectBytes && total+int64(len(data)) <= domain.MaxCommitObjectsBytes {
72+ if take, _ := budget.Admit(id, typ == git.TypeCommit, int64(len(data))); take {
7273 out[id] = data
73- total += int64(len(data))
74+ budget.Charge(id, int64(len(data)))
7475 }
7576 }
7677 return out, nil
@@ -22,10 +22,10 @@ var _ ports.CommitObjectReader = (*Adapter)(nil)
22 // always serves it, and the wrapped reader only the repositories the library22 // always serves it, and the wrapped reader only the repositories the library
23 // cannot read.23 // cannot read.
24 //24 //
25-//hexcheck:ignore suspicious-business-logic-in-adapter applies the limits of the port and of the domain25+//hexcheck:ignore suspicious-business-logic-in-adapter streams object headers and bytes; eligibility, deduplication and byte limits are decided by domain.CommitObjectBudget, request validation by domain.ValidateCommitObjects
26 func (a *Adapter) ReadCommitObjects(ctx context.Context, repo domain.RepoName, ids []domain.ObjectID) (map[domain.ObjectID][]byte, error) {26 func (a *Adapter) ReadCommitObjects(ctx context.Context, repo domain.RepoName, ids []domain.ObjectID) (map[domain.ObjectID][]byte, error) {
27- if len(ids) > ports.MaxCommitLimit {27+ if err := domain.ValidateCommitObjects(ids, ports.MaxCommitLimit); err != nil {
28- return nil, fmt.Errorf("%w: %d commit objects asked, at most %d", domain.ErrInvalid, len(ids), ports.MaxCommitLimit)28+ return nil, err
29 }29 }
30 cli := func() (map[domain.ObjectID][]byte, error) {30 cli := func() (map[domain.ObjectID][]byte, error) {
31 r, ok := a.cli.(ports.CommitObjectReader)31 r, ok := a.cli.(ports.CommitObjectReader)
@@ -36,12 +36,12 @@ func (a *Adapter) ReadCommitObjects(ctx context.Context, repo domain.RepoName, i
36 }36 }
37 return serve(ctx, a, repo, func(r *git.Repo) (map[domain.ObjectID][]byte, error) {37 return serve(ctx, a, repo, func(r *git.Repo) (map[domain.ObjectID][]byte, error) {
38 out := make(map[domain.ObjectID][]byte, len(ids))38 out := make(map[domain.ObjectID][]byte, len(ids))
39- var total int6439+ var budget domain.CommitObjectBudget
40 for _, id := range ids {40 for _, id := range ids {
41 if err := ctx.Err(); err != nil {41 if err := ctx.Err(); err != nil {
42 return nil, err42 return nil, err
43 }43 }
44- if _, done := out[id]; done {44+ if budget.Seen(id) {
45 continue45 continue
46 }46 }
47 gid, err := git.ParseID(id.String())47 gid, err := git.ParseID(id.String())
@@ -55,12 +55,13 @@ func (a *Adapter) ReadCommitObjects(ctx context.Context, repo domain.RepoName, i
55 if err != nil {55 if err != nil {
56 return nil, err56 return nil, err
57 }57 }
58- if typ != git.TypeCommit || size > domain.MaxCommitObjectBytes {58+ take, stop := budget.Admit(id, typ == git.TypeCommit, size)
59- continue59+ if stop {
60- }
61- if total+size > domain.MaxCommitObjectsBytes {
62 break60 break
63 }61 }
62+ if !take {
63+ continue
64+ }
64 typ, data, err := r.ReadObject(gid, nil)65 typ, data, err := r.ReadObject(gid, nil)
65 if errors.Is(err, git.ErrNotFound) {66 if errors.Is(err, git.ErrNotFound) {
66 continue // removed meanwhile67 continue // removed meanwhile
@@ -68,9 +69,9 @@ func (a *Adapter) ReadCommitObjects(ctx context.Context, repo domain.RepoName, i
68 if err != nil {69 if err != nil {
69 return nil, err70 return nil, err
70 }71 }
71- if typ == git.TypeCommit && len(data) <= domain.MaxCommitObjectBytes && total+int64(len(data)) <= domain.MaxCommitObjectsBytes {72+ if take, _ := budget.Admit(id, typ == git.TypeCommit, int64(len(data))); take {
72 out[id] = data73 out[id] = data
73- total += int64(len(data))74+ budget.Charge(id, int64(len(data)))
74 }75 }
75 }76 }
76 return out, nil77 return out, nil
M internal/adapters/ermitegit/convert.go +25 -36
@@ -27,8 +27,6 @@ func domainID(id git.ObjectID) (domain.ObjectID, error) { return domain.ParseObj
2727 // listRefs lists HEAD, then the branches, then the tags, like gitcli: a tag
2828 // follows to its commit when it leads to one; HEAD takes the ids of its
2929 // branch or tag, and only the name of an unborn branch.
30-//
31-//hexcheck:ignore suspicious-business-logic-in-adapter converts ermite-git values into domain values; the limits and the orders come from domain
3230 func listRefs(r *git.Repo) ([]domain.Ref, error) {
3331 var refs []domain.Ref
3432 byName := make(map[string]domain.Ref)
@@ -121,55 +119,48 @@ func peelID(r *git.Repo, id git.ObjectID) (_ git.ObjectID, ok bool, _ error) {
121119 return commit, true, nil
122120 }
123121
124-// resolveRevision tries the candidates of gitcli in the same order, which
125-// the port documents: HEAD is HEAD; a full object id is an object before it
126-// is a ref; then the branch, then the tag; then a full ref name and an
127-// abbreviated id as written. It does not use Repo.Resolve, whose order is
128-// git's own (tags before branches, no ref for a missing full id).
129-//
130-//hexcheck:ignore suspicious-business-logic-in-adapter tries the candidates of the port in its documented order, like gitcli; the rule is the port contract
122+// resolveRevision performs the lookups ordered by the domain, rather than
123+// Repo.Resolve, whose Git-specific resolution order differs from the port.
131124 func resolveRevision(r *git.Repo, rev domain.Revision) (resolved, error) {
132- s := rev.String()
133- if rev.IsHead() || s == "HEAD" {
134- if c, ok, err := peelRef(r, "HEAD"); err != nil || ok {
135- return resolved{commit: c}, err
136- }
137- return resolved{}, notFound(rev)
138- }
139- if rev.IsObjectID() {
140- id, err := git.ParseID(s)
125+ for _, candidate := range rev.Candidates() {
126+ c, ok, err := lookupRevisionCandidate(r, candidate)
141127 if err != nil {
142128 return resolved{}, err
143129 }
144- if c, ok, err := peelID(r, id); err != nil || ok {
145- return resolved{commit: c}, err
146- }
147- }
148- for _, name := range [...]string{headsPrefix + s, tagsPrefix + s} {
149- if c, ok, err := peelRef(r, name); err != nil || ok {
150- return resolved{commit: c, ref: name}, err
130+ if ok {
131+ return resolved{commit: c, ref: candidate.ResolvedRef()}, nil
151132 }
152133 }
153- switch {
154- case len(s) > 5 && s[:5] == "refs/":
155- if c, ok, err := peelRef(r, s); err != nil || ok {
156- return resolved{commit: c, ref: s}, err
134+ return resolved{}, notFound(rev)
135+}
136+
137+// lookupRevisionCandidate translates one domain candidate to the library API.
138+func lookupRevisionCandidate(r *git.Repo, candidate domain.RevisionCandidate) (git.ObjectID, bool, error) {
139+ switch candidate.Kind {
140+ case domain.RevisionCandidateHead, domain.RevisionCandidateRef:
141+ return peelRef(r, candidate.Name)
142+ case domain.RevisionCandidateObject:
143+ id, err := git.ParseID(candidate.Name)
144+ if err != nil {
145+ return git.ObjectID{}, false, err
157146 }
158- case rev.IsAbbrevID():
159- id, err := r.ResolveCommitPrefix(s)
147+ return peelID(r, id)
148+ case domain.RevisionCandidateAbbrevID:
149+ id, err := r.ResolveCommitPrefix(candidate.Name)
160150 if err == nil {
161151 // The prefix may name an annotated tag: git peels it.
162152 id, err = r.PeelToCommit(id)
163153 }
164154 switch {
165155 case err == nil:
166- return resolved{commit: id}, nil
156+ return id, true, nil
167157 case errors.Is(err, git.ErrNotFound), errors.Is(err, git.ErrAmbiguous), errors.Is(err, git.ErrWrongType):
158+ return git.ObjectID{}, false, nil
168159 default:
169- return resolved{}, err
160+ return git.ObjectID{}, false, err
170161 }
171162 }
172- return resolved{}, notFound(rev)
163+ return git.ObjectID{}, false, fmt.Errorf("unknown revision candidate kind %d", candidate.Kind)
173164 }
174165
175166 func notFound(rev domain.Revision) error {
@@ -477,8 +468,6 @@ func fillLastCommits(ctx context.Context, r *git.Repo, commit domain.ObjectID, d
477468 // The ids, the names, the emails and the message share one backing string, cut
478469 // into substrings: one allocation for the text of a commit instead of one per
479470 // field.
480-//
481-//hexcheck:ignore suspicious-business-logic-in-adapter converts ermite-git values into domain values; the limits and the orders come from domain
482471 func convertCommit(c *git.Commit) (domain.Commit, error) {
483472 if len(c.Encoding) > 0 && !isUTF8Label(c.Encoding) {
484473 return domain.Commit{}, errFallback
@@ -27,8 +27,6 @@ func domainID(id git.ObjectID) (domain.ObjectID, error) { return domain.ParseObj
27 // listRefs lists HEAD, then the branches, then the tags, like gitcli: a tag27 // listRefs lists HEAD, then the branches, then the tags, like gitcli: a tag
28 // follows to its commit when it leads to one; HEAD takes the ids of its28 // follows to its commit when it leads to one; HEAD takes the ids of its
29 // branch or tag, and only the name of an unborn branch.29 // branch or tag, and only the name of an unborn branch.
30-//
31-//hexcheck:ignore suspicious-business-logic-in-adapter converts ermite-git values into domain values; the limits and the orders come from domain
32 func listRefs(r *git.Repo) ([]domain.Ref, error) {30 func listRefs(r *git.Repo) ([]domain.Ref, error) {
33 var refs []domain.Ref31 var refs []domain.Ref
34 byName := make(map[string]domain.Ref)32 byName := make(map[string]domain.Ref)
@@ -121,55 +119,48 @@ func peelID(r *git.Repo, id git.ObjectID) (_ git.ObjectID, ok bool, _ error) {
121 return commit, true, nil119 return commit, true, nil
122 }120 }
123 121
124-// resolveRevision tries the candidates of gitcli in the same order, which122+// resolveRevision performs the lookups ordered by the domain, rather than
125-// the port documents: HEAD is HEAD; a full object id is an object before it123+// Repo.Resolve, whose Git-specific resolution order differs from the port.
126-// is a ref; then the branch, then the tag; then a full ref name and an
127-// abbreviated id as written. It does not use Repo.Resolve, whose order is
128-// git's own (tags before branches, no ref for a missing full id).
129-//
130-//hexcheck:ignore suspicious-business-logic-in-adapter tries the candidates of the port in its documented order, like gitcli; the rule is the port contract
131 func resolveRevision(r *git.Repo, rev domain.Revision) (resolved, error) {124 func resolveRevision(r *git.Repo, rev domain.Revision) (resolved, error) {
132- s := rev.String()125+ for _, candidate := range rev.Candidates() {
133- if rev.IsHead() || s == "HEAD" {126+ c, ok, err := lookupRevisionCandidate(r, candidate)
134- if c, ok, err := peelRef(r, "HEAD"); err != nil || ok {
135- return resolved{commit: c}, err
136- }
137- return resolved{}, notFound(rev)
138- }
139- if rev.IsObjectID() {
140- id, err := git.ParseID(s)
141 if err != nil {127 if err != nil {
142 return resolved{}, err128 return resolved{}, err
143 }129 }
144- if c, ok, err := peelID(r, id); err != nil || ok {130+ if ok {
145- return resolved{commit: c}, err131+ return resolved{commit: c, ref: candidate.ResolvedRef()}, nil
146- }
147- }
148- for _, name := range [...]string{headsPrefix + s, tagsPrefix + s} {
149- if c, ok, err := peelRef(r, name); err != nil || ok {
150- return resolved{commit: c, ref: name}, err
151 }132 }
152 }133 }
153- switch {134+ return resolved{}, notFound(rev)
154- case len(s) > 5 && s[:5] == "refs/":135+}
155- if c, ok, err := peelRef(r, s); err != nil || ok {136+
156- return resolved{commit: c, ref: s}, err137+// lookupRevisionCandidate translates one domain candidate to the library API.
138+func lookupRevisionCandidate(r *git.Repo, candidate domain.RevisionCandidate) (git.ObjectID, bool, error) {
139+ switch candidate.Kind {
140+ case domain.RevisionCandidateHead, domain.RevisionCandidateRef:
141+ return peelRef(r, candidate.Name)
142+ case domain.RevisionCandidateObject:
143+ id, err := git.ParseID(candidate.Name)
144+ if err != nil {
145+ return git.ObjectID{}, false, err
157 }146 }
158- case rev.IsAbbrevID():147+ return peelID(r, id)
159- id, err := r.ResolveCommitPrefix(s)148+ case domain.RevisionCandidateAbbrevID:
149+ id, err := r.ResolveCommitPrefix(candidate.Name)
160 if err == nil {150 if err == nil {
161 // The prefix may name an annotated tag: git peels it.151 // The prefix may name an annotated tag: git peels it.
162 id, err = r.PeelToCommit(id)152 id, err = r.PeelToCommit(id)
163 }153 }
164 switch {154 switch {
165 case err == nil:155 case err == nil:
166- return resolved{commit: id}, nil156+ return id, true, nil
167 case errors.Is(err, git.ErrNotFound), errors.Is(err, git.ErrAmbiguous), errors.Is(err, git.ErrWrongType):157 case errors.Is(err, git.ErrNotFound), errors.Is(err, git.ErrAmbiguous), errors.Is(err, git.ErrWrongType):
158+ return git.ObjectID{}, false, nil
168 default:159 default:
169- return resolved{}, err160+ return git.ObjectID{}, false, err
170 }161 }
171 }162 }
172- return resolved{}, notFound(rev)163+ return git.ObjectID{}, false, fmt.Errorf("unknown revision candidate kind %d", candidate.Kind)
173 }164 }
174 165
175 func notFound(rev domain.Revision) error {166 func notFound(rev domain.Revision) error {
@@ -477,8 +468,6 @@ func fillLastCommits(ctx context.Context, r *git.Repo, commit domain.ObjectID, d
477 // The ids, the names, the emails and the message share one backing string, cut468 // The ids, the names, the emails and the message share one backing string, cut
478 // into substrings: one allocation for the text of a commit instead of one per469 // into substrings: one allocation for the text of a commit instead of one per
479 // field.470 // field.
480-//
481-//hexcheck:ignore suspicious-business-logic-in-adapter converts ermite-git values into domain values; the limits and the orders come from domain
482 func convertCommit(c *git.Commit) (domain.Commit, error) {471 func convertCommit(c *git.Commit) (domain.Commit, error) {
483 if len(c.Encoding) > 0 && !isUTF8Label(c.Encoding) {472 if len(c.Encoding) > 0 && !isUTF8Label(c.Encoding) {
484 return domain.Commit{}, errFallback473 return domain.Commit{}, errFallback
M internal/adapters/ermitegit/ermitegit.go +1 -1
@@ -291,7 +291,7 @@ func (a *Adapter) ListRefs(ctx context.Context, repo domain.RepoName) ([]domain.
291291
292292 // ResolveRevision turns a revision into a commit.
293293 //
294-//hexcheck:ignore suspicious-business-logic-in-adapter routes a call by a code constant; rules and limits come from domain and ports
294+//hexcheck:ignore suspicious-business-logic-in-adapter selects the configured CLI or library backend, with a separate route for abbreviated IDs; resolveRevision calls domain.Revision.Candidates for resolution order
295295 func (a *Adapter) ResolveRevision(ctx context.Context, repo domain.RepoName, rev domain.Revision) (domain.ResolvedRevision, error) {
296296 cli := func() (domain.ResolvedRevision, error) { return a.cli.ResolveRevision(ctx, repo, rev) }
297297 route := a.routes.ResolveRevision
@@ -291,7 +291,7 @@ func (a *Adapter) ListRefs(ctx context.Context, repo domain.RepoName) ([]domain.
291 291
292 // ResolveRevision turns a revision into a commit.292 // ResolveRevision turns a revision into a commit.
293 //293 //
294-//hexcheck:ignore suspicious-business-logic-in-adapter routes a call by a code constant; rules and limits come from domain and ports294+//hexcheck:ignore suspicious-business-logic-in-adapter selects the configured CLI or library backend, with a separate route for abbreviated IDs; resolveRevision calls domain.Revision.Candidates for resolution order
295 func (a *Adapter) ResolveRevision(ctx context.Context, repo domain.RepoName, rev domain.Revision) (domain.ResolvedRevision, error) {295 func (a *Adapter) ResolveRevision(ctx context.Context, repo domain.RepoName, rev domain.Revision) (domain.ResolvedRevision, error) {
296 cli := func() (domain.ResolvedRevision, error) { return a.cli.ResolveRevision(ctx, repo, rev) }296 cli := func() (domain.ResolvedRevision, error) { return a.cli.ResolveRevision(ctx, repo, rev) }
297 route := a.routes.ResolveRevision297 route := a.routes.ResolveRevision
M internal/adapters/gitcli/archive.go +0 -2
@@ -28,8 +28,6 @@ var _ ports.GitArchive = (*Git)(nil)
2828 // not by a gzip binary. A client that goes away ends ctx, which kills git.
2929 // Nothing is written to w when the commit does not exist
3030 // (domain.ErrNotFound).
31-//
32-//hexcheck:ignore suspicious-business-logic-in-adapter runs git archive and compresses its output: a process concern, access and naming are the use case's
3331 func (g *Git) Archive(ctx context.Context, repo domain.RepoName, commit domain.ObjectID, prefix string, w io.Writer) error {
3432 dir, err := g.repoDir(repo)
3533 if err != nil {
@@ -28,8 +28,6 @@ var _ ports.GitArchive = (*Git)(nil)
28 // not by a gzip binary. A client that goes away ends ctx, which kills git.28 // not by a gzip binary. A client that goes away ends ctx, which kills git.
29 // Nothing is written to w when the commit does not exist29 // Nothing is written to w when the commit does not exist
30 // (domain.ErrNotFound).30 // (domain.ErrNotFound).
31-//
32-//hexcheck:ignore suspicious-business-logic-in-adapter runs git archive and compresses its output: a process concern, access and naming are the use case's
33 func (g *Git) Archive(ctx context.Context, repo domain.RepoName, commit domain.ObjectID, prefix string, w io.Writer) error {31 func (g *Git) Archive(ctx context.Context, repo domain.RepoName, commit domain.ObjectID, prefix string, w io.Writer) error {
34 dir, err := g.repoDir(repo)32 dir, err := g.repoDir(repo)
35 if err != nil {33 if err != nil {
M internal/adapters/gitcli/changed.go +0 -4
@@ -21,8 +21,6 @@ const emptyTree = "4b825dc642cb6eb9a060e54bf8d69288fbee4904"
2121 // renames not detected (a rename lists both paths, as GitHub's push filters
2222 // see it), and stops reading after max paths: truncated tells there were more.
2323 // 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
2624 func (g *Git) ChangedPaths(ctx context.Context, repo domain.RepoName, base, head domain.ObjectID, limit int) ([]string, bool, error) {
2725 if limit < 1 || head.IsZero() {
2826 return nil, false, fmt.Errorf("changed paths: %w: limit %d", domain.ErrInvalid, limit)
@@ -63,8 +61,6 @@ func (g *Git) ChangedPaths(ctx context.Context, repo domain.RepoName, base, head
6361 // (refs/heads/*), newest first, at most limit. It runs
6462 // `git log --not --branches --not <head>`: tags do not count, so the commit of
6563 // another tag that no branch has is listed.
66-//
67-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain values; the limit comes from the port
6864 func (g *Git) CommitsOffBranches(ctx context.Context, repo domain.RepoName, head domain.ObjectID, limit int) ([]domain.Commit, bool, error) {
6965 if head.IsZero() {
7066 return nil, false, fmt.Errorf("commits off branches: %w: no head", domain.ErrInvalid)
@@ -21,8 +21,6 @@ const emptyTree = "4b825dc642cb6eb9a060e54bf8d69288fbee4904"
21 // renames not detected (a rename lists both paths, as GitHub's push filters21 // 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.22 // see it), and stops reading after max paths: truncated tells there were more.
23 // A zero base diffs against the empty tree.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) {24 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() {25 if limit < 1 || head.IsZero() {
28 return nil, false, fmt.Errorf("changed paths: %w: limit %d", domain.ErrInvalid, limit)26 return nil, false, fmt.Errorf("changed paths: %w: limit %d", domain.ErrInvalid, limit)
@@ -63,8 +61,6 @@ func (g *Git) ChangedPaths(ctx context.Context, repo domain.RepoName, base, head
63 // (refs/heads/*), newest first, at most limit. It runs61 // (refs/heads/*), newest first, at most limit. It runs
64 // `git log --not --branches --not <head>`: tags do not count, so the commit of62 // `git log --not --branches --not <head>`: tags do not count, so the commit of
65 // another tag that no branch has is listed.63 // another tag that no branch has is listed.
66-//
67-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain values; the limit comes from the port
68 func (g *Git) CommitsOffBranches(ctx context.Context, repo domain.RepoName, head domain.ObjectID, limit int) ([]domain.Commit, bool, error) {64 func (g *Git) CommitsOffBranches(ctx context.Context, repo domain.RepoName, head domain.ObjectID, limit int) ([]domain.Commit, bool, error) {
69 if head.IsZero() {65 if head.IsZero() {
70 return nil, false, fmt.Errorf("commits off branches: %w: no head", domain.ErrInvalid)66 return nil, false, fmt.Errorf("commits off branches: %w: no head", domain.ErrInvalid)
M internal/adapters/gitcli/commitobjects.go +11 -15
@@ -19,15 +19,10 @@ var _ ports.CommitObjectReader = (*Git)(nil)
1919 // that would take the total over domain.MaxCommitObjectsBytes (known from
2020 // `info`, before any content is read, so the stream stays consistent).
2121 //
22-//hexcheck:ignore suspicious-business-logic-in-adapter applies the limits of the port and of the domain
22+//hexcheck:ignore suspicious-business-logic-in-adapter streams object headers and bytes; eligibility, deduplication and byte limits are decided by domain.CommitObjectBudget, request validation by domain.ValidateCommitObjects
2323 func (g *Git) ReadCommitObjects(ctx context.Context, repo domain.RepoName, ids []domain.ObjectID) (map[domain.ObjectID][]byte, error) {
24- if len(ids) > ports.MaxCommitLimit {
25- return nil, fmt.Errorf("%w: %d commit objects asked, at most %d", domain.ErrInvalid, len(ids), ports.MaxCommitLimit)
26- }
27- for _, id := range ids {
28- if id.IsZero() {
29- return nil, fmt.Errorf("%w: empty commit id", domain.ErrInvalidObjectID)
30- }
24+ if err := domain.ValidateCommitObjects(ids, ports.MaxCommitLimit); err != nil {
25+ return nil, err
3126 }
3227 dir, err := g.repoDir(repo)
3328 if err != nil {
@@ -36,12 +31,12 @@ func (g *Git) ReadCommitObjects(ctx context.Context, repo domain.RepoName, ids [
3631 out := make(map[domain.ObjectID][]byte, len(ids))
3732 err = g.cat.do(ctx, dir, func(c *catConn) error {
3833 clear(out) // a retry on a fresh process starts over
39- var total int64
34+ var budget domain.CommitObjectBudget
4035 for _, id := range ids {
4136 if err := ctx.Err(); err != nil {
4237 return err
4338 }
44- if _, done := out[id]; done {
39+ if budget.Seen(id) {
4540 continue
4641 }
4742 h, err := c.info(id.String())
@@ -51,18 +46,19 @@ func (g *Git) ReadCommitObjects(ctx context.Context, repo domain.RepoName, ids [
5146 if err != nil {
5247 return err
5348 }
54- if h.Type != "commit" || h.Size > domain.MaxCommitObjectBytes {
55- continue
56- }
57- if total+h.Size > domain.MaxCommitObjectsBytes {
49+ take, stop := budget.Admit(id, h.Type == "commit", h.Size)
50+ if stop {
5851 break
5952 }
53+ if !take {
54+ continue
55+ }
6056 _, data, err := c.contents(id.String(), domain.MaxCommitObjectBytes)
6157 if err != nil {
6258 return err
6359 }
6460 out[id] = data
65- total += int64(len(data))
61+ budget.Charge(id, int64(len(data)))
6662 }
6763 return nil
6864 })
@@ -19,15 +19,10 @@ var _ ports.CommitObjectReader = (*Git)(nil)
19 // that would take the total over domain.MaxCommitObjectsBytes (known from19 // that would take the total over domain.MaxCommitObjectsBytes (known from
20 // `info`, before any content is read, so the stream stays consistent).20 // `info`, before any content is read, so the stream stays consistent).
21 //21 //
22-//hexcheck:ignore suspicious-business-logic-in-adapter applies the limits of the port and of the domain22+//hexcheck:ignore suspicious-business-logic-in-adapter streams object headers and bytes; eligibility, deduplication and byte limits are decided by domain.CommitObjectBudget, request validation by domain.ValidateCommitObjects
23 func (g *Git) ReadCommitObjects(ctx context.Context, repo domain.RepoName, ids []domain.ObjectID) (map[domain.ObjectID][]byte, error) {23 func (g *Git) ReadCommitObjects(ctx context.Context, repo domain.RepoName, ids []domain.ObjectID) (map[domain.ObjectID][]byte, error) {
24- if len(ids) > ports.MaxCommitLimit {24+ if err := domain.ValidateCommitObjects(ids, ports.MaxCommitLimit); err != nil {
25- return nil, fmt.Errorf("%w: %d commit objects asked, at most %d", domain.ErrInvalid, len(ids), ports.MaxCommitLimit)25+ return nil, err
26- }
27- for _, id := range ids {
28- if id.IsZero() {
29- return nil, fmt.Errorf("%w: empty commit id", domain.ErrInvalidObjectID)
30- }
31 }26 }
32 dir, err := g.repoDir(repo)27 dir, err := g.repoDir(repo)
33 if err != nil {28 if err != nil {
@@ -36,12 +31,12 @@ func (g *Git) ReadCommitObjects(ctx context.Context, repo domain.RepoName, ids [
36 out := make(map[domain.ObjectID][]byte, len(ids))31 out := make(map[domain.ObjectID][]byte, len(ids))
37 err = g.cat.do(ctx, dir, func(c *catConn) error {32 err = g.cat.do(ctx, dir, func(c *catConn) error {
38 clear(out) // a retry on a fresh process starts over33 clear(out) // a retry on a fresh process starts over
39- var total int6434+ var budget domain.CommitObjectBudget
40 for _, id := range ids {35 for _, id := range ids {
41 if err := ctx.Err(); err != nil {36 if err := ctx.Err(); err != nil {
42 return err37 return err
43 }38 }
44- if _, done := out[id]; done {39+ if budget.Seen(id) {
45 continue40 continue
46 }41 }
47 h, err := c.info(id.String())42 h, err := c.info(id.String())
@@ -51,18 +46,19 @@ func (g *Git) ReadCommitObjects(ctx context.Context, repo domain.RepoName, ids [
51 if err != nil {46 if err != nil {
52 return err47 return err
53 }48 }
54- if h.Type != "commit" || h.Size > domain.MaxCommitObjectBytes {49+ take, stop := budget.Admit(id, h.Type == "commit", h.Size)
55- continue50+ if stop {
56- }
57- if total+h.Size > domain.MaxCommitObjectsBytes {
58 break51 break
59 }52 }
53+ if !take {
54+ continue
55+ }
60 _, data, err := c.contents(id.String(), domain.MaxCommitObjectBytes)56 _, data, err := c.contents(id.String(), domain.MaxCommitObjectBytes)
61 if err != nil {57 if err != nil {
62 return err58 return err
63 }59 }
64 out[id] = data60 out[id] = data
65- total += int64(len(data))61+ budget.Charge(id, int64(len(data)))
66 }62 }
67 return nil63 return nil
68 })64 })
M internal/adapters/gitcli/diff.go +15 -47
@@ -32,8 +32,6 @@ const maxDiffLine = 64 << 10
3232 // Statistics come from the line counts, binary files from their "-" counts.
3333 // The patch is parsed as it streams: nothing beyond the limits is kept, and
3434 // git is stopped once nothing more is needed.
35-//
36-//hexcheck:ignore suspicious-business-logic-in-adapter parses git diff-tree output into the domain diff, applying the limits the port passes in
3735 func (g *Git) GetCommit(ctx context.Context, repo domain.RepoName, rev domain.Revision, limits domain.DiffLimits) (domain.CommitDetail, error) {
3836 dir, err := g.repoDir(repo)
3937 if err != nil {
@@ -91,7 +89,7 @@ type rawEntry struct {
9189
9290 // readDiff parses the output of diff-tree --raw --numstat -p.
9391 //
94-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain values, applying the limits the port passes in
92+//hexcheck:ignore suspicious-business-logic-in-adapter streams raw, numstat and patch sections; domain.DiffBudget decides window and hunk eligibility and accounts retained lines and bytes; adapter only skips sections and stops the process
9593 func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.DiffStats, error) {
9694 entries, err := readRaw(r)
9795 if err != nil {
@@ -107,23 +105,17 @@ func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.Di
107105 stats.Deletions += entries[i].del
108106 }
109107 var diff domain.Diff
110- if limits.MaxFiles > 0 && len(entries) > limits.MaxFiles {
111- entries = entries[:limits.MaxFiles]
112- diff.Truncated = true
113- }
114- // Only the files of the window [from, to) get their hunks; the others are
115- // listed as they are, neither truncated nor counted in the budgets.
116- from, to := max(limits.From, 0), len(entries)
117- if limits.To > 0 {
118- to = min(limits.To, len(entries))
119- }
108+ budget := domain.NewDiffBudget(limits)
109+ kept, from, to, truncated := budget.Window(len(entries))
110+ entries = entries[:kept]
111+ diff.Truncated = truncated
120112
121113 // The patch holds one section per file, two for a type change, in the
122114 // order of the entries. Entries without lines to show (binary, pure
123115 // rename, mode change) have a section without hunks.
124116 lastRead := -1
125117 for i := from; i < to; i++ {
126- if wantsHunks(&entries[i], limits, limits.MaxTotalLines, limits.MaxTotalBytes) {
118+ if budget.WantsHunks(entries[i].file.Binary, entries[i].added+entries[i].del) {
127119 lastRead = i
128120 }
129121 }
@@ -133,8 +125,7 @@ func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.Di
133125 _, _ = r.Discard(1)
134126 }
135127 }
136- pr := &patchReader{lr: newLineReader(r, maxDiffLine), bytesLeft: limits.MaxTotalBytes}
137- budget := limits.MaxTotalLines
128+ pr := &patchReader{lr: newLineReader(r, maxDiffLine), budget: budget}
138129 diff.Files = make([]domain.FileDiff, 0, len(entries))
139130 for i := range entries {
140131 e := &entries[i]
@@ -144,21 +135,19 @@ func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.Di
144135 }
145136 inWindow := i >= from && i < to
146137 switch {
147- case !inWindow, !changedLines(e):
138+ case !inWindow, !pr.budget.ChangedLines(e.file.Binary, e.added+e.del):
148139 // Nothing to show.
149- case !wantsHunks(e, limits, budget, pr.bytesLeft):
140+ case !pr.budget.WantsHunks(e.file.Binary, e.added+e.del):
150141 e.file.Truncated = true
151142 default:
152- bytesBefore := pr.bytesLeft
153- hunks, lines, truncated, err := pr.file(sections, min(limits.MaxFileLines, budget))
143+ before := pr.budget
144+ hunks, _, truncated, err := pr.file(sections, pr.budget.FileLines())
154145 if err != nil {
155146 return domain.Diff{}, domain.DiffStats{}, err
156147 }
157148 e.file.Hunks, e.file.Truncated = hunks, truncated
158149 if truncated {
159- pr.bytesLeft = bytesBefore // nothing of this file is kept
160- } else {
161- budget -= lines
150+ pr.budget = before // nothing of this file is kept
162151 }
163152 sections = 0
164153 }
@@ -177,20 +166,7 @@ func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.Di
177166 return diff, stats, nil
178167 }
179168
180-// changedLines reports whether the file has text lines in its patch.
181-func changedLines(e *rawEntry) bool { return !e.file.Binary && e.added+e.del > 0 }
182-
183-// wantsHunks reports whether the lines of e can still be shown: the changed
184-// lines alone, a lower bound of the hunk lines, must fit the per-file limit
185-// and what remains of the total, and some text bytes must remain.
186-func wantsHunks(e *rawEntry, limits domain.DiffLimits, linesLeft, bytesLeft int) bool {
187- n := e.added + e.del
188- return changedLines(e) && n <= limits.MaxFileLines && n <= linesLeft && bytesLeft > 0
189-}
190-
191169 // readRaw reads the `--raw -z` records, which start with ':'.
192-//
193-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain values
194170 func readRaw(r *bufio.Reader) ([]rawEntry, error) {
195171 var entries []rawEntry
196172 for {
@@ -241,8 +217,6 @@ func unexpectedEOF(err error) error {
241217
242218 // parseRawMeta parses ":<old mode> <new mode> <old id> <new id> <status>" and
243219 // returns the number of paths that follow.
244-//
245-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain values
246220 func parseRawMeta(meta string) (rawEntry, int, error) {
247221 f := strings.Fields(strings.TrimPrefix(meta, ":"))
248222 if len(f) != 5 || f[4] == "" {
@@ -296,8 +270,6 @@ func parseRawMeta(meta string) (rawEntry, int, error) {
296270
297271 // readNumstat reads one `--numstat -z` record per entry. A binary file has
298272 // "-" counts.
299-//
300-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain values
301273 func readNumstat(r *bufio.Reader, entries []rawEntry) error {
302274 for i := range entries {
303275 tok, err := readToken(r)
@@ -341,7 +313,7 @@ type patchReader struct {
341313 eof bool
342314 cut bool // the last line read was cut
343315
344- bytesLeft int // text bytes still allowed in the diff
316+ budget domain.DiffBudget
345317 }
346318
347319 const sectionMarker = "diff --git "
@@ -434,8 +406,6 @@ func (p *patchReader) file(sections, max int) (hunks []domain.Hunk, lines int, t
434406
435407 // section reads the hunks of the current section up to the next marker, which
436408 // stays pending. It gives up (truncated) after more than max lines.
437-//
438-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain values, applying the limits the port passes in
439409 func (p *patchReader) section(max int) (hunks []domain.Hunk, lines int, truncated bool, err error) {
440410 var cur *domain.Hunk
441411 var oldN, newN int
@@ -490,8 +460,8 @@ func (p *patchReader) section(max int) (hunks []domain.Hunk, lines int, truncate
490460 continue
491461 }
492462 lines++
493- p.bytesLeft -= len(text)
494- if lines > max || p.bytesLeft < 0 {
463+ fits := p.budget.ChargeLine(len(text))
464+ if lines > max || !fits {
495465 return nil, lines, true, nil
496466 }
497467 cur.Lines = append(cur.Lines, l)
@@ -499,8 +469,6 @@ func (p *patchReader) section(max int) (hunks []domain.Hunk, lines int, truncate
499469 }
500470
501471 // parseHunkHeader parses "@@ -a[,b] +c[,d] @@[ header]".
502-//
503-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain values
504472 func parseHunkHeader(s string) (domain.Hunk, error) {
505473 bad := func() (domain.Hunk, error) {
506474 return domain.Hunk{}, fmt.Errorf("git diff-tree: unexpected hunk header %q", truncateText(s))
@@ -32,8 +32,6 @@ const maxDiffLine = 64 << 10
32 // Statistics come from the line counts, binary files from their "-" counts.32 // Statistics come from the line counts, binary files from their "-" counts.
33 // The patch is parsed as it streams: nothing beyond the limits is kept, and33 // The patch is parsed as it streams: nothing beyond the limits is kept, and
34 // git is stopped once nothing more is needed.34 // git is stopped once nothing more is needed.
35-//
36-//hexcheck:ignore suspicious-business-logic-in-adapter parses git diff-tree output into the domain diff, applying the limits the port passes in
37 func (g *Git) GetCommit(ctx context.Context, repo domain.RepoName, rev domain.Revision, limits domain.DiffLimits) (domain.CommitDetail, error) {35 func (g *Git) GetCommit(ctx context.Context, repo domain.RepoName, rev domain.Revision, limits domain.DiffLimits) (domain.CommitDetail, error) {
38 dir, err := g.repoDir(repo)36 dir, err := g.repoDir(repo)
39 if err != nil {37 if err != nil {
@@ -91,7 +89,7 @@ type rawEntry struct {
91 89
92 // readDiff parses the output of diff-tree --raw --numstat -p.90 // readDiff parses the output of diff-tree --raw --numstat -p.
93 //91 //
94-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain values, applying the limits the port passes in92+//hexcheck:ignore suspicious-business-logic-in-adapter streams raw, numstat and patch sections; domain.DiffBudget decides window and hunk eligibility and accounts retained lines and bytes; adapter only skips sections and stops the process
95 func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.DiffStats, error) {93 func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.DiffStats, error) {
96 entries, err := readRaw(r)94 entries, err := readRaw(r)
97 if err != nil {95 if err != nil {
@@ -107,23 +105,17 @@ func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.Di
107 stats.Deletions += entries[i].del105 stats.Deletions += entries[i].del
108 }106 }
109 var diff domain.Diff107 var diff domain.Diff
110- if limits.MaxFiles > 0 && len(entries) > limits.MaxFiles {108+ budget := domain.NewDiffBudget(limits)
111- entries = entries[:limits.MaxFiles]109+ kept, from, to, truncated := budget.Window(len(entries))
112- diff.Truncated = true110+ entries = entries[:kept]
113- }111+ diff.Truncated = truncated
114- // Only the files of the window [from, to) get their hunks; the others are
115- // listed as they are, neither truncated nor counted in the budgets.
116- from, to := max(limits.From, 0), len(entries)
117- if limits.To > 0 {
118- to = min(limits.To, len(entries))
119- }
120 112
121 // The patch holds one section per file, two for a type change, in the113 // The patch holds one section per file, two for a type change, in the
122 // order of the entries. Entries without lines to show (binary, pure114 // order of the entries. Entries without lines to show (binary, pure
123 // rename, mode change) have a section without hunks.115 // rename, mode change) have a section without hunks.
124 lastRead := -1116 lastRead := -1
125 for i := from; i < to; i++ {117 for i := from; i < to; i++ {
126- if wantsHunks(&entries[i], limits, limits.MaxTotalLines, limits.MaxTotalBytes) {118+ if budget.WantsHunks(entries[i].file.Binary, entries[i].added+entries[i].del) {
127 lastRead = i119 lastRead = i
128 }120 }
129 }121 }
@@ -133,8 +125,7 @@ func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.Di
133 _, _ = r.Discard(1)125 _, _ = r.Discard(1)
134 }126 }
135 }127 }
136- pr := &patchReader{lr: newLineReader(r, maxDiffLine), bytesLeft: limits.MaxTotalBytes}128+ pr := &patchReader{lr: newLineReader(r, maxDiffLine), budget: budget}
137- budget := limits.MaxTotalLines
138 diff.Files = make([]domain.FileDiff, 0, len(entries))129 diff.Files = make([]domain.FileDiff, 0, len(entries))
139 for i := range entries {130 for i := range entries {
140 e := &entries[i]131 e := &entries[i]
@@ -144,21 +135,19 @@ func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.Di
144 }135 }
145 inWindow := i >= from && i < to136 inWindow := i >= from && i < to
146 switch {137 switch {
147- case !inWindow, !changedLines(e):138+ case !inWindow, !pr.budget.ChangedLines(e.file.Binary, e.added+e.del):
148 // Nothing to show.139 // Nothing to show.
149- case !wantsHunks(e, limits, budget, pr.bytesLeft):140+ case !pr.budget.WantsHunks(e.file.Binary, e.added+e.del):
150 e.file.Truncated = true141 e.file.Truncated = true
151 default:142 default:
152- bytesBefore := pr.bytesLeft143+ before := pr.budget
153- hunks, lines, truncated, err := pr.file(sections, min(limits.MaxFileLines, budget))144+ hunks, _, truncated, err := pr.file(sections, pr.budget.FileLines())
154 if err != nil {145 if err != nil {
155 return domain.Diff{}, domain.DiffStats{}, err146 return domain.Diff{}, domain.DiffStats{}, err
156 }147 }
157 e.file.Hunks, e.file.Truncated = hunks, truncated148 e.file.Hunks, e.file.Truncated = hunks, truncated
158 if truncated {149 if truncated {
159- pr.bytesLeft = bytesBefore // nothing of this file is kept150+ pr.budget = before // nothing of this file is kept
160- } else {
161- budget -= lines
162 }151 }
163 sections = 0152 sections = 0
164 }153 }
@@ -177,20 +166,7 @@ func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.Di
177 return diff, stats, nil166 return diff, stats, nil
178 }167 }
179 168
180-// changedLines reports whether the file has text lines in its patch.
181-func changedLines(e *rawEntry) bool { return !e.file.Binary && e.added+e.del > 0 }
182-
183-// wantsHunks reports whether the lines of e can still be shown: the changed
184-// lines alone, a lower bound of the hunk lines, must fit the per-file limit
185-// and what remains of the total, and some text bytes must remain.
186-func wantsHunks(e *rawEntry, limits domain.DiffLimits, linesLeft, bytesLeft int) bool {
187- n := e.added + e.del
188- return changedLines(e) && n <= limits.MaxFileLines && n <= linesLeft && bytesLeft > 0
189-}
190-
191 // readRaw reads the `--raw -z` records, which start with ':'.169 // readRaw reads the `--raw -z` records, which start with ':'.
192-//
193-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain values
194 func readRaw(r *bufio.Reader) ([]rawEntry, error) {170 func readRaw(r *bufio.Reader) ([]rawEntry, error) {
195 var entries []rawEntry171 var entries []rawEntry
196 for {172 for {
@@ -241,8 +217,6 @@ func unexpectedEOF(err error) error {
241 217
242 // parseRawMeta parses ":<old mode> <new mode> <old id> <new id> <status>" and218 // parseRawMeta parses ":<old mode> <new mode> <old id> <new id> <status>" and
243 // returns the number of paths that follow.219 // returns the number of paths that follow.
244-//
245-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain values
246 func parseRawMeta(meta string) (rawEntry, int, error) {220 func parseRawMeta(meta string) (rawEntry, int, error) {
247 f := strings.Fields(strings.TrimPrefix(meta, ":"))221 f := strings.Fields(strings.TrimPrefix(meta, ":"))
248 if len(f) != 5 || f[4] == "" {222 if len(f) != 5 || f[4] == "" {
@@ -296,8 +270,6 @@ func parseRawMeta(meta string) (rawEntry, int, error) {
296 270
297 // readNumstat reads one `--numstat -z` record per entry. A binary file has271 // readNumstat reads one `--numstat -z` record per entry. A binary file has
298 // "-" counts.272 // "-" counts.
299-//
300-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain values
301 func readNumstat(r *bufio.Reader, entries []rawEntry) error {273 func readNumstat(r *bufio.Reader, entries []rawEntry) error {
302 for i := range entries {274 for i := range entries {
303 tok, err := readToken(r)275 tok, err := readToken(r)
@@ -341,7 +313,7 @@ type patchReader struct {
341 eof bool313 eof bool
342 cut bool // the last line read was cut314 cut bool // the last line read was cut
343 315
344- bytesLeft int // text bytes still allowed in the diff316+ budget domain.DiffBudget
345 }317 }
346 318
347 const sectionMarker = "diff --git "319 const sectionMarker = "diff --git "
@@ -434,8 +406,6 @@ func (p *patchReader) file(sections, max int) (hunks []domain.Hunk, lines int, t
434 406
435 // section reads the hunks of the current section up to the next marker, which407 // section reads the hunks of the current section up to the next marker, which
436 // stays pending. It gives up (truncated) after more than max lines.408 // stays pending. It gives up (truncated) after more than max lines.
437-//
438-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain values, applying the limits the port passes in
439 func (p *patchReader) section(max int) (hunks []domain.Hunk, lines int, truncated bool, err error) {409 func (p *patchReader) section(max int) (hunks []domain.Hunk, lines int, truncated bool, err error) {
440 var cur *domain.Hunk410 var cur *domain.Hunk
441 var oldN, newN int411 var oldN, newN int
@@ -490,8 +460,8 @@ func (p *patchReader) section(max int) (hunks []domain.Hunk, lines int, truncate
490 continue460 continue
491 }461 }
492 lines++462 lines++
493- p.bytesLeft -= len(text)463+ fits := p.budget.ChargeLine(len(text))
494- if lines > max || p.bytesLeft < 0 {464+ if lines > max || !fits {
495 return nil, lines, true, nil465 return nil, lines, true, nil
496 }466 }
497 cur.Lines = append(cur.Lines, l)467 cur.Lines = append(cur.Lines, l)
@@ -499,8 +469,6 @@ func (p *patchReader) section(max int) (hunks []domain.Hunk, lines int, truncate
499 }469 }
500 470
501 // parseHunkHeader parses "@@ -a[,b] +c[,d] @@[ header]".471 // parseHunkHeader parses "@@ -a[,b] +c[,d] @@[ header]".
502-//
503-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain values
504 func parseHunkHeader(s string) (domain.Hunk, error) {472 func parseHunkHeader(s string) (domain.Hunk, error) {
505 bad := func() (domain.Hunk, error) {473 bad := func() (domain.Hunk, error) {
506 return domain.Hunk{}, fmt.Errorf("git diff-tree: unexpected hunk header %q", truncateText(s))474 return domain.Hunk{}, fmt.Errorf("git diff-tree: unexpected hunk header %q", truncateText(s))
M internal/adapters/gitcli/export_test.go +0 -6
@@ -110,15 +110,9 @@ func (g *Git) CatFileWaiting() int {
110110 return n
111111 }
112112
113-// MaxPushedCommits is the bound of the commits of a push, for the tests.
114-const MaxPushedCommits = maxPushedCommits
115-
116113 // ParseRevList reads rev-list output with the bounds of the guard replaced, and
117114 // returns the number of commits kept.
118115 func ParseRevList(r io.Reader, maxCommits, maxIDs int) (int, error) {
119116 c, err := parseRevList(bufio.NewReaderSize(r, 64*1024), maxCommits, maxIDs)
120117 return len(c), err
121118 }
122-
123-// MaxPushedObjects is the bound of the ids of the commits of a push.
124-const MaxPushedObjects = maxPushedObjects
@@ -110,15 +110,9 @@ func (g *Git) CatFileWaiting() int {
110 return n110 return n
111 }111 }
112 112
113-// MaxPushedCommits is the bound of the commits of a push, for the tests.
114-const MaxPushedCommits = maxPushedCommits
115-
116 // ParseRevList reads rev-list output with the bounds of the guard replaced, and113 // ParseRevList reads rev-list output with the bounds of the guard replaced, and
117 // returns the number of commits kept.114 // returns the number of commits kept.
118 func ParseRevList(r io.Reader, maxCommits, maxIDs int) (int, error) {115 func ParseRevList(r io.Reader, maxCommits, maxIDs int) (int, error) {
119 c, err := parseRevList(bufio.NewReaderSize(r, 64*1024), maxCommits, maxIDs)116 c, err := parseRevList(bufio.NewReaderSize(r, 64*1024), maxCommits, maxIDs)
120 return len(c), err117 return len(c), err
121 }118 }
122-
123-// MaxPushedObjects is the bound of the ids of the commits of a push.
124-const MaxPushedObjects = maxPushedObjects
M internal/adapters/gitcli/merge.go +0 -16
@@ -39,8 +39,6 @@ func parseID(out string) (domain.ObjectID, error) {
3939 }
4040
4141 // BranchHead resolves refs/heads/<branch>^{commit} with `cat-file`.
42-//
43-//hexcheck:ignore suspicious-business-logic-in-adapter maps a branch name to a git object name
4442 func (g *Git) BranchHead(ctx context.Context, repo domain.RepoName, branch string) (domain.ObjectID, error) {
4543 dir, err := g.repoDir(repo)
4644 if err != nil {
@@ -95,8 +93,6 @@ func (g *Git) HasCommit(ctx context.Context, repo domain.RepoName, id domain.Obj
9593 }
9694
9795 // MergeBase runs `git merge-base`, which exits 1 when there is none.
98-//
99-//hexcheck:ignore suspicious-business-logic-in-adapter maps git exit codes to the domain not found error
10096 func (g *Git) MergeBase(ctx context.Context, repo domain.RepoName, a, b domain.ObjectID) (domain.ObjectID, error) {
10197 dir, err := g.repoDir(repo)
10298 if err != nil {
@@ -129,8 +125,6 @@ func (g *Git) IsAncestor(ctx context.Context, repo domain.RepoName, ancestor, de
129125 }
130126
131127 // ListRange runs `git log <head> ^<exclude>`.
132-//
133-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain values; the limit comes from the port
134128 func (g *Git) ListRange(ctx context.Context, repo domain.RepoName, exclude, head domain.ObjectID, limit int) ([]domain.Commit, bool, error) {
135129 var excluded []string
136130 if !exclude.IsZero() {
@@ -142,8 +136,6 @@ func (g *Git) ListRange(ctx context.Context, repo domain.RepoName, exclude, head
142136 // logCommits runs `git log` for the commits reachable from head: revArgs go
143137 // before the head (options such as --not), excluded after it. Newest first,
144138 // at most limit; more says there were others.
145-//
146-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain values; the limit comes from the port
147139 func (g *Git) logCommits(ctx context.Context, repo domain.RepoName, head domain.ObjectID, limit int, revArgs, excluded []string) ([]domain.Commit, bool, error) {
148140 if limit < 1 || limit > ports.MaxCommitLimit {
149141 return nil, false, fmt.Errorf("list range: %w: limit %d", domain.ErrInvalidPageSize, limit)
@@ -183,8 +175,6 @@ func (g *Git) logCommits(ctx context.Context, repo domain.RepoName, head domain.
183175 }
184176
185177 // DiffCommits reads the diff between two commits like GetCommit does.
186-//
187-//hexcheck:ignore suspicious-business-logic-in-adapter reads a git diff into the domain diff with the limits the port passes in
188178 func (g *Git) DiffCommits(ctx context.Context, repo domain.RepoName, base, head domain.ObjectID, limits domain.DiffLimits) (domain.DiffStats, domain.Diff, error) {
189179 dir, err := g.repoDir(repo)
190180 if err != nil {
@@ -198,8 +188,6 @@ func (g *Git) DiffCommits(ctx context.Context, repo domain.RepoName, base, head
198188 }
199189
200190 // CommitTree runs `git rev-parse <commit>^{tree}`.
201-//
202-//hexcheck:ignore suspicious-business-logic-in-adapter parses the id git printed
203191 func (g *Git) CommitTree(ctx context.Context, repo domain.RepoName, id domain.ObjectID) (domain.ObjectID, error) {
204192 dir, err := g.repoDir(repo)
205193 if err != nil {
@@ -229,8 +217,6 @@ func (g *Git) WriteCommit(ctx context.Context, repo domain.RepoName, object []by
229217 // UpdateBranch runs `git update-ref refs/heads/<branch> <new> <old>`, which
230218 // takes the ref lock and refuses when the ref is not at old. A failure is
231219 // told from a moved branch by reading the branch again.
232-//
233-//hexcheck:ignore suspicious-business-logic-in-adapter maps a refused update-ref to the conflict error of the port
234220 func (g *Git) UpdateBranch(ctx context.Context, repo domain.RepoName, branch string, old, newID domain.ObjectID) error {
235221 dir, err := g.repoDir(repo)
236222 if err != nil {
@@ -252,8 +238,6 @@ func (g *Git) UpdateBranch(ctx context.Context, repo domain.RepoName, branch str
252238 // DeleteBranch runs `git update-ref -d refs/heads/<branch> <old>`, which takes
253239 // the ref lock and refuses when the ref is not at old. A failure is told from
254240 // a moved or deleted branch by reading the branch again.
255-//
256-//hexcheck:ignore suspicious-business-logic-in-adapter maps a refused update-ref to the conflict error of the port
257241 func (g *Git) DeleteBranch(ctx context.Context, repo domain.RepoName, branch string, old domain.ObjectID) error {
258242 dir, err := g.repoDir(repo)
259243 if err != nil {
@@ -39,8 +39,6 @@ func parseID(out string) (domain.ObjectID, error) {
39 }39 }
40 40
41 // BranchHead resolves refs/heads/<branch>^{commit} with `cat-file`.41 // BranchHead resolves refs/heads/<branch>^{commit} with `cat-file`.
42-//
43-//hexcheck:ignore suspicious-business-logic-in-adapter maps a branch name to a git object name
44 func (g *Git) BranchHead(ctx context.Context, repo domain.RepoName, branch string) (domain.ObjectID, error) {42 func (g *Git) BranchHead(ctx context.Context, repo domain.RepoName, branch string) (domain.ObjectID, error) {
45 dir, err := g.repoDir(repo)43 dir, err := g.repoDir(repo)
46 if err != nil {44 if err != nil {
@@ -95,8 +93,6 @@ func (g *Git) HasCommit(ctx context.Context, repo domain.RepoName, id domain.Obj
95 }93 }
96 94
97 // MergeBase runs `git merge-base`, which exits 1 when there is none.95 // MergeBase runs `git merge-base`, which exits 1 when there is none.
98-//
99-//hexcheck:ignore suspicious-business-logic-in-adapter maps git exit codes to the domain not found error
100 func (g *Git) MergeBase(ctx context.Context, repo domain.RepoName, a, b domain.ObjectID) (domain.ObjectID, error) {96 func (g *Git) MergeBase(ctx context.Context, repo domain.RepoName, a, b domain.ObjectID) (domain.ObjectID, error) {
101 dir, err := g.repoDir(repo)97 dir, err := g.repoDir(repo)
102 if err != nil {98 if err != nil {
@@ -129,8 +125,6 @@ func (g *Git) IsAncestor(ctx context.Context, repo domain.RepoName, ancestor, de
129 }125 }
130 126
131 // ListRange runs `git log <head> ^<exclude>`.127 // ListRange runs `git log <head> ^<exclude>`.
132-//
133-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain values; the limit comes from the port
134 func (g *Git) ListRange(ctx context.Context, repo domain.RepoName, exclude, head domain.ObjectID, limit int) ([]domain.Commit, bool, error) {128 func (g *Git) ListRange(ctx context.Context, repo domain.RepoName, exclude, head domain.ObjectID, limit int) ([]domain.Commit, bool, error) {
135 var excluded []string129 var excluded []string
136 if !exclude.IsZero() {130 if !exclude.IsZero() {
@@ -142,8 +136,6 @@ func (g *Git) ListRange(ctx context.Context, repo domain.RepoName, exclude, head
142 // logCommits runs `git log` for the commits reachable from head: revArgs go136 // logCommits runs `git log` for the commits reachable from head: revArgs go
143 // before the head (options such as --not), excluded after it. Newest first,137 // before the head (options such as --not), excluded after it. Newest first,
144 // at most limit; more says there were others.138 // at most limit; more says there were others.
145-//
146-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain values; the limit comes from the port
147 func (g *Git) logCommits(ctx context.Context, repo domain.RepoName, head domain.ObjectID, limit int, revArgs, excluded []string) ([]domain.Commit, bool, error) {139 func (g *Git) logCommits(ctx context.Context, repo domain.RepoName, head domain.ObjectID, limit int, revArgs, excluded []string) ([]domain.Commit, bool, error) {
148 if limit < 1 || limit > ports.MaxCommitLimit {140 if limit < 1 || limit > ports.MaxCommitLimit {
149 return nil, false, fmt.Errorf("list range: %w: limit %d", domain.ErrInvalidPageSize, limit)141 return nil, false, fmt.Errorf("list range: %w: limit %d", domain.ErrInvalidPageSize, limit)
@@ -183,8 +175,6 @@ func (g *Git) logCommits(ctx context.Context, repo domain.RepoName, head domain.
183 }175 }
184 176
185 // DiffCommits reads the diff between two commits like GetCommit does.177 // DiffCommits reads the diff between two commits like GetCommit does.
186-//
187-//hexcheck:ignore suspicious-business-logic-in-adapter reads a git diff into the domain diff with the limits the port passes in
188 func (g *Git) DiffCommits(ctx context.Context, repo domain.RepoName, base, head domain.ObjectID, limits domain.DiffLimits) (domain.DiffStats, domain.Diff, error) {178 func (g *Git) DiffCommits(ctx context.Context, repo domain.RepoName, base, head domain.ObjectID, limits domain.DiffLimits) (domain.DiffStats, domain.Diff, error) {
189 dir, err := g.repoDir(repo)179 dir, err := g.repoDir(repo)
190 if err != nil {180 if err != nil {
@@ -198,8 +188,6 @@ func (g *Git) DiffCommits(ctx context.Context, repo domain.RepoName, base, head
198 }188 }
199 189
200 // CommitTree runs `git rev-parse <commit>^{tree}`.190 // CommitTree runs `git rev-parse <commit>^{tree}`.
201-//
202-//hexcheck:ignore suspicious-business-logic-in-adapter parses the id git printed
203 func (g *Git) CommitTree(ctx context.Context, repo domain.RepoName, id domain.ObjectID) (domain.ObjectID, error) {191 func (g *Git) CommitTree(ctx context.Context, repo domain.RepoName, id domain.ObjectID) (domain.ObjectID, error) {
204 dir, err := g.repoDir(repo)192 dir, err := g.repoDir(repo)
205 if err != nil {193 if err != nil {
@@ -229,8 +217,6 @@ func (g *Git) WriteCommit(ctx context.Context, repo domain.RepoName, object []by
229 // UpdateBranch runs `git update-ref refs/heads/<branch> <new> <old>`, which217 // UpdateBranch runs `git update-ref refs/heads/<branch> <new> <old>`, which
230 // takes the ref lock and refuses when the ref is not at old. A failure is218 // takes the ref lock and refuses when the ref is not at old. A failure is
231 // told from a moved branch by reading the branch again.219 // told from a moved branch by reading the branch again.
232-//
233-//hexcheck:ignore suspicious-business-logic-in-adapter maps a refused update-ref to the conflict error of the port
234 func (g *Git) UpdateBranch(ctx context.Context, repo domain.RepoName, branch string, old, newID domain.ObjectID) error {220 func (g *Git) UpdateBranch(ctx context.Context, repo domain.RepoName, branch string, old, newID domain.ObjectID) error {
235 dir, err := g.repoDir(repo)221 dir, err := g.repoDir(repo)
236 if err != nil {222 if err != nil {
@@ -252,8 +238,6 @@ func (g *Git) UpdateBranch(ctx context.Context, repo domain.RepoName, branch str
252 // DeleteBranch runs `git update-ref -d refs/heads/<branch> <old>`, which takes238 // DeleteBranch runs `git update-ref -d refs/heads/<branch> <old>`, which takes
253 // the ref lock and refuses when the ref is not at old. A failure is told from239 // the ref lock and refuses when the ref is not at old. A failure is told from
254 // a moved or deleted branch by reading the branch again.240 // a moved or deleted branch by reading the branch again.
255-//
256-//hexcheck:ignore suspicious-business-logic-in-adapter maps a refused update-ref to the conflict error of the port
257 func (g *Git) DeleteBranch(ctx context.Context, repo domain.RepoName, branch string, old domain.ObjectID) error {241 func (g *Git) DeleteBranch(ctx context.Context, repo domain.RepoName, branch string, old domain.ObjectID) error {
258 dir, err := g.repoDir(repo)242 dir, err := g.repoDir(repo)
259 if err != nil {243 if err != nil {
M internal/adapters/gitcli/pull.go +0 -2
@@ -18,8 +18,6 @@ var _ ports.PullGit = (*Git)(nil)
1818 // database and touches no ref or index: it prints the tree of the result and
1919 // exits 0, or exits 1 when the merge conflicts. Commits with no common history
2020 // do not merge (GitHub makes no test merge for them either).
21-//
22-//hexcheck:ignore suspicious-business-logic-in-adapter maps git exit codes to the clean/conflict answer of the port
2321 func (g *Git) TestMerge(ctx context.Context, repo domain.RepoName, base, head domain.ObjectID) (domain.ObjectID, bool, error) {
2422 dir, err := g.repoDir(repo)
2523 if err != nil {
@@ -18,8 +18,6 @@ var _ ports.PullGit = (*Git)(nil)
18 // database and touches no ref or index: it prints the tree of the result and18 // database and touches no ref or index: it prints the tree of the result and
19 // exits 0, or exits 1 when the merge conflicts. Commits with no common history19 // exits 0, or exits 1 when the merge conflicts. Commits with no common history
20 // do not merge (GitHub makes no test merge for them either).20 // do not merge (GitHub makes no test merge for them either).
21-//
22-//hexcheck:ignore suspicious-business-logic-in-adapter maps git exit codes to the clean/conflict answer of the port
23 func (g *Git) TestMerge(ctx context.Context, repo domain.RepoName, base, head domain.ObjectID) (domain.ObjectID, bool, error) {21 func (g *Git) TestMerge(ctx context.Context, repo domain.RepoName, base, head domain.ObjectID) (domain.ObjectID, bool, error) {
24 dir, err := g.repoDir(repo)22 dir, err := g.repoDir(repo)
25 if err != nil {23 if err != nil {
A internal/adapters/gitcli/push_trees.go +400 -0
@@ -0,0 +1,400 @@
1+// SPDX-License-Identifier: AGPL-3.0-or-later
2+
3+package gitcli
4+
5+import (
6+ "bufio"
7+ "context"
8+ "errors"
9+ "fmt"
10+ "io"
11+ "path/filepath"
12+ "strconv"
13+ "strings"
14+
15+ "bnema.dev/ermite/internal/domain"
16+ "bnema.dev/ermite/internal/ports"
17+)
18+
19+var _ ports.PushTreeReader = (*Git)(nil)
20+
21+const (
22+ maxDiffPath = 4096
23+ maxRevListLine = 8192
24+)
25+
26+// PushSubtrees peels push endpoints, walks new commits, and reads entries at path.
27+func (g *Git) PushSubtrees(ctx context.Context, repo domain.RepoName, updates []domain.RefUpdate, objects ports.PushObjects, path string, bounds domain.PushWalkBounds) (domain.PushSubtrees, error) {
28+ out := domain.PushSubtrees{Ends: map[string]domain.PushObjectType{}, Subtrees: map[string]domain.Subtree{}}
29+ dir, err := g.repoDir(repo)
30+ if err != nil {
31+ return out, err
32+ }
33+ extra, err := quarantineEnv(dir, objects)
34+ if err != nil {
35+ return out, err
36+ }
37+ var kept []domain.RefUpdate
38+ for _, up := range updates {
39+ if up.New != domain.ZeroOID {
40+ kept = append(kept, up)
41+ }
42+ }
43+ if len(kept) == 0 {
44+ return out, nil
45+ }
46+ return g.readPushSubtrees(ctx, dir, extra, kept, path, bounds, out)
47+}
48+
49+// readPushSubtrees batches the endpoint and walked-object cat-file queries.
50+func (g *Git) readPushSubtrees(ctx context.Context, dir string, extra []string, kept []domain.RefUpdate, path string, bounds domain.PushWalkBounds, out domain.PushSubtrees) (domain.PushSubtrees, error) {
51+ types, err := g.batchCheck(ctx, dir, extra, kept, "^{}", "%(objecttype)")
52+ if err != nil {
53+ return out, err
54+ }
55+ tips := pushCommitTips(kept, types, out.Ends)
56+ if len(tips) != 0 {
57+ out.Commits, err = g.newCommits(ctx, dir, extra, tips, bounds)
58+ if err != nil {
59+ return out, err
60+ }
61+ }
62+ ids := commitAndParentIDs(out.Commits)
63+ ws, err := g.subtreesAt(ctx, dir, extra, ids, path)
64+ if err != nil {
65+ return out, err
66+ }
67+ for id, w := range ws {
68+ out.Subtrees[id] = w
69+ }
70+ ids = pushEndSubtreeIDs(kept, out.Ends)
71+ ws, err = g.subtreesAt(ctx, dir, extra, ids, path)
72+ if err != nil {
73+ return out, err
74+ }
75+ for id, w := range ws {
76+ out.Subtrees[id] = w
77+ }
78+ return out, nil
79+}
80+
81+// pushCommitTips translates peeled endpoint answers and deduplicates rev-list tips.
82+func pushCommitTips(kept []domain.RefUpdate, types []string, ends map[string]domain.PushObjectType) []string {
83+ var tips []string
84+ seen := map[string]bool{}
85+ for i, up := range kept {
86+ for j, id := range []string{up.Old, up.New} {
87+ if id != domain.ZeroOID {
88+ ends[id] = domain.PushObjectType(types[2*i+j])
89+ }
90+ }
91+ if types[2*i+1] == "commit" && !seen[up.New] {
92+ seen[up.New] = true
93+ tips = append(tips, up.New)
94+ }
95+ }
96+ return tips
97+}
98+
99+// commitAndParentIDs deduplicates the object names for a cat-file batch.
100+func commitAndParentIDs(commits []domain.PushedCommit) []string {
101+ var ids []string
102+ asked := map[string]bool{}
103+ for _, c := range commits {
104+ for _, id := range append([]domain.ObjectID{c.ID}, c.Parents...) {
105+ key := id.String()
106+ if !asked[key] {
107+ asked[key] = true
108+ ids = append(ids, key)
109+ }
110+ }
111+ }
112+ return ids
113+}
114+
115+// pushEndSubtreeIDs omits blobs (not tree-ish) and includes HEAD only for creates.
116+func pushEndSubtreeIDs(kept []domain.RefUpdate, ends map[string]domain.PushObjectType) []string {
117+ var ids []string
118+ for _, up := range kept {
119+ for _, id := range []string{up.Old, up.New} {
120+ if id != domain.ZeroOID && ends[id] != domain.PushObjectBlob {
121+ ids = append(ids, id)
122+ }
123+ }
124+ }
125+ // HEAD is needed only for a new tree-ish ref. Keep it in the end batch.
126+ for _, up := range kept {
127+ if up.Old == domain.ZeroOID && ends[up.New] != domain.PushObjectBlob {
128+ ids = append(ids, domain.HeadKey)
129+ break
130+ }
131+ }
132+ return ids
133+}
134+
135+// newCommits lists the commits reachable from the tips and from no ref
136+// (`rev-list --parents <tips> --not --all`), at most bounds.Commits: more
137+// fails closed.
138+func (g *Git) newCommits(ctx context.Context, dir string, extra []string, tips []string, bounds domain.PushWalkBounds) ([]domain.PushedCommit, error) {
139+ if err := revListLimitsError(bounds); err != nil {
140+ return nil, err
141+ }
142+ args := readArgs(dir, append(append([]string{"rev-list", "--parents", "--max-count=" + strconv.Itoa(bounds.Commits+1)}, tips...), "--not", "--all")...)
143+ var out []domain.PushedCommit
144+ err := g.pipeWith(ctx, extra, nil, args, func(r *bufio.Reader) error {
145+ var err error
146+ out, err = parseRevList(r, bounds.Commits, bounds.IDs)
147+ return err
148+ })
149+ if err != nil {
150+ return nil, fmt.Errorf("commits of a push: %w", err)
151+ }
152+ return out, nil
153+}
154+
155+// revListLimitsError checks the numeric range of rev-list and parser limits.
156+func revListLimitsError(bounds domain.PushWalkBounds) error {
157+ if bounds.Commits < 0 || bounds.IDs < 0 || bounds.Commits == int(^uint(0)>>1) {
158+ return fmt.Errorf("%w: invalid push walk bounds", domain.ErrInvalid)
159+ }
160+ return nil
161+}
162+
163+// parseRevList reads the lines of `rev-list --parents` ("<commit> <parent>..."):
164+// at most maxCommits lines and maxIDs ids in all (a commit and each of its
165+// parents count, repeats too). A line over maxRevListLine, a limit crossed or an
166+// id that is not one is an error, and it is raised as soon as the line that
167+// crosses it is read: nothing past it is read or kept.
168+func parseRevList(r *bufio.Reader, maxCommits, maxIDs int) ([]domain.PushedCommit, error) {
169+ var out []domain.PushedCommit
170+ ids := 0
171+ for {
172+ line, err := r.ReadSlice('\n')
173+ switch {
174+ case errors.Is(err, bufio.ErrBufferFull) || len(line) > maxRevListLine:
175+ return nil, fmt.Errorf("%w: a pushed commit has too many parents", domain.ErrInvalid)
176+ case errors.Is(err, io.EOF) && len(line) == 0:
177+ return out, nil
178+ case err != nil && !errors.Is(err, io.EOF):
179+ return nil, err
180+ }
181+ f := strings.Fields(string(line))
182+ if len(f) == 0 {
183+ continue
184+ }
185+ if len(out) == maxCommits {
186+ return nil, fmt.Errorf("%w: the push brings more than %d commits", domain.ErrInvalid, maxCommits)
187+ }
188+ if ids += len(f); ids > maxIDs {
189+ return nil, fmt.Errorf("%w: the push brings commits with more than %d ids (commits and parents) in all", domain.ErrInvalid, maxIDs)
190+ }
191+ for _, id := range f {
192+ if _, err := domain.ParseObjectID(id); err != nil {
193+ return nil, fmt.Errorf("%w: unexpected rev-list output", domain.ErrInvalid)
194+ }
195+ }
196+ id, _ := domain.ParseObjectID(f[0])
197+ c := domain.PushedCommit{ID: id}
198+ for _, p := range f[1:] {
199+ id, _ := domain.ParseObjectID(p)
200+ c.Parents = append(c.Parents, id)
201+ }
202+ out = append(out, c)
203+ }
204+}
205+
206+// subtreesAt reads the mode, id and type of entries without traversing them.
207+func (g *Git) subtreesAt(ctx context.Context, dir string, extra []string, ids []string, path string) (map[string]domain.Subtree, error) {
208+ out := make(map[string]domain.Subtree, len(ids))
209+ if len(ids) == 0 {
210+ return out, nil
211+ }
212+ specs := make([]string, len(ids))
213+ for i, id := range ids {
214+ specs[i] = id + ":" + path
215+ }
216+ args := readArgs(dir, "cat-file", "--batch-check=%(objectmode) %(objectname) %(objecttype)")
217+ err := g.pipeWith(ctx, extra, strings.NewReader(strings.Join(specs, "\n")+"\n"), args, func(r *bufio.Reader) error {
218+ for _, key := range ids {
219+ line, err := r.ReadSlice('\n')
220+ if err != nil {
221+ return fmt.Errorf("read object answers: %w", err)
222+ }
223+ f := strings.Fields(string(line))
224+ if len(f) != 3 {
225+ continue
226+ } // missing entry
227+ mode, err := strconv.ParseUint(f[0], 8, 32)
228+ if err != nil {
229+ continue
230+ }
231+ id, err := domain.ParseObjectID(f[1])
232+ if err != nil {
233+ return err
234+ }
235+ kind := domain.EntryKindOf(uint32(mode))
236+ if kind == domain.EntryDirectory && (f[0] != "040000" || f[2] != "tree") {
237+ kind = ""
238+ }
239+ out[key] = domain.Subtree{Kind: kind, ID: id}
240+ }
241+ return nil
242+ })
243+ if err != nil {
244+ return nil, fmt.Errorf("subtrees of a push: %w", err)
245+ }
246+ return out, nil
247+}
248+
249+// FirstDifferentEntry streams the first differing top-level entry of two trees.
250+func (g *Git) FirstDifferentEntry(ctx context.Context, repo domain.RepoName, objects ports.PushObjects, oldID, newID domain.ObjectID) (string, error) {
251+ dir, err := g.repoDir(repo)
252+ if err != nil {
253+ return "", err
254+ }
255+ extra, err := quarantineEnv(dir, objects)
256+ if err != nil {
257+ return "", err
258+ }
259+ from := oldID.String()
260+ if from == "" {
261+ from = emptyTree
262+ }
263+ to := newID.String()
264+ args := readArgs(dir, "diff-tree", "--name-only", "-z", "--no-renames", "--no-commit-id", "--end-of-options", from, to)
265+ var name string
266+ err = g.pipeWith(ctx, extra, nil, args, func(r *bufio.Reader) error {
267+ n, found, err := firstPath(r)
268+ name = n
269+ if err != nil || !found {
270+ return err // no name: git's exit status is checked, so a failure refuses
271+ }
272+ return errStop // the first name is all that is needed: stop git
273+ })
274+ if err != nil {
275+ return "", fmt.Errorf("different tree entries %s..%s: %w", from, to, err)
276+ }
277+ return name, nil
278+}
279+
280+// firstPath reads the first NUL-terminated path of r; found is false at the end
281+// of the output without one. A path longer than maxDiffPath is an error.
282+func firstPath(r *bufio.Reader) (path string, found bool, err error) {
283+ var b []byte
284+ for {
285+ c, err := r.ReadByte()
286+ switch {
287+ case err != nil:
288+ return "", false, nil // the end of the output; a failing exit status is reported by pipe
289+ case c == 0:
290+ return string(b), true, nil
291+ case len(b) >= maxDiffPath:
292+ return "", false, fmt.Errorf("%w: a changed path is longer than %d bytes", domain.ErrInvalid, maxDiffPath)
293+ }
294+ b = append(b, c)
295+ }
296+}
297+
298+// batchCheck asks `cat-file --batch-check=<format>` about <oid><suffix> for the
299+// old (unless the ref is created) and the new end of each update and returns
300+// the answers in that order, two per update, "" for the old end of a created
301+// ref. With suffix "^{}" an object that is missing, malformed or neither a
302+// commit, a tree nor a blob is an error (the push is refused); with any other
303+// suffix an answer "<input> missing" is kept as the answer.
304+func (g *Git) batchCheck(ctx context.Context, dir string, extra []string, ups []domain.RefUpdate, suffix, format string) ([]string, error) {
305+ var in strings.Builder
306+ want := 0
307+ for _, up := range ups {
308+ for _, oid := range []string{up.Old, up.New} {
309+ if oid == domain.ZeroOID {
310+ continue
311+ }
312+ if _, err := domain.ParseObjectID(oid); err != nil {
313+ return nil, fmt.Errorf("%w: %v", domain.ErrInvalid, err)
314+ }
315+ in.WriteString(oid + suffix + "\n")
316+ want++
317+ }
318+ }
319+ got := make([]string, 0, want)
320+ args := readArgs(dir, "cat-file", "--batch-check="+format)
321+ err := g.pipeWith(ctx, extra, strings.NewReader(in.String()), args, func(r *bufio.Reader) error {
322+ for range want {
323+ line, err := r.ReadSlice('\n')
324+ if err != nil {
325+ if errors.Is(err, bufio.ErrBufferFull) {
326+ return fmt.Errorf("%w: unexpected cat-file output", domain.ErrInvalid)
327+ }
328+ return fmt.Errorf("read object answers: %w", err)
329+ }
330+ got = append(got, strings.TrimSuffix(string(line), "\n"))
331+ }
332+ return nil
333+ })
334+ if err != nil {
335+ return nil, fmt.Errorf("objects of a push: %w", err)
336+ }
337+ out := make([]string, 0, 2*len(ups))
338+ i := 0
339+ for _, up := range ups {
340+ for _, oid := range []string{up.Old, up.New} {
341+ if oid == domain.ZeroOID {
342+ out = append(out, "")
343+ continue
344+ }
345+ if suffix == "^{}" {
346+ switch got[i] {
347+ case "commit", "tree", "blob":
348+ default: // "<input> missing", or a type that cannot be compared
349+ return nil, fmt.Errorf("%w: object %s of the push is missing or of an unexpected type", domain.ErrInvalid, oid)
350+ }
351+ }
352+ out = append(out, got[i])
353+ i++
354+ }
355+ }
356+ return out, nil
357+}
358+
359+// quarantineEnv returns the environment that makes git read the objects of a
360+// push in quarantine. Each directory must exist and, once symbolic links are
361+// resolved, be inside the objects directory of the repository dir (itself
362+// resolved): a hook request cannot send git to read anything else. The resolved
363+// paths are the ones given to git.
364+func quarantineEnv(repoDir string, o ports.PushObjects) ([]string, error) {
365+ if o.Dir == "" && o.Alternates == "" {
366+ return nil, nil
367+ }
368+ root, err := filepath.EvalSymlinks(filepath.Join(repoDir, "objects"))
369+ if err != nil {
370+ return nil, fmt.Errorf("resolve the objects directory of %s: %w", repoDir, err)
371+ }
372+ inside := func(p string) (string, bool) {
373+ if !filepath.IsAbs(p) {
374+ return "", false
375+ }
376+ real, err := filepath.EvalSymlinks(p)
377+ if err != nil {
378+ return "", false
379+ }
380+ rel, err := filepath.Rel(root, real)
381+ return real, err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator))
382+ }
383+ dir, ok := inside(o.Dir)
384+ if o.Dir == "" || !ok {
385+ return nil, fmt.Errorf("%w: the objects directory of the push is outside the repository", domain.ErrInvalid)
386+ }
387+ env := []string{"GIT_OBJECT_DIRECTORY=" + dir}
388+ if o.Alternates != "" {
389+ var alts []string
390+ for _, a := range filepath.SplitList(o.Alternates) {
391+ real, ok := inside(a)
392+ if !ok {
393+ return nil, fmt.Errorf("%w: an alternate objects directory of the push is outside the repository", domain.ErrInvalid)
394+ }
395+ alts = append(alts, real)
396+ }
397+ env = append(env, "GIT_ALTERNATE_OBJECT_DIRECTORIES="+strings.Join(alts, string(filepath.ListSeparator)))
398+ }
399+ return env, nil
400+}
@@ -0,0 +1,400 @@
1+// SPDX-License-Identifier: AGPL-3.0-or-later
2+
3+package gitcli
4+
5+import (
6+ "bufio"
7+ "context"
8+ "errors"
9+ "fmt"
10+ "io"
11+ "path/filepath"
12+ "strconv"
13+ "strings"
14+
15+ "bnema.dev/ermite/internal/domain"
16+ "bnema.dev/ermite/internal/ports"
17+)
18+
19+var _ ports.PushTreeReader = (*Git)(nil)
20+
21+const (
22+ maxDiffPath = 4096
23+ maxRevListLine = 8192
24+)
25+
26+// PushSubtrees peels push endpoints, walks new commits, and reads entries at path.
27+func (g *Git) PushSubtrees(ctx context.Context, repo domain.RepoName, updates []domain.RefUpdate, objects ports.PushObjects, path string, bounds domain.PushWalkBounds) (domain.PushSubtrees, error) {
28+ out := domain.PushSubtrees{Ends: map[string]domain.PushObjectType{}, Subtrees: map[string]domain.Subtree{}}
29+ dir, err := g.repoDir(repo)
30+ if err != nil {
31+ return out, err
32+ }
33+ extra, err := quarantineEnv(dir, objects)
34+ if err != nil {
35+ return out, err
36+ }
37+ var kept []domain.RefUpdate
38+ for _, up := range updates {
39+ if up.New != domain.ZeroOID {
40+ kept = append(kept, up)
41+ }
42+ }
43+ if len(kept) == 0 {
44+ return out, nil
45+ }
46+ return g.readPushSubtrees(ctx, dir, extra, kept, path, bounds, out)
47+}
48+
49+// readPushSubtrees batches the endpoint and walked-object cat-file queries.
50+func (g *Git) readPushSubtrees(ctx context.Context, dir string, extra []string, kept []domain.RefUpdate, path string, bounds domain.PushWalkBounds, out domain.PushSubtrees) (domain.PushSubtrees, error) {
51+ types, err := g.batchCheck(ctx, dir, extra, kept, "^{}", "%(objecttype)")
52+ if err != nil {
53+ return out, err
54+ }
55+ tips := pushCommitTips(kept, types, out.Ends)
56+ if len(tips) != 0 {
57+ out.Commits, err = g.newCommits(ctx, dir, extra, tips, bounds)
58+ if err != nil {
59+ return out, err
60+ }
61+ }
62+ ids := commitAndParentIDs(out.Commits)
63+ ws, err := g.subtreesAt(ctx, dir, extra, ids, path)
64+ if err != nil {
65+ return out, err
66+ }
67+ for id, w := range ws {
68+ out.Subtrees[id] = w
69+ }
70+ ids = pushEndSubtreeIDs(kept, out.Ends)
71+ ws, err = g.subtreesAt(ctx, dir, extra, ids, path)
72+ if err != nil {
73+ return out, err
74+ }
75+ for id, w := range ws {
76+ out.Subtrees[id] = w
77+ }
78+ return out, nil
79+}
80+
81+// pushCommitTips translates peeled endpoint answers and deduplicates rev-list tips.
82+func pushCommitTips(kept []domain.RefUpdate, types []string, ends map[string]domain.PushObjectType) []string {
83+ var tips []string
84+ seen := map[string]bool{}
85+ for i, up := range kept {
86+ for j, id := range []string{up.Old, up.New} {
87+ if id != domain.ZeroOID {
88+ ends[id] = domain.PushObjectType(types[2*i+j])
89+ }
90+ }
91+ if types[2*i+1] == "commit" && !seen[up.New] {
92+ seen[up.New] = true
93+ tips = append(tips, up.New)
94+ }
95+ }
96+ return tips
97+}
98+
99+// commitAndParentIDs deduplicates the object names for a cat-file batch.
100+func commitAndParentIDs(commits []domain.PushedCommit) []string {
101+ var ids []string
102+ asked := map[string]bool{}
103+ for _, c := range commits {
104+ for _, id := range append([]domain.ObjectID{c.ID}, c.Parents...) {
105+ key := id.String()
106+ if !asked[key] {
107+ asked[key] = true
108+ ids = append(ids, key)
109+ }
110+ }
111+ }
112+ return ids
113+}
114+
115+// pushEndSubtreeIDs omits blobs (not tree-ish) and includes HEAD only for creates.
116+func pushEndSubtreeIDs(kept []domain.RefUpdate, ends map[string]domain.PushObjectType) []string {
117+ var ids []string
118+ for _, up := range kept {
119+ for _, id := range []string{up.Old, up.New} {
120+ if id != domain.ZeroOID && ends[id] != domain.PushObjectBlob {
121+ ids = append(ids, id)
122+ }
123+ }
124+ }
125+ // HEAD is needed only for a new tree-ish ref. Keep it in the end batch.
126+ for _, up := range kept {
127+ if up.Old == domain.ZeroOID && ends[up.New] != domain.PushObjectBlob {
128+ ids = append(ids, domain.HeadKey)
129+ break
130+ }
131+ }
132+ return ids
133+}
134+
135+// newCommits lists the commits reachable from the tips and from no ref
136+// (`rev-list --parents <tips> --not --all`), at most bounds.Commits: more
137+// fails closed.
138+func (g *Git) newCommits(ctx context.Context, dir string, extra []string, tips []string, bounds domain.PushWalkBounds) ([]domain.PushedCommit, error) {
139+ if err := revListLimitsError(bounds); err != nil {
140+ return nil, err
141+ }
142+ args := readArgs(dir, append(append([]string{"rev-list", "--parents", "--max-count=" + strconv.Itoa(bounds.Commits+1)}, tips...), "--not", "--all")...)
143+ var out []domain.PushedCommit
144+ err := g.pipeWith(ctx, extra, nil, args, func(r *bufio.Reader) error {
145+ var err error
146+ out, err = parseRevList(r, bounds.Commits, bounds.IDs)
147+ return err
148+ })
149+ if err != nil {
150+ return nil, fmt.Errorf("commits of a push: %w", err)
151+ }
152+ return out, nil
153+}
154+
155+// revListLimitsError checks the numeric range of rev-list and parser limits.
156+func revListLimitsError(bounds domain.PushWalkBounds) error {
157+ if bounds.Commits < 0 || bounds.IDs < 0 || bounds.Commits == int(^uint(0)>>1) {
158+ return fmt.Errorf("%w: invalid push walk bounds", domain.ErrInvalid)
159+ }
160+ return nil
161+}
162+
163+// parseRevList reads the lines of `rev-list --parents` ("<commit> <parent>..."):
164+// at most maxCommits lines and maxIDs ids in all (a commit and each of its
165+// parents count, repeats too). A line over maxRevListLine, a limit crossed or an
166+// id that is not one is an error, and it is raised as soon as the line that
167+// crosses it is read: nothing past it is read or kept.
168+func parseRevList(r *bufio.Reader, maxCommits, maxIDs int) ([]domain.PushedCommit, error) {
169+ var out []domain.PushedCommit
170+ ids := 0
171+ for {
172+ line, err := r.ReadSlice('\n')
173+ switch {
174+ case errors.Is(err, bufio.ErrBufferFull) || len(line) > maxRevListLine:
175+ return nil, fmt.Errorf("%w: a pushed commit has too many parents", domain.ErrInvalid)
176+ case errors.Is(err, io.EOF) && len(line) == 0:
177+ return out, nil
178+ case err != nil && !errors.Is(err, io.EOF):
179+ return nil, err
180+ }
181+ f := strings.Fields(string(line))
182+ if len(f) == 0 {
183+ continue
184+ }
185+ if len(out) == maxCommits {
186+ return nil, fmt.Errorf("%w: the push brings more than %d commits", domain.ErrInvalid, maxCommits)
187+ }
188+ if ids += len(f); ids > maxIDs {
189+ return nil, fmt.Errorf("%w: the push brings commits with more than %d ids (commits and parents) in all", domain.ErrInvalid, maxIDs)
190+ }
191+ for _, id := range f {
192+ if _, err := domain.ParseObjectID(id); err != nil {
193+ return nil, fmt.Errorf("%w: unexpected rev-list output", domain.ErrInvalid)
194+ }
195+ }
196+ id, _ := domain.ParseObjectID(f[0])
197+ c := domain.PushedCommit{ID: id}
198+ for _, p := range f[1:] {
199+ id, _ := domain.ParseObjectID(p)
200+ c.Parents = append(c.Parents, id)
201+ }
202+ out = append(out, c)
203+ }
204+}
205+
206+// subtreesAt reads the mode, id and type of entries without traversing them.
207+func (g *Git) subtreesAt(ctx context.Context, dir string, extra []string, ids []string, path string) (map[string]domain.Subtree, error) {
208+ out := make(map[string]domain.Subtree, len(ids))
209+ if len(ids) == 0 {
210+ return out, nil
211+ }
212+ specs := make([]string, len(ids))
213+ for i, id := range ids {
214+ specs[i] = id + ":" + path
215+ }
216+ args := readArgs(dir, "cat-file", "--batch-check=%(objectmode) %(objectname) %(objecttype)")
217+ err := g.pipeWith(ctx, extra, strings.NewReader(strings.Join(specs, "\n")+"\n"), args, func(r *bufio.Reader) error {
218+ for _, key := range ids {
219+ line, err := r.ReadSlice('\n')
220+ if err != nil {
221+ return fmt.Errorf("read object answers: %w", err)
222+ }
223+ f := strings.Fields(string(line))
224+ if len(f) != 3 {
225+ continue
226+ } // missing entry
227+ mode, err := strconv.ParseUint(f[0], 8, 32)
228+ if err != nil {
229+ continue
230+ }
231+ id, err := domain.ParseObjectID(f[1])
232+ if err != nil {
233+ return err
234+ }
235+ kind := domain.EntryKindOf(uint32(mode))
236+ if kind == domain.EntryDirectory && (f[0] != "040000" || f[2] != "tree") {
237+ kind = ""
238+ }
239+ out[key] = domain.Subtree{Kind: kind, ID: id}
240+ }
241+ return nil
242+ })
243+ if err != nil {
244+ return nil, fmt.Errorf("subtrees of a push: %w", err)
245+ }
246+ return out, nil
247+}
248+
249+// FirstDifferentEntry streams the first differing top-level entry of two trees.
250+func (g *Git) FirstDifferentEntry(ctx context.Context, repo domain.RepoName, objects ports.PushObjects, oldID, newID domain.ObjectID) (string, error) {
251+ dir, err := g.repoDir(repo)
252+ if err != nil {
253+ return "", err
254+ }
255+ extra, err := quarantineEnv(dir, objects)
256+ if err != nil {
257+ return "", err
258+ }
259+ from := oldID.String()
260+ if from == "" {
261+ from = emptyTree
262+ }
263+ to := newID.String()
264+ args := readArgs(dir, "diff-tree", "--name-only", "-z", "--no-renames", "--no-commit-id", "--end-of-options", from, to)
265+ var name string
266+ err = g.pipeWith(ctx, extra, nil, args, func(r *bufio.Reader) error {
267+ n, found, err := firstPath(r)
268+ name = n
269+ if err != nil || !found {
270+ return err // no name: git's exit status is checked, so a failure refuses
271+ }
272+ return errStop // the first name is all that is needed: stop git
273+ })
274+ if err != nil {
275+ return "", fmt.Errorf("different tree entries %s..%s: %w", from, to, err)
276+ }
277+ return name, nil
278+}
279+
280+// firstPath reads the first NUL-terminated path of r; found is false at the end
281+// of the output without one. A path longer than maxDiffPath is an error.
282+func firstPath(r *bufio.Reader) (path string, found bool, err error) {
283+ var b []byte
284+ for {
285+ c, err := r.ReadByte()
286+ switch {
287+ case err != nil:
288+ return "", false, nil // the end of the output; a failing exit status is reported by pipe
289+ case c == 0:
290+ return string(b), true, nil
291+ case len(b) >= maxDiffPath:
292+ return "", false, fmt.Errorf("%w: a changed path is longer than %d bytes", domain.ErrInvalid, maxDiffPath)
293+ }
294+ b = append(b, c)
295+ }
296+}
297+
298+// batchCheck asks `cat-file --batch-check=<format>` about <oid><suffix> for the
299+// old (unless the ref is created) and the new end of each update and returns
300+// the answers in that order, two per update, "" for the old end of a created
301+// ref. With suffix "^{}" an object that is missing, malformed or neither a
302+// commit, a tree nor a blob is an error (the push is refused); with any other
303+// suffix an answer "<input> missing" is kept as the answer.
304+func (g *Git) batchCheck(ctx context.Context, dir string, extra []string, ups []domain.RefUpdate, suffix, format string) ([]string, error) {
305+ var in strings.Builder
306+ want := 0
307+ for _, up := range ups {
308+ for _, oid := range []string{up.Old, up.New} {
309+ if oid == domain.ZeroOID {
310+ continue
311+ }
312+ if _, err := domain.ParseObjectID(oid); err != nil {
313+ return nil, fmt.Errorf("%w: %v", domain.ErrInvalid, err)
314+ }
315+ in.WriteString(oid + suffix + "\n")
316+ want++
317+ }
318+ }
319+ got := make([]string, 0, want)
320+ args := readArgs(dir, "cat-file", "--batch-check="+format)
321+ err := g.pipeWith(ctx, extra, strings.NewReader(in.String()), args, func(r *bufio.Reader) error {
322+ for range want {
323+ line, err := r.ReadSlice('\n')
324+ if err != nil {
325+ if errors.Is(err, bufio.ErrBufferFull) {
326+ return fmt.Errorf("%w: unexpected cat-file output", domain.ErrInvalid)
327+ }
328+ return fmt.Errorf("read object answers: %w", err)
329+ }
330+ got = append(got, strings.TrimSuffix(string(line), "\n"))
331+ }
332+ return nil
333+ })
334+ if err != nil {
335+ return nil, fmt.Errorf("objects of a push: %w", err)
336+ }
337+ out := make([]string, 0, 2*len(ups))
338+ i := 0
339+ for _, up := range ups {
340+ for _, oid := range []string{up.Old, up.New} {
341+ if oid == domain.ZeroOID {
342+ out = append(out, "")
343+ continue
344+ }
345+ if suffix == "^{}" {
346+ switch got[i] {
347+ case "commit", "tree", "blob":
348+ default: // "<input> missing", or a type that cannot be compared
349+ return nil, fmt.Errorf("%w: object %s of the push is missing or of an unexpected type", domain.ErrInvalid, oid)
350+ }
351+ }
352+ out = append(out, got[i])
353+ i++
354+ }
355+ }
356+ return out, nil
357+}
358+
359+// quarantineEnv returns the environment that makes git read the objects of a
360+// push in quarantine. Each directory must exist and, once symbolic links are
361+// resolved, be inside the objects directory of the repository dir (itself
362+// resolved): a hook request cannot send git to read anything else. The resolved
363+// paths are the ones given to git.
364+func quarantineEnv(repoDir string, o ports.PushObjects) ([]string, error) {
365+ if o.Dir == "" && o.Alternates == "" {
366+ return nil, nil
367+ }
368+ root, err := filepath.EvalSymlinks(filepath.Join(repoDir, "objects"))
369+ if err != nil {
370+ return nil, fmt.Errorf("resolve the objects directory of %s: %w", repoDir, err)
371+ }
372+ inside := func(p string) (string, bool) {
373+ if !filepath.IsAbs(p) {
374+ return "", false
375+ }
376+ real, err := filepath.EvalSymlinks(p)
377+ if err != nil {
378+ return "", false
379+ }
380+ rel, err := filepath.Rel(root, real)
381+ return real, err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator))
382+ }
383+ dir, ok := inside(o.Dir)
384+ if o.Dir == "" || !ok {
385+ return nil, fmt.Errorf("%w: the objects directory of the push is outside the repository", domain.ErrInvalid)
386+ }
387+ env := []string{"GIT_OBJECT_DIRECTORY=" + dir}
388+ if o.Alternates != "" {
389+ var alts []string
390+ for _, a := range filepath.SplitList(o.Alternates) {
391+ real, ok := inside(a)
392+ if !ok {
393+ return nil, fmt.Errorf("%w: an alternate objects directory of the push is outside the repository", domain.ErrInvalid)
394+ }
395+ alts = append(alts, real)
396+ }
397+ env = append(env, "GIT_ALTERNATE_OBJECT_DIRECTORIES="+strings.Join(alts, string(filepath.ListSeparator)))
398+ }
399+ return env, nil
400+}
R internal/adapters/gitcli/workflow_guard_test.go → internal/adapters/gitcli/push_trees_test.go +158 -217 renamed, 53% similar
@@ -51,7 +51,7 @@ func TestChangedWorkflow(t *testing.T) {
5151 return domain.RefUpdate{Old: old, New: new, Ref: "refs/heads/x"}
5252 }
5353 changed := func(ups ...domain.RefUpdate) string {
54- p, err := g.ChangedWorkflow(ctx, f.Name, ups, ports.PushObjects{})
54+ p, err := changedWorkflow(g, ctx, f.Name, ups, ports.PushObjects{})
5555 require.NoError(t, err)
5656 return p
5757 }
@@ -68,11 +68,11 @@ func TestChangedWorkflow(t *testing.T) {
6868 assert.Equal(t, ".github/workflows/ci.yml", changed(up(oid(base), oid(docs)), up(oid(docs), oid(edited))), "the first ref that changes one")
6969 assert.Equal(t, "", changed(), "no update")
7070
71- _, err := g.ChangedWorkflow(ctx, f.Name, []domain.RefUpdate{up(oid(base), "1111111111111111111111111111111111111111")}, ports.PushObjects{})
71+ _, err := changedWorkflow(g, ctx, f.Name, []domain.RefUpdate{up(oid(base), "1111111111111111111111111111111111111111")}, ports.PushObjects{})
7272 assert.Error(t, err, "an object git does not have is a failure, not a pass")
7373 }
7474
75-func TestChangedWorkflowReadsTheQuarantineOfThePushOnly(t *testing.T) {
75+func TestPushSubtreesReadsTheQuarantineOfThePushOnly(t *testing.T) {
7676 g, data := newGit(t)
7777 ctx := context.Background()
7878 f := readertest.NewFixture(t, data, "proj")
@@ -90,23 +90,23 @@ func TestChangedWorkflowReadsTheQuarantineOfThePushOnly(t *testing.T) {
9090 "an alternate outside": {Dir: objects + "/incoming-1", Alternates: "/tmp"},
9191 "alternates without a dir": {Alternates: objects},
9292 } {
93- _, err := g.ChangedWorkflow(ctx, f.Name, up, o)
93+ _, err := g.PushSubtrees(ctx, f.Name, up, o, domain.WorkflowDir, domain.DefaultPushWalkBounds())
9494 assert.ErrorIs(t, err, domain.ErrInvalid, name)
9595 }
9696 // An existing directory inside the repository is accepted as a location
9797 // (the object is missing: a refusal of the object, not of the path).
9898 require.NoError(t, os.Mkdir(filepath.Join(objects, "incoming-1"), 0o700))
99- _, err := g.ChangedWorkflow(ctx, f.Name, up, ports.PushObjects{Dir: objects + "/incoming-1", Alternates: objects})
99+ _, err := g.PushSubtrees(ctx, f.Name, up, ports.PushObjects{Dir: objects + "/incoming-1", Alternates: objects}, domain.WorkflowDir, domain.DefaultPushWalkBounds())
100100 require.Error(t, err)
101101 assert.NotContains(t, err.Error(), "outside the repository")
102102 // One that does not exist is refused.
103- _, err = g.ChangedWorkflow(ctx, f.Name, up, ports.PushObjects{Dir: objects + "/incoming-2"})
103+ _, err = g.PushSubtrees(ctx, f.Name, up, ports.PushObjects{Dir: objects + "/incoming-2"}, domain.WorkflowDir, domain.DefaultPushWalkBounds())
104104 assert.ErrorIs(t, err, domain.ErrInvalid)
105105 }
106106
107107 // A symbolic link in the way cannot take the objects of a push out of the
108108 // repository, and a data directory reached through links still works.
109-func TestChangedWorkflowResolvesSymbolicLinks(t *testing.T) {
109+func TestPushSubtreesResolvesSymbolicLinks(t *testing.T) {
110110 g, data := newGit(t)
111111 ctx := context.Background()
112112 f := readertest.NewFixture(t, data, "proj")
@@ -119,20 +119,20 @@ func TestChangedWorkflowResolvesSymbolicLinks(t *testing.T) {
119119 // Inside objects/, a link to elsewhere is outside once resolved.
120120 elsewhere := t.TempDir()
121121 require.NoError(t, os.Symlink(elsewhere, filepath.Join(objects, "incoming-link")))
122- _, err := g.ChangedWorkflow(ctx, f.Name, up, ports.PushObjects{Dir: filepath.Join(objects, "incoming-link")})
122+ _, err := g.PushSubtrees(ctx, f.Name, up, ports.PushObjects{Dir: filepath.Join(objects, "incoming-link")}, domain.WorkflowDir, domain.DefaultPushWalkBounds())
123123 assert.ErrorIs(t, err, domain.ErrInvalid, "a link out of objects/")
124- _, err = g.ChangedWorkflow(ctx, f.Name, up, ports.PushObjects{Dir: objects + "/pack", Alternates: filepath.Join(objects, "incoming-link")})
124+ _, err = g.PushSubtrees(ctx, f.Name, up, ports.PushObjects{Dir: objects + "/pack", Alternates: filepath.Join(objects, "incoming-link")}, domain.WorkflowDir, domain.DefaultPushWalkBounds())
125125 assert.ErrorIs(t, err, domain.ErrInvalid, "an alternate that is a link out of objects/")
126126
127127 // A link to somewhere inside objects/ is fine.
128128 require.NoError(t, os.Mkdir(filepath.Join(objects, "q1"), 0o700))
129129 require.NoError(t, os.Symlink(filepath.Join(objects, "q1"), filepath.Join(objects, "q-link")))
130- _, err = g.ChangedWorkflow(ctx, f.Name, up, ports.PushObjects{Dir: filepath.Join(objects, "q-link")})
130+ _, err = g.PushSubtrees(ctx, f.Name, up, ports.PushObjects{Dir: filepath.Join(objects, "q-link")}, domain.WorkflowDir, domain.DefaultPushWalkBounds())
131131 require.Error(t, err)
132132 assert.NotContains(t, err.Error(), "outside the repository")
133133 }
134134
135-func TestChangedWorkflowThroughALinkedDataDirectory(t *testing.T) {
135+func TestPushSubtreesThroughALinkedDataDirectory(t *testing.T) {
136136 _, data := newGit(t)
137137 ctx := context.Background()
138138 f := readertest.NewFixture(t, data, "proj")
@@ -152,10 +152,10 @@ func TestChangedWorkflowThroughALinkedDataDirectory(t *testing.T) {
152152 q := filepath.Join(f.Bare, "objects", "incoming-9")
153153 require.NoError(t, os.Mkdir(q, 0o700))
154154 for _, dir := range []string{q, filepath.Join(link, "repos", "proj.git", "objects", "incoming-9")} {
155- p, err := g2.ChangedWorkflow(ctx, f.Name, []domain.RefUpdate{{Old: oidOf(t, base).String(), New: oidOf(t, wf).String(), Ref: "refs/heads/x"}},
156- ports.PushObjects{Dir: dir, Alternates: filepath.Join(link, "repos", "proj.git", "objects")})
155+ p, err := g2.PushSubtrees(ctx, f.Name, []domain.RefUpdate{{Old: oidOf(t, base).String(), New: oidOf(t, wf).String(), Ref: "refs/heads/x"}},
156+ ports.PushObjects{Dir: dir, Alternates: filepath.Join(link, "repos", "proj.git", "objects")}, domain.WorkflowDir, domain.DefaultPushWalkBounds())
157157 require.NoError(t, err, dir)
158- assert.Equal(t, ".github/workflows/ci.yml", p, dir)
158+ assert.Equal(t, domain.EntryDirectory, p.Subtrees[oidOf(t, wf).String()].Kind, dir)
159159 }
160160 }
161161
@@ -182,17 +182,7 @@ func commitOf(t *testing.T, f *readertest.Fixture, tree string, parents ...strin
182182 return strings.TrimSpace(string(out))
183183 }
184184
185-// withWorkflows is a commit whose .github/workflows is the tree given.
186-func withWorkflows(t *testing.T, f *readertest.Fixture, workflows string) string {
187- t.Helper()
188- gh := treeOf(t, f, "040000 tree "+workflows+"\tworkflows")
189- return commitOf(t, f, treeOf(t, f, "040000 tree "+gh+"\t.github"))
190-}
191-
192-// The trees of a push are the pusher's: a workflows directory that fans out
193-// into ten million paths is answered with its first path, at once, by a git
194-// that is stopped, without the diff ever being held in memory.
195-func TestChangedWorkflowDoesNotBufferTheDiff(t *testing.T) {
185+func TestFirstDifferentEntryDoesNotBufferTheDiff(t *testing.T) {
196186 g, data := newGit(t)
197187 ctx := context.Background()
198188 f := readertest.NewFixture(t, data, "proj")
@@ -206,17 +196,17 @@ func TestChangedWorkflowDoesNotBufferTheDiff(t *testing.T) {
206196 }
207197 level = treeOf(t, f, entries...)
208198 }
209- amp := withWorkflows(t, f, level)
199+ amp := level
210200
211201 var before, after runtime.MemStats
212202 runtime.GC()
213203 runtime.ReadMemStats(&before)
214204 start := time.Now()
215- p, err := g.ChangedWorkflow(ctx, f.Name, []domain.RefUpdate{{Old: domain.ZeroOID, New: amp, Ref: "refs/heads/x"}}, ports.PushObjects{})
205+ p, err := g.FirstDifferentEntry(ctx, f.Name, ports.PushObjects{}, domain.ObjectID{}, oidOf(t, level))
216206 took := time.Since(start)
217207 runtime.ReadMemStats(&after)
218208 require.NoError(t, err)
219- assert.True(t, strings.HasPrefix(p, ".github/workflows/d"), p)
209+ assert.True(t, strings.HasPrefix(p, "d"), p)
220210 assert.Less(t, took, 2*time.Second, "answered from the first path")
221211 assert.Less(t, after.TotalAlloc-before.TotalAlloc, uint64(4<<20), "bounded memory")
222212 // And no git is left behind.
@@ -225,69 +215,50 @@ func TestChangedWorkflowDoesNotBufferTheDiff(t *testing.T) {
225215 }
226216
227217 // A path longer than the bound fails closed: the push is refused, not read on.
228-func TestChangedWorkflowRefusesAnEndlessPath(t *testing.T) {
218+func TestFirstDifferentEntryRefusesAnEndlessPath(t *testing.T) {
229219 g, data := newGit(t)
230220 ctx := context.Background()
231221 f := readertest.NewFixture(t, data, "proj")
232222 t.Cleanup(func() { _ = g.Close(ctx) })
233223 level := treeOf(t, f, "100644 blob "+f.WriteObject("blob", "x\n")+"\tf")
234224 level = treeOf(t, f, "040000 tree "+level+"\t"+strings.Repeat("d", 5000)) // an entry name of 5000 bytes
235- _, err := g.ChangedWorkflow(ctx, f.Name, []domain.RefUpdate{{Old: domain.ZeroOID, New: withWorkflows(t, f, level), Ref: "refs/heads/x"}}, ports.PushObjects{})
225+ _, err := g.FirstDifferentEntry(ctx, f.Name, ports.PushObjects{}, domain.ObjectID{}, oidOf(t, level))
236226 assert.ErrorIs(t, err, domain.ErrInvalid)
237227 }
238228
239229 // A tag may point to a blob, a tree or a commit; only the commits and trees can
240230 // hold a workflow, and an object that is not there refuses the push.
241-func TestChangedWorkflowPeelsAndClassifiesBothEnds(t *testing.T) {
231+func TestPushSubtreesPeelsAndClassifiesBothEnds(t *testing.T) {
242232 g, data := newGit(t)
243233 ctx := context.Background()
244234 f := readertest.NewFixture(t, data, "proj")
245235 t.Cleanup(func() { _ = g.Close(ctx) })
246236 f.Write("README", "r\n")
247- plain := f.Commit("plain")
248- f.Write(".github/workflows/ci.yml", "on: push\n")
249- wf := f.Commit("workflow")
250- f.Write(".github/workflows/ci.yml", "on: [push]\n")
251- f.Commit("the default branch moves on to other workflows") // HEAD is not wf
237+ plain := oidOf(t, f.Commit("plain")).String()
252238 blob := f.WriteObject("blob", "hello\n")
253- annotatedBlob := f.WriteObject("tag", "object "+blob+"\ntype blob\ntag b\ntagger a <a@b> 1 +0000\n\nm\n")
254- annotatedWF := f.WriteObject("tag", "object "+oidOf(t, wf).String()+"\ntype commit\ntag c\ntagger a <a@b> 1 +0000\n\nm\n")
255- rootTree := f.Git("rev-parse", wf+"^{tree}")
256- ref := func(old, new string) []domain.RefUpdate {
257- return []domain.RefUpdate{{Old: old, New: new, Ref: "refs/tags/t"}}
258- }
259- changed := func(ups []domain.RefUpdate) (string, error) {
260- return g.ChangedWorkflow(ctx, f.Name, ups, ports.PushObjects{})
261- }
262- for name, ups := range map[string][]domain.RefUpdate{
263- "create a lightweight tag on a blob": ref(domain.ZeroOID, blob),
264- "create an annotated tag on a blob": ref(domain.ZeroOID, annotatedBlob),
265- "a tag from a blob to another blob": ref(blob, annotatedBlob),
266- "a tag from a commit with workflows to a blob": ref(oidOf(t, wf).String(), blob),
267- } {
268- p, err := changed(ups)
269- require.NoError(t, err, name)
270- assert.Empty(t, p, name)
239+ tag := func(id, kind string) string {
240+ return f.WriteObject("tag", "object "+id+"\ntype "+kind+"\ntag t\ntagger a <a@b> 1 +0000\n\nm\n")
271241 }
272- for name, ups := range map[string][]domain.RefUpdate{
273- "a tag from a blob to a commit with workflows": ref(blob, oidOf(t, wf).String()),
274- "a tag from an annotated blob to a commit with workflows": ref(annotatedBlob, oidOf(t, wf).String()),
275- "an annotated tag on a commit with workflows": ref(domain.ZeroOID, annotatedWF),
276- "a tag on a tree with workflows": ref(domain.ZeroOID, rootTree),
277- "from a commit without to one with": ref(oidOf(t, plain).String(), annotatedWF),
278- } {
279- p, err := changed(ups)
280- require.NoError(t, err, name)
281- assert.Equal(t, ".github/workflows/ci.yml", p, name)
242+ tree := f.Git("rev-parse", plain+"^{tree}")
243+ for _, r := range []struct {
244+ id string
245+ kind domain.PushObjectType
246+ }{{plain, domain.PushObjectCommit}, {tag(plain, "commit"), domain.PushObjectCommit}, {blob, domain.PushObjectBlob}, {tag(blob, "blob"), domain.PushObjectBlob}, {tree, domain.PushObjectTree}, {tag(tag(plain, "commit"), "tag"), domain.PushObjectCommit}} {
247+ got, err := g.PushSubtrees(ctx, f.Name, []domain.RefUpdate{{Old: domain.ZeroOID, New: r.id}}, ports.PushObjects{}, domain.WorkflowDir, domain.DefaultPushWalkBounds())
248+ require.NoError(t, err)
249+ assert.Equal(t, r.kind, got.Ends[r.id])
250+ if r.kind == domain.PushObjectBlob {
251+ assert.NotContains(t, got.Subtrees, r.id)
252+ }
282253 }
283- for name, ups := range map[string][]domain.RefUpdate{
284- "a missing new object": ref(domain.ZeroOID, "1111111111111111111111111111111111111111"),
285- "a missing old object": ref("2222222222222222222222222222222222222222", oidOf(t, wf).String()),
286- "a malformed object": ref(domain.ZeroOID, "ZZZZ"),
287- } {
288- _, err := changed(ups)
289- assert.Error(t, err, name)
254+ for _, r := range []domain.RefUpdate{{Old: domain.ZeroOID, New: strings.Repeat("1", 40)}, {Old: strings.Repeat("2", 40), New: plain}, {Old: domain.ZeroOID, New: "ZZZZ"}} {
255+ _, err := g.PushSubtrees(ctx, f.Name, []domain.RefUpdate{r}, ports.PushObjects{}, domain.WorkflowDir, domain.DefaultPushWalkBounds())
256+ assert.ErrorIs(t, err, domain.ErrInvalid)
290257 }
258+ got, err := g.PushSubtrees(ctx, f.Name, []domain.RefUpdate{{Old: "invalid but deleted", New: domain.ZeroOID}}, ports.PushObjects{}, domain.WorkflowDir, domain.DefaultPushWalkBounds())
259+ require.NoError(t, err)
260+ assert.Empty(t, got.Ends)
261+ assert.Empty(t, got.Commits)
291262 }
292263
293264 // rawTree writes a tree object by hand: entries are "mode name hexid", in the
@@ -308,73 +279,35 @@ func rawTree(t *testing.T, f *readertest.Fixture, entries ...[3]string) string {
308279 // mode 040000 below a directory .github of mode 040000. The same tree id as a
309280 // gitlink, a symbolic link or a file is nothing, and turning it into a
310281 // directory activates it; the object id alone does not tell.
311-func TestChangedWorkflowComparesTheModeOfTheEntries(t *testing.T) {
282+func TestPushSubtreesEntryKinds(t *testing.T) {
312283 g, data := newGit(t)
313284 ctx := context.Background()
314285 f := readertest.NewFixture(t, data, "proj")
315286 t.Cleanup(func() { _ = g.Close(ctx) })
316- f.Write("README", "r\n")
317- f.Commit("base")
318- yml := f.WriteObject("blob", "on: push\n")
319- wfTree := rawTree(t, f, [3]string{"100644", "ci.yml", yml}) // the workflows
320- link := f.WriteObject("blob", ".github/other")
321- file := f.WriteObject("blob", "not a directory\n")
322- readme := f.WriteObject("blob", "r\n")
323-
324- // A commit whose .github/workflows has the mode and id given.
325- withWorkflows := func(mode, id string) string {
326- gh := rawTree(t, f, [3]string{mode, "workflows", id})
327- return commitOf(t, f, rawTree(t, f, [3]string{"040000", ".github", gh}))
328- }
329- // A commit whose .github has the mode and id given.
330- withGithub := func(mode, id string) string {
331- return commitOf(t, f, rawTree(t, f, [3]string{mode, ".github", id}))
332- }
333- asDir := withWorkflows("040000", wfTree)
334- asGitlink := withWorkflows("160000", wfTree) // the same id, a submodule
335- asSymlink := withWorkflows("120000", link)
336- asFile := withWorkflows("100644", file)
337- none := commitOf(t, f, rawTree(t, f, [3]string{"100644", "README", readme}))
338- ghDir := rawTree(t, f, [3]string{"040000", "workflows", wfTree})
339- githubDir := withGithub("040000", ghDir)
340- githubGitlink := withGithub("160000", ghDir) // the same id, a submodule
341- githubFile := withGithub("100644", file)
342- githubLink := withGithub("120000", link)
343-
344- for name, c := range map[string]struct {
345- old, new string
346- want string
347- }{
348- "gitlink to a directory, same id (the bypass)": {asGitlink, asDir, ".github/workflows/ci.yml"},
349- "a directory to a gitlink removes it": {asDir, asGitlink, ""},
350- "symlink to a directory": {asSymlink, asDir, ".github/workflows/ci.yml"},
351- "a directory to a symlink removes it": {asDir, asSymlink, ""},
352- "file to a directory": {asFile, asDir, ".github/workflows/ci.yml"},
353- "a directory to a file removes it": {asDir, asFile, ""},
354- "nothing to a directory": {none, asDir, ".github/workflows/ci.yml"},
355- "a directory to nothing": {asDir, none, ""},
356- "the same gitlink": {asGitlink, asGitlink, ""},
357- "gitlink to symlink": {asGitlink, asSymlink, ""},
358- ".github a gitlink to a directory, same id": {githubGitlink, githubDir, ".github/workflows/ci.yml"},
359- ".github a directory to a gitlink": {githubDir, githubGitlink, ""},
360- ".github a file to a directory": {githubFile, githubDir, ".github/workflows/ci.yml"},
361- ".github a symlink to a directory": {githubLink, githubDir, ".github/workflows/ci.yml"},
362- ".github a directory to a symlink": {githubDir, githubLink, ""},
363- "a new ref with a gitlink": {domain.ZeroOID, asGitlink, ""},
364- "a new ref with a symlink": {domain.ZeroOID, asSymlink, ""},
365- "a new ref with .github a gitlink": {domain.ZeroOID, githubGitlink, ""},
366- "a new ref with a directory": {domain.ZeroOID, asDir, ".github/workflows/ci.yml"},
367- } {
368- p, err := g.ChangedWorkflow(ctx, f.Name, []domain.RefUpdate{{Old: c.old, New: c.new, Ref: "refs/heads/x"}}, ports.PushObjects{})
369- require.NoError(t, err, name)
370- assert.Equal(t, c.want, p, name)
287+ blob := f.WriteObject("blob", "x\n")
288+ tree := rawTree(t, f, [3]string{"100644", "ci.yml", blob})
289+ for _, r := range []struct {
290+ mode, id string
291+ kind domain.EntryKind
292+ }{{"040000", tree, domain.EntryDirectory}, {"160000", tree, domain.EntrySubmodule}, {"120000", blob, domain.EntrySymlink}, {"100644", blob, domain.EntryFile}, {"100755", blob, domain.EntryExecutable}} {
293+ t.Run(r.mode, func(t *testing.T) {
294+ gh := rawTree(t, f, [3]string{r.mode, "workflows", r.id})
295+ root := rawTree(t, f, [3]string{"040000", ".github", gh})
296+ oid := commitOf(t, f, root)
297+ got, err := g.PushSubtrees(ctx, f.Name, []domain.RefUpdate{{Old: domain.ZeroOID, New: oid}}, ports.PushObjects{}, domain.WorkflowDir, domain.DefaultPushWalkBounds())
298+ require.NoError(t, err)
299+ assert.Equal(t, r.kind, got.Subtrees[oid].Kind)
300+ assert.Equal(t, r.id, got.Subtrees[oid].ID.String())
301+ assert.NotContains(t, got.Subtrees, domain.HeadKey)
302+ if r.mode != "040000" {
303+ root = rawTree(t, f, [3]string{r.mode, ".github", r.id})
304+ oid = commitOf(t, f, root)
305+ got, err = g.PushSubtrees(ctx, f.Name, []domain.RefUpdate{{Old: domain.ZeroOID, New: oid}}, ports.PushObjects{}, domain.WorkflowDir, domain.DefaultPushWalkBounds())
306+ require.NoError(t, err)
307+ assert.NotContains(t, got.Subtrees, oid)
308+ }
309+ })
371310 }
372-
373- // A tag that peels to the commit is judged the same way.
374- tag := f.WriteObject("tag", "object "+asDir+"\ntype commit\ntag t\ntagger a <a@b> 1 +0000\n\nm\n")
375- p, err := g.ChangedWorkflow(ctx, f.Name, []domain.RefUpdate{{Old: asGitlink, New: tag, Ref: "refs/tags/t"}}, ports.PushObjects{})
376- require.NoError(t, err)
377- assert.Equal(t, ".github/workflows/ci.yml", p)
378311 }
379312
380313 // A history for the rules on the commits a push brings: main holds workflows A,
@@ -418,91 +351,37 @@ func (h *guardHistory) commit(workflows, _ string, parents ...string) string {
418351 return commitOf(h.t, h.f, rawTree(h.t, h.f, entries...), parents...)
419352 }
420353
421-func TestChangedWorkflowJudgesThePushedCommits(t *testing.T) {
422- g, data := newGit(t)
423- ctx := context.Background()
424- h := newGuardHistory(t, g, data)
425- t.Cleanup(func() { _ = g.Close(ctx) })
426- f := h.f
427- run := func(ups ...domain.RefUpdate) (string, error) {
428- return g.ChangedWorkflow(ctx, f.Name, ups, ports.PushObjects{})
429- }
430- create := func(ref, to string) domain.RefUpdate { return domain.RefUpdate{Old: domain.ZeroOID, New: to, Ref: ref} }
431- move := func(ref, from, to string) domain.RefUpdate { return domain.RefUpdate{Old: from, New: to, Ref: ref} }
432- const ci = ".github/workflows/ci.yml"
433-
434- allowed := func(name string, ups ...domain.RefUpdate) {
435- t.Helper()
436- p, err := run(ups...)
437- require.NoError(t, err, name)
438- assert.Empty(t, p, name)
439- }
440- refused := func(name string, ups ...domain.RefUpdate) {
441- t.Helper()
442- p, err := run(ups...)
443- require.NoError(t, err, name)
444- assert.Equal(t, ci, p, name)
445- }
446-
447- // The staging repro: a new branch off main with a commit that touches no workflow.
448- allowed("new branch off main with a plain commit", create("refs/heads/smoke", h.commit("A", "", h.m)))
449- allowed("a new branch at the head of main", create("refs/heads/same", h.m))
450- allowed("a new tag at the head of main", create("refs/tags/v1", h.m))
451- allowed("a new branch whose commits carry no workflows at all", create("refs/heads/bare", h.commit("", "")))
452- allowed("a fast-forward with plain commits", move("refs/heads/main", h.m, h.commit("A", "", h.commit("A", "", h.m))))
453- allowed("a commit that removes the workflows", move("refs/heads/main", h.m, h.commit("", "", h.m)))
454-
455- // A pushed commit that changes them is refused wherever it goes.
456- refused("new branch with a commit that changes workflows", create("refs/heads/w", h.commit("C", "", h.m)))
457- refused("a root commit that carries workflows", create("refs/heads/root", h.commit("C", "")))
458- refused("an update with a commit that changes workflows", move("refs/heads/main", h.m, h.commit("C", "", h.m)))
459- n4 := h.commit("C", "", h.m)
460- n5 := h.commit("A", "", n4) // reverts the change of the commit before
461- refused("a fast-forward through a commit that changes them and a later one that reverts it", move("refs/heads/main", h.m, n5))
462- refused("a new branch through the same", create("refs/heads/through", n5))
463- n6 := h.commit("A", "", h.commit("C", "", h.o))
464- refused("a commit that changes them below plain ones, on an old base", create("refs/heads/below", h.commit("C", "", n6)))
465-
466- // A ref cannot be moved onto existing commits with other workflows.
467- refused("a new branch at an old commit whose workflows differ from main", create("refs/heads/b", h.o))
468- refused("a new tag at that commit", create("refs/tags/old", h.o))
469- refused("an update moving back to an older commit with other workflows", move("refs/heads/main", h.m, h.o))
470- allowed("an update to a commit with the same workflows", move("refs/heads/old", h.o, h.commit("B", "", h.o)))
471- allowed("an update that moves a ref to a commit without workflows (removes them)", move("refs/heads/old", h.o, h.commit("", "", h.o)))
472-
473- // Merges: a change only if it differs from every parent.
474- allowed("a merge of existing commits that keeps main's workflows", move("refs/heads/main", h.m, h.commit("A", "", h.m, h.o)))
475- allowed("a new branch with a merge keeping one parent's workflows", create("refs/heads/merge", h.commit("A", "", h.m, h.o)))
476- refused("a merge that differs from both parents", create("refs/heads/merge2", h.commit("D", "", h.m, h.o)))
477- refused("a merge of a pushed side that changes them (the side commit is itself new)", create("refs/heads/merge3", h.commit("A", "", h.m, h.commit("C", "", h.o))))
478- // One that takes the old side's workflows is a change of the ref, not of a commit.
479- refused("a merge taking the other parent's workflows, onto main", move("refs/heads/main", h.m, h.commit("B", "", h.m, h.o)))
480-
481- // Several updates: the first refused refuses the push.
482- refused("a plain ref and a refused one", create("refs/heads/ok", h.commit("A", "", h.m)), create("refs/heads/w2", h.commit("C", "", h.m)))
483- allowed("deletes", move("refs/heads/old", h.o, domain.ZeroOID))
484-}
485-
486-// With no default branch head, a new ref is compared with no workflows at all.
487-func TestChangedWorkflowOfANewRefWithoutADefaultBranchHead(t *testing.T) {
354+func TestPushSubtreesCommitsAndParents(t *testing.T) {
488355 g, data := newGit(t)
489356 ctx := context.Background()
490357 h := newGuardHistory(t, g, data)
491358 t.Cleanup(func() { _ = g.Close(ctx) })
492- h.f.Git("symbolic-ref", "HEAD", "refs/heads/nothing") // the default branch does not exist
493- run := func(to string) string {
494- p, err := g.ChangedWorkflow(ctx, h.f.Name, []domain.RefUpdate{{Old: domain.ZeroOID, New: to, Ref: "refs/tags/t"}}, ports.PushObjects{})
495- require.NoError(t, err)
496- return p
359+ side := h.commit("B", "", h.o)
360+ merge := h.commit("A", "", h.m, side)
361+ got, err := g.PushSubtrees(ctx, h.f.Name, []domain.RefUpdate{{Old: h.m, New: merge}, {Old: domain.ZeroOID, New: merge}}, ports.PushObjects{}, domain.WorkflowDir, domain.DefaultPushWalkBounds())
362+ require.NoError(t, err)
363+ want := h.f.Git("rev-list", "--parents", merge, "--not", "--all")
364+ var lines []string
365+ for _, c := range got.Commits {
366+ line := c.ID.String()
367+ for _, p := range c.Parents {
368+ line += " " + p.String()
369+ }
370+ lines = append(lines, line)
497371 }
498- assert.Equal(t, ".github/workflows/ci.yml", run(h.m), "an existing commit with workflows")
499- plain := h.commit("", "")
500- h.f.Git("update-ref", "refs/heads/plain", plain)
501- assert.Empty(t, run(plain), "an existing commit without")
372+ assert.Equal(t, strings.TrimSpace(want), strings.Join(lines, "\n"))
373+ assert.Len(t, got.Commits, 2)
374+ assert.Equal(t, h.trees["A"], got.Subtrees[domain.HeadKey].ID.String())
375+ assert.Equal(t, h.trees["B"], got.Subtrees[h.o].ID.String())
376+ // Any directory path is accepted; the reader has no workflow path policy.
377+ other, err := g.PushSubtrees(ctx, h.f.Name, []domain.RefUpdate{{Old: h.m, New: merge}}, ports.PushObjects{}, "README", domain.DefaultPushWalkBounds())
378+ require.NoError(t, err)
379+ assert.Equal(t, domain.EntryFile, other.Subtrees[merge].Kind)
380+ assert.NotContains(t, other.Subtrees, domain.HeadKey, "HEAD is lazy for non-creates")
502381 }
503382
504383 // The commits of a push are bounded: more than the bound fails closed.
505-func TestChangedWorkflowBoundsThePushedCommits(t *testing.T) {
384+func TestPushSubtreesBoundsThePushedCommits(t *testing.T) {
506385 g, data := newGit(t)
507386 ctx := context.Background()
508387 h := newGuardHistory(t, g, data)
@@ -525,10 +404,10 @@ func TestChangedWorkflowBoundsThePushedCommits(t *testing.T) {
525404 }
526405 // Plain commits carry the workflows of main (the fast-import tree has none,
527406 // so the chain is "removing" them): allowed, whatever their number, up to the bound.
528- p, err := g.ChangedWorkflow(ctx, h.f.Name, []domain.RefUpdate{{Old: h.m, New: chain(gitcli.MaxPushedCommits), Ref: "refs/heads/main"}}, ports.PushObjects{})
407+ p, err := g.PushSubtrees(ctx, h.f.Name, []domain.RefUpdate{{Old: h.m, New: chain(20), Ref: "refs/heads/main"}}, ports.PushObjects{}, domain.WorkflowDir, domain.PushWalkBounds{Commits: 20, IDs: 80})
529408 require.NoError(t, err)
530- assert.Empty(t, p)
531- _, err = g.ChangedWorkflow(ctx, h.f.Name, []domain.RefUpdate{{Old: h.m, New: chain(gitcli.MaxPushedCommits + 1), Ref: "refs/heads/main"}}, ports.PushObjects{})
409+ assert.Len(t, p.Commits, 20)
410+ _, err = g.PushSubtrees(ctx, h.f.Name, []domain.RefUpdate{{Old: h.m, New: chain(21), Ref: "refs/heads/main"}}, ports.PushObjects{}, domain.WorkflowDir, domain.PushWalkBounds{Commits: 20, IDs: 80})
532411 assert.ErrorIs(t, err, domain.ErrInvalid, "more commits than the bound refuses the push")
533412 }
534413
@@ -565,18 +444,18 @@ func TestParseRevListRejectsManyParentsEarly(t *testing.T) {
565444 b.WriteString(" " + id(1+i%3)) // few distinct parents, repeated: the distinct count alone would stay small
566445 }
567446 b.WriteString("\n")
568- r := &countingReader{lines: gitcli.MaxPushedCommits, line: []byte(b.String())}
569- kept, err := gitcli.ParseRevList(r, gitcli.MaxPushedCommits, gitcli.MaxPushedObjects)
447+ r := &countingReader{lines: domain.MaxPushedCommits, line: []byte(b.String())}
448+ kept, err := gitcli.ParseRevList(r, domain.MaxPushedCommits, domain.MaxPushedIDs)
570449 require.ErrorIs(t, err, domain.ErrInvalid)
571450 assert.Contains(t, err.Error(), "ids (commits and parents)", "refused by the bound on ids, not by the line length")
572451 assert.Zero(t, kept, "nothing is returned with the error")
573- total := gitcli.MaxPushedCommits * len(b.String())
452+ total := domain.MaxPushedCommits * len(b.String())
574453 assert.Less(t, r.read, 2<<20, "read only the head of %d bytes", total)
575454 assert.Less(t, r.read*20, total)
576455
577456 // Within the bounds the same lines are kept: 100 commits of 198 parents.
578457 r = &countingReader{lines: 100, line: []byte(b.String())}
579- kept, err = gitcli.ParseRevList(r, gitcli.MaxPushedCommits, gitcli.MaxPushedObjects)
458+ kept, err = gitcli.ParseRevList(r, domain.MaxPushedCommits, domain.MaxPushedIDs)
580459 require.NoError(t, err)
581460 assert.Equal(t, 100, kept)
582461
@@ -590,3 +469,65 @@ func TestParseRevListRejectsManyParentsEarly(t *testing.T) {
590469 _, err = gitcli.ParseRevList(strings.NewReader(id(0)+strings.Repeat(" "+id(1), 300)+"\n"), 10, 1000)
591470 assert.ErrorIs(t, err, domain.ErrInvalid, "a line over the bound")
592471 }
472+
473+func changedWorkflow(g *gitcli.Git, ctx context.Context, repo domain.RepoName, ups []domain.RefUpdate, objects ports.PushObjects) (string, error) {
474+ t, err := g.PushSubtrees(ctx, repo, ups, objects, domain.WorkflowDir, domain.DefaultPushWalkBounds())
475+ if err != nil {
476+ return "", err
477+ }
478+ c, found, err := domain.WorkflowChangeOf(ups, t)
479+ if err != nil || !found {
480+ return "", err
481+ }
482+ entry, err := g.FirstDifferentEntry(ctx, repo, objects, c.Old, c.New)
483+ if err != nil {
484+ return "", err
485+ }
486+ if entry == "" {
487+ return domain.WorkflowDir, nil
488+ }
489+ return domain.WorkflowDir + "/" + entry, nil
490+}
491+
492+func TestFirstDifferentEntry(t *testing.T) {
493+ g, data := newGit(t)
494+ ctx := context.Background()
495+ f := readertest.NewFixture(t, data, "proj")
496+ t.Cleanup(func() { _ = g.Close(ctx) })
497+ tree := oidOf(t, treeOf(t, f, "100644 blob "+f.WriteObject("blob", "x\n")+"\tci.yml"))
498+ entry, err := g.FirstDifferentEntry(ctx, f.Name, ports.PushObjects{}, domain.ObjectID{}, tree)
499+ require.NoError(t, err)
500+ assert.Equal(t, "ci.yml", entry)
501+ entry, err = g.FirstDifferentEntry(ctx, f.Name, ports.PushObjects{}, tree, tree)
502+ require.NoError(t, err)
503+ assert.Empty(t, entry)
504+ _, err = g.FirstDifferentEntry(ctx, f.Name, ports.PushObjects{Dir: t.TempDir()}, domain.ObjectID{}, tree)
505+ assert.ErrorIs(t, err, domain.ErrInvalid)
506+}
507+
508+func TestChangedWorkflowPushHistories(t *testing.T) {
509+ g, data := newGit(t)
510+ ctx := context.Background()
511+ h := newGuardHistory(t, g, data)
512+ t.Cleanup(func() { _ = g.Close(ctx) })
513+ run := func(old, tip, want string) {
514+ t.Helper()
515+ got, err := changedWorkflow(g, ctx, h.f.Name, []domain.RefUpdate{{Old: old, New: tip, Ref: "refs/heads/pushed"}}, ports.PushObjects{})
516+ require.NoError(t, err)
517+ assert.Equal(t, want, got)
518+ }
519+ run(domain.ZeroOID, h.commit("A", "", h.m), "")
520+ change := h.commit("C", "", h.m)
521+ revert := h.commit("A", "", change)
522+ run(h.m, revert, ".github/workflows/ci.yml")
523+ run(domain.ZeroOID, h.commit("D", "", h.m, h.o), ".github/workflows/ci.yml")
524+ h.f.Git("symbolic-ref", "HEAD", "refs/heads/nothing")
525+ run(domain.ZeroOID, h.m, ".github/workflows/ci.yml")
526+ ups := []domain.RefUpdate{{Old: domain.ZeroOID, New: h.m, Ref: "refs/tags/t"}}
527+ trees, err := g.PushSubtrees(ctx, h.f.Name, ups, ports.PushObjects{}, domain.WorkflowDir, domain.DefaultPushWalkBounds())
528+ require.NoError(t, err)
529+ assert.NotContains(t, trees.Subtrees, domain.HeadKey)
530+ plain := h.commit("", "")
531+ h.f.Git("update-ref", "refs/heads/plain", plain)
532+ run(domain.ZeroOID, plain, "")
533+}
@@ -51,7 +51,7 @@ func TestChangedWorkflow(t *testing.T) {
51 return domain.RefUpdate{Old: old, New: new, Ref: "refs/heads/x"}51 return domain.RefUpdate{Old: old, New: new, Ref: "refs/heads/x"}
52 }52 }
53 changed := func(ups ...domain.RefUpdate) string {53 changed := func(ups ...domain.RefUpdate) string {
54- p, err := g.ChangedWorkflow(ctx, f.Name, ups, ports.PushObjects{})54+ p, err := changedWorkflow(g, ctx, f.Name, ups, ports.PushObjects{})
55 require.NoError(t, err)55 require.NoError(t, err)
56 return p56 return p
57 }57 }
@@ -68,11 +68,11 @@ func TestChangedWorkflow(t *testing.T) {
68 assert.Equal(t, ".github/workflows/ci.yml", changed(up(oid(base), oid(docs)), up(oid(docs), oid(edited))), "the first ref that changes one")68 assert.Equal(t, ".github/workflows/ci.yml", changed(up(oid(base), oid(docs)), up(oid(docs), oid(edited))), "the first ref that changes one")
69 assert.Equal(t, "", changed(), "no update")69 assert.Equal(t, "", changed(), "no update")
70 70
71- _, err := g.ChangedWorkflow(ctx, f.Name, []domain.RefUpdate{up(oid(base), "1111111111111111111111111111111111111111")}, ports.PushObjects{})71+ _, err := changedWorkflow(g, ctx, f.Name, []domain.RefUpdate{up(oid(base), "1111111111111111111111111111111111111111")}, ports.PushObjects{})
72 assert.Error(t, err, "an object git does not have is a failure, not a pass")72 assert.Error(t, err, "an object git does not have is a failure, not a pass")
73 }73 }
74 74
75-func TestChangedWorkflowReadsTheQuarantineOfThePushOnly(t *testing.T) {75+func TestPushSubtreesReadsTheQuarantineOfThePushOnly(t *testing.T) {
76 g, data := newGit(t)76 g, data := newGit(t)
77 ctx := context.Background()77 ctx := context.Background()
78 f := readertest.NewFixture(t, data, "proj")78 f := readertest.NewFixture(t, data, "proj")
@@ -90,23 +90,23 @@ func TestChangedWorkflowReadsTheQuarantineOfThePushOnly(t *testing.T) {
90 "an alternate outside": {Dir: objects + "/incoming-1", Alternates: "/tmp"},90 "an alternate outside": {Dir: objects + "/incoming-1", Alternates: "/tmp"},
91 "alternates without a dir": {Alternates: objects},91 "alternates without a dir": {Alternates: objects},
92 } {92 } {
93- _, err := g.ChangedWorkflow(ctx, f.Name, up, o)93+ _, err := g.PushSubtrees(ctx, f.Name, up, o, domain.WorkflowDir, domain.DefaultPushWalkBounds())
94 assert.ErrorIs(t, err, domain.ErrInvalid, name)94 assert.ErrorIs(t, err, domain.ErrInvalid, name)
95 }95 }
96 // An existing directory inside the repository is accepted as a location96 // An existing directory inside the repository is accepted as a location
97 // (the object is missing: a refusal of the object, not of the path).97 // (the object is missing: a refusal of the object, not of the path).
98 require.NoError(t, os.Mkdir(filepath.Join(objects, "incoming-1"), 0o700))98 require.NoError(t, os.Mkdir(filepath.Join(objects, "incoming-1"), 0o700))
99- _, err := g.ChangedWorkflow(ctx, f.Name, up, ports.PushObjects{Dir: objects + "/incoming-1", Alternates: objects})99+ _, err := g.PushSubtrees(ctx, f.Name, up, ports.PushObjects{Dir: objects + "/incoming-1", Alternates: objects}, domain.WorkflowDir, domain.DefaultPushWalkBounds())
100 require.Error(t, err)100 require.Error(t, err)
101 assert.NotContains(t, err.Error(), "outside the repository")101 assert.NotContains(t, err.Error(), "outside the repository")
102 // One that does not exist is refused.102 // One that does not exist is refused.
103- _, err = g.ChangedWorkflow(ctx, f.Name, up, ports.PushObjects{Dir: objects + "/incoming-2"})103+ _, err = g.PushSubtrees(ctx, f.Name, up, ports.PushObjects{Dir: objects + "/incoming-2"}, domain.WorkflowDir, domain.DefaultPushWalkBounds())
104 assert.ErrorIs(t, err, domain.ErrInvalid)104 assert.ErrorIs(t, err, domain.ErrInvalid)
105 }105 }
106 106
107 // A symbolic link in the way cannot take the objects of a push out of the107 // A symbolic link in the way cannot take the objects of a push out of the
108 // repository, and a data directory reached through links still works.108 // repository, and a data directory reached through links still works.
109-func TestChangedWorkflowResolvesSymbolicLinks(t *testing.T) {109+func TestPushSubtreesResolvesSymbolicLinks(t *testing.T) {
110 g, data := newGit(t)110 g, data := newGit(t)
111 ctx := context.Background()111 ctx := context.Background()
112 f := readertest.NewFixture(t, data, "proj")112 f := readertest.NewFixture(t, data, "proj")
@@ -119,20 +119,20 @@ func TestChangedWorkflowResolvesSymbolicLinks(t *testing.T) {
119 // Inside objects/, a link to elsewhere is outside once resolved.119 // Inside objects/, a link to elsewhere is outside once resolved.
120 elsewhere := t.TempDir()120 elsewhere := t.TempDir()
121 require.NoError(t, os.Symlink(elsewhere, filepath.Join(objects, "incoming-link")))121 require.NoError(t, os.Symlink(elsewhere, filepath.Join(objects, "incoming-link")))
122- _, err := g.ChangedWorkflow(ctx, f.Name, up, ports.PushObjects{Dir: filepath.Join(objects, "incoming-link")})122+ _, err := g.PushSubtrees(ctx, f.Name, up, ports.PushObjects{Dir: filepath.Join(objects, "incoming-link")}, domain.WorkflowDir, domain.DefaultPushWalkBounds())
123 assert.ErrorIs(t, err, domain.ErrInvalid, "a link out of objects/")123 assert.ErrorIs(t, err, domain.ErrInvalid, "a link out of objects/")
124- _, err = g.ChangedWorkflow(ctx, f.Name, up, ports.PushObjects{Dir: objects + "/pack", Alternates: filepath.Join(objects, "incoming-link")})124+ _, err = g.PushSubtrees(ctx, f.Name, up, ports.PushObjects{Dir: objects + "/pack", Alternates: filepath.Join(objects, "incoming-link")}, domain.WorkflowDir, domain.DefaultPushWalkBounds())
125 assert.ErrorIs(t, err, domain.ErrInvalid, "an alternate that is a link out of objects/")125 assert.ErrorIs(t, err, domain.ErrInvalid, "an alternate that is a link out of objects/")
126 126
127 // A link to somewhere inside objects/ is fine.127 // A link to somewhere inside objects/ is fine.
128 require.NoError(t, os.Mkdir(filepath.Join(objects, "q1"), 0o700))128 require.NoError(t, os.Mkdir(filepath.Join(objects, "q1"), 0o700))
129 require.NoError(t, os.Symlink(filepath.Join(objects, "q1"), filepath.Join(objects, "q-link")))129 require.NoError(t, os.Symlink(filepath.Join(objects, "q1"), filepath.Join(objects, "q-link")))
130- _, err = g.ChangedWorkflow(ctx, f.Name, up, ports.PushObjects{Dir: filepath.Join(objects, "q-link")})130+ _, err = g.PushSubtrees(ctx, f.Name, up, ports.PushObjects{Dir: filepath.Join(objects, "q-link")}, domain.WorkflowDir, domain.DefaultPushWalkBounds())
131 require.Error(t, err)131 require.Error(t, err)
132 assert.NotContains(t, err.Error(), "outside the repository")132 assert.NotContains(t, err.Error(), "outside the repository")
133 }133 }
134 134
135-func TestChangedWorkflowThroughALinkedDataDirectory(t *testing.T) {135+func TestPushSubtreesThroughALinkedDataDirectory(t *testing.T) {
136 _, data := newGit(t)136 _, data := newGit(t)
137 ctx := context.Background()137 ctx := context.Background()
138 f := readertest.NewFixture(t, data, "proj")138 f := readertest.NewFixture(t, data, "proj")
@@ -152,10 +152,10 @@ func TestChangedWorkflowThroughALinkedDataDirectory(t *testing.T) {
152 q := filepath.Join(f.Bare, "objects", "incoming-9")152 q := filepath.Join(f.Bare, "objects", "incoming-9")
153 require.NoError(t, os.Mkdir(q, 0o700))153 require.NoError(t, os.Mkdir(q, 0o700))
154 for _, dir := range []string{q, filepath.Join(link, "repos", "proj.git", "objects", "incoming-9")} {154 for _, dir := range []string{q, filepath.Join(link, "repos", "proj.git", "objects", "incoming-9")} {
155- p, err := g2.ChangedWorkflow(ctx, f.Name, []domain.RefUpdate{{Old: oidOf(t, base).String(), New: oidOf(t, wf).String(), Ref: "refs/heads/x"}},155+ p, err := g2.PushSubtrees(ctx, f.Name, []domain.RefUpdate{{Old: oidOf(t, base).String(), New: oidOf(t, wf).String(), Ref: "refs/heads/x"}},
156- ports.PushObjects{Dir: dir, Alternates: filepath.Join(link, "repos", "proj.git", "objects")})156+ ports.PushObjects{Dir: dir, Alternates: filepath.Join(link, "repos", "proj.git", "objects")}, domain.WorkflowDir, domain.DefaultPushWalkBounds())
157 require.NoError(t, err, dir)157 require.NoError(t, err, dir)
158- assert.Equal(t, ".github/workflows/ci.yml", p, dir)158+ assert.Equal(t, domain.EntryDirectory, p.Subtrees[oidOf(t, wf).String()].Kind, dir)
159 }159 }
160 }160 }
161 161
@@ -182,17 +182,7 @@ func commitOf(t *testing.T, f *readertest.Fixture, tree string, parents ...strin
182 return strings.TrimSpace(string(out))182 return strings.TrimSpace(string(out))
183 }183 }
184 184
185-// withWorkflows is a commit whose .github/workflows is the tree given.185+func TestFirstDifferentEntryDoesNotBufferTheDiff(t *testing.T) {
186-func withWorkflows(t *testing.T, f *readertest.Fixture, workflows string) string {
187- t.Helper()
188- gh := treeOf(t, f, "040000 tree "+workflows+"\tworkflows")
189- return commitOf(t, f, treeOf(t, f, "040000 tree "+gh+"\t.github"))
190-}
191-
192-// The trees of a push are the pusher's: a workflows directory that fans out
193-// into ten million paths is answered with its first path, at once, by a git
194-// that is stopped, without the diff ever being held in memory.
195-func TestChangedWorkflowDoesNotBufferTheDiff(t *testing.T) {
196 g, data := newGit(t)186 g, data := newGit(t)
197 ctx := context.Background()187 ctx := context.Background()
198 f := readertest.NewFixture(t, data, "proj")188 f := readertest.NewFixture(t, data, "proj")
@@ -206,17 +196,17 @@ func TestChangedWorkflowDoesNotBufferTheDiff(t *testing.T) {
206 }196 }
207 level = treeOf(t, f, entries...)197 level = treeOf(t, f, entries...)
208 }198 }
209- amp := withWorkflows(t, f, level)199+ amp := level
210 200
211 var before, after runtime.MemStats201 var before, after runtime.MemStats
212 runtime.GC()202 runtime.GC()
213 runtime.ReadMemStats(&before)203 runtime.ReadMemStats(&before)
214 start := time.Now()204 start := time.Now()
215- p, err := g.ChangedWorkflow(ctx, f.Name, []domain.RefUpdate{{Old: domain.ZeroOID, New: amp, Ref: "refs/heads/x"}}, ports.PushObjects{})205+ p, err := g.FirstDifferentEntry(ctx, f.Name, ports.PushObjects{}, domain.ObjectID{}, oidOf(t, level))
216 took := time.Since(start)206 took := time.Since(start)
217 runtime.ReadMemStats(&after)207 runtime.ReadMemStats(&after)
218 require.NoError(t, err)208 require.NoError(t, err)
219- assert.True(t, strings.HasPrefix(p, ".github/workflows/d"), p)209+ assert.True(t, strings.HasPrefix(p, "d"), p)
220 assert.Less(t, took, 2*time.Second, "answered from the first path")210 assert.Less(t, took, 2*time.Second, "answered from the first path")
221 assert.Less(t, after.TotalAlloc-before.TotalAlloc, uint64(4<<20), "bounded memory")211 assert.Less(t, after.TotalAlloc-before.TotalAlloc, uint64(4<<20), "bounded memory")
222 // And no git is left behind.212 // And no git is left behind.
@@ -225,69 +215,50 @@ func TestChangedWorkflowDoesNotBufferTheDiff(t *testing.T) {
225 }215 }
226 216
227 // A path longer than the bound fails closed: the push is refused, not read on.217 // A path longer than the bound fails closed: the push is refused, not read on.
228-func TestChangedWorkflowRefusesAnEndlessPath(t *testing.T) {218+func TestFirstDifferentEntryRefusesAnEndlessPath(t *testing.T) {
229 g, data := newGit(t)219 g, data := newGit(t)
230 ctx := context.Background()220 ctx := context.Background()
231 f := readertest.NewFixture(t, data, "proj")221 f := readertest.NewFixture(t, data, "proj")
232 t.Cleanup(func() { _ = g.Close(ctx) })222 t.Cleanup(func() { _ = g.Close(ctx) })
233 level := treeOf(t, f, "100644 blob "+f.WriteObject("blob", "x\n")+"\tf")223 level := treeOf(t, f, "100644 blob "+f.WriteObject("blob", "x\n")+"\tf")
234 level = treeOf(t, f, "040000 tree "+level+"\t"+strings.Repeat("d", 5000)) // an entry name of 5000 bytes224 level = treeOf(t, f, "040000 tree "+level+"\t"+strings.Repeat("d", 5000)) // an entry name of 5000 bytes
235- _, err := g.ChangedWorkflow(ctx, f.Name, []domain.RefUpdate{{Old: domain.ZeroOID, New: withWorkflows(t, f, level), Ref: "refs/heads/x"}}, ports.PushObjects{})225+ _, err := g.FirstDifferentEntry(ctx, f.Name, ports.PushObjects{}, domain.ObjectID{}, oidOf(t, level))
236 assert.ErrorIs(t, err, domain.ErrInvalid)226 assert.ErrorIs(t, err, domain.ErrInvalid)
237 }227 }
238 228
239 // A tag may point to a blob, a tree or a commit; only the commits and trees can229 // A tag may point to a blob, a tree or a commit; only the commits and trees can
240 // hold a workflow, and an object that is not there refuses the push.230 // hold a workflow, and an object that is not there refuses the push.
241-func TestChangedWorkflowPeelsAndClassifiesBothEnds(t *testing.T) {231+func TestPushSubtreesPeelsAndClassifiesBothEnds(t *testing.T) {
242 g, data := newGit(t)232 g, data := newGit(t)
243 ctx := context.Background()233 ctx := context.Background()
244 f := readertest.NewFixture(t, data, "proj")234 f := readertest.NewFixture(t, data, "proj")
245 t.Cleanup(func() { _ = g.Close(ctx) })235 t.Cleanup(func() { _ = g.Close(ctx) })
246 f.Write("README", "r\n")236 f.Write("README", "r\n")
247- plain := f.Commit("plain")237+ plain := oidOf(t, f.Commit("plain")).String()
248- f.Write(".github/workflows/ci.yml", "on: push\n")
249- wf := f.Commit("workflow")
250- f.Write(".github/workflows/ci.yml", "on: [push]\n")
251- f.Commit("the default branch moves on to other workflows") // HEAD is not wf
252 blob := f.WriteObject("blob", "hello\n")238 blob := f.WriteObject("blob", "hello\n")
253- annotatedBlob := f.WriteObject("tag", "object "+blob+"\ntype blob\ntag b\ntagger a <a@b> 1 +0000\n\nm\n")239+ tag := func(id, kind string) string {
254- annotatedWF := f.WriteObject("tag", "object "+oidOf(t, wf).String()+"\ntype commit\ntag c\ntagger a <a@b> 1 +0000\n\nm\n")240+ return f.WriteObject("tag", "object "+id+"\ntype "+kind+"\ntag t\ntagger a <a@b> 1 +0000\n\nm\n")
255- rootTree := f.Git("rev-parse", wf+"^{tree}")
256- ref := func(old, new string) []domain.RefUpdate {
257- return []domain.RefUpdate{{Old: old, New: new, Ref: "refs/tags/t"}}
258- }
259- changed := func(ups []domain.RefUpdate) (string, error) {
260- return g.ChangedWorkflow(ctx, f.Name, ups, ports.PushObjects{})
261- }
262- for name, ups := range map[string][]domain.RefUpdate{
263- "create a lightweight tag on a blob": ref(domain.ZeroOID, blob),
264- "create an annotated tag on a blob": ref(domain.ZeroOID, annotatedBlob),
265- "a tag from a blob to another blob": ref(blob, annotatedBlob),
266- "a tag from a commit with workflows to a blob": ref(oidOf(t, wf).String(), blob),
267- } {
268- p, err := changed(ups)
269- require.NoError(t, err, name)
270- assert.Empty(t, p, name)
271 }241 }
272- for name, ups := range map[string][]domain.RefUpdate{242+ tree := f.Git("rev-parse", plain+"^{tree}")
273- "a tag from a blob to a commit with workflows": ref(blob, oidOf(t, wf).String()),243+ for _, r := range []struct {
274- "a tag from an annotated blob to a commit with workflows": ref(annotatedBlob, oidOf(t, wf).String()),244+ id string
275- "an annotated tag on a commit with workflows": ref(domain.ZeroOID, annotatedWF),245+ kind domain.PushObjectType
276- "a tag on a tree with workflows": ref(domain.ZeroOID, rootTree),246+ }{{plain, domain.PushObjectCommit}, {tag(plain, "commit"), domain.PushObjectCommit}, {blob, domain.PushObjectBlob}, {tag(blob, "blob"), domain.PushObjectBlob}, {tree, domain.PushObjectTree}, {tag(tag(plain, "commit"), "tag"), domain.PushObjectCommit}} {
277- "from a commit without to one with": ref(oidOf(t, plain).String(), annotatedWF),247+ got, err := g.PushSubtrees(ctx, f.Name, []domain.RefUpdate{{Old: domain.ZeroOID, New: r.id}}, ports.PushObjects{}, domain.WorkflowDir, domain.DefaultPushWalkBounds())
278- } {248+ require.NoError(t, err)
279- p, err := changed(ups)249+ assert.Equal(t, r.kind, got.Ends[r.id])
280- require.NoError(t, err, name)250+ if r.kind == domain.PushObjectBlob {
281- assert.Equal(t, ".github/workflows/ci.yml", p, name)251+ assert.NotContains(t, got.Subtrees, r.id)
252+ }
282 }253 }
283- for name, ups := range map[string][]domain.RefUpdate{254+ for _, r := range []domain.RefUpdate{{Old: domain.ZeroOID, New: strings.Repeat("1", 40)}, {Old: strings.Repeat("2", 40), New: plain}, {Old: domain.ZeroOID, New: "ZZZZ"}} {
284- "a missing new object": ref(domain.ZeroOID, "1111111111111111111111111111111111111111"),255+ _, err := g.PushSubtrees(ctx, f.Name, []domain.RefUpdate{r}, ports.PushObjects{}, domain.WorkflowDir, domain.DefaultPushWalkBounds())
285- "a missing old object": ref("2222222222222222222222222222222222222222", oidOf(t, wf).String()),256+ assert.ErrorIs(t, err, domain.ErrInvalid)
286- "a malformed object": ref(domain.ZeroOID, "ZZZZ"),
287- } {
288- _, err := changed(ups)
289- assert.Error(t, err, name)
290 }257 }
258+ got, err := g.PushSubtrees(ctx, f.Name, []domain.RefUpdate{{Old: "invalid but deleted", New: domain.ZeroOID}}, ports.PushObjects{}, domain.WorkflowDir, domain.DefaultPushWalkBounds())
259+ require.NoError(t, err)
260+ assert.Empty(t, got.Ends)
261+ assert.Empty(t, got.Commits)
291 }262 }
292 263
293 // rawTree writes a tree object by hand: entries are "mode name hexid", in the264 // rawTree writes a tree object by hand: entries are "mode name hexid", in the
@@ -308,73 +279,35 @@ func rawTree(t *testing.T, f *readertest.Fixture, entries ...[3]string) string {
308 // mode 040000 below a directory .github of mode 040000. The same tree id as a279 // mode 040000 below a directory .github of mode 040000. The same tree id as a
309 // gitlink, a symbolic link or a file is nothing, and turning it into a280 // gitlink, a symbolic link or a file is nothing, and turning it into a
310 // directory activates it; the object id alone does not tell.281 // directory activates it; the object id alone does not tell.
311-func TestChangedWorkflowComparesTheModeOfTheEntries(t *testing.T) {282+func TestPushSubtreesEntryKinds(t *testing.T) {
312 g, data := newGit(t)283 g, data := newGit(t)
313 ctx := context.Background()284 ctx := context.Background()
314 f := readertest.NewFixture(t, data, "proj")285 f := readertest.NewFixture(t, data, "proj")
315 t.Cleanup(func() { _ = g.Close(ctx) })286 t.Cleanup(func() { _ = g.Close(ctx) })
316- f.Write("README", "r\n")287+ blob := f.WriteObject("blob", "x\n")
317- f.Commit("base")288+ tree := rawTree(t, f, [3]string{"100644", "ci.yml", blob})
318- yml := f.WriteObject("blob", "on: push\n")289+ for _, r := range []struct {
319- wfTree := rawTree(t, f, [3]string{"100644", "ci.yml", yml}) // the workflows290+ mode, id string
320- link := f.WriteObject("blob", ".github/other")291+ kind domain.EntryKind
321- file := f.WriteObject("blob", "not a directory\n")292+ }{{"040000", tree, domain.EntryDirectory}, {"160000", tree, domain.EntrySubmodule}, {"120000", blob, domain.EntrySymlink}, {"100644", blob, domain.EntryFile}, {"100755", blob, domain.EntryExecutable}} {
322- readme := f.WriteObject("blob", "r\n")293+ t.Run(r.mode, func(t *testing.T) {
323-294+ gh := rawTree(t, f, [3]string{r.mode, "workflows", r.id})
324- // A commit whose .github/workflows has the mode and id given.295+ root := rawTree(t, f, [3]string{"040000", ".github", gh})
325- withWorkflows := func(mode, id string) string {296+ oid := commitOf(t, f, root)
326- gh := rawTree(t, f, [3]string{mode, "workflows", id})297+ got, err := g.PushSubtrees(ctx, f.Name, []domain.RefUpdate{{Old: domain.ZeroOID, New: oid}}, ports.PushObjects{}, domain.WorkflowDir, domain.DefaultPushWalkBounds())
327- return commitOf(t, f, rawTree(t, f, [3]string{"040000", ".github", gh}))298+ require.NoError(t, err)
328- }299+ assert.Equal(t, r.kind, got.Subtrees[oid].Kind)
329- // A commit whose .github has the mode and id given.300+ assert.Equal(t, r.id, got.Subtrees[oid].ID.String())
330- withGithub := func(mode, id string) string {301+ assert.NotContains(t, got.Subtrees, domain.HeadKey)
331- return commitOf(t, f, rawTree(t, f, [3]string{mode, ".github", id}))302+ if r.mode != "040000" {
332- }303+ root = rawTree(t, f, [3]string{r.mode, ".github", r.id})
333- asDir := withWorkflows("040000", wfTree)304+ oid = commitOf(t, f, root)
334- asGitlink := withWorkflows("160000", wfTree) // the same id, a submodule305+ got, err = g.PushSubtrees(ctx, f.Name, []domain.RefUpdate{{Old: domain.ZeroOID, New: oid}}, ports.PushObjects{}, domain.WorkflowDir, domain.DefaultPushWalkBounds())
335- asSymlink := withWorkflows("120000", link)306+ require.NoError(t, err)
336- asFile := withWorkflows("100644", file)307+ assert.NotContains(t, got.Subtrees, oid)
337- none := commitOf(t, f, rawTree(t, f, [3]string{"100644", "README", readme}))308+ }
338- ghDir := rawTree(t, f, [3]string{"040000", "workflows", wfTree})309+ })
339- githubDir := withGithub("040000", ghDir)
340- githubGitlink := withGithub("160000", ghDir) // the same id, a submodule
341- githubFile := withGithub("100644", file)
342- githubLink := withGithub("120000", link)
343-
344- for name, c := range map[string]struct {
345- old, new string
346- want string
347- }{
348- "gitlink to a directory, same id (the bypass)": {asGitlink, asDir, ".github/workflows/ci.yml"},
349- "a directory to a gitlink removes it": {asDir, asGitlink, ""},
350- "symlink to a directory": {asSymlink, asDir, ".github/workflows/ci.yml"},
351- "a directory to a symlink removes it": {asDir, asSymlink, ""},
352- "file to a directory": {asFile, asDir, ".github/workflows/ci.yml"},
353- "a directory to a file removes it": {asDir, asFile, ""},
354- "nothing to a directory": {none, asDir, ".github/workflows/ci.yml"},
355- "a directory to nothing": {asDir, none, ""},
356- "the same gitlink": {asGitlink, asGitlink, ""},
357- "gitlink to symlink": {asGitlink, asSymlink, ""},
358- ".github a gitlink to a directory, same id": {githubGitlink, githubDir, ".github/workflows/ci.yml"},
359- ".github a directory to a gitlink": {githubDir, githubGitlink, ""},
360- ".github a file to a directory": {githubFile, githubDir, ".github/workflows/ci.yml"},
361- ".github a symlink to a directory": {githubLink, githubDir, ".github/workflows/ci.yml"},
362- ".github a directory to a symlink": {githubDir, githubLink, ""},
363- "a new ref with a gitlink": {domain.ZeroOID, asGitlink, ""},
364- "a new ref with a symlink": {domain.ZeroOID, asSymlink, ""},
365- "a new ref with .github a gitlink": {domain.ZeroOID, githubGitlink, ""},
366- "a new ref with a directory": {domain.ZeroOID, asDir, ".github/workflows/ci.yml"},
367- } {
368- p, err := g.ChangedWorkflow(ctx, f.Name, []domain.RefUpdate{{Old: c.old, New: c.new, Ref: "refs/heads/x"}}, ports.PushObjects{})
369- require.NoError(t, err, name)
370- assert.Equal(t, c.want, p, name)
371 }310 }
372-
373- // A tag that peels to the commit is judged the same way.
374- tag := f.WriteObject("tag", "object "+asDir+"\ntype commit\ntag t\ntagger a <a@b> 1 +0000\n\nm\n")
375- p, err := g.ChangedWorkflow(ctx, f.Name, []domain.RefUpdate{{Old: asGitlink, New: tag, Ref: "refs/tags/t"}}, ports.PushObjects{})
376- require.NoError(t, err)
377- assert.Equal(t, ".github/workflows/ci.yml", p)
378 }311 }
379 312
380 // A history for the rules on the commits a push brings: main holds workflows A,313 // A history for the rules on the commits a push brings: main holds workflows A,
@@ -418,91 +351,37 @@ func (h *guardHistory) commit(workflows, _ string, parents ...string) string {
418 return commitOf(h.t, h.f, rawTree(h.t, h.f, entries...), parents...)351 return commitOf(h.t, h.f, rawTree(h.t, h.f, entries...), parents...)
419 }352 }
420 353
421-func TestChangedWorkflowJudgesThePushedCommits(t *testing.T) {354+func TestPushSubtreesCommitsAndParents(t *testing.T) {
422- g, data := newGit(t)
423- ctx := context.Background()
424- h := newGuardHistory(t, g, data)
425- t.Cleanup(func() { _ = g.Close(ctx) })
426- f := h.f
427- run := func(ups ...domain.RefUpdate) (string, error) {
428- return g.ChangedWorkflow(ctx, f.Name, ups, ports.PushObjects{})
429- }
430- create := func(ref, to string) domain.RefUpdate { return domain.RefUpdate{Old: domain.ZeroOID, New: to, Ref: ref} }
431- move := func(ref, from, to string) domain.RefUpdate { return domain.RefUpdate{Old: from, New: to, Ref: ref} }
432- const ci = ".github/workflows/ci.yml"
433-
434- allowed := func(name string, ups ...domain.RefUpdate) {
435- t.Helper()
436- p, err := run(ups...)
437- require.NoError(t, err, name)
438- assert.Empty(t, p, name)
439- }
440- refused := func(name string, ups ...domain.RefUpdate) {
441- t.Helper()
442- p, err := run(ups...)
443- require.NoError(t, err, name)
444- assert.Equal(t, ci, p, name)
445- }
446-
447- // The staging repro: a new branch off main with a commit that touches no workflow.
448- allowed("new branch off main with a plain commit", create("refs/heads/smoke", h.commit("A", "", h.m)))
449- allowed("a new branch at the head of main", create("refs/heads/same", h.m))
450- allowed("a new tag at the head of main", create("refs/tags/v1", h.m))
451- allowed("a new branch whose commits carry no workflows at all", create("refs/heads/bare", h.commit("", "")))
452- allowed("a fast-forward with plain commits", move("refs/heads/main", h.m, h.commit("A", "", h.commit("A", "", h.m))))
453- allowed("a commit that removes the workflows", move("refs/heads/main", h.m, h.commit("", "", h.m)))
454-
455- // A pushed commit that changes them is refused wherever it goes.
456- refused("new branch with a commit that changes workflows", create("refs/heads/w", h.commit("C", "", h.m)))
457- refused("a root commit that carries workflows", create("refs/heads/root", h.commit("C", "")))
458- refused("an update with a commit that changes workflows", move("refs/heads/main", h.m, h.commit("C", "", h.m)))
459- n4 := h.commit("C", "", h.m)
460- n5 := h.commit("A", "", n4) // reverts the change of the commit before
461- refused("a fast-forward through a commit that changes them and a later one that reverts it", move("refs/heads/main", h.m, n5))
462- refused("a new branch through the same", create("refs/heads/through", n5))
463- n6 := h.commit("A", "", h.commit("C", "", h.o))
464- refused("a commit that changes them below plain ones, on an old base", create("refs/heads/below", h.commit("C", "", n6)))
465-
466- // A ref cannot be moved onto existing commits with other workflows.
467- refused("a new branch at an old commit whose workflows differ from main", create("refs/heads/b", h.o))
468- refused("a new tag at that commit", create("refs/tags/old", h.o))
469- refused("an update moving back to an older commit with other workflows", move("refs/heads/main", h.m, h.o))
470- allowed("an update to a commit with the same workflows", move("refs/heads/old", h.o, h.commit("B", "", h.o)))
471- allowed("an update that moves a ref to a commit without workflows (removes them)", move("refs/heads/old", h.o, h.commit("", "", h.o)))
472-
473- // Merges: a change only if it differs from every parent.
474- allowed("a merge of existing commits that keeps main's workflows", move("refs/heads/main", h.m, h.commit("A", "", h.m, h.o)))
475- allowed("a new branch with a merge keeping one parent's workflows", create("refs/heads/merge", h.commit("A", "", h.m, h.o)))
476- refused("a merge that differs from both parents", create("refs/heads/merge2", h.commit("D", "", h.m, h.o)))
477- refused("a merge of a pushed side that changes them (the side commit is itself new)", create("refs/heads/merge3", h.commit("A", "", h.m, h.commit("C", "", h.o))))
478- // One that takes the old side's workflows is a change of the ref, not of a commit.
479- refused("a merge taking the other parent's workflows, onto main", move("refs/heads/main", h.m, h.commit("B", "", h.m, h.o)))
480-
481- // Several updates: the first refused refuses the push.
482- refused("a plain ref and a refused one", create("refs/heads/ok", h.commit("A", "", h.m)), create("refs/heads/w2", h.commit("C", "", h.m)))
483- allowed("deletes", move("refs/heads/old", h.o, domain.ZeroOID))
484-}
485-
486-// With no default branch head, a new ref is compared with no workflows at all.
487-func TestChangedWorkflowOfANewRefWithoutADefaultBranchHead(t *testing.T) {
488 g, data := newGit(t)355 g, data := newGit(t)
489 ctx := context.Background()356 ctx := context.Background()
490 h := newGuardHistory(t, g, data)357 h := newGuardHistory(t, g, data)
491 t.Cleanup(func() { _ = g.Close(ctx) })358 t.Cleanup(func() { _ = g.Close(ctx) })
492- h.f.Git("symbolic-ref", "HEAD", "refs/heads/nothing") // the default branch does not exist359+ side := h.commit("B", "", h.o)
493- run := func(to string) string {360+ merge := h.commit("A", "", h.m, side)
494- p, err := g.ChangedWorkflow(ctx, h.f.Name, []domain.RefUpdate{{Old: domain.ZeroOID, New: to, Ref: "refs/tags/t"}}, ports.PushObjects{})361+ got, err := g.PushSubtrees(ctx, h.f.Name, []domain.RefUpdate{{Old: h.m, New: merge}, {Old: domain.ZeroOID, New: merge}}, ports.PushObjects{}, domain.WorkflowDir, domain.DefaultPushWalkBounds())
495- require.NoError(t, err)362+ require.NoError(t, err)
496- return p363+ want := h.f.Git("rev-list", "--parents", merge, "--not", "--all")
364+ var lines []string
365+ for _, c := range got.Commits {
366+ line := c.ID.String()
367+ for _, p := range c.Parents {
368+ line += " " + p.String()
369+ }
370+ lines = append(lines, line)
497 }371 }
498- assert.Equal(t, ".github/workflows/ci.yml", run(h.m), "an existing commit with workflows")372+ assert.Equal(t, strings.TrimSpace(want), strings.Join(lines, "\n"))
499- plain := h.commit("", "")373+ assert.Len(t, got.Commits, 2)
500- h.f.Git("update-ref", "refs/heads/plain", plain)374+ assert.Equal(t, h.trees["A"], got.Subtrees[domain.HeadKey].ID.String())
501- assert.Empty(t, run(plain), "an existing commit without")375+ assert.Equal(t, h.trees["B"], got.Subtrees[h.o].ID.String())
376+ // Any directory path is accepted; the reader has no workflow path policy.
377+ other, err := g.PushSubtrees(ctx, h.f.Name, []domain.RefUpdate{{Old: h.m, New: merge}}, ports.PushObjects{}, "README", domain.DefaultPushWalkBounds())
378+ require.NoError(t, err)
379+ assert.Equal(t, domain.EntryFile, other.Subtrees[merge].Kind)
380+ assert.NotContains(t, other.Subtrees, domain.HeadKey, "HEAD is lazy for non-creates")
502 }381 }
503 382
504 // The commits of a push are bounded: more than the bound fails closed.383 // The commits of a push are bounded: more than the bound fails closed.
505-func TestChangedWorkflowBoundsThePushedCommits(t *testing.T) {384+func TestPushSubtreesBoundsThePushedCommits(t *testing.T) {
506 g, data := newGit(t)385 g, data := newGit(t)
507 ctx := context.Background()386 ctx := context.Background()
508 h := newGuardHistory(t, g, data)387 h := newGuardHistory(t, g, data)
@@ -525,10 +404,10 @@ func TestChangedWorkflowBoundsThePushedCommits(t *testing.T) {
525 }404 }
526 // Plain commits carry the workflows of main (the fast-import tree has none,405 // Plain commits carry the workflows of main (the fast-import tree has none,
527 // so the chain is "removing" them): allowed, whatever their number, up to the bound.406 // so the chain is "removing" them): allowed, whatever their number, up to the bound.
528- p, err := g.ChangedWorkflow(ctx, h.f.Name, []domain.RefUpdate{{Old: h.m, New: chain(gitcli.MaxPushedCommits), Ref: "refs/heads/main"}}, ports.PushObjects{})407+ p, err := g.PushSubtrees(ctx, h.f.Name, []domain.RefUpdate{{Old: h.m, New: chain(20), Ref: "refs/heads/main"}}, ports.PushObjects{}, domain.WorkflowDir, domain.PushWalkBounds{Commits: 20, IDs: 80})
529 require.NoError(t, err)408 require.NoError(t, err)
530- assert.Empty(t, p)409+ assert.Len(t, p.Commits, 20)
531- _, err = g.ChangedWorkflow(ctx, h.f.Name, []domain.RefUpdate{{Old: h.m, New: chain(gitcli.MaxPushedCommits + 1), Ref: "refs/heads/main"}}, ports.PushObjects{})410+ _, err = g.PushSubtrees(ctx, h.f.Name, []domain.RefUpdate{{Old: h.m, New: chain(21), Ref: "refs/heads/main"}}, ports.PushObjects{}, domain.WorkflowDir, domain.PushWalkBounds{Commits: 20, IDs: 80})
532 assert.ErrorIs(t, err, domain.ErrInvalid, "more commits than the bound refuses the push")411 assert.ErrorIs(t, err, domain.ErrInvalid, "more commits than the bound refuses the push")
533 }412 }
534 413
@@ -565,18 +444,18 @@ func TestParseRevListRejectsManyParentsEarly(t *testing.T) {
565 b.WriteString(" " + id(1+i%3)) // few distinct parents, repeated: the distinct count alone would stay small444 b.WriteString(" " + id(1+i%3)) // few distinct parents, repeated: the distinct count alone would stay small
566 }445 }
567 b.WriteString("\n")446 b.WriteString("\n")
568- r := &countingReader{lines: gitcli.MaxPushedCommits, line: []byte(b.String())}447+ r := &countingReader{lines: domain.MaxPushedCommits, line: []byte(b.String())}
569- kept, err := gitcli.ParseRevList(r, gitcli.MaxPushedCommits, gitcli.MaxPushedObjects)448+ kept, err := gitcli.ParseRevList(r, domain.MaxPushedCommits, domain.MaxPushedIDs)
570 require.ErrorIs(t, err, domain.ErrInvalid)449 require.ErrorIs(t, err, domain.ErrInvalid)
571 assert.Contains(t, err.Error(), "ids (commits and parents)", "refused by the bound on ids, not by the line length")450 assert.Contains(t, err.Error(), "ids (commits and parents)", "refused by the bound on ids, not by the line length")
572 assert.Zero(t, kept, "nothing is returned with the error")451 assert.Zero(t, kept, "nothing is returned with the error")
573- total := gitcli.MaxPushedCommits * len(b.String())452+ total := domain.MaxPushedCommits * len(b.String())
574 assert.Less(t, r.read, 2<<20, "read only the head of %d bytes", total)453 assert.Less(t, r.read, 2<<20, "read only the head of %d bytes", total)
575 assert.Less(t, r.read*20, total)454 assert.Less(t, r.read*20, total)
576 455
577 // Within the bounds the same lines are kept: 100 commits of 198 parents.456 // Within the bounds the same lines are kept: 100 commits of 198 parents.
578 r = &countingReader{lines: 100, line: []byte(b.String())}457 r = &countingReader{lines: 100, line: []byte(b.String())}
579- kept, err = gitcli.ParseRevList(r, gitcli.MaxPushedCommits, gitcli.MaxPushedObjects)458+ kept, err = gitcli.ParseRevList(r, domain.MaxPushedCommits, domain.MaxPushedIDs)
580 require.NoError(t, err)459 require.NoError(t, err)
581 assert.Equal(t, 100, kept)460 assert.Equal(t, 100, kept)
582 461
@@ -590,3 +469,65 @@ func TestParseRevListRejectsManyParentsEarly(t *testing.T) {
590 _, err = gitcli.ParseRevList(strings.NewReader(id(0)+strings.Repeat(" "+id(1), 300)+"\n"), 10, 1000)469 _, err = gitcli.ParseRevList(strings.NewReader(id(0)+strings.Repeat(" "+id(1), 300)+"\n"), 10, 1000)
591 assert.ErrorIs(t, err, domain.ErrInvalid, "a line over the bound")470 assert.ErrorIs(t, err, domain.ErrInvalid, "a line over the bound")
592 }471 }
472+
473+func changedWorkflow(g *gitcli.Git, ctx context.Context, repo domain.RepoName, ups []domain.RefUpdate, objects ports.PushObjects) (string, error) {
474+ t, err := g.PushSubtrees(ctx, repo, ups, objects, domain.WorkflowDir, domain.DefaultPushWalkBounds())
475+ if err != nil {
476+ return "", err
477+ }
478+ c, found, err := domain.WorkflowChangeOf(ups, t)
479+ if err != nil || !found {
480+ return "", err
481+ }
482+ entry, err := g.FirstDifferentEntry(ctx, repo, objects, c.Old, c.New)
483+ if err != nil {
484+ return "", err
485+ }
486+ if entry == "" {
487+ return domain.WorkflowDir, nil
488+ }
489+ return domain.WorkflowDir + "/" + entry, nil
490+}
491+
492+func TestFirstDifferentEntry(t *testing.T) {
493+ g, data := newGit(t)
494+ ctx := context.Background()
495+ f := readertest.NewFixture(t, data, "proj")
496+ t.Cleanup(func() { _ = g.Close(ctx) })
497+ tree := oidOf(t, treeOf(t, f, "100644 blob "+f.WriteObject("blob", "x\n")+"\tci.yml"))
498+ entry, err := g.FirstDifferentEntry(ctx, f.Name, ports.PushObjects{}, domain.ObjectID{}, tree)
499+ require.NoError(t, err)
500+ assert.Equal(t, "ci.yml", entry)
501+ entry, err = g.FirstDifferentEntry(ctx, f.Name, ports.PushObjects{}, tree, tree)
502+ require.NoError(t, err)
503+ assert.Empty(t, entry)
504+ _, err = g.FirstDifferentEntry(ctx, f.Name, ports.PushObjects{Dir: t.TempDir()}, domain.ObjectID{}, tree)
505+ assert.ErrorIs(t, err, domain.ErrInvalid)
506+}
507+
508+func TestChangedWorkflowPushHistories(t *testing.T) {
509+ g, data := newGit(t)
510+ ctx := context.Background()
511+ h := newGuardHistory(t, g, data)
512+ t.Cleanup(func() { _ = g.Close(ctx) })
513+ run := func(old, tip, want string) {
514+ t.Helper()
515+ got, err := changedWorkflow(g, ctx, h.f.Name, []domain.RefUpdate{{Old: old, New: tip, Ref: "refs/heads/pushed"}}, ports.PushObjects{})
516+ require.NoError(t, err)
517+ assert.Equal(t, want, got)
518+ }
519+ run(domain.ZeroOID, h.commit("A", "", h.m), "")
520+ change := h.commit("C", "", h.m)
521+ revert := h.commit("A", "", change)
522+ run(h.m, revert, ".github/workflows/ci.yml")
523+ run(domain.ZeroOID, h.commit("D", "", h.m, h.o), ".github/workflows/ci.yml")
524+ h.f.Git("symbolic-ref", "HEAD", "refs/heads/nothing")
525+ run(domain.ZeroOID, h.m, ".github/workflows/ci.yml")
526+ ups := []domain.RefUpdate{{Old: domain.ZeroOID, New: h.m, Ref: "refs/tags/t"}}
527+ trees, err := g.PushSubtrees(ctx, h.f.Name, ups, ports.PushObjects{}, domain.WorkflowDir, domain.DefaultPushWalkBounds())
528+ require.NoError(t, err)
529+ assert.NotContains(t, trees.Subtrees, domain.HeadKey)
530+ plain := h.commit("", "")
531+ h.f.Git("update-ref", "refs/heads/plain", plain)
532+ run(domain.ZeroOID, plain, "")
533+}
M internal/adapters/gitcli/read.go +31 -60
@@ -35,8 +35,6 @@ func notFound(err error) bool { return errors.Is(err, errObjectMissing) }
3535 // ListRefs lists HEAD, the branches and the tags with `git for-each-ref`.
3636 // An annotated tag is followed to its commit; a tag of a tag is followed with
3737 // one `cat-file` request each.
38-//
39-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain refs
4038 func (g *Git) ListRefs(ctx context.Context, repo domain.RepoName) ([]domain.Ref, error) {
4139 dir, err := g.repoDir(repo)
4240 if err != nil {
@@ -155,66 +153,45 @@ func readHead(dir string) (string, error) {
155153 return ref, nil
156154 }
157155
158-// revCandidate is one way to read a revision as a commit.
159-type revCandidate struct {
160- spec string // object expression for cat-file
161- ref string // full ref name it stands for, "" for ids and HEAD
162-}
163-
164-// candidates lists, in order, how a revision is tried. A full object id is an
165-// object id first: a branch or a tag that happens to be named like a commit
166-// must not redirect a link to that commit; refs are tried only when no such
167-// object exists. Otherwise it is the branch, then the tag, then (for "HEAD",
168-// full ref names and abbreviations) as written. Other forms Git understands,
169-// such as "main~2" or "FETCH_HEAD", are not revisions of ermite.
170-func candidates(rev domain.Revision) []revCandidate {
171- s := rev.String()
172- switch {
173- case rev.IsHead() || s == "HEAD":
174- return []revCandidate{{spec: "HEAD^{commit}"}}
175- }
176- refs := []revCandidate{
177- {spec: headsRefPrefix + s + "^{commit}", ref: headsRefPrefix + s},
178- {spec: tagsRefPrefix + s + "^{commit}", ref: tagsRefPrefix + s},
179- }
180- switch {
181- case rev.IsObjectID():
182- return append([]revCandidate{{spec: s + "^{commit}"}}, refs...)
183- case strings.HasPrefix(s, "refs/"):
184- return append(refs, revCandidate{spec: s + "^{commit}", ref: s})
185- case rev.IsAbbrevID():
186- return append(refs, revCandidate{spec: s + "^{commit}"})
187- }
188- return refs
189-}
190-
191-// resolve resolves rev on an open cat-file connection.
192-//
193-//hexcheck:ignore suspicious-business-logic-in-adapter maps a revision to git object names, a git detail of the reader
156+// resolve resolves the domain candidates on an open cat-file connection.
194157 func resolve(c *catConn, rev domain.Revision) (domain.ResolvedRevision, error) {
195- for _, cand := range candidates(rev) {
196- h, err := c.info(cand.spec)
197- if notFound(err) {
198- continue
199- }
158+ for _, cand := range rev.Candidates() {
159+ id, ok, err := lookupRevisionCandidate(c, cand)
200160 if err != nil {
201161 return domain.ResolvedRevision{}, err
202162 }
203- if h.Type != "commit" {
163+ if !ok {
204164 continue
205165 }
206- id, err := domain.ParseObjectID(h.ID)
207- if err != nil {
208- return domain.ResolvedRevision{}, c.fail(err)
209- }
210- return domain.ResolvedRevision{CommitID: id, Ref: cand.ref}, nil
166+ return domain.ResolvedRevision{CommitID: id, Ref: cand.ResolvedRef()}, nil
211167 }
212- return domain.ResolvedRevision{}, fmt.Errorf("revision %q: %w", rev, domain.ErrNotFound)
168+ return domain.ResolvedRevision{}, revisionNotFound(rev)
169+}
170+
171+func revisionNotFound(rev domain.Revision) error {
172+ return fmt.Errorf("revision %q: %w", rev, domain.ErrNotFound)
173+}
174+
175+// lookupRevisionCandidate asks cat-file to peel a candidate to a commit.
176+func lookupRevisionCandidate(c *catConn, candidate domain.RevisionCandidate) (domain.ObjectID, bool, error) {
177+ h, err := c.info(candidate.Name + "^{commit}")
178+ if notFound(err) {
179+ return domain.ObjectID{}, false, nil
180+ }
181+ if err != nil {
182+ return domain.ObjectID{}, false, err
183+ }
184+ if h.Type != "commit" {
185+ return domain.ObjectID{}, false, nil
186+ }
187+ id, err := domain.ParseObjectID(h.ID)
188+ if err != nil {
189+ return domain.ObjectID{}, false, c.fail(err)
190+ }
191+ return id, true, nil
213192 }
214193
215194 // ResolveRevision resolves a revision with `cat-file`.
216-//
217-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain values; limits and rules come from the port
218195 func (g *Git) ResolveRevision(ctx context.Context, repo domain.RepoName, rev domain.Revision) (domain.ResolvedRevision, error) {
219196 dir, err := g.repoDir(repo)
220197 if err != nil {
@@ -235,8 +212,6 @@ func (g *Git) ResolveRevision(ctx context.Context, repo domain.RepoName, rev dom
235212 // tag, full ref name, HEAD; a single segment may also be a commit id), longest
236213 // first, with up to one `cat-file` request per candidate on the repository's
237214 // process. At most domain.MaxRefPathTries segments are tried.
238-//
239-//hexcheck:ignore suspicious-business-logic-in-adapter the split is domain.SplitRefPath; this only runs it on a cat-file connection
240215 func (g *Git) ResolveRefPath(ctx context.Context, repo domain.RepoName, path string) (domain.RefPath, error) {
241216 dir, err := g.repoDir(repo)
242217 if err != nil {
@@ -264,6 +239,8 @@ type object struct {
264239 // lookup resolves rev and reads the header of the object at path, in one
265240 // request series on the repository's process. A missing path is
266241 // domain.ErrNotFound.
242+//
243+//hexcheck:ignore suspicious-business-logic-in-adapter translates a root path to Git tree syntax and a non-root path to Git colon syntax, then reads the cat-file header
267244 func (g *Git) lookup(ctx context.Context, dir string, rev domain.Revision, path domain.TreePath) (object, error) {
268245 var obj object
269246 err := g.cat.do(ctx, dir, func(c *catConn) error {
@@ -392,8 +369,6 @@ func (g *Git) GetTree(ctx context.Context, repo domain.RepoName, rev domain.Revi
392369 }
393370
394371 // parseTreeEntry parses "<mode> <type> <id> <size>\t<name>" of ls-tree -l.
395-//
396-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain values; limits and rules come from the port
397372 func parseTreeEntry(rec string, parent domain.TreePath) (domain.TreeEntry, error) {
398373 meta, name, ok := strings.Cut(rec, "\t")
399374 f := strings.Fields(meta)
@@ -487,8 +462,6 @@ feed:
487462
488463 // lastCommit returns the newest commit reachable from head that changed path;
489464 // an empty path means head itself.
490-//
491-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain values; limits and rules come from the port
492465 func (g *Git) lastCommit(ctx context.Context, dir string, head domain.ObjectID, path string) (domain.Commit, error) {
493466 var c domain.Commit
494467 found := false
@@ -520,8 +493,6 @@ const commitFormat = "%H%x00%P%x00%an%x00%ae%x00%at%x00%cn%x00%ce%x00%ct%x00%B"
520493
521494 // readCommit reads one commit in commitFormat. It returns io.EOF at the end
522495 // of the output.
523-//
524-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain values; limits and rules come from the port
525496 func readCommit(r *bufio.Reader) (domain.Commit, error) {
526497 var f [9]string
527498 for i := range f {
@@ -35,8 +35,6 @@ func notFound(err error) bool { return errors.Is(err, errObjectMissing) }
35 // ListRefs lists HEAD, the branches and the tags with `git for-each-ref`.35 // ListRefs lists HEAD, the branches and the tags with `git for-each-ref`.
36 // An annotated tag is followed to its commit; a tag of a tag is followed with36 // An annotated tag is followed to its commit; a tag of a tag is followed with
37 // one `cat-file` request each.37 // one `cat-file` request each.
38-//
39-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain refs
40 func (g *Git) ListRefs(ctx context.Context, repo domain.RepoName) ([]domain.Ref, error) {38 func (g *Git) ListRefs(ctx context.Context, repo domain.RepoName) ([]domain.Ref, error) {
41 dir, err := g.repoDir(repo)39 dir, err := g.repoDir(repo)
42 if err != nil {40 if err != nil {
@@ -155,66 +153,45 @@ func readHead(dir string) (string, error) {
155 return ref, nil153 return ref, nil
156 }154 }
157 155
158-// revCandidate is one way to read a revision as a commit.156+// resolve resolves the domain candidates on an open cat-file connection.
159-type revCandidate struct {
160- spec string // object expression for cat-file
161- ref string // full ref name it stands for, "" for ids and HEAD
162-}
163-
164-// candidates lists, in order, how a revision is tried. A full object id is an
165-// object id first: a branch or a tag that happens to be named like a commit
166-// must not redirect a link to that commit; refs are tried only when no such
167-// object exists. Otherwise it is the branch, then the tag, then (for "HEAD",
168-// full ref names and abbreviations) as written. Other forms Git understands,
169-// such as "main~2" or "FETCH_HEAD", are not revisions of ermite.
170-func candidates(rev domain.Revision) []revCandidate {
171- s := rev.String()
172- switch {
173- case rev.IsHead() || s == "HEAD":
174- return []revCandidate{{spec: "HEAD^{commit}"}}
175- }
176- refs := []revCandidate{
177- {spec: headsRefPrefix + s + "^{commit}", ref: headsRefPrefix + s},
178- {spec: tagsRefPrefix + s + "^{commit}", ref: tagsRefPrefix + s},
179- }
180- switch {
181- case rev.IsObjectID():
182- return append([]revCandidate{{spec: s + "^{commit}"}}, refs...)
183- case strings.HasPrefix(s, "refs/"):
184- return append(refs, revCandidate{spec: s + "^{commit}", ref: s})
185- case rev.IsAbbrevID():
186- return append(refs, revCandidate{spec: s + "^{commit}"})
187- }
188- return refs
189-}
190-
191-// resolve resolves rev on an open cat-file connection.
192-//
193-//hexcheck:ignore suspicious-business-logic-in-adapter maps a revision to git object names, a git detail of the reader
194 func resolve(c *catConn, rev domain.Revision) (domain.ResolvedRevision, error) {157 func resolve(c *catConn, rev domain.Revision) (domain.ResolvedRevision, error) {
195- for _, cand := range candidates(rev) {158+ for _, cand := range rev.Candidates() {
196- h, err := c.info(cand.spec)159+ id, ok, err := lookupRevisionCandidate(c, cand)
197- if notFound(err) {
198- continue
199- }
200 if err != nil {160 if err != nil {
201 return domain.ResolvedRevision{}, err161 return domain.ResolvedRevision{}, err
202 }162 }
203- if h.Type != "commit" {163+ if !ok {
204 continue164 continue
205 }165 }
206- id, err := domain.ParseObjectID(h.ID)166+ return domain.ResolvedRevision{CommitID: id, Ref: cand.ResolvedRef()}, nil
207- if err != nil {
208- return domain.ResolvedRevision{}, c.fail(err)
209- }
210- return domain.ResolvedRevision{CommitID: id, Ref: cand.ref}, nil
211 }167 }
212- return domain.ResolvedRevision{}, fmt.Errorf("revision %q: %w", rev, domain.ErrNotFound)168+ return domain.ResolvedRevision{}, revisionNotFound(rev)
169+}
170+
171+func revisionNotFound(rev domain.Revision) error {
172+ return fmt.Errorf("revision %q: %w", rev, domain.ErrNotFound)
173+}
174+
175+// lookupRevisionCandidate asks cat-file to peel a candidate to a commit.
176+func lookupRevisionCandidate(c *catConn, candidate domain.RevisionCandidate) (domain.ObjectID, bool, error) {
177+ h, err := c.info(candidate.Name + "^{commit}")
178+ if notFound(err) {
179+ return domain.ObjectID{}, false, nil
180+ }
181+ if err != nil {
182+ return domain.ObjectID{}, false, err
183+ }
184+ if h.Type != "commit" {
185+ return domain.ObjectID{}, false, nil
186+ }
187+ id, err := domain.ParseObjectID(h.ID)
188+ if err != nil {
189+ return domain.ObjectID{}, false, c.fail(err)
190+ }
191+ return id, true, nil
213 }192 }
214 193
215 // ResolveRevision resolves a revision with `cat-file`.194 // ResolveRevision resolves a revision with `cat-file`.
216-//
217-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain values; limits and rules come from the port
218 func (g *Git) ResolveRevision(ctx context.Context, repo domain.RepoName, rev domain.Revision) (domain.ResolvedRevision, error) {195 func (g *Git) ResolveRevision(ctx context.Context, repo domain.RepoName, rev domain.Revision) (domain.ResolvedRevision, error) {
219 dir, err := g.repoDir(repo)196 dir, err := g.repoDir(repo)
220 if err != nil {197 if err != nil {
@@ -235,8 +212,6 @@ func (g *Git) ResolveRevision(ctx context.Context, repo domain.RepoName, rev dom
235 // tag, full ref name, HEAD; a single segment may also be a commit id), longest212 // tag, full ref name, HEAD; a single segment may also be a commit id), longest
236 // first, with up to one `cat-file` request per candidate on the repository's213 // first, with up to one `cat-file` request per candidate on the repository's
237 // process. At most domain.MaxRefPathTries segments are tried.214 // process. At most domain.MaxRefPathTries segments are tried.
238-//
239-//hexcheck:ignore suspicious-business-logic-in-adapter the split is domain.SplitRefPath; this only runs it on a cat-file connection
240 func (g *Git) ResolveRefPath(ctx context.Context, repo domain.RepoName, path string) (domain.RefPath, error) {215 func (g *Git) ResolveRefPath(ctx context.Context, repo domain.RepoName, path string) (domain.RefPath, error) {
241 dir, err := g.repoDir(repo)216 dir, err := g.repoDir(repo)
242 if err != nil {217 if err != nil {
@@ -264,6 +239,8 @@ type object struct {
264 // lookup resolves rev and reads the header of the object at path, in one239 // lookup resolves rev and reads the header of the object at path, in one
265 // request series on the repository's process. A missing path is240 // request series on the repository's process. A missing path is
266 // domain.ErrNotFound.241 // domain.ErrNotFound.
242+//
243+//hexcheck:ignore suspicious-business-logic-in-adapter translates a root path to Git tree syntax and a non-root path to Git colon syntax, then reads the cat-file header
267 func (g *Git) lookup(ctx context.Context, dir string, rev domain.Revision, path domain.TreePath) (object, error) {244 func (g *Git) lookup(ctx context.Context, dir string, rev domain.Revision, path domain.TreePath) (object, error) {
268 var obj object245 var obj object
269 err := g.cat.do(ctx, dir, func(c *catConn) error {246 err := g.cat.do(ctx, dir, func(c *catConn) error {
@@ -392,8 +369,6 @@ func (g *Git) GetTree(ctx context.Context, repo domain.RepoName, rev domain.Revi
392 }369 }
393 370
394 // parseTreeEntry parses "<mode> <type> <id> <size>\t<name>" of ls-tree -l.371 // parseTreeEntry parses "<mode> <type> <id> <size>\t<name>" of ls-tree -l.
395-//
396-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain values; limits and rules come from the port
397 func parseTreeEntry(rec string, parent domain.TreePath) (domain.TreeEntry, error) {372 func parseTreeEntry(rec string, parent domain.TreePath) (domain.TreeEntry, error) {
398 meta, name, ok := strings.Cut(rec, "\t")373 meta, name, ok := strings.Cut(rec, "\t")
399 f := strings.Fields(meta)374 f := strings.Fields(meta)
@@ -487,8 +462,6 @@ feed:
487 462
488 // lastCommit returns the newest commit reachable from head that changed path;463 // lastCommit returns the newest commit reachable from head that changed path;
489 // an empty path means head itself.464 // an empty path means head itself.
490-//
491-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain values; limits and rules come from the port
492 func (g *Git) lastCommit(ctx context.Context, dir string, head domain.ObjectID, path string) (domain.Commit, error) {465 func (g *Git) lastCommit(ctx context.Context, dir string, head domain.ObjectID, path string) (domain.Commit, error) {
493 var c domain.Commit466 var c domain.Commit
494 found := false467 found := false
@@ -520,8 +493,6 @@ const commitFormat = "%H%x00%P%x00%an%x00%ae%x00%at%x00%cn%x00%ce%x00%ct%x00%B"
520 493
521 // readCommit reads one commit in commitFormat. It returns io.EOF at the end494 // readCommit reads one commit in commitFormat. It returns io.EOF at the end
522 // of the output.495 // of the output.
523-//
524-//hexcheck:ignore suspicious-business-logic-in-adapter parses git output into domain values; limits and rules come from the port
525 func readCommit(r *bufio.Reader) (domain.Commit, error) {496 func readCommit(r *bufio.Reader) (domain.Commit, error) {
526 var f [9]string497 var f [9]string
527 for i := range f {498 for i := range f {
M internal/adapters/gitcli/repos.go +1 -3
@@ -102,7 +102,7 @@ func (g *Git) Discard(_ context.Context, id ports.StagedID) error {
102102
103103 // PurgeStaging removes everything under repos/.staging/.
104104 //
105-//hexcheck:ignore suspicious-business-logic-in-adapter port method name only
105+//hexcheck:ignore suspicious-business-logic-in-adapter removes staging leftovers from disk, tolerates a missing directory, and logs each removal
106106 func (g *Git) PurgeStaging(ctx context.Context) error {
107107 entries, err := os.ReadDir(g.stagingDir())
108108 if err != nil {
@@ -280,8 +280,6 @@ func (g *Git) DefaultBranch(ctx context.Context, name domain.RepoName) (string,
280280 // HasBranch checks refs/heads/<branch> with `git for-each-ref` after
281281 // `git check-ref-format`, which rejects the glob characters for-each-ref
282282 // would otherwise interpret.
283-//
284-//hexcheck:ignore suspicious-business-logic-in-adapter port method name only
285283 func (g *Git) HasBranch(ctx context.Context, name domain.RepoName, branch string) (bool, error) {
286284 dir, err := g.repoDir(name)
287285 if err != nil {
@@ -102,7 +102,7 @@ func (g *Git) Discard(_ context.Context, id ports.StagedID) error {
102 102
103 // PurgeStaging removes everything under repos/.staging/.103 // PurgeStaging removes everything under repos/.staging/.
104 //104 //
105-//hexcheck:ignore suspicious-business-logic-in-adapter port method name only105+//hexcheck:ignore suspicious-business-logic-in-adapter removes staging leftovers from disk, tolerates a missing directory, and logs each removal
106 func (g *Git) PurgeStaging(ctx context.Context) error {106 func (g *Git) PurgeStaging(ctx context.Context) error {
107 entries, err := os.ReadDir(g.stagingDir())107 entries, err := os.ReadDir(g.stagingDir())
108 if err != nil {108 if err != nil {
@@ -280,8 +280,6 @@ func (g *Git) DefaultBranch(ctx context.Context, name domain.RepoName) (string,
280 // HasBranch checks refs/heads/<branch> with `git for-each-ref` after280 // HasBranch checks refs/heads/<branch> with `git for-each-ref` after
281 // `git check-ref-format`, which rejects the glob characters for-each-ref281 // `git check-ref-format`, which rejects the glob characters for-each-ref
282 // would otherwise interpret.282 // would otherwise interpret.
283-//
284-//hexcheck:ignore suspicious-business-logic-in-adapter port method name only
285 func (g *Git) HasBranch(ctx context.Context, name domain.RepoName, branch string) (bool, error) {283 func (g *Git) HasBranch(ctx context.Context, name domain.RepoName, branch string) (bool, error) {
286 dir, err := g.repoDir(name)284 dir, err := g.repoDir(name)
287 if err != nil {285 if err != nil {
M internal/adapters/gitcli/stats.go +0 -6
@@ -18,8 +18,6 @@ import (
1818
1919 // CountCommits runs `git rev-list --count`, from the commit graph when the
2020 // repository has one.
21-//
22-//hexcheck:ignore suspicious-business-logic-in-adapter reads the count git prints
2321 func (g *Git) CountCommits(ctx context.Context, repo domain.RepoName, head domain.ObjectID) (int, error) {
2422 if head.IsZero() {
2523 return 0, fmt.Errorf("count commits: %w: no head", domain.ErrInvalid)
@@ -45,8 +43,6 @@ func (g *Git) CountCommits(ctx context.Context, repo domain.RepoName, head domai
4543 // CommitTimes runs `git rev-list --since-as-filter --format=%ct`. The filter
4644 // looks at every reachable commit: --since stops the walk at the first older
4745 // one, and misses newer commits behind it when dates are out of order.
48-//
49-//hexcheck:ignore suspicious-business-logic-in-adapter reads the dates git prints; the bound comes from the port
5046 func (g *Git) CommitTimes(ctx context.Context, repo domain.RepoName, head domain.ObjectID, since time.Time) ([]time.Time, bool, error) {
5147 if head.IsZero() {
5248 return nil, false, fmt.Errorf("commit times: %w: no head", domain.ErrInvalid)
@@ -91,8 +87,6 @@ func (g *Git) CommitTimes(ctx context.Context, repo domain.RepoName, head domain
9187 }
9288
9389 // LatestTag runs `git for-each-ref --sort=-creatordate --count=1` on the tags.
94-//
95-//hexcheck:ignore suspicious-business-logic-in-adapter reads the line git prints
9690 func (g *Git) LatestTag(ctx context.Context, repo domain.RepoName) (domain.TagInfo, bool, error) {
9791 dir, err := g.repoDir(repo)
9892 if err != nil {
@@ -18,8 +18,6 @@ import (
18 18
19 // CountCommits runs `git rev-list --count`, from the commit graph when the19 // CountCommits runs `git rev-list --count`, from the commit graph when the
20 // repository has one.20 // repository has one.
21-//
22-//hexcheck:ignore suspicious-business-logic-in-adapter reads the count git prints
23 func (g *Git) CountCommits(ctx context.Context, repo domain.RepoName, head domain.ObjectID) (int, error) {21 func (g *Git) CountCommits(ctx context.Context, repo domain.RepoName, head domain.ObjectID) (int, error) {
24 if head.IsZero() {22 if head.IsZero() {
25 return 0, fmt.Errorf("count commits: %w: no head", domain.ErrInvalid)23 return 0, fmt.Errorf("count commits: %w: no head", domain.ErrInvalid)
@@ -45,8 +43,6 @@ func (g *Git) CountCommits(ctx context.Context, repo domain.RepoName, head domai
45 // CommitTimes runs `git rev-list --since-as-filter --format=%ct`. The filter43 // CommitTimes runs `git rev-list --since-as-filter --format=%ct`. The filter
46 // looks at every reachable commit: --since stops the walk at the first older44 // looks at every reachable commit: --since stops the walk at the first older
47 // one, and misses newer commits behind it when dates are out of order.45 // one, and misses newer commits behind it when dates are out of order.
48-//
49-//hexcheck:ignore suspicious-business-logic-in-adapter reads the dates git prints; the bound comes from the port
50 func (g *Git) CommitTimes(ctx context.Context, repo domain.RepoName, head domain.ObjectID, since time.Time) ([]time.Time, bool, error) {46 func (g *Git) CommitTimes(ctx context.Context, repo domain.RepoName, head domain.ObjectID, since time.Time) ([]time.Time, bool, error) {
51 if head.IsZero() {47 if head.IsZero() {
52 return nil, false, fmt.Errorf("commit times: %w: no head", domain.ErrInvalid)48 return nil, false, fmt.Errorf("commit times: %w: no head", domain.ErrInvalid)
@@ -91,8 +87,6 @@ func (g *Git) CommitTimes(ctx context.Context, repo domain.RepoName, head domain
91 }87 }
92 88
93 // LatestTag runs `git for-each-ref --sort=-creatordate --count=1` on the tags.89 // LatestTag runs `git for-each-ref --sort=-creatordate --count=1` on the tags.
94-//
95-//hexcheck:ignore suspicious-business-logic-in-adapter reads the line git prints
96 func (g *Git) LatestTag(ctx context.Context, repo domain.RepoName) (domain.TagInfo, bool, error) {90 func (g *Git) LatestTag(ctx context.Context, repo domain.RepoName) (domain.TagInfo, bool, error) {
97 dir, err := g.repoDir(repo)91 dir, err := g.repoDir(repo)
98 if err != nil {92 if err != nil {
D internal/adapters/gitcli/workflow_guard.go +0 -438
@@ -1,438 +0,0 @@
1-// SPDX-License-Identifier: AGPL-3.0-or-later
2-
3-package gitcli
4-
5-import (
6- "bufio"
7- "context"
8- "errors"
9- "fmt"
10- "io"
11- "path/filepath"
12- "strconv"
13- "strings"
14-
15- "bnema.dev/ermite/internal/domain"
16- "bnema.dev/ermite/internal/ports"
17-)
18-
19-var _ ports.WorkflowGuard = (*Git)(nil)
20-
21-// workflowsDir is where workflow files live, as a literal pathspec directory.
22-const workflowsDir = ".github/workflows/"
23-
24-// maxWorkflowPath bounds the one path the guard reads from git: a longer one
25-// fails closed (the push is refused), whatever the trees hold.
26-const maxWorkflowPath = 4096
27-
28-// Bounds of the walk over the commits a push brings: more than that, or a
29-// commit with more parents than a line of rev-list holds, fails closed (the
30-// push is refused), whatever the commits hold. maxPushedObjects counts every
31-// commit and every parent listed, repeats included (so it bounds the distinct
32-// ids too), and is enforced as the lines are read, before anything more is
33-// kept: a few thousand commits with two hundred parents each are refused after
34-// the first lines, not after they have been held.
35-const (
36- maxPushedCommits = 10000
37- maxPushedObjects = 4 * maxPushedCommits
38- maxRevListLine = 8192
39-)
40-
41-// ChangedWorkflow tells whether a push changes the workflows of a repository.
42-// It returns the path of the first change, "" for none. Deleting a ref changes
43-// nothing, and a workflows directory that disappears is not a change.
44-//
45-// Two things are judged, as GitHub does for the token of a job (it refuses a
46-// push when the commits pushed change workflow files, not when a new branch is
47-// cut from a commit that has them):
48-//
49-// 1. Every commit the push brings (`rev-list <new> --not --all`, so the commits
50-// no ref has yet) must leave .github/workflows as it found it: it is
51-// compared with each of its parents (a root commit with nothing) and is a
52-// change when it is present and differs from every one of them. A merge of
53-// existing commits is thus no change; a side that brings a change is itself
54-// a new commit, and judged as one.
55-// 2. The directory a ref ends with must not be one it did not have: for an
56-// update the final .github/workflows must be the old one or absent, for a
57-// new ref the one of the default branch (HEAD) or absent. This keeps a job
58-// token from moving a ref onto existing commits that carry other workflows
59-// (an older commit, a commit of a branch with different ones).
60-//
61-// Presence is the directory the workflow loader lists (CI.workflowFiles, on both
62-// git readers): .github and workflows both entries of mode 040000 and type
63-// tree. A path through a gitlink (160000), a symbolic link (120000, which the
64-// loader does not follow) or a file is no workflows, so the same tree id that is
65-// no workflows as a gitlink and becomes workflows as a directory is a change,
66-// which the object id alone would not tell. Mode and id of
67-// "<commit>:.github/workflows" are what is compared: equal means the same
68-// content, whatever it holds, so nothing is read.
69-//
70-// Nothing is walked recursively: the trees of a push are the pusher's, and a
71-// recursive diff of trees that share subtrees fans out into more paths than the
72-// push has bytes (git builds the whole diff before it prints a line, so reading
73-// it as a stream would not help). The shallow diff that names the path reads one
74-// directory, as large as its tree object, as a stream, up to its first name, and
75-// git is stopped and reaped then; a name longer than maxWorkflowPath fails
76-// closed.
77-//
78-// The ends of each update are peeled first (`cat-file --batch-check` on
79-// `<oid>^{}`): a commit or a tree is compared, a blob (a tag may point to one)
80-// holds no workflows; a missing or malformed object fails closed. Git reads the
81-// objects of a push still in quarantine through the environment of the hook
82-// (ports.PushObjects), accepted only inside the repository's objects directory.
83-//
84-//hexcheck:ignore suspicious-business-logic-in-adapter reads the object ids and the path git prints for the compared trees; the rule that refuses is the use case's
85-func (g *Git) ChangedWorkflow(ctx context.Context, repo domain.RepoName, updates []domain.RefUpdate, objects ports.PushObjects) (string, error) {
86- dir, err := g.repoDir(repo)
87- if err != nil {
88- return "", err
89- }
90- extra, err := quarantineEnv(dir, objects)
91- if err != nil {
92- return "", err
93- }
94- kept := make([]domain.RefUpdate, 0, len(updates))
95- for _, up := range updates {
96- if !up.IsDelete() {
97- kept = append(kept, up)
98- }
99- }
100- if len(kept) == 0 {
101- return "", nil
102- }
103- types, err := g.batchCheck(ctx, dir, extra, kept, "^{}", "%(objecttype)")
104- if err != nil {
105- return "", err
106- }
107- if p, err := g.changedByNewCommits(ctx, dir, extra, kept, types); p != "" || err != nil {
108- return p, err
109- }
110- trees, err := g.batchCheck(ctx, dir, extra, kept, ":"+strings.TrimSuffix(workflowsDir, "/"), "%(objectmode) %(objectname) %(objecttype)")
111- if err != nil {
112- return "", err
113- }
114- headW, headRead := "", false
115- for i, up := range kept {
116- if types[2*i+1] == "blob" {
117- continue // a blob holds no workflow, whatever it replaces
118- }
119- base := workflowsOf(types[2*i], trees[2*i])
120- if up.IsCreate() {
121- if !headRead {
122- w, err := g.workflowsAt(ctx, dir, extra, []string{"HEAD:" + strings.TrimSuffix(workflowsDir, "/")})
123- if err != nil {
124- return "", err
125- }
126- headW, headRead = w["HEAD:"+strings.TrimSuffix(workflowsDir, "/")], true
127- }
128- base = headW // a new ref may end with the workflows of the default branch
129- }
130- newW := workflowsOf(types[2*i+1], trees[2*i+1])
131- if base == newW || newW == "" {
132- continue // the same directory, or none left
133- }
134- return g.firstChanged(ctx, dir, extra, base, newW)
135- }
136- return "", nil
137-}
138-
139-// changedByNewCommits judges the commits the updates bring (rule 1 of
140-// ChangedWorkflow).
141-func (g *Git) changedByNewCommits(ctx context.Context, dir string, extra []string, ups []domain.RefUpdate, types []string) (string, error) {
142- var tips []string
143- seen := map[string]bool{}
144- for i, up := range ups {
145- if types[2*i+1] == "commit" && !seen[up.New] {
146- seen[up.New] = true
147- tips = append(tips, up.New)
148- }
149- }
150- if len(tips) == 0 {
151- return "", nil
152- }
153- commits, err := g.newCommits(ctx, dir, extra, tips)
154- if err != nil {
155- return "", err
156- }
157- const suffix = ":.github/workflows"
158- var specs []string
159- asked := map[string]bool{}
160- for _, c := range commits {
161- for _, id := range append([]string{c.id}, c.parents...) {
162- if !asked[id] {
163- asked[id] = true
164- specs = append(specs, id+suffix)
165- }
166- }
167- }
168- ws, err := g.workflowsAt(ctx, dir, extra, specs)
169- if err != nil {
170- return "", err
171- }
172- for _, c := range commits {
173- cur := ws[c.id+suffix]
174- if cur == "" {
175- continue // none: nothing new is active
176- }
177- first, same := "", false
178- for i, p := range c.parents {
179- if i == 0 {
180- first = ws[p+suffix]
181- }
182- if ws[p+suffix] == cur {
183- same = true
184- }
185- }
186- if !same {
187- return g.firstChanged(ctx, dir, extra, first, cur)
188- }
189- }
190- return "", nil
191-}
192-
193-type pushedCommit struct {
194- id string
195- parents []string
196-}
197-
198-// newCommits lists the commits reachable from the tips and from no ref
199-// (`rev-list --parents <tips> --not --all`), at most maxPushedCommits: more
200-// fails closed.
201-func (g *Git) newCommits(ctx context.Context, dir string, extra []string, tips []string) ([]pushedCommit, error) {
202- args := readArgs(dir, append(append([]string{"rev-list", "--parents", "--max-count=" + strconv.Itoa(maxPushedCommits+1)}, tips...), "--not", "--all")...)
203- var out []pushedCommit
204- err := g.pipeWith(ctx, extra, nil, args, func(r *bufio.Reader) error {
205- var err error
206- out, err = parseRevList(r, maxPushedCommits, maxPushedObjects)
207- return err
208- })
209- if err != nil {
210- return nil, fmt.Errorf("commits of a push: %w", err)
211- }
212- return out, nil
213-}
214-
215-// parseRevList reads the lines of `rev-list --parents` ("<commit> <parent>..."):
216-// at most maxCommits lines and maxIDs ids in all (a commit and each of its
217-// parents count, repeats too). A line over maxRevListLine, a limit crossed or an
218-// id that is not one is an error, and it is raised as soon as the line that
219-// crosses it is read: nothing past it is read or kept.
220-func parseRevList(r *bufio.Reader, maxCommits, maxIDs int) ([]pushedCommit, error) {
221- var out []pushedCommit
222- ids := 0
223- for {
224- line, err := r.ReadSlice('\n')
225- switch {
226- case errors.Is(err, bufio.ErrBufferFull) || len(line) > maxRevListLine:
227- return nil, fmt.Errorf("%w: a pushed commit has too many parents", domain.ErrInvalid)
228- case errors.Is(err, io.EOF) && len(line) == 0:
229- return out, nil
230- case err != nil && !errors.Is(err, io.EOF):
231- return nil, err
232- }
233- f := strings.Fields(string(line))
234- if len(f) == 0 {
235- continue
236- }
237- if len(out) == maxCommits {
238- return nil, fmt.Errorf("%w: the push brings more than %d commits", domain.ErrInvalid, maxCommits)
239- }
240- if ids += len(f); ids > maxIDs {
241- return nil, fmt.Errorf("%w: the push brings commits with more than %d ids (commits and parents) in all", domain.ErrInvalid, maxIDs)
242- }
243- for _, id := range f {
244- if _, err := domain.ParseObjectID(id); err != nil {
245- return nil, fmt.Errorf("%w: unexpected rev-list output", domain.ErrInvalid)
246- }
247- }
248- out = append(out, pushedCommit{id: f[0], parents: f[1:]})
249- }
250-}
251-
252-// workflowsAt returns, for each spec ("<rev>:.github/workflows"), the id of the
253-// directory it names, or "" when there is none (see workflowsOf).
254-func (g *Git) workflowsAt(ctx context.Context, dir string, extra []string, specs []string) (map[string]string, error) {
255- out := make(map[string]string, len(specs))
256- if len(specs) == 0 {
257- return out, nil
258- }
259- args := readArgs(dir, "cat-file", "--batch-check=%(objectmode) %(objectname) %(objecttype)")
260- err := g.pipeWith(ctx, extra, strings.NewReader(strings.Join(specs, "\n")+"\n"), args, func(r *bufio.Reader) error {
261- for _, spec := range specs {
262- line, err := r.ReadSlice('\n')
263- if err != nil {
264- return fmt.Errorf("read object answers: %w", err)
265- }
266- out[spec] = workflowsOf("", strings.TrimSuffix(string(line), "\n"))
267- }
268- return nil
269- })
270- if err != nil {
271- return nil, fmt.Errorf("workflows of a push: %w", err)
272- }
273- return out, nil
274-}
275-
276-// workflowsOf is the id of the tree that is the .github/workflows of a commit
277-// or a tree, "" when there is none: the answer for "<it>:.github/workflows" must
278-// be a tree of mode 040000, and the object peeled to a tree-ish, not a blob.
279-func workflowsOf(objType, answer string) string {
280- if objType == "blob" {
281- return ""
282- }
283- mode, rest, _ := strings.Cut(answer, " ")
284- id, typ, _ := strings.Cut(rest, " ")
285- if mode != "040000" || typ != "tree" {
286- return ""
287- }
288- return id
289-}
290-
291-// firstChanged names the first top-level entry of .github/workflows that
292-// differs between two trees (oldID may be "": none, the empty tree).
293-func (g *Git) firstChanged(ctx context.Context, dir string, extra []string, oldID, newID string) (string, error) {
294- from := oldID
295- if from == "" {
296- from = emptyTree
297- }
298- to := newID
299- args := readArgs(dir, "diff-tree", "--name-only", "-z", "--no-renames", "--no-commit-id", "--end-of-options", from, to)
300- var name string
301- err := g.pipeWith(ctx, extra, nil, args, func(r *bufio.Reader) error {
302- n, found, err := firstPath(r)
303- name = n
304- if err != nil || !found {
305- return err // no name: git's exit status is checked, so a failure refuses
306- }
307- return errStop // the first name is all that is needed: stop git
308- })
309- if err != nil {
310- return "", fmt.Errorf("changed workflows %s..%s: %w", from, to, err)
311- }
312- if name == "" {
313- return strings.TrimSuffix(workflowsDir, "/"), nil
314- }
315- return workflowsDir + name, nil
316-}
317-
318-// firstPath reads the first NUL-terminated path of r; found is false at the end
319-// of the output without one. A path longer than maxWorkflowPath is an error.
320-func firstPath(r *bufio.Reader) (path string, found bool, err error) {
321- var b []byte
322- for {
323- c, err := r.ReadByte()
324- switch {
325- case err != nil:
326- return "", false, nil // the end of the output; a failing exit status is reported by pipe
327- case c == 0:
328- return string(b), true, nil
329- case len(b) >= maxWorkflowPath:
330- return "", false, fmt.Errorf("%w: a changed path is longer than %d bytes", domain.ErrInvalid, maxWorkflowPath)
331- }
332- b = append(b, c)
333- }
334-}
335-
336-// batchCheck asks `cat-file --batch-check=<format>` about <oid><suffix> for the
337-// old (unless the ref is created) and the new end of each update and returns
338-// the answers in that order, two per update, "" for the old end of a created
339-// ref. With suffix "^{}" an object that is missing, malformed or neither a
340-// commit, a tree nor a blob is an error (the push is refused); with any other
341-// suffix an answer "<input> missing" is kept as the answer.
342-func (g *Git) batchCheck(ctx context.Context, dir string, extra []string, ups []domain.RefUpdate, suffix, format string) ([]string, error) {
343- var in strings.Builder
344- want := 0
345- for _, up := range ups {
346- for _, oid := range []string{up.Old, up.New} {
347- if oid == domain.ZeroOID {
348- continue
349- }
350- if _, err := domain.ParseObjectID(oid); err != nil {
351- return nil, fmt.Errorf("%w: %v", domain.ErrInvalid, err)
352- }
353- in.WriteString(oid + suffix + "\n")
354- want++
355- }
356- }
357- got := make([]string, 0, want)
358- args := readArgs(dir, "cat-file", "--batch-check="+format)
359- err := g.pipeWith(ctx, extra, strings.NewReader(in.String()), args, func(r *bufio.Reader) error {
360- for range want {
361- line, err := r.ReadSlice('\n')
362- if err != nil {
363- if errors.Is(err, bufio.ErrBufferFull) {
364- return fmt.Errorf("%w: unexpected cat-file output", domain.ErrInvalid)
365- }
366- return fmt.Errorf("read object answers: %w", err)
367- }
368- got = append(got, strings.TrimSuffix(string(line), "\n"))
369- }
370- return nil
371- })
372- if err != nil {
373- return nil, fmt.Errorf("objects of a push: %w", err)
374- }
375- out := make([]string, 0, 2*len(ups))
376- i := 0
377- for _, up := range ups {
378- for _, oid := range []string{up.Old, up.New} {
379- if oid == domain.ZeroOID {
380- out = append(out, "")
381- continue
382- }
383- if suffix == "^{}" {
384- switch got[i] {
385- case "commit", "tree", "blob":
386- default: // "<input> missing", or a type that cannot be compared
387- return nil, fmt.Errorf("%w: object %s of the push is missing or of an unexpected type", domain.ErrInvalid, oid)
388- }
389- }
390- out = append(out, got[i])
391- i++
392- }
393- }
394- return out, nil
395-}
396-
397-// quarantineEnv returns the environment that makes git read the objects of a
398-// push in quarantine. Each directory must exist and, once symbolic links are
399-// resolved, be inside the objects directory of the repository dir (itself
400-// resolved): a hook request cannot send git to read anything else. The resolved
401-// paths are the ones given to git.
402-func quarantineEnv(repoDir string, o ports.PushObjects) ([]string, error) {
403- if o.Dir == "" && o.Alternates == "" {
404- return nil, nil
405- }
406- root, err := filepath.EvalSymlinks(filepath.Join(repoDir, "objects"))
407- if err != nil {
408- return nil, fmt.Errorf("resolve the objects directory of %s: %w", repoDir, err)
409- }
410- inside := func(p string) (string, bool) {
411- if !filepath.IsAbs(p) {
412- return "", false
413- }
414- real, err := filepath.EvalSymlinks(p)
415- if err != nil {
416- return "", false
417- }
418- rel, err := filepath.Rel(root, real)
419- return real, err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator))
420- }
421- dir, ok := inside(o.Dir)
422- if o.Dir == "" || !ok {
423- return nil, fmt.Errorf("%w: the objects directory of the push is outside the repository", domain.ErrInvalid)
424- }
425- env := []string{"GIT_OBJECT_DIRECTORY=" + dir}
426- if o.Alternates != "" {
427- var alts []string
428- for _, a := range filepath.SplitList(o.Alternates) {
429- real, ok := inside(a)
430- if !ok {
431- return nil, fmt.Errorf("%w: an alternate objects directory of the push is outside the repository", domain.ErrInvalid)
432- }
433- alts = append(alts, real)
434- }
435- env = append(env, "GIT_ALTERNATE_OBJECT_DIRECTORIES="+strings.Join(alts, string(filepath.ListSeparator)))
436- }
437- return env, nil
438-}
@@ -1,438 +0,0 @@
1-// SPDX-License-Identifier: AGPL-3.0-or-later
2-
3-package gitcli
4-
5-import (
6- "bufio"
7- "context"
8- "errors"
9- "fmt"
10- "io"
11- "path/filepath"
12- "strconv"
13- "strings"
14-
15- "bnema.dev/ermite/internal/domain"
16- "bnema.dev/ermite/internal/ports"
17-)
18-
19-var _ ports.WorkflowGuard = (*Git)(nil)
20-
21-// workflowsDir is where workflow files live, as a literal pathspec directory.
22-const workflowsDir = ".github/workflows/"
23-
24-// maxWorkflowPath bounds the one path the guard reads from git: a longer one
25-// fails closed (the push is refused), whatever the trees hold.
26-const maxWorkflowPath = 4096
27-
28-// Bounds of the walk over the commits a push brings: more than that, or a
29-// commit with more parents than a line of rev-list holds, fails closed (the
30-// push is refused), whatever the commits hold. maxPushedObjects counts every
31-// commit and every parent listed, repeats included (so it bounds the distinct
32-// ids too), and is enforced as the lines are read, before anything more is
33-// kept: a few thousand commits with two hundred parents each are refused after
34-// the first lines, not after they have been held.
35-const (
36- maxPushedCommits = 10000
37- maxPushedObjects = 4 * maxPushedCommits
38- maxRevListLine = 8192
39-)
40-
41-// ChangedWorkflow tells whether a push changes the workflows of a repository.
42-// It returns the path of the first change, "" for none. Deleting a ref changes
43-// nothing, and a workflows directory that disappears is not a change.
44-//
45-// Two things are judged, as GitHub does for the token of a job (it refuses a
46-// push when the commits pushed change workflow files, not when a new branch is
47-// cut from a commit that has them):
48-//
49-// 1. Every commit the push brings (`rev-list <new> --not --all`, so the commits
50-// no ref has yet) must leave .github/workflows as it found it: it is
51-// compared with each of its parents (a root commit with nothing) and is a
52-// change when it is present and differs from every one of them. A merge of
53-// existing commits is thus no change; a side that brings a change is itself
54-// a new commit, and judged as one.
55-// 2. The directory a ref ends with must not be one it did not have: for an
56-// update the final .github/workflows must be the old one or absent, for a
57-// new ref the one of the default branch (HEAD) or absent. This keeps a job
58-// token from moving a ref onto existing commits that carry other workflows
59-// (an older commit, a commit of a branch with different ones).
60-//
61-// Presence is the directory the workflow loader lists (CI.workflowFiles, on both
62-// git readers): .github and workflows both entries of mode 040000 and type
63-// tree. A path through a gitlink (160000), a symbolic link (120000, which the
64-// loader does not follow) or a file is no workflows, so the same tree id that is
65-// no workflows as a gitlink and becomes workflows as a directory is a change,
66-// which the object id alone would not tell. Mode and id of
67-// "<commit>:.github/workflows" are what is compared: equal means the same
68-// content, whatever it holds, so nothing is read.
69-//
70-// Nothing is walked recursively: the trees of a push are the pusher's, and a
71-// recursive diff of trees that share subtrees fans out into more paths than the
72-// push has bytes (git builds the whole diff before it prints a line, so reading
73-// it as a stream would not help). The shallow diff that names the path reads one
74-// directory, as large as its tree object, as a stream, up to its first name, and
75-// git is stopped and reaped then; a name longer than maxWorkflowPath fails
76-// closed.
77-//
78-// The ends of each update are peeled first (`cat-file --batch-check` on
79-// `<oid>^{}`): a commit or a tree is compared, a blob (a tag may point to one)
80-// holds no workflows; a missing or malformed object fails closed. Git reads the
81-// objects of a push still in quarantine through the environment of the hook
82-// (ports.PushObjects), accepted only inside the repository's objects directory.
83-//
84-//hexcheck:ignore suspicious-business-logic-in-adapter reads the object ids and the path git prints for the compared trees; the rule that refuses is the use case's
85-func (g *Git) ChangedWorkflow(ctx context.Context, repo domain.RepoName, updates []domain.RefUpdate, objects ports.PushObjects) (string, error) {
86- dir, err := g.repoDir(repo)
87- if err != nil {
88- return "", err
89- }
90- extra, err := quarantineEnv(dir, objects)
91- if err != nil {
92- return "", err
93- }
94- kept := make([]domain.RefUpdate, 0, len(updates))
95- for _, up := range updates {
96- if !up.IsDelete() {
97- kept = append(kept, up)
98- }
99- }
100- if len(kept) == 0 {
101- return "", nil
102- }
103- types, err := g.batchCheck(ctx, dir, extra, kept, "^{}", "%(objecttype)")
104- if err != nil {
105- return "", err
106- }
107- if p, err := g.changedByNewCommits(ctx, dir, extra, kept, types); p != "" || err != nil {
108- return p, err
109- }
110- trees, err := g.batchCheck(ctx, dir, extra, kept, ":"+strings.TrimSuffix(workflowsDir, "/"), "%(objectmode) %(objectname) %(objecttype)")
111- if err != nil {
112- return "", err
113- }
114- headW, headRead := "", false
115- for i, up := range kept {
116- if types[2*i+1] == "blob" {
117- continue // a blob holds no workflow, whatever it replaces
118- }
119- base := workflowsOf(types[2*i], trees[2*i])
120- if up.IsCreate() {
121- if !headRead {
122- w, err := g.workflowsAt(ctx, dir, extra, []string{"HEAD:" + strings.TrimSuffix(workflowsDir, "/")})
123- if err != nil {
124- return "", err
125- }
126- headW, headRead = w["HEAD:"+strings.TrimSuffix(workflowsDir, "/")], true
127- }
128- base = headW // a new ref may end with the workflows of the default branch
129- }
130- newW := workflowsOf(types[2*i+1], trees[2*i+1])
131- if base == newW || newW == "" {
132- continue // the same directory, or none left
133- }
134- return g.firstChanged(ctx, dir, extra, base, newW)
135- }
136- return "", nil
137-}
138-
139-// changedByNewCommits judges the commits the updates bring (rule 1 of
140-// ChangedWorkflow).
141-func (g *Git) changedByNewCommits(ctx context.Context, dir string, extra []string, ups []domain.RefUpdate, types []string) (string, error) {
142- var tips []string
143- seen := map[string]bool{}
144- for i, up := range ups {
145- if types[2*i+1] == "commit" && !seen[up.New] {
146- seen[up.New] = true
147- tips = append(tips, up.New)
148- }
149- }
150- if len(tips) == 0 {
151- return "", nil
152- }
153- commits, err := g.newCommits(ctx, dir, extra, tips)
154- if err != nil {
155- return "", err
156- }
157- const suffix = ":.github/workflows"
158- var specs []string
159- asked := map[string]bool{}
160- for _, c := range commits {
161- for _, id := range append([]string{c.id}, c.parents...) {
162- if !asked[id] {
163- asked[id] = true
164- specs = append(specs, id+suffix)
165- }
166- }
167- }
168- ws, err := g.workflowsAt(ctx, dir, extra, specs)
169- if err != nil {
170- return "", err
171- }
172- for _, c := range commits {
173- cur := ws[c.id+suffix]
174- if cur == "" {
175- continue // none: nothing new is active
176- }
177- first, same := "", false
178- for i, p := range c.parents {
179- if i == 0 {
180- first = ws[p+suffix]
181- }
182- if ws[p+suffix] == cur {
183- same = true
184- }
185- }
186- if !same {
187- return g.firstChanged(ctx, dir, extra, first, cur)
188- }
189- }
190- return "", nil
191-}
192-
193-type pushedCommit struct {
194- id string
195- parents []string
196-}
197-
198-// newCommits lists the commits reachable from the tips and from no ref
199-// (`rev-list --parents <tips> --not --all`), at most maxPushedCommits: more
200-// fails closed.
201-func (g *Git) newCommits(ctx context.Context, dir string, extra []string, tips []string) ([]pushedCommit, error) {
202- args := readArgs(dir, append(append([]string{"rev-list", "--parents", "--max-count=" + strconv.Itoa(maxPushedCommits+1)}, tips...), "--not", "--all")...)
203- var out []pushedCommit
204- err := g.pipeWith(ctx, extra, nil, args, func(r *bufio.Reader) error {
205- var err error
206- out, err = parseRevList(r, maxPushedCommits, maxPushedObjects)
207- return err
208- })
209- if err != nil {
210- return nil, fmt.Errorf("commits of a push: %w", err)
211- }
212- return out, nil
213-}
214-
215-// parseRevList reads the lines of `rev-list --parents` ("<commit> <parent>..."):
216-// at most maxCommits lines and maxIDs ids in all (a commit and each of its
217-// parents count, repeats too). A line over maxRevListLine, a limit crossed or an
218-// id that is not one is an error, and it is raised as soon as the line that
219-// crosses it is read: nothing past it is read or kept.
220-func parseRevList(r *bufio.Reader, maxCommits, maxIDs int) ([]pushedCommit, error) {
221- var out []pushedCommit
222- ids := 0
223- for {
224- line, err := r.ReadSlice('\n')
225- switch {
226- case errors.Is(err, bufio.ErrBufferFull) || len(line) > maxRevListLine:
227- return nil, fmt.Errorf("%w: a pushed commit has too many parents", domain.ErrInvalid)
228- case errors.Is(err, io.EOF) && len(line) == 0:
229- return out, nil
230- case err != nil && !errors.Is(err, io.EOF):
231- return nil, err
232- }
233- f := strings.Fields(string(line))
234- if len(f) == 0 {
235- continue
236- }
237- if len(out) == maxCommits {
238- return nil, fmt.Errorf("%w: the push brings more than %d commits", domain.ErrInvalid, maxCommits)
239- }
240- if ids += len(f); ids > maxIDs {
241- return nil, fmt.Errorf("%w: the push brings commits with more than %d ids (commits and parents) in all", domain.ErrInvalid, maxIDs)
242- }
243- for _, id := range f {
244- if _, err := domain.ParseObjectID(id); err != nil {
245- return nil, fmt.Errorf("%w: unexpected rev-list output", domain.ErrInvalid)
246- }
247- }
248- out = append(out, pushedCommit{id: f[0], parents: f[1:]})
249- }
250-}
251-
252-// workflowsAt returns, for each spec ("<rev>:.github/workflows"), the id of the
253-// directory it names, or "" when there is none (see workflowsOf).
254-func (g *Git) workflowsAt(ctx context.Context, dir string, extra []string, specs []string) (map[string]string, error) {
255- out := make(map[string]string, len(specs))
256- if len(specs) == 0 {
257- return out, nil
258- }
259- args := readArgs(dir, "cat-file", "--batch-check=%(objectmode) %(objectname) %(objecttype)")
260- err := g.pipeWith(ctx, extra, strings.NewReader(strings.Join(specs, "\n")+"\n"), args, func(r *bufio.Reader) error {
261- for _, spec := range specs {
262- line, err := r.ReadSlice('\n')
263- if err != nil {
264- return fmt.Errorf("read object answers: %w", err)
265- }
266- out[spec] = workflowsOf("", strings.TrimSuffix(string(line), "\n"))
267- }
268- return nil
269- })
270- if err != nil {
271- return nil, fmt.Errorf("workflows of a push: %w", err)
272- }
273- return out, nil
274-}
275-
276-// workflowsOf is the id of the tree that is the .github/workflows of a commit
277-// or a tree, "" when there is none: the answer for "<it>:.github/workflows" must
278-// be a tree of mode 040000, and the object peeled to a tree-ish, not a blob.
279-func workflowsOf(objType, answer string) string {
280- if objType == "blob" {
281- return ""
282- }
283- mode, rest, _ := strings.Cut(answer, " ")
284- id, typ, _ := strings.Cut(rest, " ")
285- if mode != "040000" || typ != "tree" {
286- return ""
287- }
288- return id
289-}
290-
291-// firstChanged names the first top-level entry of .github/workflows that
292-// differs between two trees (oldID may be "": none, the empty tree).
293-func (g *Git) firstChanged(ctx context.Context, dir string, extra []string, oldID, newID string) (string, error) {
294- from := oldID
295- if from == "" {
296- from = emptyTree
297- }
298- to := newID
299- args := readArgs(dir, "diff-tree", "--name-only", "-z", "--no-renames", "--no-commit-id", "--end-of-options", from, to)
300- var name string
301- err := g.pipeWith(ctx, extra, nil, args, func(r *bufio.Reader) error {
302- n, found, err := firstPath(r)
303- name = n
304- if err != nil || !found {
305- return err // no name: git's exit status is checked, so a failure refuses
306- }
307- return errStop // the first name is all that is needed: stop git
308- })
309- if err != nil {
310- return "", fmt.Errorf("changed workflows %s..%s: %w", from, to, err)
311- }
312- if name == "" {
313- return strings.TrimSuffix(workflowsDir, "/"), nil
314- }
315- return workflowsDir + name, nil
316-}
317-
318-// firstPath reads the first NUL-terminated path of r; found is false at the end
319-// of the output without one. A path longer than maxWorkflowPath is an error.
320-func firstPath(r *bufio.Reader) (path string, found bool, err error) {
321- var b []byte
322- for {
323- c, err := r.ReadByte()
324- switch {
325- case err != nil:
326- return "", false, nil // the end of the output; a failing exit status is reported by pipe
327- case c == 0:
328- return string(b), true, nil
329- case len(b) >= maxWorkflowPath:
330- return "", false, fmt.Errorf("%w: a changed path is longer than %d bytes", domain.ErrInvalid, maxWorkflowPath)
331- }
332- b = append(b, c)
333- }
334-}
335-
336-// batchCheck asks `cat-file --batch-check=<format>` about <oid><suffix> for the
337-// old (unless the ref is created) and the new end of each update and returns
338-// the answers in that order, two per update, "" for the old end of a created
339-// ref. With suffix "^{}" an object that is missing, malformed or neither a
340-// commit, a tree nor a blob is an error (the push is refused); with any other
341-// suffix an answer "<input> missing" is kept as the answer.
342-func (g *Git) batchCheck(ctx context.Context, dir string, extra []string, ups []domain.RefUpdate, suffix, format string) ([]string, error) {
343- var in strings.Builder
344- want := 0
345- for _, up := range ups {
346- for _, oid := range []string{up.Old, up.New} {
347- if oid == domain.ZeroOID {
348- continue
349- }
350- if _, err := domain.ParseObjectID(oid); err != nil {
351- return nil, fmt.Errorf("%w: %v", domain.ErrInvalid, err)
352- }
353- in.WriteString(oid + suffix + "\n")
354- want++
355- }
356- }
357- got := make([]string, 0, want)
358- args := readArgs(dir, "cat-file", "--batch-check="+format)
359- err := g.pipeWith(ctx, extra, strings.NewReader(in.String()), args, func(r *bufio.Reader) error {
360- for range want {
361- line, err := r.ReadSlice('\n')
362- if err != nil {
363- if errors.Is(err, bufio.ErrBufferFull) {
364- return fmt.Errorf("%w: unexpected cat-file output", domain.ErrInvalid)
365- }
366- return fmt.Errorf("read object answers: %w", err)
367- }
368- got = append(got, strings.TrimSuffix(string(line), "\n"))
369- }
370- return nil
371- })
372- if err != nil {
373- return nil, fmt.Errorf("objects of a push: %w", err)
374- }
375- out := make([]string, 0, 2*len(ups))
376- i := 0
377- for _, up := range ups {
378- for _, oid := range []string{up.Old, up.New} {
379- if oid == domain.ZeroOID {
380- out = append(out, "")
381- continue
382- }
383- if suffix == "^{}" {
384- switch got[i] {
385- case "commit", "tree", "blob":
386- default: // "<input> missing", or a type that cannot be compared
387- return nil, fmt.Errorf("%w: object %s of the push is missing or of an unexpected type", domain.ErrInvalid, oid)
388- }
389- }
390- out = append(out, got[i])
391- i++
392- }
393- }
394- return out, nil
395-}
396-
397-// quarantineEnv returns the environment that makes git read the objects of a
398-// push in quarantine. Each directory must exist and, once symbolic links are
399-// resolved, be inside the objects directory of the repository dir (itself
400-// resolved): a hook request cannot send git to read anything else. The resolved
401-// paths are the ones given to git.
402-func quarantineEnv(repoDir string, o ports.PushObjects) ([]string, error) {
403- if o.Dir == "" && o.Alternates == "" {
404- return nil, nil
405- }
406- root, err := filepath.EvalSymlinks(filepath.Join(repoDir, "objects"))
407- if err != nil {
408- return nil, fmt.Errorf("resolve the objects directory of %s: %w", repoDir, err)
409- }
410- inside := func(p string) (string, bool) {
411- if !filepath.IsAbs(p) {
412- return "", false
413- }
414- real, err := filepath.EvalSymlinks(p)
415- if err != nil {
416- return "", false
417- }
418- rel, err := filepath.Rel(root, real)
419- return real, err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator))
420- }
421- dir, ok := inside(o.Dir)
422- if o.Dir == "" || !ok {
423- return nil, fmt.Errorf("%w: the objects directory of the push is outside the repository", domain.ErrInvalid)
424- }
425- env := []string{"GIT_OBJECT_DIRECTORY=" + dir}
426- if o.Alternates != "" {
427- var alts []string
428- for _, a := range filepath.SplitList(o.Alternates) {
429- real, ok := inside(a)
430- if !ok {
431- return nil, fmt.Errorf("%w: an alternate objects directory of the push is outside the repository", domain.ErrInvalid)
432- }
433- alts = append(alts, real)
434- }
435- env = append(env, "GIT_ALTERNATE_OBJECT_DIRECTORIES="+strings.Join(alts, string(filepath.ListSeparator)))
436- }
437- return env, nil
438-}
M internal/adapters/githttp/handler.go +4 -4
@@ -174,6 +174,8 @@ var gitSuffixes = []string{"/info/refs", "/" + serviceUpload, "/" + serviceRecei
174174 // and /<repo>/git-receive-pack, and the same under /<owner>/ for the owner of
175175 // the forge. info/refs without a service is the dumb protocol, which ermite
176176 // does not serve.
177+//
178+//hexcheck:ignore suspicious-business-logic-in-adapter decodes smart-HTTP repository routes and owner aliases, validating methods and service query parameters
177179 func (h *Handler) parseRoute(r *http.Request) (route, int) {
178180 p := strings.TrimPrefix(r.URL.Path, "/")
179181 var head, rest string
@@ -224,8 +226,6 @@ func (h *Handler) parseRoute(r *http.Request) (route, int) {
224226 }
225227
226228 // ServeHTTP implements http.Handler.
227-//
228-//hexcheck:ignore suspicious-business-logic-in-adapter transport orchestration: the authorization decisions are the use cases'
229229 func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
230230 var rt route
231231 var name domain.RepoName
@@ -328,6 +328,8 @@ func credentials(r *http.Request) (secret string, present bool) {
328328 // denied answers an authorization failure. A caller without valid
329329 // credentials is asked for them wherever the repository does not show itself,
330330 // which keeps a private repository and a missing one indistinguishable.
331+//
332+//hexcheck:ignore suspicious-business-logic-in-adapter implements the smart-HTTP protocol visibility/auth challenge policy (401 vs 404) for hidden or missing repositories
331333 func (h *Handler) denied(ctx context.Context, w http.ResponseWriter, p domain.Principal, err error) {
332334 switch {
333335 case errors.Is(err, domain.ErrUnauthenticated):
@@ -418,8 +420,6 @@ func (h *Handler) acquire(ctx context.Context, w http.ResponseWriter, rt route,
418420 }
419421
420422 // serveBackend runs git http-backend for an authorized request.
421-//
422-//hexcheck:ignore suspicious-business-logic-in-adapter process and CGI plumbing: no business rule is decided here
423423 func (h *Handler) serveBackend(w http.ResponseWriter, r *http.Request, rt route, name domain.RepoName, p domain.Principal, client string) {
424424 ctx := r.Context()
425425 logger := zerowrap.FromCtx(ctx)
@@ -174,6 +174,8 @@ var gitSuffixes = []string{"/info/refs", "/" + serviceUpload, "/" + serviceRecei
174 // and /<repo>/git-receive-pack, and the same under /<owner>/ for the owner of174 // and /<repo>/git-receive-pack, and the same under /<owner>/ for the owner of
175 // the forge. info/refs without a service is the dumb protocol, which ermite175 // the forge. info/refs without a service is the dumb protocol, which ermite
176 // does not serve.176 // does not serve.
177+//
178+//hexcheck:ignore suspicious-business-logic-in-adapter decodes smart-HTTP repository routes and owner aliases, validating methods and service query parameters
177 func (h *Handler) parseRoute(r *http.Request) (route, int) {179 func (h *Handler) parseRoute(r *http.Request) (route, int) {
178 p := strings.TrimPrefix(r.URL.Path, "/")180 p := strings.TrimPrefix(r.URL.Path, "/")
179 var head, rest string181 var head, rest string
@@ -224,8 +226,6 @@ func (h *Handler) parseRoute(r *http.Request) (route, int) {
224 }226 }
225 227
226 // ServeHTTP implements http.Handler.228 // ServeHTTP implements http.Handler.
227-//
228-//hexcheck:ignore suspicious-business-logic-in-adapter transport orchestration: the authorization decisions are the use cases'
229 func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {229 func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
230 var rt route230 var rt route
231 var name domain.RepoName231 var name domain.RepoName
@@ -328,6 +328,8 @@ func credentials(r *http.Request) (secret string, present bool) {
328 // denied answers an authorization failure. A caller without valid328 // denied answers an authorization failure. A caller without valid
329 // credentials is asked for them wherever the repository does not show itself,329 // credentials is asked for them wherever the repository does not show itself,
330 // which keeps a private repository and a missing one indistinguishable.330 // which keeps a private repository and a missing one indistinguishable.
331+//
332+//hexcheck:ignore suspicious-business-logic-in-adapter implements the smart-HTTP protocol visibility/auth challenge policy (401 vs 404) for hidden or missing repositories
331 func (h *Handler) denied(ctx context.Context, w http.ResponseWriter, p domain.Principal, err error) {333 func (h *Handler) denied(ctx context.Context, w http.ResponseWriter, p domain.Principal, err error) {
332 switch {334 switch {
333 case errors.Is(err, domain.ErrUnauthenticated):335 case errors.Is(err, domain.ErrUnauthenticated):
@@ -418,8 +420,6 @@ func (h *Handler) acquire(ctx context.Context, w http.ResponseWriter, rt route,
418 }420 }
419 421
420 // serveBackend runs git http-backend for an authorized request.422 // serveBackend runs git http-backend for an authorized request.
421-//
422-//hexcheck:ignore suspicious-business-logic-in-adapter process and CGI plumbing: no business rule is decided here
423 func (h *Handler) serveBackend(w http.ResponseWriter, r *http.Request, rt route, name domain.RepoName, p domain.Principal, client string) {423 func (h *Handler) serveBackend(w http.ResponseWriter, r *http.Request, rt route, name domain.RepoName, p domain.Principal, client string) {
424 ctx := r.Context()424 ctx := r.Context()
425 logger := zerowrap.FromCtx(ctx)425 logger := zerowrap.FromCtx(ctx)
M internal/adapters/githttp/hooks.go +0 -2
@@ -175,8 +175,6 @@ func (s *HookServer) Close(ctx context.Context) {
175175 }
176176
177177 // handle serves one connection: one request line, one reply line.
178-//
179-//hexcheck:ignore suspicious-business-logic-in-adapter hook protocol dispatch: the decisions are the use cases'
180178 func (s *HookServer) handle(ctx context.Context, conn net.Conn) {
181179 log := zerowrap.FromCtx(ctx)
182180 _ = conn.SetReadDeadline(time.Now().Add(readTimeout))
@@ -175,8 +175,6 @@ func (s *HookServer) Close(ctx context.Context) {
175 }175 }
176 176
177 // handle serves one connection: one request line, one reply line.177 // handle serves one connection: one request line, one reply line.
178-//
179-//hexcheck:ignore suspicious-business-logic-in-adapter hook protocol dispatch: the decisions are the use cases'
180 func (s *HookServer) handle(ctx context.Context, conn net.Conn) {178 func (s *HookServer) handle(ctx context.Context, conn net.Conn) {
181 log := zerowrap.FromCtx(ctx)179 log := zerowrap.FromCtx(ctx)
182 _ = conn.SetReadDeadline(time.Now().Add(readTimeout))180 _ = conn.SetReadDeadline(time.Now().Add(readTimeout))
M internal/adapters/kvstore/ci.go +11 -18
@@ -310,6 +310,9 @@ func decodeRun(item kv.Item) (domain.CIRun, error) {
310310 }, nil
311311 }
312312
313+// encodeJob serializes a job for storage.
314+//
315+//hexcheck:ignore suspicious-business-logic-in-adapter encodes a job as protobuf, rejecting an empty repository and omitting an absent token hash
313316 func encodeJob(j domain.CIJob) ([]byte, error) {
314317 if j.Repo.IsZero() {
315318 return nil, fmt.Errorf("%w: empty name", domain.ErrInvalidName)
@@ -346,7 +349,6 @@ func encodeJob(j domain.CIJob) ([]byte, error) {
346349 return mustMarshal(pb), nil
347350 }
348351
349-//hexcheck:ignore suspicious-business-logic-in-adapter maps the stored record to the domain job field by field: no rule is decided here
350352 func decodeJob(item kv.Item) (domain.CIJob, error) {
351353 var pb storagepb.CIJob
352354 if err := proto.Unmarshal(item.Value, &pb); err != nil {
@@ -530,8 +532,6 @@ func (c *CI) stageRecords(ctx context.Context, b *kv.Batch, w ports.CIWrite) (gu
530532 }
531533
532534 // Apply writes the records in one batch.
533-//
534-//hexcheck:ignore suspicious-business-logic-in-adapter stages records and their derived indexes under version preconditions: a storage concern, no policy
535535 func (c *CI) Apply(ctx context.Context, w ports.CIWrite) (uint64, error) {
536536 var b kv.Batch
537537 guarded, err := c.stageRecords(ctx, &b, w)
@@ -576,18 +576,14 @@ func stageSeq(b *kv.Batch, key []byte, item kv.Item, ok bool, last int64) {
576576
577577 const createRunTries = 20
578578
579-// CreateRun takes the next run id and number and stores the draft.
579+// CreateRun takes the next run id and number and stores the draft. The batch
580+// requires the workflow's disabled key absent, so a disable that races the
581+// caller's check fails the batch and is reported as domain.ErrWorkflowDisabled.
580582 //
581-//hexcheck:ignore suspicious-business-logic-in-adapter takes the next numbers under version preconditions: a storage concern, no policy
583+//hexcheck:ignore suspicious-business-logic-in-adapter atomic precondition only: maps a conflict on the disabled-workflow key to its domain error; the usecase decides
582584 func (c *CI) CreateRun(ctx context.Context, repo domain.RepoName, workflowPath string, build func(id, number int64) ports.CIRunDraft) (ports.CIRunDraft, error) {
583585 offKey := workflowOffKey(repo, workflowPath)
584586 for range createRunTries {
585- // A workflow the repository disabled gets no run, whatever started it.
586- // The key is also required absent by the batch below, so a disable that
587- // lands after this read fails the batch, and the next try sees it.
588- if _, off := c.db.Get(ctx, offKey); off {
589- return ports.CIRunDraft{}, domain.ErrWorkflowDisabled
590- }
591587 lastID, idItem, idOK, err := c.readSeq(ctx, []byte(runSeqKey))
592588 if err != nil {
593589 return ports.CIRunDraft{}, err
@@ -620,6 +616,10 @@ func (c *CI) CreateRun(ctx context.Context, repo domain.RepoName, workflowPath s
620616 }
621617 v, err := c.db.Apply(ctx, &b)
622618 if errors.Is(err, kv.ErrConflict) {
619+ // Translate a failed disabled-workflow precondition; other conflicts retry.
620+ if _, off := c.db.Get(ctx, offKey); off {
621+ return ports.CIRunDraft{}, domain.ErrWorkflowDisabled
622+ }
623623 continue
624624 }
625625 if err != nil {
@@ -669,8 +669,6 @@ func (c *CI) GetRunSource(ctx context.Context, runID int64) (domain.CIRunSource,
669669 }
670670
671671 // ListRuns returns the runs of the repository, newest first.
672-//
673-//hexcheck:ignore suspicious-business-logic-in-adapter port method name only: reads a key range and reverses it
674672 func (c *CI) ListRuns(ctx context.Context, repo domain.RepoName, before int64, limit int) ([]domain.CIRun, error) {
675673 start := []byte(runsPrefix(repo))
676674 end := []byte(runPrefix + repo.String() + "0") // '/' + 1
@@ -906,8 +904,6 @@ func ciKeys(ctx context.Context, snap *kv.Snapshot, repo domain.RepoName) [][]by
906904 const sweepChunk = 500
907905
908906 // Sweep removes the runs completed before the time.
909-//
910-//hexcheck:ignore suspicious-business-logic-in-adapter chunked delete of expired records: a storage concern, the retention is decided by the caller
911907 func (c *CI) Sweep(ctx context.Context, before time.Time) ([]int64, error) {
912908 snap := c.db.Snapshot(ctx)
913909 var ids []int64
@@ -955,8 +951,6 @@ func runnerNameKey(n string) []byte { return []byte(runnerName + stri
955951 func runnerTokenKey(h domain.TokenHash) []byte { return []byte(runnerTokPref + h.String()) }
956952
957953 // CreateRunner stores the runner, its name index and its token index.
958-//
959-//hexcheck:ignore suspicious-business-logic-in-adapter replaces a runner of the same name in one batch: a storage concern, the rule is decided by the caller
960954 func (c *CI) CreateRunner(ctx context.Context, r domain.CIRunner, replace bool) ([]domain.CIRunner, error) {
961955 for range createRunTries {
962956 var b kv.Batch
@@ -1115,7 +1109,6 @@ func encodeCall(c *domain.CICall) *storagepb.CICall {
11151109 }
11161110 }
11171111
1118-//hexcheck:ignore suspicious-business-logic-in-adapter maps the stored call to the domain call field by field: no rule is decided here
11191112 func decodeCall(pb *storagepb.CICall) *domain.CICall {
11201113 if pb == nil {
11211114 return nil
@@ -310,6 +310,9 @@ func decodeRun(item kv.Item) (domain.CIRun, error) {
310 }, nil310 }, nil
311 }311 }
312 312
313+// encodeJob serializes a job for storage.
314+//
315+//hexcheck:ignore suspicious-business-logic-in-adapter encodes a job as protobuf, rejecting an empty repository and omitting an absent token hash
313 func encodeJob(j domain.CIJob) ([]byte, error) {316 func encodeJob(j domain.CIJob) ([]byte, error) {
314 if j.Repo.IsZero() {317 if j.Repo.IsZero() {
315 return nil, fmt.Errorf("%w: empty name", domain.ErrInvalidName)318 return nil, fmt.Errorf("%w: empty name", domain.ErrInvalidName)
@@ -346,7 +349,6 @@ func encodeJob(j domain.CIJob) ([]byte, error) {
346 return mustMarshal(pb), nil349 return mustMarshal(pb), nil
347 }350 }
348 351
349-//hexcheck:ignore suspicious-business-logic-in-adapter maps the stored record to the domain job field by field: no rule is decided here
350 func decodeJob(item kv.Item) (domain.CIJob, error) {352 func decodeJob(item kv.Item) (domain.CIJob, error) {
351 var pb storagepb.CIJob353 var pb storagepb.CIJob
352 if err := proto.Unmarshal(item.Value, &pb); err != nil {354 if err := proto.Unmarshal(item.Value, &pb); err != nil {
@@ -530,8 +532,6 @@ func (c *CI) stageRecords(ctx context.Context, b *kv.Batch, w ports.CIWrite) (gu
530 }532 }
531 533
532 // Apply writes the records in one batch.534 // Apply writes the records in one batch.
533-//
534-//hexcheck:ignore suspicious-business-logic-in-adapter stages records and their derived indexes under version preconditions: a storage concern, no policy
535 func (c *CI) Apply(ctx context.Context, w ports.CIWrite) (uint64, error) {535 func (c *CI) Apply(ctx context.Context, w ports.CIWrite) (uint64, error) {
536 var b kv.Batch536 var b kv.Batch
537 guarded, err := c.stageRecords(ctx, &b, w)537 guarded, err := c.stageRecords(ctx, &b, w)
@@ -576,18 +576,14 @@ func stageSeq(b *kv.Batch, key []byte, item kv.Item, ok bool, last int64) {
576 576
577 const createRunTries = 20577 const createRunTries = 20
578 578
579-// CreateRun takes the next run id and number and stores the draft.579+// CreateRun takes the next run id and number and stores the draft. The batch
580+// requires the workflow's disabled key absent, so a disable that races the
581+// caller's check fails the batch and is reported as domain.ErrWorkflowDisabled.
580 //582 //
581-//hexcheck:ignore suspicious-business-logic-in-adapter takes the next numbers under version preconditions: a storage concern, no policy583+//hexcheck:ignore suspicious-business-logic-in-adapter atomic precondition only: maps a conflict on the disabled-workflow key to its domain error; the usecase decides
582 func (c *CI) CreateRun(ctx context.Context, repo domain.RepoName, workflowPath string, build func(id, number int64) ports.CIRunDraft) (ports.CIRunDraft, error) {584 func (c *CI) CreateRun(ctx context.Context, repo domain.RepoName, workflowPath string, build func(id, number int64) ports.CIRunDraft) (ports.CIRunDraft, error) {
583 offKey := workflowOffKey(repo, workflowPath)585 offKey := workflowOffKey(repo, workflowPath)
584 for range createRunTries {586 for range createRunTries {
585- // A workflow the repository disabled gets no run, whatever started it.
586- // The key is also required absent by the batch below, so a disable that
587- // lands after this read fails the batch, and the next try sees it.
588- if _, off := c.db.Get(ctx, offKey); off {
589- return ports.CIRunDraft{}, domain.ErrWorkflowDisabled
590- }
591 lastID, idItem, idOK, err := c.readSeq(ctx, []byte(runSeqKey))587 lastID, idItem, idOK, err := c.readSeq(ctx, []byte(runSeqKey))
592 if err != nil {588 if err != nil {
593 return ports.CIRunDraft{}, err589 return ports.CIRunDraft{}, err
@@ -620,6 +616,10 @@ func (c *CI) CreateRun(ctx context.Context, repo domain.RepoName, workflowPath s
620 }616 }
621 v, err := c.db.Apply(ctx, &b)617 v, err := c.db.Apply(ctx, &b)
622 if errors.Is(err, kv.ErrConflict) {618 if errors.Is(err, kv.ErrConflict) {
619+ // Translate a failed disabled-workflow precondition; other conflicts retry.
620+ if _, off := c.db.Get(ctx, offKey); off {
621+ return ports.CIRunDraft{}, domain.ErrWorkflowDisabled
622+ }
623 continue623 continue
624 }624 }
625 if err != nil {625 if err != nil {
@@ -669,8 +669,6 @@ func (c *CI) GetRunSource(ctx context.Context, runID int64) (domain.CIRunSource,
669 }669 }
670 670
671 // ListRuns returns the runs of the repository, newest first.671 // ListRuns returns the runs of the repository, newest first.
672-//
673-//hexcheck:ignore suspicious-business-logic-in-adapter port method name only: reads a key range and reverses it
674 func (c *CI) ListRuns(ctx context.Context, repo domain.RepoName, before int64, limit int) ([]domain.CIRun, error) {672 func (c *CI) ListRuns(ctx context.Context, repo domain.RepoName, before int64, limit int) ([]domain.CIRun, error) {
675 start := []byte(runsPrefix(repo))673 start := []byte(runsPrefix(repo))
676 end := []byte(runPrefix + repo.String() + "0") // '/' + 1674 end := []byte(runPrefix + repo.String() + "0") // '/' + 1
@@ -906,8 +904,6 @@ func ciKeys(ctx context.Context, snap *kv.Snapshot, repo domain.RepoName) [][]by
906 const sweepChunk = 500904 const sweepChunk = 500
907 905
908 // Sweep removes the runs completed before the time.906 // Sweep removes the runs completed before the time.
909-//
910-//hexcheck:ignore suspicious-business-logic-in-adapter chunked delete of expired records: a storage concern, the retention is decided by the caller
911 func (c *CI) Sweep(ctx context.Context, before time.Time) ([]int64, error) {907 func (c *CI) Sweep(ctx context.Context, before time.Time) ([]int64, error) {
912 snap := c.db.Snapshot(ctx)908 snap := c.db.Snapshot(ctx)
913 var ids []int64909 var ids []int64
@@ -955,8 +951,6 @@ func runnerNameKey(n string) []byte { return []byte(runnerName + stri
955 func runnerTokenKey(h domain.TokenHash) []byte { return []byte(runnerTokPref + h.String()) }951 func runnerTokenKey(h domain.TokenHash) []byte { return []byte(runnerTokPref + h.String()) }
956 952
957 // CreateRunner stores the runner, its name index and its token index.953 // CreateRunner stores the runner, its name index and its token index.
958-//
959-//hexcheck:ignore suspicious-business-logic-in-adapter replaces a runner of the same name in one batch: a storage concern, the rule is decided by the caller
960 func (c *CI) CreateRunner(ctx context.Context, r domain.CIRunner, replace bool) ([]domain.CIRunner, error) {954 func (c *CI) CreateRunner(ctx context.Context, r domain.CIRunner, replace bool) ([]domain.CIRunner, error) {
961 for range createRunTries {955 for range createRunTries {
962 var b kv.Batch956 var b kv.Batch
@@ -1115,7 +1109,6 @@ func encodeCall(c *domain.CICall) *storagepb.CICall {
1115 }1109 }
1116 }1110 }
1117 1111
1118-//hexcheck:ignore suspicious-business-logic-in-adapter maps the stored call to the domain call field by field: no rule is decided here
1119 func decodeCall(pb *storagepb.CICall) *domain.CICall {1112 func decodeCall(pb *storagepb.CICall) *domain.CICall {
1120 if pb == nil {1113 if pb == nil {
1121 return nil1114 return nil
M internal/adapters/kvstore/ci_artifact.go +3 -5
@@ -79,6 +79,9 @@ var artifactStates = []struct {
7979 {domain.CIArtifactFinal, storagepb.CIArtifactState_CI_ARTIFACT_STATE_FINAL},
8080 }
8181
82+// encodeArtifact serializes an artifact for storage.
83+//
84+//hexcheck:ignore suspicious-business-logic-in-adapter validates artifact storage identifiers, UTF-8 and enum encoding before protobuf serialization
8285 func encodeArtifact(a domain.CIArtifact) ([]byte, error) {
8386 if a.Repo.IsZero() || a.ID < 1 || a.RunID < 1 {
8487 return nil, fmt.Errorf("%w: artifact without id, run or repository", domain.ErrInvalid)
@@ -106,7 +109,6 @@ func encodeArtifact(a domain.CIArtifact) ([]byte, error) {
106109 return mustMarshal(pb), nil
107110 }
108111
109-//hexcheck:ignore suspicious-business-logic-in-adapter decodes a record into the domain type: a storage concern, no policy
110112 func decodeArtifact(item kv.Item) (domain.CIArtifact, error) {
111113 var pb storagepb.CIArtifact
112114 if err := proto.Unmarshal(item.Value, &pb); err != nil {
@@ -141,8 +143,6 @@ func decodeArtifact(item kv.Item) (domain.CIArtifact, error) {
141143 const artifactIDTries = 1000
142144
143145 // NextArtifactID takes the next id in its own batch.
144-//
145-//hexcheck:ignore suspicious-business-logic-in-adapter takes the next number under a version precondition: a storage concern, no policy
146146 func (c *CIArtifacts) NextArtifactID(ctx context.Context) (int64, error) {
147147 ci := &CI{db: c.db}
148148 for range artifactIDTries {
@@ -274,8 +274,6 @@ func (c *CIArtifacts) BlockArtifactIDs(ctx context.Context) ([]int64, error) {
274274 // --- writes ---
275275
276276 // ApplyArtifacts writes w in one batch.
277-//
278-//hexcheck:ignore suspicious-business-logic-in-adapter stages records and their derived keys under version preconditions: a storage concern, no policy
279277 func (c *CIArtifacts) ApplyArtifacts(ctx context.Context, w ports.CIArtifactWrite) error {
280278 var b kv.Batch
281279 guard := false
@@ -79,6 +79,9 @@ var artifactStates = []struct {
79 {domain.CIArtifactFinal, storagepb.CIArtifactState_CI_ARTIFACT_STATE_FINAL},79 {domain.CIArtifactFinal, storagepb.CIArtifactState_CI_ARTIFACT_STATE_FINAL},
80 }80 }
81 81
82+// encodeArtifact serializes an artifact for storage.
83+//
84+//hexcheck:ignore suspicious-business-logic-in-adapter validates artifact storage identifiers, UTF-8 and enum encoding before protobuf serialization
82 func encodeArtifact(a domain.CIArtifact) ([]byte, error) {85 func encodeArtifact(a domain.CIArtifact) ([]byte, error) {
83 if a.Repo.IsZero() || a.ID < 1 || a.RunID < 1 {86 if a.Repo.IsZero() || a.ID < 1 || a.RunID < 1 {
84 return nil, fmt.Errorf("%w: artifact without id, run or repository", domain.ErrInvalid)87 return nil, fmt.Errorf("%w: artifact without id, run or repository", domain.ErrInvalid)
@@ -106,7 +109,6 @@ func encodeArtifact(a domain.CIArtifact) ([]byte, error) {
106 return mustMarshal(pb), nil109 return mustMarshal(pb), nil
107 }110 }
108 111
109-//hexcheck:ignore suspicious-business-logic-in-adapter decodes a record into the domain type: a storage concern, no policy
110 func decodeArtifact(item kv.Item) (domain.CIArtifact, error) {112 func decodeArtifact(item kv.Item) (domain.CIArtifact, error) {
111 var pb storagepb.CIArtifact113 var pb storagepb.CIArtifact
112 if err := proto.Unmarshal(item.Value, &pb); err != nil {114 if err := proto.Unmarshal(item.Value, &pb); err != nil {
@@ -141,8 +143,6 @@ func decodeArtifact(item kv.Item) (domain.CIArtifact, error) {
141 const artifactIDTries = 1000143 const artifactIDTries = 1000
142 144
143 // NextArtifactID takes the next id in its own batch.145 // NextArtifactID takes the next id in its own batch.
144-//
145-//hexcheck:ignore suspicious-business-logic-in-adapter takes the next number under a version precondition: a storage concern, no policy
146 func (c *CIArtifacts) NextArtifactID(ctx context.Context) (int64, error) {146 func (c *CIArtifacts) NextArtifactID(ctx context.Context) (int64, error) {
147 ci := &CI{db: c.db}147 ci := &CI{db: c.db}
148 for range artifactIDTries {148 for range artifactIDTries {
@@ -274,8 +274,6 @@ func (c *CIArtifacts) BlockArtifactIDs(ctx context.Context) ([]int64, error) {
274 // --- writes ---274 // --- writes ---
275 275
276 // ApplyArtifacts writes w in one batch.276 // ApplyArtifacts writes w in one batch.
277-//
278-//hexcheck:ignore suspicious-business-logic-in-adapter stages records and their derived keys under version preconditions: a storage concern, no policy
279 func (c *CIArtifacts) ApplyArtifacts(ctx context.Context, w ports.CIArtifactWrite) error {277 func (c *CIArtifacts) ApplyArtifacts(ctx context.Context, w ports.CIArtifactWrite) error {
280 var b kv.Batch278 var b kv.Batch
281 guard := false279 guard := false
M internal/adapters/kvstore/ci_concurrency.go +0 -4
M internal/adapters/kvstore/ci_settings.go +6 -4
M internal/adapters/kvstore/issues.go +0 -8
M internal/adapters/kvstore/issues_pending.go +0 -5
M internal/adapters/kvstore/mergerequests.go +3 -9
M internal/adapters/kvstore/passkeys.go +0 -2
M internal/adapters/kvstore/repos.go +2 -10
M internal/adapters/kvstore/store.go +0 -2
M internal/adapters/kvstore/tokens.go +0 -6
M internal/adapters/rpc/api.go +2 -4
M internal/adapters/rpc/convert.go +1 -4
M internal/adapters/rpc/git.go +0 -1
M internal/adapters/rpc/mergerequest.go +0 -2
M internal/adapters/rpc/repo.go +0 -2
M internal/adapters/rpc/server.go +5 -8
M internal/adapters/rpc/server_test.go +11 -0
M internal/adapters/sigverify/gpg.go +0 -2
M internal/adapters/sigverify/ssh.go +0 -2
M internal/adapters/sigverify/verifier.go +0 -2
M internal/adapters/web/actions_artifacts.go +0 -4
M internal/adapters/web/browse.go +0 -2
M internal/adapters/web/checks.go +2 -0
M internal/adapters/web/commit.go +0 -2
M internal/adapters/web/compare.go +3 -55
M internal/adapters/web/context.go +1 -1
M internal/adapters/web/diff_highlight.go +0 -2
M internal/adapters/web/events.go +3 -1
M internal/adapters/web/events_test.go +13 -0
M internal/adapters/web/fragments.go +43 -32
M internal/adapters/web/issues.go +3 -5
M internal/adapters/web/issues_form.go +3 -1
M internal/adapters/web/issues_post.go +0 -1
M internal/adapters/web/mr.go +96 -88
M internal/adapters/web/mr_files.go +0 -2
M internal/adapters/web/mr_post.go +0 -2
M internal/adapters/web/mr_test.go +4 -4
M internal/adapters/web/paths.go +0 -2
M internal/adapters/web/raw.go +0 -2
M internal/adapters/web/refs.go +0 -4
M internal/adapters/web/repo.go +3 -7
M internal/adapters/web/tree.go +0 -2
M internal/adapters/webauthn/webauthn.go +0 -2
M internal/app/forge.go +3 -8
A internal/domain/budget.go +85 -0
A internal/domain/budget_test.go +141 -0
A internal/domain/ci_workflow_guard.go +123 -0
A internal/domain/ci_workflow_guard_test.go +120 -0
M internal/domain/mergerequest.go +9 -0
M internal/domain/repo.go +8 -0
M internal/domain/repo_test.go +34 -0
A internal/domain/revision.go +54 -0
A internal/domain/revision_test.go +58 -0
M internal/ports/ci.go +41 -15
M internal/ports/dto.go +28 -0
M internal/ports/mocks/MergeRequestService.go +168 -0
M internal/ports/mocks/OIDCKeyService.go +44 -0
A internal/ports/mocks/PushTreeReader.go +223 -0
D internal/ports/mocks/WorkflowGuard.go +0 -127
M internal/ports/oidc.go +2 -0
M internal/ports/services.go +4 -0
M internal/usecase/ci.go +1 -1
A internal/usecase/ci_creation_policy_test.go +48 -0
M internal/usecase/ci_oidc.go +3 -9
M internal/usecase/ci_pull.go +9 -1
M internal/usecase/ci_runner.go +1 -1
M internal/usecase/events.go +2 -2
M internal/usecase/inbox.go +1 -1
M internal/usecase/issue_moderation.go +1 -1
M internal/usecase/issue_submissions.go +1 -1
M internal/usecase/issues.go +1 -1
M internal/usecase/mergerequests.go +57 -8
M internal/usecase/mergerequests_test.go +144 -0
M internal/usecase/oidc.go +27 -3
M internal/usecase/oidc_test.go +44 -1
M internal/usecase/pushes.go +30 -14
M internal/usecase/pushes_test.go +79 -17
M internal/usecase/repos_write.go +1 -3

Comments1 open / 1

bnema outdated

Scope extended: the business rules flagged in adapters now live in domain/usecase.

  • domain: Revision.Candidates, CommitObjectBudget/ValidateCommitObjects/DiffBudget, WorkflowChangeOf (port WorkflowGuard -> PushTreeReader), Repo.EffectiveSettings, IssuesEnabled/ShowsIssueSubmissionReceipt/AcceptsAnonymousIssues.
  • usecase: CI.createRun refuses disabled workflows (kvstore keeps the atomic precondition); OIDC service always built, decides disabled-mode errors; MergeRequests.Compare/PushSummary, Get returns NewCommits/HeadUnread.
  • Deliberate changes: the commits banner detects a rewrite by ancestry; the compare page uses the real target..source range.
  • Ignore directives: 129 -> 49, none marked 'pending move'.

Reviewed (correctness/security + architecture/maintainability/tests), findings fixed or deferred (GO-005 wiring, GO-011/012 budget loop factoring). Rebased on main 4534992. Build, vet, gofmt, hexcheck -fail-on warn, go test ./internal/... and -race on touched packages pass.

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
l
merge request: the list of files (j k move, enter opens, esc back)
M
merge request: the merge menu; there 1–3 pick the mode, D deletes the branch, enter merges
R
merge request: mark the head read
esc
close a dialog, leave a field

Branches 2

Tags 42

↑↓ moveenter openesc close