Support for the Meson build system #9

pks-t · 2025-03-11T11:01:39Z

Similar to the recent MR in gitk, this patch series wires up support for Meson in git-gui. Once landed, I'll send a patch series to Git to wire up both of these over there.

pks-t · 2025-03-11T11:03:31Z

I've tested the build instructions on NixOS, Windows 10 and (ancient) macOS 10.15 and have compared results between old/new Makefile results as well as Make and Meson. There are some minor differences around injected paths in Meson because we use the dynamically derived paths there. But other than that results looked mostly the same to me.

j6t · 2025-03-11T19:03:13Z

Thank you. It will take some time until I can tend to this. Please bear with me.

pks-t · 2025-03-11T19:10:27Z

Thank you. It will take some time until I can tend to this. Please bear with me.

No worries, take your time, there is no urgency here. Thanks!

j6t · 2025-05-10T10:31:48Z

I started looking into this series, but then got side-tracked and forgot to follow up, hence, it's not included in my recent Git PR. My apologies for the delay.

j6t

If the problem is "the format of GIT-GUI-VARS does not allow us to propagate those build options to other scripts", why is this patch so much more than just "let's write it in a more usable format"? Why do we need a template file? What is the reason to rename the file?

j6t · 2025-05-11T18:08:47Z

GIT-VERSION-GEN

@@ -60,21 +74,21 @@ else
 	VN="$DEF_VER"
 fi

-dirty=$(sh -c 'git diff-index --name-only HEAD' 2>/dev/null) || dirty=
+dirty=$(sh -c "git -C '$SOURCE_DIR' diff-index --name-only HEAD" 2>/dev/null) || dirty=


This change isn't kosher because single-quotes in $SOURCE_DIR aren't quoted. I don't see why we have to rungit in sh -c in the first place. Just run it in $() like we do everywhere else, including the redirection.

Good point, fixed.

j6t · 2025-05-11T18:10:11Z

GIT-VERSION-GEN

 else
 	VC=unset
 fi
 test "$VN" = "$VC" || {
 	echo >&2 "GITGUI_VERSION = $VN"
-	echo "GITGUI_VERSION = $VN" >$GVF
+	echo "GITGUI_VERSION = $VN" >$OUTPUT
 }


This would be a good time to put all "$OUTPUT" in double-quotes.

Yup, makes sense indeed.

j6t · 2025-05-11T18:25:06Z

Makefile

-	$(QUIET_INDEX)if echo \
-	  $(foreach p,$(PRELOAD_FILES),source $p\;) \
-	  auto_mkindex lib $(patsubst lib/%,%,$(sort $(ALL_LIBFILES))) \
-	| $(TCL_PATH) $(QUIET_2DEVNULL); then : ok; \


This removal obsoletes Makefile variable QUIET_2DEVNULL.

Good catch, fixed.

j6t · 2025-05-11T18:26:07Z

Makefile

-	 echo >>$@ ; \
-	fi
+lib/tclIndex: $(ALL_LIBFILES) generate-tclindex.sh GIT-GUI-BUILD-OPTIONS
+	$(QUIET_INDEX)$(SHELL_PATH) -x generate-tclindex.sh . ./GIT-GUI-BUILD-OPTIONS $(ALL_LIBFILES)


-x is left over from debugging and not intended to remain, I assume?

Oh, indeed, good catch.

j6t · 2025-05-11T18:32:14Z

Have to stop for today. Still need to go through the last 4 commits.

pks-t · 2025-05-12T09:32:42Z

If the problem is "the format of GIT-GUI-VARS does not allow us to propagate those build options to other scripts", why is this patch so much more than just "let's write it in a more usable format"? Why do we need a template file? What is the reason to rename the file?

There are multiple reasons:

It makes our life significantly easier with Meson, as we can then trivially use configure_file() to perform this operation.
Meson knows to warn about unreplaced variables, which gives us useful signal in case this ever were to break due to a new variable that we don't modify in Meson.
It makes it overall way more discoverable what variables we expect to exist.
It brings us in line with what downstream Git is using nowadays for a bit more consistency.

j6t · 2025-05-13T06:15:49Z

Makefile

