Emit a depfile for -MMD, not only for -MD - #33
Open
dkulp wants to merge 1 commit into
Open
Conversation
ShouldGenerateDepFile() tested flagMD || flagMF, so an invocation carrying only -MMD produced no .d file at all. -MMD is not a modifier on -MD; the two are alternatives that differ only in whether system headers are listed, and both emit a dependency file as a side effect of compilation. nocc strips -MMD off cxxArgs along with the rest of the dep flags and generates the file itself on the client, so when this predicate is false nothing generates it -- neither nocc nor the compiler on the server. "-MMD -MP" with no -MF is the usual hand-written-Makefile spelling (as opposed to the "-MD -MT ... -MF ..." that CMake emits), so such a project built through nocc silently ends up with zero .d files and no header dependency tracking: editing a header rebuilds nothing. The failure is invisible, because a missing .d is not an error to make -- the -include of the dep files just matches nothing. Every existing case in tests/depfiles_test.go passes -MD as well, including the two that also pass -MMD, so the predicate was never exercised with -MMD on its own. Add the two cases that were missing; both fail against the current code with "Error parsing dt/dep1/1.cpp.d after nocc: no such file or directory" and pass with this change, comparing contents against real g++ output as the rest of the file does. The system-header filtering that -MMD asks for was already implemented in calcDepListFromHFiles and keys off flagMMD, so it applies as soon as the file is generated. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
There was a problem hiding this comment.
Pull request overview
Fixes a bug in nocc’s depfile generation logic where passing -MMD (without -MD or -MF) would result in no dependency file being produced, because nocc strips dep flags from the remote compile and relies on the client-side generator.
Changes:
- Treat
-MMDas a depfile-emitting flag inShouldGenerateDepFile()(same as-MD). - Add test coverage for
-MMD-only and-MMD -MPcommand lines, comparing nocc output to realg++output.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
internal/client/dep-cmd-flags.go |
Fixes the depfile-generation predicate to include -MMD. |
tests/depfiles_test.go |
Adds regression tests ensuring depfiles are emitted for -MMD without -MD / -MF. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
An invocation carrying only
-MMDproduces no.dfile at all.-MMDis not a modifier on-MD; the two are alternatives that differ only in whether system headers are listed, and both emit a dependency file as a side effect of compilation. nocc strips-MMDoffcxxArgsalong with the rest of the dep flags and generates the file itself on the client, so when this predicate is false nothing generates it — neither nocc nor the compiler on the server.Why it goes unnoticed
-MMD -MPwith no-MFis the usual hand-written-Makefile spelling, as opposed to the-MD -MT ... -MF ...that CMake emits. Such a project built through nocc silently ends up with zero.dfiles and no header dependency tracking: editing a header rebuilds nothing.The failure is invisible, because a missing
.dis not an error to make — the-includeof the dep files just matches nothing.Why the existing tests did not catch it
Every case in
tests/depfiles_test.goalso passes-MD, including the two that pass-MMD(Test_MDMTMFMPMMD,Test_MQMQMQMQMMP,Test_MTMTMTMTMMP). SoflagMDwas always true and the predicate was never exercised with-MMDon its own.Test
Two cases added in the same style as the rest of the file, comparing contents against real
g++output:Both fail against current master with:
and pass with this change. The rest of the suite is unaffected — same results as master on my machine.
The system-header filtering that
-MMDasks for was already implemented incalcDepListFromHFilesand keys offflagMMD, so it applies as soon as the file is generated.