MergeInto also names programs a commit of the branch added and a later one removed: the merge commit would keep the blob in history (issue 1577)

go/buildstep/merge.go

@@ -106,6 +106,11 @@ func MergeInto(ctx context.Context, run forgekey.Runner, dir, remote, base, comm
 		return res, fmt.Errorf("the files the merge adds or modifies cannot be read: %w", err)
 	}
 	res.Programs = programsIn(dir, raw)
+	history, err := historyPrograms(git, base, commit, res.Programs)
+	if err != nil {
+		return res, fmt.Errorf("the commits of the branch cannot be read for compiled programs: %w", err)
+	}
+	res.Programs = append(res.Programs, history...)
 	return res, nil
 }
 

go/buildstep/merge_integration_test.go

@@ -79,6 +79,30 @@ func (r *mergeRig) head(files map[string]string) string {
 	return h
 }
 
+// headThatAddsAndRemoves is head with an earlier commit on the branch that adds a program and a later one that removes it: the
+// branch's tree is clean, its history is not. It answers the head and the earlier commit.
+func (r *mergeRig) headThatAddsAndRemoves(files map[string]string) (head, added string) {
+	realGit(r.t, r.work, "checkout", "-q", "-B", "pr", r.fork)
+	writeFile(r.t, r.work, "gone/prog", "\x7fELF\x02\x01")
+	realGit(r.t, r.work, "add", ".")
+	realGit(r.t, r.work, "commit", "-q", "-m", "oops")
+	added = realGit(r.t, r.work, "rev-parse", "HEAD")
+	realGit(r.t, r.work, "rm", "-q", "gone/prog")
+	return r.head2(files), added
+}
+
+// head2 commits files on the current branch and pushes it as the pull request's head.
+func (r *mergeRig) head2(files map[string]string) string {
+	for n, c := range files {
+		writeFile(r.t, r.work, n, c)
+	}
+	realGit(r.t, r.work, "add", ".")
+	realGit(r.t, r.work, "commit", "-q", "-m", "head")
+	h := realGit(r.t, r.work, "rev-parse", "HEAD")
+	realGit(r.t, r.work, "push", "-q", r.bare, "HEAD:refs/pull/1/head")
+	return h
+}
+
 // advance moves the trunk past the fork point with a commit changing files, pushes it and answers its sha.
 func (r *mergeRig) advance(files map[string]string) string {
 	realGit(r.t, r.work, "checkout", "-q", "-B", "main", r.fork)
@@ -102,7 +126,7 @@ func (r *mergeRig) fetch(head string) {
 // holds both sides' changes.
 func TestMergeIntoACleanMergeAnswersTheMergeCommitAndTheChangedFiles(t *testing.T) {
 	r := newMergeRig(t)
-	head := r.head(map[string]string{"b.txt": "head\n", "prog": "\x7fELF\x02\x01"})
+	head, added := r.headThatAddsAndRemoves(map[string]string{"b.txt": "head\n", "prog": "\x7fELF\x02\x01"})
 	base := r.advance(map[string]string{"a.txt": "one\ntwo\nthree\nfour\n"})
 	r.fetch(head)
 	res, err := MergeInto(context.Background(), forgekey.ExecRunner, r.ws, r.remote, base, head, AllowFileRemote())
@@ -115,8 +139,8 @@ func TestMergeIntoACleanMergeAnswersTheMergeCommitAndTheChangedFiles(t *testing.
 	if !reflect.DeepEqual(res.Changed, []string{"b.txt", "prog"}) {
 		t.Errorf("changed = %v, want only what the head adds to the trunk: b.txt, prog", res.Changed)
 	}
-	if !reflect.DeepEqual(res.Programs, []Program{{"prog", "ELF"}}) { // issue 1577
-		t.Errorf("programs = %v, want the ELF the head adds", res.Programs)
+	if !reflect.DeepEqual(res.Programs, []Program{{"prog", "ELF", ""}, {"gone/prog", "ELF", added}}) { // issue 1577
+		t.Errorf("programs = %v, want the ELF the head adds and the one an earlier commit added and a later one removed", res.Programs)
 	}
 	if at := realGit(t, r.ws, "rev-parse", "HEAD"); at != res.Merge {
 		t.Errorf("the workspace is at %s, not at the merge %s", at, res.Merge)

go/buildstep/programs.go

@@ -13,6 +13,9 @@ import (
 type Program struct {
 	Path string `json:"path"`
 	Kind string `json:"kind"` // ELF, Mach-O or PE
+	// Commit is set for a program a commit of the branch added and a later one removed (or replaced): the merge's tree is clean,
+	// but the merge commit carries the blob into the trunk's history for good. Empty for a program the merge's tree holds.
+	Commit string `json:"commit,omitempty"`
 }
 
 // ExecutableKind says what executable format the first bytes of a file are, or "" when they are none. A PE needs its header
@@ -40,6 +43,46 @@ func ExecutableKind(head []byte) string {
 // programHeadSize is how many bytes of a file are read to tell its format: enough for a PE header at its usual offsets.
 const programHeadSize = 0x100
 
+// historyPrograms names the compiled programs that a commit of base..head added or modified and the merge's tree no longer
+// holds at that path (those it holds are in final already): each with the commit that added it. git is the merge's git runner.
+func historyPrograms(git func(args ...string) (string, error), base, head string, final []Program) ([]Program, error) {
+	revs, err := git("rev-list", "--no-merges", "--reverse", base+".."+head)
+	if err != nil {
+		return nil, err
+	}
+	held := map[string]bool{}
+	for _, p := range final {
+		held[p.Path] = true
+	}
+	out, seen := []Program{}, map[string]bool{}
+	for _, c := range strings.Fields(revs) {
+		raw, err := git("diff-tree", "-r", "-z", "--no-commit-id", "--no-renames", "--root", "--diff-filter=AM", c)
+		if err != nil {
+			return nil, err
+		}
+		f := strings.Split(raw, "\x00")
+		for i := 0; i+1 < len(f); i += 2 {
+			meta := strings.Fields(strings.TrimPrefix(f[i], ":"))
+			if len(meta) < 4 || (meta[1] != "100644" && meta[1] != "100755") || held[f[i+1]] || seen[meta[3]] {
+				continue
+			}
+			seen[meta[3]] = true
+			blob, err := git("cat-file", "blob", meta[3])
+			if err != nil {
+				return nil, err
+			}
+			head := []byte(blob)
+			if len(head) > programHeadSize {
+				head = head[:programHeadSize]
+			}
+			if k := ExecutableKind(head); k != "" {
+				out = append(out, Program{Path: f[i+1], Kind: k, Commit: c})
+			}
+		}
+	}
+	return out, nil
+}
+
 // programsIn reads the output of `git diff --raw -z --no-renames --diff-filter=AM` (":<old mode> <new mode> <old sha> <new
 // sha> <status>\0<path>\0" per file) and names each added or modified regular file of the tree dir that is a compiled
 // program, from its first bytes read locally. A symlink (mode 120000) and a submodule (160000) are not read. It never

go/buildstep/programs_test.go

@@ -60,8 +60,8 @@ func TestProgramsInNamesTheAddedAndModifiedRegularFilesThatArePrograms(t *testin
 		want      []Program
 	}{
 		{"none", "", []Program{}},
-		{"a program and a text", line("100755", "m/prog") + line("100644", "m/notes"), []Program{{"m/prog", "ELF"}}},
-		{"two programs", line("100644", "m/prog") + line("100755", "m/mac"), []Program{{"m/prog", "ELF"}, {"m/mac", "Mach-O"}}},
+		{"a program and a text", line("100755", "m/prog") + line("100644", "m/notes"), []Program{{"m/prog", "ELF", ""}}},
+		{"two programs", line("100644", "m/prog") + line("100755", "m/mac"), []Program{{"m/prog", "ELF", ""}, {"m/mac", "Mach-O", ""}}},
 		{"a symlink and a submodule are not read", line("120000", "m/link") + line("160000", "m/sub"), []Program{}},
 		{"a path the tree lacks", line("100644", "m/gone"), []Program{}},
 	} {