Subject: bug#74413: [PATCH] Allow to store and read repository information of VCS builds

From: Eli Zaretskii <[email-masked]>
Date: Mon, 18 Nov 2024 14:45:27 +0200

> Date: Mon, 18 Nov 2024 10:18:11 +0200
> From:  Björn Bidar via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <[email-masked]>
> 
> Emacs has the feature to read the repository version and
> branch if git is installed during the build and afterwards if also
> the sources including the VCS repository is present.
> 
> For the Android builds the feature was added to store and read the
> information mentioned above in a special version file.

Po Lu, why is that needed in the Android build, and how is it used
there?

> This patch reuses that mechanism so it can be reused on other platforms
> to use for the same reasons its available for Android and to also be
> able to use the information on CI workers without git installed and/or a
> full source checkout.

Doesn't that go against the tendency to have _less_ detailed/private
information in the build?  We've lately removed some relatively useful
infos from what we report in commands that use the build information.

More generally, could you present the motivation and the rationale for
making this information available in production builds?

> The things I'm not sure about for this patch are:
> - Is the generating of the version file in the right place in
> Makefile.in

It should be in the build tree, yes.

> - Is the data directory the right place to store the file

Not sure, but I don't see why not.

> - Should the creation of the version file be shared between the Android
>   builds and the other platforms

Let's first discuss whether this is at all needed and a good idea,
okay?

> -	${INSTALL_PROGRAM} $(INSTALL_STRIP) src/emacs${EXEEXT} "$(DESTDIR)${bindir}/$(EMACSFULL)"
> +	${INSTALL_PROGRAM} $(INSTALL_STRIP) src/emacs${EXEEXT} "$(DESTDIR)${bindir}/$(EMACS)"

Why this change (and other similar ones)?  What does EMACSFULL have to
do with the repository version data?

> @@ -826,6 +837,7 @@ install-man:
>  	umask 022; ${MKDIR_P} "$(DESTDIR)${man1dir}"
>  	thisdir=`pwd -P`; \
>  	cd ${mansrcdir}; \
> +	cp ctags.1 gnuctags.1; \

This hunk also looks unrelated.