Skip to content

Commit 646928f

Browse files
committed
pkg/aflow/flow/patching: harden against empty generated patches
Make codeeditor error on nop changes that don't actually change the code. Make patch testing error on empty patch. Perhaps we need a notion of "mandatory" tools that must be called successfully at least once... not sure yet.
1 parent e7652f8 commit 646928f

File tree

4 files changed

+39
-6
lines changed

4 files changed

+39
-6
lines changed

pkg/aflow/action/crash/test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ func testPatch(ctx *aflow.Context, args testArgs) (testResult, error) {
4747
if err != nil {
4848
return res, err
4949
}
50+
if diff == "" {
51+
res.TestError = "No patch to test."
52+
return res, nil
53+
}
5054
res.PatchDiff = diff
5155

5256
if err := kernel.BuildKernel(args.KernelScratchSrc, args.KernelScratchSrc, args.KernelConfig, false); err != nil {

pkg/aflow/flow/patching/patching.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -129,15 +129,18 @@ The explanation of the root cause of the bug is:
129129
130130
{{.BugExplanation}}
131131
132+
{{/* A TestError without PatchDiff means the previous invocation did not generate any patch. */}}
132133
{{if .TestError}}
133-
Another developer tried to fix this bug, and come up with the following patch:
134134
135-
{{.PatchDiff}}
136-
137-
and the following explanation:
135+
Another developer tried to fix this bug, and come up with the following strategy for fixing:
138136
139137
{{.PatchExplanation}}
140138
139+
{{if .PatchDiff}}
140+
and the following patch:
141+
142+
{{.PatchDiff}}
143+
141144
However, the patch testing failed with the following error:
142145
143146
{{.TestError}}
@@ -149,6 +152,9 @@ then create a new patch from scratch.
149152
Note: in both cases the source tree does not contain the patch yet
150153
(so if you want to create a new fixed patch, you need to recreate it
151154
in its entirety from scratch using the codeeditor tool).
155+
{{else}}
156+
If the strategy looks reasonable to you, proceed with patch generation.
157+
{{end}}
152158
{{end}}
153159
`
154160

pkg/aflow/tool/codeeditor/codeeditor.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,11 @@ func codeeditor(ctx *aflow.Context, state state, args args) (struct{}, error) {
7979
return struct{}{}, aflow.BadCallError("CurrentCode snippet matched %v places,"+
8080
" increase context in CurrentCode to avoid ambiguity", matches)
8181
}
82-
err = osutil.WriteFile(file, slices.Concat(newLines...))
82+
newFileData := slices.Concat(newLines...)
83+
if bytes.Equal(fileData, newFileData) {
84+
return struct{}{}, aflow.BadCallError("The edit does not change the code.")
85+
}
86+
err = osutil.WriteFile(file, newFileData)
8387
return struct{}{}, err
8488
}
8589

@@ -97,7 +101,7 @@ func replace(lines, src, dst [][]byte, fuzzy bool) (newLines [][]byte, matches i
97101
si++
98102
continue
99103
}
100-
if len(l) == 0 {
104+
if len(l) == 0 && li != i {
101105
li++
102106
continue
103107
}

pkg/aflow/tool/codeeditor/codeeditor_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,25 @@ foo`)
105105
)
106106
}
107107

108+
func TestCodeeditorNopEdit(t *testing.T) {
109+
dir := writeTestFile(t, "src.c", `
110+
line0
111+
line1
112+
`)
113+
aflow.TestTool(t, Tool,
114+
state{
115+
KernelScratchSrc: dir,
116+
},
117+
args{
118+
SourceFile: "src.c",
119+
CurrentCode: " line0",
120+
NewCode: "line0",
121+
},
122+
struct{}{},
123+
`The edit does not change the code.`,
124+
)
125+
}
126+
108127
func TestCodeeditorReplacement(t *testing.T) {
109128
type Test struct {
110129
curFile string

0 commit comments

Comments
 (0)