From 8ba142c3dc9c468e010e60e78cba269c96b66be5 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Fri, 26 May 2023 12:17:19 -0400 Subject: [PATCH] build: gen-version-i.sh: make it an error if git describe fails When doing `make` as a regular user, building from the git repo, the src/common/version.i file may contain: #define BT_VERSION_GIT "v1.2.0-3717-ga7074326ba9a" Then, doing `make` as root, when logged in with `su -`, generates a new version.i with an empty version string: #define BT_VERSION_GIT "" This is because the `git describe --tags --dirty --abbrev=12` command fails with: make[1]: Entering directory '/home/smarchi/build/babeltrace/src/common' GEN version.i fatal: detected dubious ownership in repository at '/home/smarchi/src/babeltrace' To add an exception for this directory, call: git config --global --add safe.directory /home/smarchi/src/babeltrace By default, git refuses to work on a git repository owned by a different user. Note that running a command under sudo doesn't hit this problem, because git honors the SUDO_UID & co variables. I think that the current state is buggy, because it results in the installed library having an empty version string, and it happens almost silently. I suggest making it an error if that `git describe` command fails, since the script can't work correctly in those conditions. The user would then need to adjust their setup: either add the directory to git's safe.directory variable (as the git command suggested) or run the command under sudo. Doing "make install" as root not under sudo is one use case, but another one would be to do the complete build as a regular user from a source git repository that is owned by another reguar user. In that case, the script wouldn't be able to generate a version string in the first place. I think that the right thing to do is to error out. And in that case, the only solution I see if or the user to use the git safe.directory variable. Note that the second git describe command is expected to fail in normal situations, when it doesn't find an exact tag match, so the `|| true` there is fine. Change-Id: I879ddc2184223266829afceca252fe971795f395 Signed-off-by: Simon Marchi Fixes: https://bugs.lttng.org/issues/1376 Reviewed-on: https://review.lttng.org/c/babeltrace/+/10104 Tested-by: jenkins --- src/common/gen-version-i.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/gen-version-i.sh b/src/common/gen-version-i.sh index aa7d5b3e..938d70c5 100755 --- a/src/common/gen-version-i.sh +++ b/src/common/gen-version-i.sh @@ -42,7 +42,7 @@ fi # configurations leading to different results. if test -r "$TOP_SRCDIR/bootstrap" && test -r "$TOP_SRCDIR/.git" && (command -v git > /dev/null 2>&1); then - GIT_VERSION_STR="$(cd "$TOP_SRCDIR" && (git describe --tags --dirty --abbrev=12 || true))" + GIT_VERSION_STR="$(cd "$TOP_SRCDIR" && git describe --tags --dirty --abbrev=12)" GIT_CURRENT_TAG="$(cd "$TOP_SRCDIR" && (git describe --tags --exact-match --match="v[0-9]*" HEAD || true) 2> /dev/null)" echo "#define BT_VERSION_GIT \"$GIT_VERSION_STR\"" > version.i.tmp -- 2.34.1