Re: [PATCH v2 2/5] gitweb: fix generation of "gitweb.js"

[Patrick Steinhardt <ps@pks.im>]

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