build: make depclean remove the userspace dependency files too - #4419
Open
greatEndian wants to merge 1 commit into
Open
build: make depclean remove the userspace dependency files too#4419greatEndian wants to merge 1 commit into
greatEndian wants to merge 1 commit into
Conversation
depclean removed only 'depends', which holds the realtime dependency
files. The userspace ones are written next to their objects as
objects/**/*.d (see TODEPS and the -MF "${@:.o=.d}" compile rules), and
survived it -- 535 of them in a typical run-in-place tree.
That matters when a source file is renamed or moved. A stale dependency
file still declares, say
objects/hal/utils/halrmt.o: hal/utils/halrmt.c
and gcc's -MP writes dummy targets for the *headers* only, never for the
main source, so once hal/utils/halrmt.c is gone nothing can satisfy that
prerequisite and the whole build stops with
make: *** No rule to make target 'hal/utils/halrmt.c',
needed by 'objects/hal/utils/halrmt.o'. Stop.
The documented remedy for stale dependencies is exactly this target, and
it did not work: the only way out was 'make clean' and a full rebuild.
Hit in practice on a run-in-place tree carried across the halrmt.c ->
halrmt.cc rename and the src/libnml/posemath -> src/libposemath move;
eight dependency files pointed at sources that no longer existed.
Removing the files costs no recompilation -- nothing has a .d as a
prerequisite, and UNREAD_DEPS is computed but never used -- so this only
gives up header-dependency tracking until each object is next rebuilt,
which is what asking for depclean means.
Verified by planting a dependency file naming a since-renamed source,
reproducing the failure verbatim, and confirming that depclean did not
clear it before this change and does after, with the build then
completing normally.
The comment above modclean is corrected as well: 'clean' does remove the
userspace dependency files today, because genclean deletes objects/
wholesale.
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.
Problem
src/Makefile'sdepcleantarget removes onlydepends, which holds therealtime dependency files. The userspace ones are written next to their
objects as
objects/**/*.d— seeTODEPS(Makefile:229) and the-MF "${@:.o=.d}"compile rules (Makefile:291, 301, 309, 317) — and surviveit. A typical run-in-place tree keeps 535 of them.
The target's own comment claims otherwise ("
cleancleans everything butdependency files, and
depcleancleans them too"), and that comment is wrongin both directions:
cleandoes remove the userspace dependency files,because
gencleandeletesobjects/wholesale.Why it matters
A stale dependency file still declares its original source:
gcc's
-MPwrites dummy targets for the headers only, never for the mainsource, so once
hal/utils/halrmt.chas been renamed the prerequisite cannotbe satisfied and the entire build stops:
The documented remedy for stale dependency information is
make depclean—and it does not work. The only way out is
make cleanand a full rebuild.This is not hypothetical. It was hit on a run-in-place tree carried across two
upstream changes: the
halrmt.c->halrmt.ccrename, and thesrc/libnml/posemath->src/libposemathmove. Eight dependency files pointedat sources that no longer existed, and the tree could not be built at all.
Fix
Have
depcleanremoveobjects/**/*.das well, and correct the comment.Removing them costs no recompilation: nothing takes a
.das a prerequisite,and
UNREAD_DEPS(Makefile:255) is computed but never used. The only thinggiven up is header-dependency tracking until each object is next rebuilt,
which is precisely what asking for
depcleanmeans.Verification
failure verbatim, including the
-MPsubtlety (a first attempt that includeda dummy rule for the main source did not reproduce it, which is what
confirmed the mechanism).
make depcleanleft all 535objects/**/*.din place andthe build still aborted.
make depcleanremoved all 535, and the previously fataltarget built normally.
make defaultimmediately after a patcheddepcleanon an up-to-date tree recompiled 0 files.