littlekernel / lk

LK embedded kernel
MIT License
3.14k stars 615 forks source link

[make] [lib/version] remove a misplaced .PHONY #170

Closed girtsf closed 7 years ago

girtsf commented 7 years ago

When lib/version was included, a rule depended on .PHONY. This would unexpectedly invoke other rules defined as dependent on .PHONY (standard phony-ness signal).

As a side effect, this used to generate srcfiles.txt and include_paths.txt in build dir. Since they are useful, we might always build those anyway, and not make them phony at all.

girtsf commented 7 years ago

doh, forgot a $@.tmp and didn't retest with a clean build. fix coming.

girtsf commented 7 years ago

Could this use a level of indirection for the phony target? (Similar to how some linker script generation does it.)

E.g.:

--- a/lib/version/rules.mk
+++ b/lib/version/rules.mk
@@ -21,9 +21,13 @@ endif
 # included yet set BUILDID.
 BUILDID_DEFINE="BUILDID=\"$(BUILDID)\""
 BUILDID_H := $(BUILDDIR)/buildid.h
-$(BUILDID_H):
+$(BUILDID_H): buildid_h.phony
        @$(call MAKECONFIGHEADER,$@,BUILDID_DEFINE)
-.PHONY: $(BUILDID_H)
+
+# Moving the phony to an extra dependency allows version.o not to be
+# rebuilt if buildid.h doesn't change.
+buildid_h.phony:
+.PHONY: buildid_h.phony

 GENERATED += $(BUILDID_H)
travisg commented 7 years ago

The latter one with buildid.h isn't really needed because the MAKECONFIGHEADER thing explicitly does the Right Thing and only replaces the file if it was different. It generates it every time but throws away the result of it's not different.