Skip to content

Commit 71ad32b

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 65e1023 commit 71ad32b

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
@@ -131,14 +131,17 @@ The explanation of the root cause of the bug is:
131131
{{.BugExplanation}}
132132
133133
{{if .TestError}}
134-
Another developer tried to fix this bug, and come up with the following patch:
135134
136-
{{.PatchDiff}}
137-
138-
and the following explanation:
135+
Another developer tried to fix this bug, and come up with the following strategy for fixing:
139136
140137
{{.PatchExplanation}}
141138
139+
{{/* A TestError without PatchDiff means the previous invocation did not generate any patch. */}}
140+
{{if .PatchDiff}}
141+
and the following patch:
142+
143+
{{.PatchDiff}}
144+
142145
However, the patch testing failed with the following error:
143146
144147
{{.TestError}}
@@ -150,6 +153,9 @@ then create a new patch from scratch.
150153
Note: in both cases the source tree does not contain the patch yet
151154
(so if you want to create a new fixed patch, you need to recreate it
152155
in its entirety from scratch using the codeeditor tool).
156+
{{else}}
157+
If the strategy looks reasonable to you, proceed with patch generation.
158+
{{end}}
153159
{{end}}
154160
`
155161

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)