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 60166292071681168ef30fdf5cfc3e8558a5a3d9 Author: Szymon Janc <[email protected]> AuthorDate: Mon Oct 19 18:00:11 2026 +0200 newt/toolchain: Fix builds of projects in paths with spaces Include paths were passed through util.SortFields(), which splits strings on white space, so every include path below a directory with a space in its name was broken into several bogus arguments and no project in such a directory could be built. Sort and deduplicate include paths without splitting them. Commands are executed with argument slices already; quote arguments containing white space, quotes or backslashes where newt generates text that is parsed later: the object file list passed to the linker as @file, the commands in compile_commands.json and the include directories in the CMake output. Assisted-by: Claude:claude-opus-5-5 --- newt/build_space_test.go | 104 +++++++++++++++++++++++++++++++++++++++++++ newt/builder/cmake.go | 12 ++++- newt/toolchain/compiler.go | 38 ++++++++++++++-- newt/toolchain/flags_test.go | 74 ++++++++++++++++++++++++++++++ 4 files changed, 223 insertions(+), 5 deletions(-) diff --git a/newt/build_space_test.go b/newt/build_space_test.go new file mode 100644 index 00000000..55a31497 --- /dev/null +++ b/newt/build_space_test.go @@ -0,0 +1,104 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package main + +import ( + "os" + "os/exec" + "path/filepath" + "runtime" + "testing" + + "github.com/otiai10/copy" +) + +// buildSpaceWriteFile writes a file, creating its directory if needed. +func buildSpaceWriteFile(t *testing.T, path string, content string) { + t.Helper() + + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0644); err != nil { + t.Fatal(err) + } +} + +// TestBuildSpaceInPath builds a project located in a directory with a space +// in its name, including a source file with a space in its name, using the +// host C compiler. +func TestBuildSpaceInPath(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("host build not supported on Windows") + } + + var cc string + for _, name := range []string{"cc", "gcc"} { + if path, err := exec.LookPath(name); err == nil { + cc = path + break + } + } + if cc == "" { + t.Skip("no C compiler found in PATH") + } + + dir := filepath.Join(t.TempDir(), "sp ace", "proj") + if err := copy.Copy(filepath.Join("testdata", "syscfg_collision"), + dir); err != nil { + + t.Fatal(err) + } + + core := filepath.Join(dir, "repos", "apache-mynewt-core") + buildSpaceWriteFile(t, filepath.Join(core, "compiler", "sim", + "compiler.yml"), + "compiler.path.cc: \""+cc+"\"\n"+ + "compiler.path.as: \""+cc+"\"\n"+ + "compiler.path.archive: ar\n"+ + "compiler.path.objsize: size\n"+ + "compiler.flags.default: [-O1, -Wall]\n"+ + "compiler.ld.binfile: false\n") + buildSpaceWriteFile(t, filepath.Join(core, "sys", "foo", "src", + "foo.c"), "void foo_init(void) {}\n") + // Linked as a list of object files (@file). + buildErrAppendFile(t, filepath.Join(core, "sys", "foo", "pkg.yml"), + "\npkg.whole_archive: true\n") + buildSpaceWriteFile(t, filepath.Join(core, "sys", "foo", "include", + "foo dir", "foo.h"), "#define FOO 1\n") + buildSpaceWriteFile(t, filepath.Join(dir, "targets", "success", + "include", "flash_map", "flash_map.h"), + "#include <stdint.h>\n"+ + "struct flash_area { uint8_t fa_id; uint8_t fa_device_id;\n"+ + " uint16_t pad16; uint32_t fa_off; uint32_t fa_size; };\n") + buildSpaceWriteFile(t, filepath.Join(dir, "apps", "app", "src", + "main.c"), "#include \"foo dir/foo.h\"\n"+ + "int main(void) { return FOO - 1; }\n") + buildSpaceWriteFile(t, filepath.Join(dir, "apps", "app", "src", + "my file.c"), "int my_file(void) { return 0; }\n") + + // Second and third builds use the generated dependency files. + for i := 0; i < 3; i++ { + out, err := newtCombinedOutput(dir, nil, "build", "success") + if err != nil { + t.Fatalf("newt build #%d failed: %s\n%s", i+1, err, out) + } + } +} diff --git a/newt/builder/cmake.go b/newt/builder/cmake.go index 6e5a37b3..876d090c 100644 --- a/newt/builder/cmake.go +++ b/newt/builder/cmake.go @@ -27,6 +27,7 @@ import ( "os" "path" "path/filepath" + "sort" "strings" log "github.com/sirupsen/logrus" @@ -349,10 +350,17 @@ func CmakeCompilerInfoWrite(w io.Writer, archiveFile string, bpkg *BuildPackage, includes = append(includes, c.GetLocalCompilerInfo().Includes...) includes = append(includes, otherIncludes...) - // Sort and remove duplicate flags - includes = util.SortFields(includes...) + // Sort and remove duplicate paths; paths may contain spaces, so they + // must not be split. + includes = util.UniqueStrings(includes) + sort.Strings(includes) trimProjectPathSlice(includes) replaceBackslashesSlice(includes) + for i, inc := range includes { + if strings.ContainsAny(inc, " \t") { + includes[i] = "\"" + inc + "\"" + } + } fmt.Fprintf(w, `set_target_properties(%s diff --git a/newt/toolchain/compiler.go b/newt/toolchain/compiler.go index 43b8e613..f1537ac2 100644 --- a/newt/toolchain/compiler.go +++ b/newt/toolchain/compiler.go @@ -133,11 +133,34 @@ func (c *Compiler) addCompileCommand(cmd []string, file string) { c.compileCommands = append(c.compileCommands, CompileCommand{ - Command: strings.Join(cmd, " "), + Command: commandString(cmd), File: file, }) } +// quoteArg quotes a command argument containing white space, quotes or +// backslashes, so that it is not split when the command line or a response +// file (@file) is parsed. Other arguments are returned unchanged. +func quoteArg(arg string) string { + if !strings.ContainsAny(arg, " \t\n\"'\\") { + return arg + } + + r := strings.NewReplacer("\\", "\\\\", "\"", "\\\"") + return "\"" + r.Replace(arg) + "\"" +} + +// commandString joins command arguments into a single command line, quoting +// arguments where necessary. +func commandString(cmd []string) string { + args := make([]string, len(cmd)) + for i, arg := range cmd { + args[i] = quoteArg(arg) + } + + return strings.Join(args, " ") +} + func (c *Compiler) GetCcPath() string { return c.ccPath } @@ -531,7 +554,16 @@ func (c *Compiler) includesStrings() []string { return nil } - includes := util.SortFields(c.info.Includes...) + // Include paths may contain spaces; do not split them. + includes := []string{} + for _, s := range c.info.Includes { + s = strings.TrimSpace(s) + if s != "" { + includes = append(includes, s) + } + } + includes = util.UniqueStrings(includes) + sort.Strings(includes) tokens := make([]string, len(includes)) for i, s := range includes { @@ -1578,7 +1610,7 @@ func (c *Compiler) CompileArchive(outputFile string, createList bool) error { defer file.Close() for _, objFilePath := range objList { - _, err = file.WriteString(objFilePath + "\n") + _, err = file.WriteString(quoteArg(objFilePath) + "\n") if err != nil { return util.NewNewtError(err.Error()) } diff --git a/newt/toolchain/flags_test.go b/newt/toolchain/flags_test.go new file mode 100644 index 00000000..a93e511f --- /dev/null +++ b/newt/toolchain/flags_test.go @@ -0,0 +1,74 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package toolchain + +import ( + "slices" + "testing" + + "github.com/kballard/go-shellquote" +) + +func TestIncludesStringsSpaces(t *testing.T) { + c := newTestCompiler() + c.baseDir = "/sp ace/proj" + c.info.Includes = []string{ + "/sp ace/proj/b dir/include", + "/sp ace/proj/a/include", + "/other dir/include", + "/sp ace/proj/a/include", + " ", + } + + exp := []string{ + "-I/other dir/include", + "-Ia/include", + "-Ib dir/include", + } + if got := c.includesStrings(); !slices.Equal(got, exp) { + t.Errorf("unexpected include flags: expected %q, got %q", exp, got) + } +} + +func TestCommandString(t *testing.T) { + cmd := []string{ + "gcc", + "-DSTR=\"a b\"", + "-I/sp ace/inc", + "-DQ='x'", + "-c", + "a\\b.c", + "plain.c", + } + + s := commandString(cmd) + got, err := shellquote.Split(s) + if err != nil { + t.Fatal(err) + } + if !slices.Equal(got, cmd) { + t.Errorf("round trip failed: %q -> %q -> %q", cmd, s, got) + } + + // Arguments without special characters are not quoted. + if s := commandString([]string{"gcc", "-O2", "a.c"}); s != "gcc -O2 a.c" { + t.Errorf("unexpected command string: %q", s) + } +}
