> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of Ramiro Polla > Sent: Dienstag, 20. Mai 2025 22:29 > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH v2] ffbuild/commonmak: Fix rebuild check > with implicit rule chains > > On Tue, May 20, 2025 at 9:46 PM softworkz . > <softworkz-at-hotmail....@ffmpeg.org> wrote: > > > -----Original Message----- > > > From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of Ramiro > Polla > > > Sent: Dienstag, 20. Mai 2025 21:36 > > > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> > > > Subject: Re: [FFmpeg-devel] [PATCH v2] ffbuild/commonmak: Fix rebuild > check > > > with implicit rule chains > > > On Sun, May 18, 2025 at 8:30 AM softworkz <ffmpegag...@gmail.com> wrote: > > > > From: softworkz <softwo...@hotmail.com> > > > > 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 <softwo...@hotmail.com> > > > > --- > > > > 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. > > I had forgotten about that part. It's not triggered with gcc/clang, > but it seems to be triggered when using msvc. I don't have an msvc > setup ready, but I believe you do, right?
My local MSVC setup is using vcxproj projects, it's not makefile driven, but I have the other ways as well. Though, I love to have it off the table and running elsewhere. I made the V=1 change and also added a second make after make to check: https://dev.azure.com/githubsync/ffmpeg/_build/results?buildId=89332&view=logs BTW, it's public, just create a PR on https://github.com/ffstaging/FFmpeg and you get 6 CI builds which are the same like on Patchwork. (nobody needs to use that submission thing, just close the PR right after) Best, sw _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