@@ -322,7 +318,7 @@ dist-version:

 clean::
 	$(RM_RF) $(GITGUI_MAIN) lib/tclIndex po/*.msg $(PO_TEMPLATE)
-	$(RM_RF) GIT-VERSION-FILE GIT-GUI-VARS
+	$(RM_RF) GIT-VERSION-FILE GIT-GUI-VARS GIT-GUI-BUILD-OPTIONS


This also cleans up GIT-GUI-VARS, which is not used anymore. It was not in the earlier round. Is this a deliberate decision or a glitched conflict resolution?

The latter. Fixed now.

The GIT-GUI-VARS file is used to track whether any of our build options has changed. Unfortunately, the format of that file does not allow us to propagate those build options to other scripts. But as we are about to introduce support for the Meson build system, we will extract a couple of scripts to deduplicate core build logic across Makefiles and Meson. With this refactoring, it will become necessary to make build options more widely accessible. Replace GIT-GUI-VARS with a new GIT-GUI-BUILD-OPTIONS file that is being populated from a template. This file can easily be sourced from build scripts in subsequent steps. Signed-off-by: Patrick Steinhardt <ps@pks.im>

The GIT-VERSION-GEN unconditionally writes version information into the source directory in the form of the "GIT-VERSION-FILE". We are about to introduce the Meson build system though, which enforces out-of-tree builds by default, and in that context we cannot continue to write version information into the source tree. Prepare the script for out-of-tree builds by treating the source directory different from the output file. Signed-off-by: Patrick Steinhardt <ps@pks.im>

The output of GIT-VERSION-GEN can be sourced by our Makefile to make the version available there. The output has a couple of spaces around the equals sign, which is perfectly valid for parsing it in our Makefile. But in subsequent steps we'll also want to source the file in a couple of newly-introduced shell scripts, but having spaces around variable assignments is invalid there. Prepare for this step by dropping the spaces surrounding the equals sign. Like this, we can easily use the same file both in our Makefile and in shell scripts. Signed-off-by: Patrick Steinhardt <ps@pks.im>

The value of the GITGUI_SCRIPT variable is only used in a single place as part of an sed(1) script that massages the "git-gui.sh" script. Interestingly, this specific replacement does seem to be a no-op: we replace "^ argv0=$$0" with " argv=$(GITGUI_SCRIPT)", which has a value of "$$0". The result would thus be completely unchanged. Drop the replacement and its variable. Signed-off-by: Patrick Steinhardt <ps@pks.im>

Extract script to generate "git-gui". This change allows us to reuse the build logic with the Meson build system. Signed-off-by: Patrick Steinhardt <ps@pks.im>

Extract script to generate "tclIndex". This change allows us to reuse the build logic with the Meson build system. Signed-off-by: Patrick Steinhardt <ps@pks.im>

j6t · 2025-05-13T06:32:58Z

generate-macos-wrapper.sh

+	echo 'git-gui version $GITGUI_VERSION'
+else
+	libdir="\${GIT_GUI_LIB_DIR:-$GITGUI_LIBDIR}"
+	exec "\$libdir/Git Gui.app/Contents/MacOS/$TKEXECUTABLE" "\$0" "\$@"


We have quite a few levels of quoting here. The values of $GITGUI_LIBDIR and $TKEXECUTABLE are taken from the file $BUILD_OPTIONS, but are not protected against meta-characters that could be interpreted as part of the shell syntax.

I suggest to construct the file by including the contents of $BUILD_OPTIONS and $VERSION_FILE:

( echo "#!$SHELL_PATH" cat "$BUILD_OPTIONS" "$VERSION_FILE" - <<-'EOF' ... rest as above without the extra quoting plus dq around "$GITGUI_VERSION" EOF ) >"$OUTPUT"

That reads much better indeed, thanks!

j6t · 2025-05-13T06:45:16Z

generate-macos-app.sh

+	-e "s|@@gitexecdir@@|$GITGUI_GITEXECDIR|" \
+	-e "s|@@GITGUI_LIBDIR@@|$GITGUI_LIBDIR|" \
+	"$SOURCE_DIR/macosx/AppMain.tcl" \
+	>"$OUTPUT"+/Contents/Resources/Scripts/AppMain.tcl


Not worth a reroll by itself, but everywhere else double-quotes surround the entire word "$OUTPUT+/more/stuff".

Fixed, as well.

Extract script to generate the macOS wrapper for git-gui. This change allows us to reuse the build logic with the Meson build system. Signed-off-by: Patrick Steinhardt <ps@pks.im>

Extract script to generate the macOS app. This change allows us to reuse the build logic with the Meson build system. Note that as part of this change we also modify the TKEXECUTABLE variable to track its full path. Like this we don't have to propagate both the TKEXECUTABLE and TKFRAMEWORK variables into the script, and the basename can be trivially computed from TKEXECUTABLE anyway. Signed-off-by: Patrick Steinhardt <ps@pks.im>

The "GITGUI_VERSION" variable is made available by generating and including the "GIT-VERSION-FILE" file. Its value has been used in various build steps, but in the preceding commits we have refactored those to instead source the "GIT-VERSION-FILE" directly. As a result, the variable is now only used in a single recipe, and this use can be trivially replaced with sed(1). Refactor the recipe to do so and stop including "GIT-VERSION-FILE" to simplify the build process. Signed-off-by: Patrick Steinhardt <ps@pks.im>

The Git project has started to wire up Meson as a build system in Git v2.48.0. Wire up support for Meson in "git-gui" so that we can trivially include it as a subproject in Git. Signed-off-by: Patrick Steinhardt <ps@pks.im>

j6t · 2025-05-13T17:23:38Z

This round looks good.

I tested the scripts in a basic fashion, and they look good. I don't have MacOS to test, but I think that the transcription of the old Makefile instructions are faithful.

I haven't tested the meson build, though.

Do you want this version pulled into upstream Git early (before 2.50)?

pks-t · 2025-05-14T02:19:47Z

I haven't tested the meson build, though.

I did, for all I can see it works alright :) Same caveat though, I didn't test on macOS.

Do you want this version pulled into upstream Git early (before 2.50)?

This isn't time critical in any way, so I'll let you decide when you create the upstream pull request.

Thanks for your review!

j6t reviewed May 11, 2025

View reviewed changes

pks-t force-pushed the pks-meson-support branch from c6b6131 to acd8451 Compare May 12, 2025 09:32

pks-t requested a review from j6t May 12, 2025 09:38

j6t reviewed May 13, 2025

View reviewed changes

pks-t added 6 commits May 13, 2025 08:27

git-gui: extract script to generate "git-gui"

854e883

Extract script to generate "git-gui". This change allows us to reuse the build logic with the Meson build system. Signed-off-by: Patrick Steinhardt <ps@pks.im>

git-gui: extract script to generate "tclIndex"

2cc5b0f

Extract script to generate "tclIndex". This change allows us to reuse the build logic with the Meson build system. Signed-off-by: Patrick Steinhardt <ps@pks.im>

j6t reviewed May 13, 2025

View reviewed changes

pks-t added 4 commits May 13, 2025 08:45

git-gui: extract script to generate macOS wrapper

743e1cb

Extract script to generate the macOS wrapper for git-gui. This change allows us to reuse the build logic with the Meson build system. Signed-off-by: Patrick Steinhardt <ps@pks.im>

git-gui: wire up support for the Meson build system

8bf062d

The Git project has started to wire up Meson as a build system in Git v2.48.0. Wire up support for Meson in "git-gui" so that we can trivially include it as a subproject in Git. Signed-off-by: Patrick Steinhardt <ps@pks.im>

pks-t force-pushed the pks-meson-support branch from acd8451 to 8bf062d Compare May 13, 2025 07:00

j6t merged commit 61f8788 into j6t:master May 29, 2025

Support for the Meson build system #9

Support for the Meson build system #9

Uh oh!

Conversation

pks-t commented Mar 11, 2025

Uh oh!

pks-t commented Mar 11, 2025

Uh oh!

j6t commented Mar 11, 2025

Uh oh!

pks-t commented Mar 11, 2025

Uh oh!

j6t commented May 10, 2025

Uh oh!

j6t left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

j6t commented May 11, 2025

Uh oh!

pks-t commented May 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

j6t commented May 13, 2025

Uh oh!

pks-t commented May 14, 2025

Uh oh!

Uh oh!