This is an automated email from the ASF dual-hosted git repository. sjanc pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mynewt-newt.git
commit d4d477cf500c9ee12a711996d90597a681fd34a2 Author: Szymon Janc <[email protected]> AuthorDate: Mon Oct 19 17:57:07 2026 +0200 newt/toolchain: Fix dependency files of sources with spaces in names gcc escapes spaces in .d files ("my\ file.o: my\ file.c"), but newt split dependency lines on white space without undoing the escaping, and appended the object file name to the .d file unescaped. A source file with a space in its name failed every build after the first one with "line missing ':'". Parse the Makefile escaping ("\ ", "\#", "$$") and escape the names newt writes itself. If a dependency file still cannot be parsed (e.g. one written by an older newt), remove it and rebuild the source instead of failing the build. Assisted-by: Claude:claude-opus-5-5 --- newt/toolchain/compiler.go | 9 +++- newt/toolchain/deps.go | 66 +++++++++++++++++++++-- newt/toolchain/deps_test.go | 124 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 194 insertions(+), 5 deletions(-) diff --git a/newt/toolchain/compiler.go b/newt/toolchain/compiler.go index b8bf733d..43b8e613 100644 --- a/newt/toolchain/compiler.go +++ b/newt/toolchain/compiler.go @@ -572,7 +572,11 @@ func (c *Compiler) lflagsStrings() []string { } func (c *Compiler) depsString() string { - extraDeps := util.SortFields(c.extraDeps...) + extraDeps := util.UniqueStrings(c.extraDeps) + sort.Strings(extraDeps) + for i, d := range extraDeps { + extraDeps[i] = escapeDepsPath(d) + } return strings.Join(extraDeps, " ") + "\n" } @@ -697,7 +701,8 @@ func (c *Compiler) GenDepsForFile(file string, compilerType int) error { // Append the extra dependencies (.yml files) to the .d file. objFile := strings.TrimSuffix(file, filepath.Ext(file)) + ".o" - if _, err := f.WriteString(objFile + ": " + c.depsString()); err != nil { + if _, err := f.WriteString(escapeDepsPath(objFile) + ": " + + c.depsString()); err != nil { return util.NewNewtError(err.Error()) } diff --git a/newt/toolchain/deps.go b/newt/toolchain/deps.go index 1da52934..2a14413e 100644 --- a/newt/toolchain/deps.go +++ b/newt/toolchain/deps.go @@ -66,17 +66,73 @@ func (d *DepTracker) setMostRecent(name string, t time.Time) { d.MostRecentTime = time.Unix(t.Unix(), 0) } +// splitDepsTokens splits a line of a Makefile dependency file into white space +// separated tokens. Escaping used in such files is undone: "\ " stands for a +// space, "\#" for '#' and "$$" for '$'. Other backslashes are kept as is. +func splitDepsTokens(line string) []string { + tokens := []string{} + var tok strings.Builder + inTok := false + + for i := 0; i < len(line); i++ { + ch := line[i] + switch { + case ch == '\\' && i+1 < len(line) && + strings.IndexByte(" \t#", line[i+1]) >= 0: + + i++ + tok.WriteByte(line[i]) + inTok = true + + case ch == '$' && i+1 < len(line) && line[i+1] == '$': + i++ + tok.WriteByte('$') + inTok = true + + case ch == ' ' || ch == '\t' || ch == '\r' || ch == '\n': + if inTok { + tokens = append(tokens, tok.String()) + tok.Reset() + inTok = false + } + + default: + tok.WriteByte(ch) + inTok = true + } + } + + if inTok { + tokens = append(tokens, tok.String()) + } + + return tokens +} + +// escapeDepsPath escapes a file name for use in a Makefile dependency file. +// It is the inverse of the unescaping done by splitDepsTokens. +func escapeDepsPath(path string) string { + r := strings.NewReplacer( + "$", "$$", + " ", "\\ ", + "\t", "\\\t", + "#", "\\#", + ) + + return r.Replace(path) +} + // @return string The name of the dependent file (i.e., the first // .o file encountered). // @return []string Populated with the dependencies' filenames. func parseDepsLine(line string) (string, []string, error) { - tokens := strings.Fields(line) + tokens := splitDepsTokens(line) if len(tokens) == 0 { return "", nil, nil } dFileTok := tokens[0] - if dFileTok[len(dFileTok)-1:] != ":" { + if !strings.HasSuffix(dFileTok, ":") { return "", nil, util.NewNewtError("line missing ':'") } @@ -266,7 +322,11 @@ func (tracker *DepTracker) CompileRequired(srcFile string, // Extract the dependency filenames from the dependency file. deps, err := ParseDepsFile(depPath) if err != nil { - return false, err + // The dependency file is corrupt (e.g., generated by an older + // version of newt); delete it so it gets regenerated and rebuild. + logRebuildReqd(srcFile, err.Error()) + os.Remove(depPath) + return true, nil } // Check if any dependencies are newer than the destination object file. diff --git a/newt/toolchain/deps_test.go b/newt/toolchain/deps_test.go index eafc10b1..f44fe3fa 100644 --- a/newt/toolchain/deps_test.go +++ b/newt/toolchain/deps_test.go @@ -115,3 +115,127 @@ func TestGenDepsForFileIgnoresStderr(t *testing.T) { t.Errorf("unexpected dependencies: expected %q, got %q", exp, deps) } } + +func TestParseDepsFileEscaping(t *testing.T) { + dir := t.TempDir() + obj := dir + "/my dir/my file.o" + + // As generated by gcc -MM for "my dir/my file.c", followed by the line + // newt appends itself. + content := "my\\ file.o: my\\ dir/my\\ file.c inc/a\\#b.h \\\n" + + " inc/c$$d.h \\\n" + + " inc/plain.h\n" + + escapeDepsPath(obj) + ": \n" + + path := filepath.Join(dir, "my file.d") + if err := os.WriteFile(path, []byte(content), 0644); err != nil { + t.Fatal(err) + } + + deps, err := ParseDepsFile(path) + if err != nil { + t.Fatal(err) + } + + exp := []string{"my dir/my file.c", "inc/a#b.h", "inc/c$d.h", + "inc/plain.h"} + if !slices.Equal(deps, exp) { + t.Errorf("unexpected dependencies: expected %q, got %q", exp, deps) + } +} + +func TestParseDepsLine(t *testing.T) { + tests := []struct { + line string + obj string + deps []string + err bool + }{ + {"", "", nil, false}, + {" ", "", nil, false}, + {"a.o: a.c b.h", "a.o", []string{"a.c", "b.h"}, false}, + {"a.o:", "a.o", []string{}, false}, + {"C:/x/a.o: C:/x/a.c", "C:/x/a.o", []string{"C:/x/a.c"}, false}, + {"a\\ b.o: a\\ b.c\tc.h\r", "a b.o", []string{"a b.c", "c.h"}, false}, + {"a\\\\b.o: c\\d.h", "a\\\\b.o", []string{"c\\d.h"}, false}, + {"a.c b.h", "", nil, true}, + } + + for _, tc := range tests { + obj, deps, err := parseDepsLine(tc.line) + if (err != nil) != tc.err { + t.Errorf("%q: unexpected error result: %v", tc.line, err) + continue + } + if obj != tc.obj || !slices.Equal(deps, tc.deps) { + t.Errorf("%q: expected %q %q, got %q %q", tc.line, tc.obj, + tc.deps, obj, deps) + } + } +} + +func TestEscapeDepsPath(t *testing.T) { + for _, p := range []string{"plain/a.o", "/sp ace/a b.o", "a#b$c.o", + "tab\there.o"} { + + tokens := splitDepsTokens(escapeDepsPath(p) + ": x") + if len(tokens) != 2 || tokens[0] != p+":" { + t.Errorf("%q: escape round trip failed: %q", p, tokens) + } + } +} + +// A dependency file which cannot be parsed (e.g., written by an older newt) +// must cause a rebuild rather than a permanent build failure. +func TestCompileRequiredCorruptDepsFile(t *testing.T) { + dir := t.TempDir() + c := newTestCompiler() + c.ccPath = "cc" + c.baseDir = dir + c.dstDir = dir + "/bin" + src := dir + "/src/a.c" + dst := c.dstFilePath(src) + + if err := os.MkdirAll(filepath.Dir(src), 0755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Dir(dst), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(src, nil, 0644); err != nil { + t.Fatal(err) + } + + cmd, err := c.CompileFileCmd(src, COMPILER_TYPE_C) + if err != nil { + t.Fatal(err) + } + if err := writeCommandFile(dst+".o", cmd); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(dst+".d", []byte("no colon here\n"), + 0644); err != nil { + + t.Fatal(err) + } + if err := os.WriteFile(dst+".o", nil, 0644); err != nil { + t.Fatal(err) + } + + // Make sure the source is older than the object and dependency files. + old := time.Now().Add(-time.Hour) + if err := os.Chtimes(src, old, old); err != nil { + t.Fatal(err) + } + + required, err := c.depTracker.CompileRequired(src, COMPILER_TYPE_C) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + if !required { + t.Errorf("compile not required with corrupt dependency file") + } + if _, err := os.Stat(dst + ".d"); !os.IsNotExist(err) { + t.Errorf("corrupt dependency file not removed") + } +}
