> -----Original Message-----
> From: ffmpeg-devel <[email protected]> On Behalf Of Ramiro Polla
> Sent: Dienstag, 20. Mai 2025 21:36
> To: FFmpeg development discussions and patches <[email protected]>
> Subject: Re: [FFmpeg-devel] [PATCH v2] ffbuild/commonmak: Fix rebuild check
> with implicit rule chains
>
> Hi,
>
> On Sun, May 18, 2025 at 8:30 AM softworkz <[email protected]> wrote:
> >
> > From: softworkz <[email protected]>
> >
> > When there's a chain of implicit rules, make treats files generated
> > inside that chain as intermediate files. Those intermediate files are
> > removed after completion of make. When make is run again, it normally
> > determines the need for a rebuild by comparing the timestamps of the
> > original source file and the final output of the chain and if still
> > up-to-date, it doesn't rebuild, even when the intermediate files are
> > not present. That makes sense of course - why would it delete them
> > otherwise, it would end up in builds being never up-to-date.
> > But this original by-the-book logic appeared to be broken and has been
> > worked around by adding all intermediate files to the .SECONDARY
> > special target, which required extra logic and issues with make clean.
> >
> > What broke the up-to-date checking is the dependency file generation.
> > For the .c files generated by BIN2C the compile target created a .d
> > file which indicated that the .ptx.o file has a dependency on the
> > ptx.c file. And that dependency broke the normal make behavior with
> > intermediate files.
> >
> > This patch compiles the BIN2C generated .c files without generating
> > .d files. In turn the files do not longer need to be added to the
> > .SECONDARY target in common.mak.
> >
> > When interested in those files for debugging, a line can be added to
> > the corresponding Makefile like
> >
> > .SECONDARY: %.ptx.c %.ptx.gz
> >
> > Signed-off-by: softworkz <[email protected]>
> > ---
> > ffbuild/commonmak: Fix rebuild check with implicit rule chains
> >
> > When there's a chain of implicit rules, make treats files generated
> > inside that chain as intermediate files. Those intermediate files are
> > removed after completion of make. When make is run again, it normally
> > determines the need for a rebuild by comparing the timestamps of the
> > original source file and the final output of the chain and if still
> > up-to-date, it doesn't rebuild, even when the intermediate files are not
> > present. That makes sense of course - why would it delete them
> > otherwise, it would end up in builds being never up-to-date. But this
> > original by-the-book logic appeared to be broken and has been worked
> > around by adding all intermediate files to the .SECONDARY special
> > target, which required extra logic and issues with make clean.
> >
> > What broke the up-to-date checking is the dependency file generation.
> > For the .c files generated by BIN2C the compile target created a .d file
> > which indicated that the .ptx.o file has a dependency on the ptx.c file.
> > And that dependency broke the normal make behavior with intermediate
> > files.
> >
> > This patch compiles the BIN2C generated .c files without generating .d
> > files. In turn the files do not longer need to be added to the
> > .SECONDARY target in common.mak.
> >
> > When interested in those files for debugging, a line can be added to the
> > corresponding Makefile like
> >
> > .SECONDARY: %.ptx.c %.ptx.gz
> >
> >
> > V2
> > ==
> >
> > * Fix MSVC build
> > (use the universal command pattern)
> >
> > Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-
> 80%2Fsoftworkz%2Fsubmit_commonmak-v2
> > Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-
> 80/softworkz/submit_commonmak-v2
> > Pull-Request: https://github.com/ffstaging/FFmpeg/pull/80
> >
> > Range-diff vs v1:
> >
> > 1: e276f54ffc ! 1: f468ea2431 ffbuild/commonmak: Fix rebuild check with
> implicit rule chains
> > @@ ffbuild/common.mak: else
> > endif
> >
> > +%.ptx.o: %.ptx.c
> > -+ $(CC) $(CCFLAGS) -x c -c -o $@ $<
> > ++ $(CC) $(CCFLAGS) $(CC_C) $(CC_O) $(patsubst
> $(SRC_PATH)/%,$(SRC_LINK)/%,$<)
> > +
> > +
> > # 1) Preprocess CSS to a minified version
> > @@ ffbuild/common.mak: else # NO COMPRESSION
> > endif
> >
> > +%.html.o: %.html.c
> > -+ $(CC) $(CCFLAGS) -x c -c -o $@ $<
> > ++ $(CC) $(CCFLAGS) $(CC_C) $(CC_O) $(patsubst
> $(SRC_PATH)/%,$(SRC_LINK)/%,$<)
> > +
> > +%.css.o: %.css.c
> > -+ $(CC) $(CCFLAGS) -x c -c -o $@ $<
> > ++ $(CC) $(CCFLAGS) $(CC_C) $(CC_O) $(patsubst
> $(SRC_PATH)/%,$(SRC_LINK)/%,$<)
> > +
> > +
> > clean::
> >
> >
> > ffbuild/common.mak | 15 ++++++++++++---
> > 1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/ffbuild/common.mak b/ffbuild/common.mak
> > index 0e1eb1f62b..d9462271d5 100644
> > --- a/ffbuild/common.mak
> > +++ b/ffbuild/common.mak
> > @@ -139,6 +139,10 @@ else
> > $(BIN2C) $(patsubst $(SRC_PATH)/%,$(SRC_LINK)/%,$<) $@ $(subst
> .,_,$(basename $(notdir $@)))
> > endif
> >
> > +%.ptx.o: %.ptx.c
> > + $(CC) $(CCFLAGS) $(CC_C) $(CC_O) $(patsubst
> $(SRC_PATH)/%,$(SRC_LINK)/%,$<)
> > +
> > +
> > # 1) Preprocess CSS to a minified version
> > %.css.min: %.css
> > # Must start with a tab in the real Makefile
> > @@ -177,6 +181,13 @@ else # NO COMPRESSION
> > $(BIN2C) $< $@ $(subst .,_,$(basename $(notdir $@)))
> > endif
> >
> > +%.html.o: %.html.c
> > + $(CC) $(CCFLAGS) $(CC_C) $(CC_O) $(patsubst
> $(SRC_PATH)/%,$(SRC_LINK)/%,$<)
> > +
> > +%.css.o: %.css.c
> > + $(CC) $(CCFLAGS) $(CC_C) $(CC_O) $(patsubst
> $(SRC_PATH)/%,$(SRC_LINK)/%,$<)
> > +
>
> I'm not a big fan of adding new targets with a stripped-down version
> of the original rule. It seems like unnecessary duplication, and
> increases the chance of divergence over time.
>
> Instead, I think it's cleaner to use target-specific variables to
> disable a part of the original rule. For example, in this case, you
> would disable the dependency generation for just those targets, and
> end up with something like this:
> %.html.o: CC_DEPFLAGS =
> %.css.o: CC_DEPFLAGS =
Hi Ramiro,
I didn't know that one can do that. That's clearly a better way, then.
What about the first line in the COMPILE macro:
define COMPILE
$(call $(1)DEP,$(1))
$($(1)) $($(1)FLAGS) $($(2)) $($(1)_DEPFLAGS) $($(1)_C) $($(1)_O)
$(patsubst $(SRC_PATH)/%,$(SRC_LINK)/%,$<)
endef
I haven't traced back what it actually does - is it harmless when
it's executed in those cases where no dependency file is desired?
If that's fine, I'd send an updated patch with your suggestion.
Thank you
sw
_______________________________________________
ffmpeg-devel mailing list
[email protected]
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
[email protected] with subject "unsubscribe".
