diff mbox series

gitweb.js build mistake

Message ID 070641d0-730c-7d92-af4a-9157dc1edd3d@debian.org (mailing list archive)
State New
Headers show
Series gitweb.js build mistake | expand

Commit Message

Thorsten Glaser Feb. 28, 2025, 5:34 a.m. UTC
Hi,

the new gitweb.js build (moved into a shell script) now
appends said shell script to the end of the gitweb.js
that’s actually installed, causing js syntax errors and
no small amount of confusion.

This is because (rightfully) the output got a new dependency…
> $(MAK_DIR_GITWEB)static/gitweb.js: $(MAK_DIR_GITWEB)generate-gitweb-js.sh
… but the rule uses $^ to append sources.

Possible fix attached.

bye,
//mirabilos

Comments

Simon Richter Feb. 28, 2025, 5:46 a.m. UTC | #1
Hi,

On 2/28/25 14:34, Thorsten Glaser wrote:

> This is because (rightfully) the output got a new dependency…
>> $(MAK_DIR_GITWEB)static/gitweb.js: $(MAK_DIR_GITWEB)generate-gitweb-js.sh
> … but the rule uses $^ to append sources.

> Possible fix attached.

Would an order-only dependency also work?

     $(MAK_DIR_GITWEB)static/gitweb.js: | 
$(MAK_DIR_GITWEB)generate-gitweb-js.sh

    Simon
Thorsten Glaser Feb. 28, 2025, 6:34 a.m. UTC | #2
On Fri, 28 Feb 2025, Simon Richter wrote:

> Would an order-only dependency also work?
>
>    $(MAK_DIR_GITWEB)static/gitweb.js: | $(MAK_DIR_GITWEB)generate-gitweb-js.sh

Erk, that’s a gmake-ism. I have no idea, possibly. As a BSD make
person I consider use of $^ in nōn-suffix rules generally often
problematic…

Perhaps test if it indeed works; if it does, choose by whatever
is the project-desired style.

Thanks,
//mirabilos
Patrick Steinhardt Feb. 28, 2025, 7:02 a.m. UTC | #3
On Fri, Feb 28, 2025 at 06:34:43AM +0100, Thorsten Glaser wrote:
> From ed9863971d37ed53628a5871a4a569ccd6287f53 Mon Sep 17 00:00:00 2001
> From: mirabilos <tg@debian.org>
> Date: Fri, 28 Feb 2025 05:33:10 +0000
> Subject: [PATCH] Unbreak content of gitweb.js
> 
> The former $^ adds all prerequisites, including the
> (proper) new dependency on the generator script.

The commit message could use a bit of polishing. How about the
following:

    gitweb: fix generation of "gitweb.js"

    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 explicitly only passing the JavaScript files.

> Signed-off-by: mirabilos <tg@debian.org>

We typically require plain names instead of aliases in the SOB.

> ---
>  gitweb/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gitweb/Makefile b/gitweb/Makefile
> index d5748e9359..2a8f97cef8 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 $@+ $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_JSLIB_FILES)) && \
>  	mv $@+ $@

We could avoid repetition by filtering out any files that we don't care
about, like so:

    $(filter %.js,$^)

In any case, thanks for discovering and fixing this issue!

Patrick
diff mbox series

Patch

From ed9863971d37ed53628a5871a4a569ccd6287f53 Mon Sep 17 00:00:00 2001
From: mirabilos <tg@debian.org>
Date: Fri, 28 Feb 2025 05:33:10 +0000
Subject: [PATCH] Unbreak content of gitweb.js

The former $^ adds all prerequisites, including the
(proper) new dependency on the generator script.

Signed-off-by: mirabilos <tg@debian.org>
---
 gitweb/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gitweb/Makefile b/gitweb/Makefile
index d5748e9359..2a8f97cef8 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 $@+ $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_JSLIB_FILES)) && \
 	mv $@+ $@
 
 ### Installation rules
-- 
2.30.2