Merge pull request 'Apply a patch in the forge tool itself when the forge's diffpatch answers 500, so a patch whose context holds a non-ASCII character is taken (issue 1604)' (#407) from fix/1604-patch-applied into main
This commit was merged in pull request #407.
This commit is contained in:
@@ -0,0 +1,432 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"encoding/base64"
|
||||
"fmt"
|
||||
"regexp"
|
||||
"sort"
|
||||
"strconv"
|
||||
"strings"
|
||||
"unicode/utf8"
|
||||
)
|
||||
|
||||
// A unified diff applied by the tool itself (novox/hq issue 1604). The forge's diffpatch answers 500 with an empty
|
||||
// message when an unchanged context line holds a non-ASCII character, so a patch the forge did not take is applied
|
||||
// here: each touched file is read at the commit the commit goes on, its hunks are applied, and the resulting
|
||||
// files are committed through the same call a files commit uses. The forge's files call carries no file mode, so
|
||||
// a patch that creates an executable file, changes a mode or touches a symlink is not applied here: the tool says so.
|
||||
|
||||
// fileSource reads a file at the commit the patch is applied to; ok is false when there is no such file.
|
||||
type fileSource func(path string) (content []byte, mode string, ok bool, err error)
|
||||
|
||||
// patchFile is one file section of a patch.
|
||||
type patchFile struct {
|
||||
oldPath, newPath string // "" when /dev/null
|
||||
create, remove bool
|
||||
renamed bool
|
||||
oldSeen bool
|
||||
hunks []hunk
|
||||
modeBlock string // why the files call cannot carry this section, "" when it can
|
||||
}
|
||||
|
||||
type hunk struct {
|
||||
oldStart int
|
||||
old, new []string
|
||||
oldNoNL, newNoNL bool
|
||||
leading int // unchanged lines before the first change
|
||||
trailing int // unchanged lines after the last change
|
||||
}
|
||||
|
||||
var hunkHeader = regexp.MustCompile(`^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@`)
|
||||
|
||||
// gitPath reads a path of a diff header: git quotes one holding unusual bytes in C style.
|
||||
func gitPath(s string) string {
|
||||
s = strings.TrimRight(s, "\t ")
|
||||
if strings.HasPrefix(s, `"`) {
|
||||
if u, err := strconv.Unquote(s); err == nil {
|
||||
return u
|
||||
}
|
||||
} else if i := strings.IndexByte(s, '\t'); i >= 0 {
|
||||
s = s[:i]
|
||||
}
|
||||
return s
|
||||
}
|
||||
|
||||
func stripPrefix(p string) string {
|
||||
if p == "/dev/null" {
|
||||
return ""
|
||||
}
|
||||
if i := strings.IndexByte(p, '/'); i >= 0 && (p[:i] == "a" || p[:i] == "b") {
|
||||
return p[i+1:]
|
||||
}
|
||||
return p
|
||||
}
|
||||
|
||||
func nonRegular(mode string) bool { return mode != "" && mode != "100644" }
|
||||
|
||||
// parsePatch reads a unified diff into its file sections.
|
||||
func parsePatch(text string) ([]*patchFile, error) {
|
||||
lines := strings.Split(text, "\n")
|
||||
if n := len(lines); n > 0 && lines[n-1] == "" {
|
||||
lines = lines[:n-1]
|
||||
}
|
||||
var files []*patchFile
|
||||
var cur *patchFile
|
||||
block := func(f *patchFile, what string) {
|
||||
if f.modeBlock == "" {
|
||||
f.modeBlock = what
|
||||
}
|
||||
}
|
||||
for i := 0; i < len(lines); i++ {
|
||||
l := strings.TrimSuffix(lines[i], "\r")
|
||||
switch {
|
||||
case strings.HasPrefix(l, "diff --git "):
|
||||
cur = &patchFile{}
|
||||
files = append(files, cur)
|
||||
rest := strings.TrimPrefix(l, "diff --git ")
|
||||
if k := strings.Index(rest, " b/"); k > 0 && !strings.HasPrefix(rest, `"`) {
|
||||
cur.oldPath, cur.newPath = stripPrefix(rest[:k]), stripPrefix(rest[k+1:])
|
||||
}
|
||||
case cur != nil && strings.HasPrefix(l, "new file mode "):
|
||||
cur.create = true
|
||||
if m := strings.TrimPrefix(l, "new file mode "); nonRegular(m) {
|
||||
block(cur, "creates a file of mode "+m)
|
||||
}
|
||||
case cur != nil && strings.HasPrefix(l, "deleted file mode "):
|
||||
cur.remove = true
|
||||
case cur != nil && strings.HasPrefix(l, "old mode "):
|
||||
block(cur, "changes the file mode ("+l+")")
|
||||
case cur != nil && strings.HasPrefix(l, "new mode "):
|
||||
block(cur, "changes the file mode ("+l+")")
|
||||
case cur != nil && strings.HasPrefix(l, "index "):
|
||||
if f := strings.Fields(l); len(f) == 3 && nonRegular(f[2]) {
|
||||
block(cur, "touches a file of mode "+f[2])
|
||||
}
|
||||
case cur != nil && strings.HasPrefix(l, "rename from "):
|
||||
cur.oldPath, cur.renamed = gitPath(strings.TrimPrefix(l, "rename from ")), true
|
||||
case cur != nil && strings.HasPrefix(l, "rename to "):
|
||||
cur.newPath, cur.renamed = gitPath(strings.TrimPrefix(l, "rename to ")), true
|
||||
case strings.HasPrefix(l, "copy from "), strings.HasPrefix(l, "copy to "):
|
||||
return nil, fmt.Errorf("a copy in a patch (%s) is not applied by the tool", l)
|
||||
case strings.HasPrefix(l, "GIT binary patch"), strings.HasPrefix(l, "Binary files "):
|
||||
return nil, fmt.Errorf("a binary patch is not applied by the tool")
|
||||
case strings.HasPrefix(l, "--- "):
|
||||
if cur == nil || cur.oldSeen {
|
||||
cur = &patchFile{}
|
||||
files = append(files, cur)
|
||||
}
|
||||
p := stripPrefix(gitPath(strings.TrimPrefix(l, "--- ")))
|
||||
cur.oldPath = p
|
||||
if p == "" {
|
||||
cur.create = true
|
||||
}
|
||||
cur.oldSeen = true
|
||||
if i+1 >= len(lines) || !strings.HasPrefix(lines[i+1], "+++ ") {
|
||||
return nil, fmt.Errorf("the line %d, %q, is not followed by a +++ line", i+1, l)
|
||||
}
|
||||
i++
|
||||
q := stripPrefix(gitPath(strings.TrimPrefix(strings.TrimSuffix(lines[i], "\r"), "+++ ")))
|
||||
cur.newPath = q
|
||||
if q == "" {
|
||||
cur.remove = true
|
||||
}
|
||||
case cur != nil && hunkHeader.MatchString(l):
|
||||
m := hunkHeader.FindStringSubmatch(l)
|
||||
atoi := func(s string, def int) int {
|
||||
if s == "" {
|
||||
return def
|
||||
}
|
||||
n, _ := strconv.Atoi(s)
|
||||
return n
|
||||
}
|
||||
h := hunk{oldStart: atoi(m[1], 0)}
|
||||
oldN, newN := atoi(m[2], 1), atoi(m[4], 1)
|
||||
startLine := i + 1
|
||||
noNL := func() { // "\ No newline at end of file" is about the line before it
|
||||
prev := lines[i-1]
|
||||
switch {
|
||||
case prev != "" && prev[0] == '-':
|
||||
h.oldNoNL = true
|
||||
case prev != "" && prev[0] == '+':
|
||||
h.newNoNL = true
|
||||
default:
|
||||
h.oldNoNL, h.newNoNL = true, true
|
||||
}
|
||||
}
|
||||
changed := false
|
||||
for oldN > 0 || newN > 0 {
|
||||
i++
|
||||
if i >= len(lines) {
|
||||
return nil, fmt.Errorf("the hunk of %s at patch line %d ends before its %d old and %d new lines are all there", cur.name(), startLine, oldN, newN)
|
||||
}
|
||||
b := strings.TrimSuffix(lines[i], "\r")
|
||||
if b == "" {
|
||||
b = " " // a blank context line some tools strip to nothing, or a bare CR
|
||||
}
|
||||
switch b[0] {
|
||||
case '\\':
|
||||
noNL()
|
||||
case ' ':
|
||||
text := lines[i]
|
||||
if strings.HasPrefix(text, " ") {
|
||||
text = text[1:] // a bare "" or "\r" is the whole blank line, its leading space stripped
|
||||
}
|
||||
h.old, h.new = append(h.old, text), append(h.new, text)
|
||||
oldN--
|
||||
newN--
|
||||
if changed {
|
||||
h.trailing++
|
||||
} else {
|
||||
h.leading++
|
||||
}
|
||||
case '-':
|
||||
h.old = append(h.old, lines[i][1:])
|
||||
oldN--
|
||||
changed, h.trailing = true, 0
|
||||
case '+':
|
||||
h.new = append(h.new, lines[i][1:])
|
||||
newN--
|
||||
changed, h.trailing = true, 0
|
||||
default:
|
||||
return nil, fmt.Errorf("the hunk of %s at patch line %d has a line %d that is neither context, - nor +: %q", cur.name(), startLine, i+1, lines[i])
|
||||
}
|
||||
}
|
||||
for i+1 < len(lines) && strings.HasPrefix(lines[i+1], `\`) {
|
||||
i++
|
||||
noNL()
|
||||
}
|
||||
cur.hunks = append(cur.hunks, h)
|
||||
}
|
||||
}
|
||||
for _, f := range files {
|
||||
if f.newPath == "" && f.oldPath == "" {
|
||||
return nil, fmt.Errorf("a file section of the patch names no file")
|
||||
}
|
||||
}
|
||||
if len(files) == 0 {
|
||||
return nil, fmt.Errorf("the patch holds no file section")
|
||||
}
|
||||
return files, nil
|
||||
}
|
||||
|
||||
func (f *patchFile) name() string {
|
||||
if f.newPath != "" {
|
||||
return f.newPath
|
||||
}
|
||||
return f.oldPath
|
||||
}
|
||||
|
||||
// textFile is a file's content as lines, and whether its last line ends with a newline.
|
||||
type textFile struct {
|
||||
lines []string
|
||||
endNL bool
|
||||
}
|
||||
|
||||
func splitText(b []byte) textFile {
|
||||
if len(b) == 0 {
|
||||
return textFile{endNL: true}
|
||||
}
|
||||
s := string(b)
|
||||
endNL := strings.HasSuffix(s, "\n")
|
||||
s = strings.TrimSuffix(s, "\n")
|
||||
return textFile{lines: strings.Split(s, "\n"), endNL: endNL}
|
||||
}
|
||||
|
||||
func (t textFile) bytes() []byte {
|
||||
if len(t.lines) == 0 {
|
||||
return nil
|
||||
}
|
||||
s := strings.Join(t.lines, "\n")
|
||||
if t.endNL {
|
||||
s += "\n"
|
||||
}
|
||||
return []byte(s)
|
||||
}
|
||||
|
||||
// apply puts one hunk on the file, at where the hunk says or, as git apply does, at the nearest place its old
|
||||
// lines are found exactly; delta is how far earlier hunks of this file moved lines.
|
||||
func (t textFile) apply(h hunk, delta int) (textFile, int, error) {
|
||||
want := h.oldStart - 1 + delta
|
||||
if len(h.old) == 0 && h.oldStart > 0 {
|
||||
want = h.oldStart + delta // a pure insertion is "after" the line it names
|
||||
}
|
||||
if want < 0 {
|
||||
want = 0
|
||||
}
|
||||
matches := func(at int) bool {
|
||||
if at < 0 || at+len(h.old) > len(t.lines) {
|
||||
return false
|
||||
}
|
||||
for k, l := range h.old {
|
||||
if t.lines[at+k] != l {
|
||||
return false
|
||||
}
|
||||
}
|
||||
// the old side's last line lacks a newline only when the file's does
|
||||
if at+len(h.old) == len(t.lines) && len(h.old) > 0 && h.oldNoNL == t.endNL {
|
||||
return false
|
||||
}
|
||||
return true
|
||||
}
|
||||
// As git apply does: a hunk that starts at line 1 (or 0) begins the file, and a hunk with context but none
|
||||
// after its last change ends it; a hunk whose old side lacks the final newline ends it too.
|
||||
atStart := h.oldStart <= 1
|
||||
atEnd := (h.trailing == 0 && h.leading > 0) || h.oldNoNL
|
||||
at := -1
|
||||
switch {
|
||||
case atStart && atEnd:
|
||||
if len(t.lines) == len(h.old) && matches(0) {
|
||||
at = 0
|
||||
}
|
||||
case atStart:
|
||||
if matches(0) {
|
||||
at = 0
|
||||
}
|
||||
case atEnd:
|
||||
if e := len(t.lines) - len(h.old); matches(e) {
|
||||
at = e
|
||||
}
|
||||
}
|
||||
for d := 0; !atStart && !atEnd && d <= len(t.lines)+1 && at < 0; d++ {
|
||||
switch {
|
||||
case matches(want - d):
|
||||
at = want - d
|
||||
case matches(want + d):
|
||||
at = want + d
|
||||
}
|
||||
}
|
||||
if at < 0 {
|
||||
return t, delta, fmt.Errorf("the hunk at line %d does not apply: its context and removed lines are not in the file (first of them: %q)", h.oldStart, firstOf(h.old))
|
||||
}
|
||||
out := append(append(append([]string{}, t.lines[:at]...), h.new...), t.lines[at+len(h.old):]...)
|
||||
endNL := t.endNL
|
||||
if at+len(h.old) == len(t.lines) && (len(h.new) > 0 || len(h.old) > 0) {
|
||||
endNL = !h.newNoNL
|
||||
}
|
||||
return textFile{lines: out, endNL: endNL}, delta + len(h.new) - len(h.old), nil
|
||||
}
|
||||
|
||||
func firstOf(l []string) string {
|
||||
if len(l) == 0 {
|
||||
return ""
|
||||
}
|
||||
return l[0]
|
||||
}
|
||||
|
||||
// patchAsFiles applies the patch to the files read through read, and says the files to commit.
|
||||
func patchAsFiles(text string, read fileSource) ([]FileChange, error) {
|
||||
sections, err := parsePatch(text)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
for _, s := range sections {
|
||||
if s.modeBlock != "" {
|
||||
return nil, fmt.Errorf("%s %s, and the forge's files call carries no file mode", s.name(), s.modeBlock)
|
||||
}
|
||||
}
|
||||
work := map[string][]byte{} // the files as they are being changed; nil means absent
|
||||
orig := map[string]bool{} // which paths existed
|
||||
load := func(p string) error {
|
||||
if _, seen := work[p]; seen {
|
||||
return nil
|
||||
}
|
||||
b, mode, ok, err := read(p)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if ok && mode != "" && mode != "100644" {
|
||||
return fmt.Errorf("%s is a file of mode %s, and the forge's files call carries no file mode, so committing it that way would make it 100644", p, mode)
|
||||
}
|
||||
orig[p] = ok
|
||||
if ok {
|
||||
if b == nil {
|
||||
b = []byte{}
|
||||
}
|
||||
work[p] = b
|
||||
} else {
|
||||
work[p] = nil
|
||||
}
|
||||
return nil
|
||||
}
|
||||
for _, s := range sections {
|
||||
from, to := s.oldPath, s.newPath
|
||||
if s.create {
|
||||
from = ""
|
||||
}
|
||||
var t textFile
|
||||
if from != "" {
|
||||
if err := load(from); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if work[from] == nil {
|
||||
return nil, fmt.Errorf("%s: the file is not there at the commit the patch goes on, so the patch does not apply", from)
|
||||
}
|
||||
if !utf8.Valid(work[from]) && len(s.hunks) > 0 {
|
||||
return nil, fmt.Errorf("%s is not UTF-8 text: the tool applies text patches only", from)
|
||||
}
|
||||
t = splitText(work[from])
|
||||
} else {
|
||||
if err := load(to); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if work[to] != nil {
|
||||
return nil, fmt.Errorf("%s: the patch creates it, but it is already there at the commit the patch goes on", to)
|
||||
}
|
||||
t = textFile{endNL: true}
|
||||
}
|
||||
delta := 0
|
||||
for _, h := range s.hunks {
|
||||
if t, delta, err = t.apply(h, delta); err != nil {
|
||||
return nil, fmt.Errorf("%s: %w", s.name(), err)
|
||||
}
|
||||
}
|
||||
switch {
|
||||
case s.remove:
|
||||
if len(t.lines) != 0 {
|
||||
return nil, fmt.Errorf("%s: the patch deletes the file, but lines are left after its hunks", from)
|
||||
}
|
||||
work[from] = nil
|
||||
default:
|
||||
if s.renamed && from != to {
|
||||
work[from] = nil
|
||||
if err := load(to); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
body := t.bytes()
|
||||
if body == nil {
|
||||
body = []byte{}
|
||||
}
|
||||
work[to] = body
|
||||
}
|
||||
}
|
||||
var paths []string
|
||||
for p := range work {
|
||||
paths = append(paths, p)
|
||||
}
|
||||
sort.Strings(paths)
|
||||
var out []FileChange
|
||||
for _, p := range paths {
|
||||
switch {
|
||||
case work[p] == nil && !orig[p]:
|
||||
case work[p] == nil:
|
||||
out = append(out, FileChange{Path: p, Delete: true})
|
||||
default:
|
||||
fc := FileChange{Path: p, size: len(work[p])}
|
||||
if utf8.Valid(work[p]) {
|
||||
s := string(work[p])
|
||||
fc.Content = &s
|
||||
} else {
|
||||
s := base64Std(work[p])
|
||||
fc.ContentBase64 = &s
|
||||
}
|
||||
out = append(out, fc)
|
||||
}
|
||||
}
|
||||
if len(out) == 0 {
|
||||
return nil, fmt.Errorf("the patch changes no file")
|
||||
}
|
||||
return out, nil
|
||||
}
|
||||
|
||||
func base64Std(b []byte) string { return base64.StdEncoding.EncodeToString(b) }
|
||||
@@ -0,0 +1,189 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"context"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// novox/hq issue 1604: the forge's diffpatch answers 500 when an unchanged context line holds a non-ASCII
|
||||
// character; the tool then applies the patch itself.
|
||||
|
||||
func sectionFake(t *testing.T) *fakeForge {
|
||||
f := newFake(t)
|
||||
f.trees["c0"]["doc.md"] = "# Title\n\nSee § 4 for the rule.\nold line\nend § here\n"
|
||||
f.trees["c0"]["run.sh"] = "#!/bin/sh\necho a\n"
|
||||
f.patchStatus = 500
|
||||
return f
|
||||
}
|
||||
|
||||
const sectionPatch = "diff --git a/doc.md b/doc.md\n--- a/doc.md\n+++ b/doc.md\n@@ -2,4 +2,4 @@\n \n See § 4 for the rule.\n-old line\n+new line\n end § here\n"
|
||||
|
||||
func TestAPatchWhoseContextHoldsANonASCIICharacterIsAppliedByTheToolWhenTheForgeAnswers500(t *testing.T) {
|
||||
f := sectionFake(t)
|
||||
got, err := commit(context.Background(), f.serve(), ask(t, map[string]any{"branch": "fix/1604", "patch": sectionPatch}))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
want := "# Title\n\nSee § 4 for the rule.\nnew line\nend § here\n"
|
||||
if tree := f.trees[got.Commit]; tree["doc.md"] != want || tree["README.md"] != "hello\n" {
|
||||
t.Fatalf("tree %v", tree)
|
||||
}
|
||||
if len(f.patches) != 1 || len(f.writes) != 1 || f.writes[0]["new_branch"] != "fix/1604" || f.writes[0]["branch"] != "main" {
|
||||
t.Fatalf("patches %v writes %v", f.patches, f.writes)
|
||||
}
|
||||
if len(got.Files) != 1 || got.Files[0].Operation != "update" || got.Parent != "c0" {
|
||||
t.Fatalf("%+v", got)
|
||||
}
|
||||
if !strings.Contains(strings.Join(got.Said, " "), "applied it") {
|
||||
t.Errorf("not said: %v", got.Said)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAPatchTheForgeTakesIsStillTheForgesToApply(t *testing.T) {
|
||||
f := sectionFake(t)
|
||||
f.patchStatus = 0
|
||||
if _, err := commit(context.Background(), f.serve(), ask(t, map[string]any{"branch": "fix/1604", "patch": sectionPatch})); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(f.writes) != 0 {
|
||||
t.Fatal("the tool applied a patch the forge took")
|
||||
}
|
||||
}
|
||||
|
||||
func TestAPatchThatCreatesDeletesAndRenamesIsApplied(t *testing.T) {
|
||||
f := sectionFake(t)
|
||||
patch := "diff --git a/new.txt b/new.txt\nnew file mode 100644\n--- /dev/null\n+++ b/new.txt\n@@ -0,0 +1,2 @@\n+one §\n+two\n" +
|
||||
"diff --git a/docs/a.md b/docs/a.md\ndeleted file mode 100644\n--- a/docs/a.md\n+++ /dev/null\n@@ -1 +0,0 @@\n-a\n" +
|
||||
"diff --git a/README.md b/README.txt\nsimilarity index 80%\nrename from README.md\nrename to README.txt\n--- a/README.md\n+++ b/README.txt\n@@ -1 +1 @@\n-hello\n+hello §\n"
|
||||
got, err := commit(context.Background(), f.serve(), ask(t, map[string]any{"branch": "fix/r", "patch": patch}))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
tree := f.trees[got.Commit]
|
||||
if tree["new.txt"] != "one §\ntwo\n" || tree["README.txt"] != "hello §\n" {
|
||||
t.Fatalf("tree %v", tree)
|
||||
}
|
||||
if _, has := tree["README.md"]; has {
|
||||
t.Error("the renamed file is still there")
|
||||
}
|
||||
if _, has := tree["docs/a.md"]; has {
|
||||
t.Error("the deleted file is still there")
|
||||
}
|
||||
}
|
||||
|
||||
func TestAHunkMovedByEarlierLinesIsFoundAndNoNewlineIsKept(t *testing.T) {
|
||||
f := sectionFake(t)
|
||||
f.trees["c0"]["x.txt"] = "pre\npre\nabc §\nd\ne"
|
||||
// the hunk below starts at line 2, though its lines are at 3: the tool finds them one line on
|
||||
patch := "--- a/x.txt\n+++ b/x.txt\n@@ -2,3 +2,3 @@\n abc §\n d\n-e\n\\ No newline at end of file\n+f\n\\ No newline at end of file\n"
|
||||
got, err := commit(context.Background(), f.serve(), ask(t, map[string]any{"branch": "fix/n", "patch": patch}))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if g := f.trees[got.Commit]["x.txt"]; g != "pre\npre\nabc §\nd\nf" {
|
||||
t.Fatalf("%q", g)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAPatchThatDoesNotApplyIsRefusedNamingTheFileAndLine(t *testing.T) {
|
||||
f := sectionFake(t)
|
||||
patch := strings.Replace(sectionPatch, "-old line", "-other line", 1)
|
||||
_, err := commit(context.Background(), f.serve(), ask(t, map[string]any{"branch": "fix/1604", "patch": patch}))
|
||||
if err == nil || !strings.Contains(err.Error(), "doc.md") || !strings.Contains(err.Error(), "at line 2 does not apply") {
|
||||
t.Fatalf("%v", err)
|
||||
}
|
||||
if len(f.writes) != 0 {
|
||||
t.Fatal("something was written")
|
||||
}
|
||||
}
|
||||
|
||||
func TestAPatchWithAnExecutableFileIsNotAppliedByTheToolAndSaysWhy(t *testing.T) {
|
||||
f := sectionFake(t)
|
||||
for _, patch := range []string{
|
||||
"diff --git a/s.sh b/s.sh\nnew file mode 100755\n--- /dev/null\n+++ b/s.sh\n@@ -0,0 +1 @@\n+echo §\n",
|
||||
"diff --git a/run.sh b/run.sh\nold mode 100644\nnew mode 100755\n",
|
||||
"diff --git a/run.sh b/run.sh\nindex 1111111..2222222 100755\n--- a/run.sh\n+++ b/run.sh\n@@ -1,2 +1,2 @@\n #!/bin/sh\n-echo a\n+echo b\n",
|
||||
} {
|
||||
_, err := commit(context.Background(), f.serve(), ask(t, map[string]any{"branch": "fix/x", "patch": patch}))
|
||||
if err == nil || !strings.Contains(err.Error(), "the forge did not take the patch") || !strings.Contains(err.Error(), "carries no file mode") {
|
||||
t.Errorf("%v", err)
|
||||
}
|
||||
}
|
||||
if len(f.writes) != 0 {
|
||||
t.Fatal("something was written")
|
||||
}
|
||||
}
|
||||
|
||||
func TestAClientErrorOfTheForgeIsNotRetriedByTheTool(t *testing.T) {
|
||||
f := sectionFake(t)
|
||||
f.patchStatus = 422
|
||||
_, err := commit(context.Background(), f.serve(), ask(t, map[string]any{"branch": "fix/1604", "patch": sectionPatch}))
|
||||
if err == nil || len(f.writes) != 0 {
|
||||
t.Fatalf("%v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestADefaultBranchIsStillRefusedForAPatch(t *testing.T) {
|
||||
f := sectionFake(t)
|
||||
_, err := commit(context.Background(), f.serve(), ask(t, map[string]any{"branch": "main", "patch": sectionPatch}))
|
||||
if err == nil || !strings.Contains(err.Error(), "ADR 0266") || len(f.patches)+len(f.writes) != 0 {
|
||||
t.Fatalf("%v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// Review round 1.
|
||||
|
||||
func appliedTo(t *testing.T, content, patch string) (string, error) {
|
||||
t.Helper()
|
||||
f := sectionFake(t)
|
||||
f.trees["c0"]["y.txt"] = content
|
||||
got, err := commit(context.Background(), f.serve(), ask(t, map[string]any{"branch": "fix/y", "patch": patch}))
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
return f.trees[got.Commit]["y.txt"], nil
|
||||
}
|
||||
|
||||
func TestAHunkAtLineOneMustMatchAtTheStartOfTheFile(t *testing.T) {
|
||||
patch := "--- a/y.txt\n+++ b/y.txt\n@@ -1,3 +1,3 @@\n-a\n+A\n b\n c\n"
|
||||
if got, err := appliedTo(t, "X\nb\nc\nzzz\na\nb\nc\n", patch); err == nil {
|
||||
t.Fatalf("applied in the middle: %q", got)
|
||||
}
|
||||
if got, err := appliedTo(t, "a\nb\nc\nzzz\n", patch); err != nil || got != "A\nb\nc\nzzz\n" {
|
||||
t.Fatalf("%q %v", got, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAHunkWithNoTrailingContextMustMatchAtTheEndOfTheFile(t *testing.T) {
|
||||
patch := "--- a/y.txt\n+++ b/y.txt\n@@ -1,3 +1,4 @@\n a\n b\n c\n+NEW\n"
|
||||
if got, err := appliedTo(t, "a\nb\nc\nd\ne\n", patch); err == nil {
|
||||
t.Fatalf("added in the middle: %q", got)
|
||||
}
|
||||
patch = "--- a/y.txt\n+++ b/y.txt\n@@ -3,3 +3,4 @@\n a\n b\n c\n+NEW\n"
|
||||
if got, err := appliedTo(t, "x\ny\na\nb\nc\n", patch); err != nil || got != "x\ny\na\nb\nc\nNEW\n" {
|
||||
t.Fatalf("%q %v", got, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAnExistingExecutableFileIsNotSilentlyMadeNonExecutable(t *testing.T) {
|
||||
for _, patch := range []string{
|
||||
"diff --git a/run.sh b/run.sh2\nsimilarity index 100%\nrename from run.sh\nrename to run.sh2\n",
|
||||
"--- a/run.sh\n+++ b/run.sh\n@@ -1,2 +1,2 @@\n #!/bin/sh\n-echo a\n+echo b\n",
|
||||
} {
|
||||
f := sectionFake(t)
|
||||
f.modes = map[string]string{"run.sh": "100755"}
|
||||
_, err := commit(context.Background(), f.serve(), ask(t, map[string]any{"branch": "fix/m", "patch": patch}))
|
||||
if err == nil || !strings.Contains(err.Error(), "100755") || len(f.writes) != 0 {
|
||||
t.Errorf("%v (%d writes)", err, len(f.writes))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestAPatchWithCRLFLinesAndABareCRContextLineDoesNotPanic(t *testing.T) {
|
||||
patch := "--- a/y.txt\r\n+++ b/y.txt\r\n@@ -1,3 +1,3 @@\r\n a\r\n\r\n-c\r\n+C\r\n"
|
||||
got, err := appliedTo(t, "a\r\n\r\nc\r\n", patch)
|
||||
if err != nil || got != "a\r\n\r\nC\r\n" {
|
||||
t.Fatalf("%q %v", got, err)
|
||||
}
|
||||
}
|
||||
@@ -326,14 +326,49 @@ func commit(ctx context.Context, f *Forge, a CommitAsk) (*Committed, error) {
|
||||
var made struct {
|
||||
Commit madeCommit `json:"commit"`
|
||||
}
|
||||
// A patch goes to the forge's diffpatch first. When the forge answers a server error (it does so, with an
|
||||
// empty message, when an unchanged context line holds a non-ASCII character: novox/hq issue 1604) the tool
|
||||
// applies the patch itself and commits the resulting files the way a files commit does.
|
||||
files := a.Files
|
||||
forgeApplied := false
|
||||
if a.Patch != "" {
|
||||
on["content"] = a.Patch
|
||||
if err := f.SendJSON(ctx, http.MethodPost, repo+"/diffpatch", on, &made); err != nil {
|
||||
err := f.SendJSON(ctx, http.MethodPost, repo+"/diffpatch", on, &made)
|
||||
var status *forgehttp.StatusError
|
||||
switch {
|
||||
case err == nil:
|
||||
forgeApplied = true
|
||||
case errors.As(err, &status) && status.Status >= 500:
|
||||
delete(on, "content")
|
||||
read := func(p string) ([]byte, string, bool, error) {
|
||||
b, ok, err := f.contentAt(ctx, repo, p, ref)
|
||||
if err != nil || !ok {
|
||||
return nil, "", false, err
|
||||
}
|
||||
mode, err := f.modeAt(ctx, repo, ref, p)
|
||||
return b, mode, true, err
|
||||
}
|
||||
var perr error
|
||||
if files, perr = patchAsFiles(a.Patch, read); perr != nil {
|
||||
return nil, fmt.Errorf("the forge did not take the patch: %w; the tool could not apply it itself: %v", err, perr)
|
||||
}
|
||||
total := 0
|
||||
for _, fc := range files {
|
||||
if fc.size > maxFileBytes {
|
||||
return nil, refuse("%s would be %s, more than the %s one file of a commit may be. Nothing was sent", fc.Path, mib(fc.size), mib(maxFileBytes))
|
||||
}
|
||||
total += fc.size
|
||||
}
|
||||
if total > maxCommitBytes {
|
||||
return nil, refuse("the patch's files are %s together, more than the %s one commit carries. Nothing was sent", mib(total), mib(maxCommitBytes))
|
||||
}
|
||||
out.Said = append(out.Said, fmt.Sprintf("the forge answered %d to the patch, so the tool applied it to the files at %s and committed them", status.Status, short(ref)))
|
||||
default:
|
||||
return nil, fmt.Errorf("the forge did not take the patch: %w", err)
|
||||
}
|
||||
} else {
|
||||
files := a.Files
|
||||
for _, fc := range a.Files {
|
||||
}
|
||||
if !forgeApplied {
|
||||
for _, fc := range files {
|
||||
if fc.CopyFrom != nil {
|
||||
// Read, and judged against the limits, before anything is sent.
|
||||
var err error
|
||||
@@ -391,7 +426,7 @@ func commit(ctx context.Context, f *Forge, a CommitAsk) (*Committed, error) {
|
||||
moved = "base " + out.Base
|
||||
}
|
||||
how := "each file it changes was checked against " + short(read) + " by its blob, so none of them changed in between"
|
||||
if a.Patch != "" {
|
||||
if forgeApplied {
|
||||
how = "the forge applied the patch there, and it refuses a patch whose context changed"
|
||||
}
|
||||
out.Said = append(out.Said, fmt.Sprintf("the %s moved from %s to %s between the read and the commit, so the "+
|
||||
@@ -478,3 +513,82 @@ func short(sha string) string {
|
||||
}
|
||||
return sha
|
||||
}
|
||||
|
||||
// contentAt is the content of a file at a ref; ok is false when there is no file there.
|
||||
func (f *Forge) contentAt(ctx context.Context, repo, path, ref string) ([]byte, bool, error) {
|
||||
seg, err := segments("file path", path)
|
||||
if err != nil {
|
||||
return nil, false, err
|
||||
}
|
||||
body, err := f.Do(ctx, http.MethodGet, repo+"/contents/"+seg+"?ref="+queryEscape(ref), nil, maxBody)
|
||||
if err != nil {
|
||||
if errors.Is(err, errNotFound) {
|
||||
return nil, false, nil
|
||||
}
|
||||
return nil, false, err
|
||||
}
|
||||
var entry struct {
|
||||
Type string `json:"type"`
|
||||
Size int `json:"size"`
|
||||
Content string `json:"content"`
|
||||
Encoding string `json:"encoding"`
|
||||
}
|
||||
if err := jsonInto(body, &entry); err != nil {
|
||||
return nil, false, fmt.Errorf("the forge's answer about %s is not readable: %w", path, err)
|
||||
}
|
||||
if entry.Type != "file" {
|
||||
return nil, false, refuse("%s is a %s, not a file. Nothing was sent", path, entry.Type)
|
||||
}
|
||||
if entry.Encoding == "base64" && (entry.Content != "" || entry.Size == 0) {
|
||||
raw, err := base64.StdEncoding.DecodeString(strings.ReplaceAll(entry.Content, "\n", ""))
|
||||
if err != nil {
|
||||
return nil, false, fmt.Errorf("the content of %s is not base64: %w", path, err)
|
||||
}
|
||||
return raw, true, nil
|
||||
}
|
||||
raw, err := f.Do(ctx, http.MethodGet, repo+"/raw/"+seg+"?ref="+queryEscape(ref), nil, maxFileBytes+1)
|
||||
if err != nil {
|
||||
return nil, false, err
|
||||
}
|
||||
return raw, true, nil
|
||||
}
|
||||
|
||||
// modeAt is the git mode of a file at a commit, read through the git trees API: the contents API carries none.
|
||||
func (f *Forge) modeAt(ctx context.Context, repo, ref, path string) (string, error) {
|
||||
parts := strings.Split(path, "/")
|
||||
tree := ref
|
||||
for depth, name := range parts {
|
||||
found := false
|
||||
for page := 1; ; page++ {
|
||||
var got struct {
|
||||
Tree []struct {
|
||||
Path string `json:"path"`
|
||||
Mode string `json:"mode"`
|
||||
Type string `json:"type"`
|
||||
SHA string `json:"sha"`
|
||||
} `json:"tree"`
|
||||
Total int `json:"total_count"`
|
||||
}
|
||||
if err := f.GetJSON(ctx, fmt.Sprintf("%s/git/trees/%s?per_page=1000&page=%d", repo, url.PathEscape(tree), page), &got); err != nil {
|
||||
return "", fmt.Errorf("the mode of %s could not be read: %w", path, err)
|
||||
}
|
||||
for _, e := range got.Tree {
|
||||
if e.Path != name {
|
||||
continue
|
||||
}
|
||||
if depth == len(parts)-1 {
|
||||
return e.Mode, nil
|
||||
}
|
||||
tree, found = e.SHA, true
|
||||
break
|
||||
}
|
||||
if found || len(got.Tree) == 0 || page*1000 >= got.Total {
|
||||
break
|
||||
}
|
||||
}
|
||||
if !found && depth < len(parts)-1 {
|
||||
return "", fmt.Errorf("the mode of %s could not be read: %s is not a folder at %s", path, name, short(ref))
|
||||
}
|
||||
}
|
||||
return "", fmt.Errorf("the mode of %s could not be read: not in the tree of %s", path, short(ref))
|
||||
}
|
||||
|
||||
@@ -46,6 +46,8 @@ type fakeForge struct {
|
||||
parentsBroken bool // and GET git/commits fails
|
||||
parents map[string]string // commit -> parent, as GET git/commits says it
|
||||
sources map[string]*sourceRepo
|
||||
modes map[string]string // path -> git mode when not 100644
|
||||
patchStatus int // the status diffpatch answers instead of taking the patch (issue 1604)
|
||||
}
|
||||
|
||||
type move struct {
|
||||
@@ -128,7 +130,48 @@ func (f *fakeForge) handle(w http.ResponseWriter, r *http.Request) {
|
||||
http.NotFound(w, r)
|
||||
return
|
||||
}
|
||||
reply(map[string]any{"type": "file", "path": p, "sha": blob(content)})
|
||||
reply(map[string]any{"type": "file", "path": p, "sha": blob(content), "size": len(content), "encoding": "base64",
|
||||
"content": base64.StdEncoding.EncodeToString([]byte(content))})
|
||||
case r.Method == "GET" && strings.HasPrefix(path, repo+"/git/trees/"):
|
||||
id := strings.TrimPrefix(path, repo+"/git/trees/")
|
||||
commitID, dir := id, ""
|
||||
if strings.HasPrefix(id, "tree:") {
|
||||
parts := strings.SplitN(id, ":", 3)
|
||||
commitID, dir = parts[1], parts[2]
|
||||
}
|
||||
if c, isBranch := f.branches[commitID]; isBranch {
|
||||
commitID = c
|
||||
}
|
||||
var entries []map[string]any
|
||||
seen := map[string]bool{}
|
||||
for p := range f.trees[commitID] {
|
||||
if dir != "" {
|
||||
if !strings.HasPrefix(p, dir+"/") {
|
||||
continue
|
||||
}
|
||||
p = strings.TrimPrefix(p, dir+"/")
|
||||
}
|
||||
name, rest, nested := strings.Cut(p, "/")
|
||||
if seen[name] {
|
||||
continue
|
||||
}
|
||||
seen[name] = true
|
||||
full := name
|
||||
if dir != "" {
|
||||
full = dir + "/" + name
|
||||
}
|
||||
if nested {
|
||||
_ = rest
|
||||
entries = append(entries, map[string]any{"path": name, "mode": "040000", "type": "tree", "sha": "tree:" + commitID + ":" + full})
|
||||
continue
|
||||
}
|
||||
mode := "100644"
|
||||
if m, ok := f.modes[full]; ok {
|
||||
mode = m
|
||||
}
|
||||
entries = append(entries, map[string]any{"path": name, "mode": mode, "type": "blob", "sha": blob(f.trees[commitID][full])})
|
||||
}
|
||||
reply(map[string]any{"tree": entries, "total_count": len(entries)})
|
||||
case r.Method == "POST" && path == repo+"/contents":
|
||||
var body map[string]any
|
||||
_ = json.NewDecoder(r.Body).Decode(&body)
|
||||
@@ -176,6 +219,10 @@ func (f *fakeForge) handle(w http.ResponseWriter, r *http.Request) {
|
||||
var body map[string]any
|
||||
_ = json.NewDecoder(r.Body).Decode(&body)
|
||||
f.patches = append(f.patches, body)
|
||||
if f.patchStatus != 0 {
|
||||
http.Error(w, `{"message":""}`, f.patchStatus)
|
||||
return
|
||||
}
|
||||
f.next++
|
||||
made := fmt.Sprintf("p%d", f.next)
|
||||
from := f.branches[body["branch"].(string)]
|
||||
|
||||
Reference in New Issue
Block a user