diff --git a/internal/core/claim/calls_test.go b/internal/core/claim/calls_test.go new file mode 100644 index 0000000000..b773611c46 --- /dev/null +++ b/internal/core/claim/calls_test.go @@ -0,0 +1,59 @@ +package claim + +import ( + "strings" + "testing" +) + +// defaultLookups counts the times the traced commands resolved origin's +// default branch: originDefault traces its symbolic-ref once per resolution. +func defaultLookups(l *gitLog) int { + l.mu.Lock() + defer l.mu.Unlock() + n := 0 + for _, c := range l.cmds { + if c[0] == "symbolic-ref" && strings.HasSuffix(c[len(c)-1], "/HEAD") { + n++ + } + } + return n +} + +// TestAReservationResolvesTheDefaultBranchOnceEachSideOfItsFetch +// (iss-2610180545134593): a reservation where origin holds no other names +// origin's default branch twice: once before its fetch, to name the branch the +// fetch brings in, and once after it, in judge, for the default branch's +// records. A fetch can create or move origin/HEAD, so the name is never +// carried across one. Each lookup is two git processes; judge used to make +// two of its own, so this reservation made three. Where origin holds other +// reservations, the crowding check's view names the branch once more, after +// the same fetch, as a view does (TestAViewResolvesTheDefaultBranchOnce). +func TestAReservationResolvesTheDefaultBranchOnceEachSideOfItsFetch(t *testing.T) { + w := newWorld(t) + p := w.clone("1234+octo@users.noreply.github.com", "") + log := traced(t) + if _, err := p.reserve("iss-1", "commands"); err != nil { + t.Fatal(err) + } + if n := defaultLookups(log); n != 2 { + t.Errorf("a reservation with no other on origin resolved origin's default branch %d times; want 2, once each side of its fetch", n) + } +} + +// TestAViewResolvesTheDefaultBranchOnce: a view fetches nothing, so it names +// origin's default branch once, for the records it reads there and the +// generated files it recognises; it used to name it once for each. +func TestAViewResolvesTheDefaultBranchOnce(t *testing.T) { + w := newWorld(t) + p := w.clone("1234+octo@users.noreply.github.com", "") + if _, err := p.reserve("iss-1", "commands"); err != nil { + t.Fatal(err) + } + log := traced(t) + if _, err := View(p.dir, w.clock()); err != nil { + t.Fatal(err) + } + if n := defaultLookups(log); n != 1 { + t.Errorf("one view resolved origin's default branch %d times; want 1", n) + } +} diff --git a/internal/core/claim/generated.go b/internal/core/claim/generated.go index 954d43a02c..7fef7a7f08 100644 --- a/internal/core/claim/generated.go +++ b/internal/core/claim/generated.go @@ -56,7 +56,13 @@ func (g Generated) Is(p string) bool { // root .gitattributes); otherwise the set is empty and the reason says why // generated files were not recognised. func GeneratedFiles(root string) (Generated, string) { - ref := originDefault(root) + return generatedFilesAt(root, originDefault(root)) +} + +// generatedFilesAt is GeneratedFiles read from ref, origin's default branch as +// the caller resolved it ("" for none), so an operation that has named the +// branch once reads it under that one name. +func generatedFilesAt(root, ref string) (Generated, string) { if ref == "" { return Generated{}, "" } diff --git a/internal/core/claim/handback.go b/internal/core/claim/handback.go index c35739e0c4..7324190a55 100644 --- a/internal/core/claim/handback.go +++ b/internal/core/claim/handback.go @@ -239,11 +239,10 @@ func validBlob(s string) bool { return true } -// defaultRecords is every record origin's default branch holds as last -// fetched, by id, each with its folders and blobs; nil when this checkout -// names no default branch of origin. -func defaultRecords(root string) (map[string][]peers.RecordAt, error) { - ref := originDefault(root) +// defaultRecords is every record origin's default branch, ref as the caller +// resolved it, holds as last fetched, by id, each with its folders and blobs; +// nil when ref is "", where this checkout names no default branch of origin. +func defaultRecords(root, ref string) (map[string][]peers.RecordAt, error) { if ref == "" { return nil, nil } diff --git a/internal/core/claim/remote.go b/internal/core/claim/remote.go index 816d33411e..20eaaa60b3 100644 --- a/internal/core/claim/remote.go +++ b/internal/core/claim/remote.go @@ -604,13 +604,16 @@ func judge(p *prepared, tree string) (judged, error) { // no one edited it since, and a person's edit this copy has not // fetched reads as unchanged until it is (dcn-2610161412283450, // refined by dcn-2610161837009957). One with no blob read stands. - recs, err := defaultRecords(p.root) + // One name for the default branch, resolved after the act's fetch, + // which can create or move origin/HEAD (iss-2610180545134593). + ref := originDefault(p.root) + recs, err := defaultRecords(p.root, ref) if err != nil && p.derivedGap == "" { return judged{}, refuse("could not read whether %s is finished on the default branch: %s; %s", p.id, redacted(err), noReservation) } switch { - case originDefault(p.root) == "": + case ref == "": j.blobUnread = "this checkout names no default branch of " + Remote case err != nil: j.blobUnread = p.derivedGap + "; " + redacted(err) diff --git a/internal/core/claim/view.go b/internal/core/claim/view.go index 16af69e5b6..c4c88c0a3a 100644 --- a/internal/core/claim/view.go +++ b/internal/core/claim/view.go @@ -184,7 +184,10 @@ func viewOf(root string, now time.Time, derive bool, only func(subject string) b // subjects alone, which no record there ends. var records map[string][]peers.RecordAt anyRecord := slices.ContainsFunc(refs, func(ref string) bool { return !isRelease(strings.TrimPrefix(ref, TrackingPrefix)) }) - if ref := originDefault(root); derive && anyRecord && ref != "" { + // One name for origin's default branch per view (iss-2610180545134593): a + // view fetches nothing, so the name cannot move under it. + defaultRef := originDefault(root) + if ref := defaultRef; derive && anyRecord && ref != "" { if records, err = peers.RecordsAt(root, ref); err != nil { return snap, fmt.Errorf("reading %s: %s", gitutil.ShortRef(ref), redacted(err)) } @@ -228,7 +231,7 @@ func viewOf(root string, now time.Time, derive bool, only func(subject string) b // origin's default branch recognises none. if slices.ContainsFunc(snap.Reservations, func(r Reservation) bool { return r.State == StateLive && len(r.Footprint) > 0 }) { if derive { - snap.Generated, snap.GeneratedUnread = GeneratedFiles(root) + snap.Generated, snap.GeneratedUnread = generatedFilesAt(root, defaultRef) } else { snap.GeneratedUnread = "origin's default branch could not be read when the reservations were fetched" }