diff --git a/go/buildstep/merge.go b/go/buildstep/merge.go index 589c50e..86384b6 100644 --- a/go/buildstep/merge.go +++ b/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 } diff --git a/go/buildstep/merge_integration_test.go b/go/buildstep/merge_integration_test.go index ce8518c..f83ece8 100644 --- a/go/buildstep/merge_integration_test.go +++ b/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) diff --git a/go/buildstep/programs.go b/go/buildstep/programs.go index 8d87dce..36f47e6 100644 --- a/go/buildstep/programs.go +++ b/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` (": \0\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 diff --git a/go/buildstep/programs_test.go b/go/buildstep/programs_test.go index 7cc26bc..e64dae2 100644 --- a/go/buildstep/programs_test.go +++ b/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{}}, } {