This is an automated email from the ASF dual-hosted git repository. kasjer pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mynewt-newt.git
commit 69b30a18ad8085a3a5ff32ac23b8bd52d4e4e642 Author: Szymon Janc <[email protected]> AuthorDate: Thu Oct 22 16:44:54 2026 +0200 newt/toolchain: Generate dependency files during compilation Dependency files were generated by a separate "-MM -MG" compiler invocation run before compiling each source file. This doubled the number of compiler invocations and preprocessed every file twice. It also limited ccache: the -MM calls cannot be cached, so even when all compiles were served from the cache every file was still preprocessed. Pass -MMD -MF to the compile command instead, so that the compiler writes the dependency file as a side effect of compilation. An object file without a dependency file is now rebuilt, as it is not known which headers it depends on. Extra dependencies are appended to the dependency file after successful compilation. Dependency options are left out of compile_commands.json. The test of the removed separate dependency generation is removed as well. Since the compile command changes, existing build directories are rebuilt once after upgrading newt. Measured on 16 cores, building btshell and smp_svr for nRF52840 and slinky for native (1076 compiled files), mean of 2-3 runs: - no ccache: 17.9 s -> 14.9 s - warm ccache: 8.8 s -> 5.0 s - -j 4, no ccache: 31.5 s -> 25.6 s - -j 4, warm ccache: 13.4 s -> 5.1 s Object files and images are byte identical. Assisted-by: Claude:claude-opus-5-5 --- newt/toolchain/compiler.go | 131 ++++++++++++++++---------------------------- newt/toolchain/deps.go | 40 ++++---------- newt/toolchain/deps_test.go | 39 +------------ 3 files changed, 57 insertions(+), 153 deletions(-) diff --git a/newt/toolchain/compiler.go b/newt/toolchain/compiler.go index 0ee968ae..6ee46fce 100644 --- a/newt/toolchain/compiler.go +++ b/newt/toolchain/compiler.go @@ -644,6 +644,26 @@ func (c *Compiler) depsString() string { return strings.Join(extraDeps, " ") + "\n" } +// Appends the extra dependencies (.yml files) to the dependency file which +// the compiler generated for the specified object file. +func (c *Compiler) appendExtraDeps(objPath string) error { + depPath := strings.TrimSuffix(objPath, ".o") + ".d" + f, err := os.OpenFile(depPath, os.O_APPEND|os.O_WRONLY, 0644) + if err != nil { + return util.ChildNewtError(err) + } + + _, err = f.WriteString(escapeDepsPath(objPath) + ": " + c.depsString()) + if cerr := f.Close(); err == nil { + err = cerr + } + if err != nil { + return util.ChildNewtError(err) + } + + return nil +} + func (c *Compiler) dstFilePath(srcPath string) string { relSrcPath := strings.TrimPrefix(filepath.ToSlash(srcPath), c.baseDir+"/") relDstPath := strings.TrimSuffix(relSrcPath, filepath.Ext(srcPath)) @@ -694,6 +714,7 @@ func (c *Compiler) CompileFileCmd(file string, compilerType int) ( cmd := []string{cmdName} cmd = append(cmd, flags...) cmd = append(cmd, c.includesStrings()...) + cmd = append(cmd, depsFlags(objPath)...) cmd = append(cmd, []string{ "-c", "-o", @@ -704,95 +725,28 @@ func (c *Compiler) CompileFileCmd(file string, compilerType int) ( return cmd, nil } -// Generates a dependency Makefile (.d) for the specified source file. -// -// @param file The name of the source file. -func (c *Compiler) GenDepsForFile(file string, compilerType int) error { - depPath := c.dstFilePath(file) + ".d" - depDir := filepath.Dir(depPath) - if util.NodeNotExist(depDir) { - os.MkdirAll(depDir, 0755) - } - - var cmdName string - var toolKey string - var flags []string - switch compilerType { - case COMPILER_TYPE_C: - cmdName = c.ccPath - toolKey = "compiler.path.cc" - flags = c.ccflagsStrings() - case COMPILER_TYPE_ASM: - cmdName = c.asPath - toolKey = "compiler.path.as" - - // Include both the compiler flags and the assembler flags. - // XXX: This is not great. We don't have a way of specifying compiler - // flags without also passing them to the assembler. - flags = append(c.cflagsStrings(), c.aflagsStrings()...) - case COMPILER_TYPE_CPP: - cmdName = c.cppPath - toolKey = "compiler.path.cpp" - flags = append(c.cflagsStrings(), c.cxxflagsStrings()...) - default: - return util.NewNewtError("Unknown compiler type") - } - if err := c.checkToolPath(cmdName, toolKey); err != nil { - return err - } - - srcPath := strings.TrimPrefix(file, c.baseDir+"/") - cmd := []string{cmdName} - cmd = append(cmd, flags...) - cmd = append(cmd, c.includesStrings()...) - cmd = append(cmd, []string{"-MM", "-MG", srcPath}...) - - o, err := depsCommandOutput(cmd) - if err != nil { - return err - } - - // Append the extra dependencies (.yml files) to the compiler output and - // write it to a dependency file. - objFile := strings.TrimSuffix(file, filepath.Ext(file)) + ".o" - o = append(o, escapeDepsPath(objFile)+": "+c.depsString()...) - - if err := os.WriteFile(depPath, o, 0644); err != nil { - return util.ChildNewtError(err) - } - - return nil +// Returns the options which make the compiler generate a dependency Makefile +// (.d) for the specified object file as a side effect of compilation. +func depsFlags(objPath string) []string { + return []string{"-MMD", "-MF", strings.TrimSuffix(objPath, ".o") + ".d"} } -// depsCommandOutput executes the specified dependency generation command and -// returns its standard output. Standard error (e.g., compiler warnings) is -// not part of the dependency list; it is only displayed in verbose mode or -// returned as the error text if the command fails. -func depsCommandOutput(cmdStrs []string) ([]byte, error) { - cmd, err := util.ShellCommandInit(cmdStrs, nil) - if err != nil { - return nil, err - } - - util.LogShellCmd(cmdStrs, nil) - - var stderr bytes.Buffer - cmd.Stderr = &stderr - - o, err := cmd.Output() - if err != nil { - nerr := util.ChildNewtError(err) - if stderr.Len() > 0 { - nerr.Text = stderr.String() +// Removes the dependency file options from a compiler command. They are only +// relevant to newt's own dependency tracking, not to tools reading the +// compilation database. +func stripDepsFlags(cmd []string) []string { + stripped := make([]string, 0, len(cmd)) + for i := 0; i < len(cmd); i++ { + switch cmd[i] { + case "-MMD": + case "-MF": + i++ + default: + stripped = append(stripped, cmd[i]) } - return nil, nerr - } - - if stderr.Len() > 0 { - util.StatusMessage(util.VERBOSITY_VERBOSE, "%s", stderr.String()) } - return o, nil + return stripped } func serializeCommand(cmd []string) []byte { @@ -868,7 +822,14 @@ func (c *Compiler) CompileFile(file string, compilerType int) error { } util.StatusMessage(util.VERBOSITY_DEFAULT, "%s", string(o)) - c.addCompileCommand(cmd, file) + if len(c.extraDeps) > 0 { + err = c.appendExtraDeps(objPath) + if err != nil { + return err + } + } + + c.addCompileCommand(stripDepsFlags(cmd), file) err = writeCommandFile(objPath, cmd) if err != nil { diff --git a/newt/toolchain/deps.go b/newt/toolchain/deps.go index 2a14413e..c6384a97 100644 --- a/newt/toolchain/deps.go +++ b/newt/toolchain/deps.go @@ -273,18 +273,14 @@ func (tracker *DepTracker) CompileRequired(srcFile string, if commandHasChanged(objPath, cmd) { logRebuildReqdCmdChanged(srcFile) - err := tracker.compiler.GenDepsForFile(srcFile, compilerType) - if err != nil { - return false, err - } return true, nil } + // The dependency file is generated during compilation. Without it, it is + // not known which headers the object file depends on. if util.NodeNotExist(depPath) { - err := tracker.compiler.GenDepsForFile(srcFile, compilerType) - if err != nil { - return false, err - } + logRebuildReqd(srcFile, "dependency file missing") + return true, nil } srcModTime, err := util.FileModificationTime(srcFile) @@ -304,21 +300,6 @@ func (tracker *DepTracker) CompileRequired(srcFile string, return true, nil } - // Determine if the dependency (.d) file needs to be generated. If it - // doesn't exist or is older than the source file, it is out of date and - // needs to be created. - depModTime, err := util.FileModificationTime(depPath) - if err != nil { - return false, err - } - - if srcModTime.After(depModTime) { - err := tracker.compiler.GenDepsForFile(srcFile, compilerType) - if err != nil { - return false, err - } - } - // Extract the dependency filenames from the dependency file. deps, err := ParseDepsFile(depPath) if err != nil { @@ -334,16 +315,15 @@ func (tracker *DepTracker) CompileRequired(srcFile string, if util.NodeNotExist(dep) { // The dependency has been deleted; a rebuild is required. Also, // the dependency file is out of date, so it needs to be deleted. - // We cannot regenerate it now because the source file might be - // including a nonexistent header. + // It is generated again when the source file is compiled. logRebuildReqdNoDep(srcFile, dep) os.Remove(depPath) return true, nil - } else { - depModTime, err = util.FileModificationTime(dep) - if err != nil { - return false, err - } + } + + depModTime, err := util.FileModificationTime(dep) + if err != nil { + return false, err } if depModTime.After(objModTime) { diff --git a/newt/toolchain/deps_test.go b/newt/toolchain/deps_test.go index f44fe3fa..eb0008c8 100644 --- a/newt/toolchain/deps_test.go +++ b/newt/toolchain/deps_test.go @@ -23,7 +23,6 @@ import ( "fmt" "os" "path/filepath" - "runtime" "slices" "sync" "testing" @@ -80,47 +79,11 @@ func TestCompilerConcurrentUpdates(t *testing.T) { } } -// Compiler warnings on stderr must not end up in the dependency file. -func TestGenDepsForFileIgnoresStderr(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("uses a shell script as a fake compiler") - } - - dir := t.TempDir() - cc := filepath.Join(dir, "fake-cc") - script := "#!/bin/sh\n" + - "echo 'warning: something odd' >&2\n" + - "echo 'src/a.o: src/a.c src/a.h'\n" - if err := os.WriteFile(cc, []byte(script), 0755); err != nil { - t.Fatal(err) - } - - c := newTestCompiler() - c.ccPath = cc - c.baseDir = dir - c.dstDir = dir + "/bin" - src := dir + "/src/a.c" - - if err := c.GenDepsForFile(src, COMPILER_TYPE_C); err != nil { - t.Fatal(err) - } - - deps, err := ParseDepsFile(c.dstFilePath(src) + ".d") - if err != nil { - t.Fatal(err) - } - - exp := []string{"src/a.c", "src/a.h"} - if !slices.Equal(deps, exp) { - 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 + // As generated by gcc -MM / -MMD 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" +
