| Message ID | 20250331-b4-pks-collect-build-fixes-v2-2-6b06136808f3@pks.im (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Collection of build fixes | expand |
Hi Patrick, On Mon, 31 Mar 2025, Patrick Steinhardt wrote: > diff --git a/gitweb/Makefile b/gitweb/Makefile > index d5748e93594..26a683d4421 100644 > --- a/gitweb/Makefile > +++ b/gitweb/Makefile > @@ -118,7 +118,7 @@ $(MAK_DIR_GITWEB)gitweb.cgi: $(MAK_DIR_GITWEB)gitweb.perl > $(MAK_DIR_GITWEB)static/gitweb.js: $(MAK_DIR_GITWEB)generate-gitweb-js.sh > $(MAK_DIR_GITWEB)static/gitweb.js: $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_JSLIB_FILES)) > $(QUIET_GEN)$(RM) $@ $@+ && \ > - $(MAK_DIR_GITWEB)generate-gitweb-js.sh $@+ $^ && \ > + $(MAK_DIR_GITWEB)generate-gitweb-js.sh $@+ $(filter %.js,$^) && \ > mv $@+ $@ A safer way might be to use `$(filter-out %.sh,$^)` just in case the Javascript libraries might at some stage be renamed (I could imagine, for example, that someone aims for ideological purity and renames them to `*.cjs`). Ciao, Johannes
Patrick Steinhardt <ps@pks.im> writes: > In 19d8fe7da65 (Makefile: extract script to generate gitweb.js, > 2024-12-06) we have extracted the logic to build "gitweb.js" into a > separate script. As part of that the rules that builds the script > has gained a new dependency on that script. > > This refactoring is broken though because we use "$^" to determine > the set of JavaScript files that need to be concatenated, and this > implicit variable now also contains the build script itself. As a > result, the build script ends up ni the generated "gitweb.js" file, Tiniest typo: ni -> in But that's all I've got about this patch series. :+1:
On Tue, Apr 01, 2025 at 06:30:01PM +0200, Johannes Schindelin wrote: > Hi Patrick, > > On Mon, 31 Mar 2025, Patrick Steinhardt wrote: > > > diff --git a/gitweb/Makefile b/gitweb/Makefile > > index d5748e93594..26a683d4421 100644 > > --- a/gitweb/Makefile > > +++ b/gitweb/Makefile > > @@ -118,7 +118,7 @@ $(MAK_DIR_GITWEB)gitweb.cgi: $(MAK_DIR_GITWEB)gitweb.perl > > $(MAK_DIR_GITWEB)static/gitweb.js: $(MAK_DIR_GITWEB)generate-gitweb-js.sh > > $(MAK_DIR_GITWEB)static/gitweb.js: $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_JSLIB_FILES)) > > $(QUIET_GEN)$(RM) $@ $@+ && \ > > - $(MAK_DIR_GITWEB)generate-gitweb-js.sh $@+ $^ && \ > > + $(MAK_DIR_GITWEB)generate-gitweb-js.sh $@+ $(filter %.js,$^) && \ > > mv $@+ $@ > > A safer way might be to use `$(filter-out %.sh,$^)` just in case the > Javascript libraries might at some stage be renamed (I could imagine, for > example, that someone aims for ideological purity and renames them to > `*.cjs`). I could see arguments both ways: - If we use "filter-out" the developer now has to remember to also filter out files whenever a new dependency is added. - If we use "filter" the developer has to remember to update the pattern if any of the files are renamed. I think the developer is going to be more on the guard in the second case -- after all, renaming files always requires you to also update the build instructions. On the other hand it's quite easy to miss that you have to adapt the "filter-out" logic when adding a new dependency. In the end neither of these solutions is perfect, but the worst part is that we don't have any tests at all that would detect a broken build. So I lean towards keeping the current mechanism, but don't feel strongly about it. Let me know in case you still prefer "filter-out" and I'll adapt accordingly. Patrick
diff --git a/gitweb/Makefile b/gitweb/Makefile index d5748e93594..26a683d4421 100644 --- a/gitweb/Makefile +++ b/gitweb/Makefile @@ -118,7 +118,7 @@ $(MAK_DIR_GITWEB)gitweb.cgi: $(MAK_DIR_GITWEB)gitweb.perl $(MAK_DIR_GITWEB)static/gitweb.js: $(MAK_DIR_GITWEB)generate-gitweb-js.sh $(MAK_DIR_GITWEB)static/gitweb.js: $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_JSLIB_FILES)) $(QUIET_GEN)$(RM) $@ $@+ && \ - $(MAK_DIR_GITWEB)generate-gitweb-js.sh $@+ $^ && \ + $(MAK_DIR_GITWEB)generate-gitweb-js.sh $@+ $(filter %.js,$^) && \ mv $@+ $@ ### Installation rules
In 19d8fe7da65 (Makefile: extract script to generate gitweb.js, 2024-12-06) we have extracted the logic to build "gitweb.js" into a separate script. As part of that the rules that builds the script has gained a new dependency on that script. This refactoring is broken though because we use "$^" to determine the set of JavaScript files that need to be concatenated, and this implicit variable now also contains the build script itself. As a result, the build script ends up ni the generated "gitweb.js" file, which is wrong. Fix the issue by filtering out non-JavaScript files. Based-on-patch-by: Thorsten Glaser <tg@debian.org> Signed-off-by: Patrick Steinhardt <ps@pks.im> --- gitweb/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)