Re: [PATCH v2 2/5] gitweb: fix generation of "gitweb.js"
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Apr 2, 2025, 06:40 UTC
- Message-ID
- <Z-zb7yPcPgJRhhXR@pks.im>
- In-Reply-To
- <e9ea3630-a831-dec5-e461-3f550ceb7ec3@gmx.de>
On Tue, Apr 01, 2025 at 06:30:01PM +0200, Johannes Schindelin wrote:
Show 20 quoted lines
> 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