[PATCH 1/5] scripts/setlocalversion: remove mercurial, svn and git-svn supports

[PATCH 1/5] scripts/setlocalversion: remove mercurial, svn and git-svn supports

[Masahiro Yamada]

The mercurial, svn, git-svn supports were added by the following commits
without explaining why they are useful for the kernel source tree.

 - 3dce174cfcba ("kbuild: support mercurial in setlocalversion")

 - ba3d05fb6369 ("kbuild: add svn revision information to setlocalversion")

 - ff80aa97c9b4 ("setlocalversion: add git-svn support")

Let's revert all of them, and see if somebody will complain about it.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/setlocalversion | 41 -----------------------------------------
 1 file changed, 41 deletions(-)

diff --git a/scripts/setlocalversion b/scripts/setlocalversion
index db941f6d9591..879cba956e60 100755
--- a/scripts/setlocalversion
+++ b/scripts/setlocalversion
@@ -79,11 +79,6 @@ scm_version()
 			fi
 		fi
 
-		# Is this git on svn?
-		if git config --get svn-remote.svn.url >/dev/null; then
-			printf -- '-svn%s' "$(git svn find-rev $head)"
-		fi
-
 		# Check for uncommitted changes.
 		# First, with git-status, but --no-optional-locks is only
 		# supported in git >= 2.14, so fall back to git-diff-index if
@@ -96,42 +91,6 @@ scm_version()
 		} | grep -qvE '^(.. )?scripts/package'; then
 			printf '%s' -dirty
 		fi
-
-		# All done with git
-		return
-	fi
-
-	# Check for mercurial and a mercurial repo.
-	if test -d .hg && hgid=$(hg id 2>/dev/null); then
-		# Do we have an tagged version?  If so, latesttagdistance == 1
-		if [ "$(hg log -r . --template '{latesttagdistance}')" = "1" ]; then
-			id=$(hg log -r . --template '{latesttag}')
-			printf '%s%s' -hg "$id"
-		else
-			tag=$(printf '%s' "$hgid" | cut -d' ' -f2)
-			if [ -z "$tag" -o "$tag" = tip ]; then
-				id=$(printf '%s' "$hgid" | sed 's/[+ ].*//')
-				printf '%s%s' -hg "$id"
-			fi
-		fi
-
-		# Are there uncommitted changes?
-		# These are represented by + after the changeset id.
-		case "$hgid" in
-			*+|*+\ *) printf '%s' -dirty ;;
-		esac
-
-		# All done with mercurial
-		return
-	fi
-
-	# Check for svn and a svn repo.
-	if rev=$(LC_ALL=C svn info 2>/dev/null | grep '^Last Changed Rev'); then
-		rev=$(echo $rev | awk '{print $NF}')
-		printf -- '-svn%s' "$rev"
-
-		# All done with svn
-		return
 	fi
 }
 
-- 
2.27.0

[PATCH 2/5] scripts/setlocalversion: remove workaround for old make-kpkg

[Masahiro Yamada]

This reverts commit b052ce4c840e ("kbuild: fix false positive -dirty
tag caused by make-kpkg").

If I understand correctly, it occurred in very old versions of
make-kpkg. When I tried a newer version, make-kpkg did not touch
scripts/package/Makefile.

Anyway, Debian uses 'make deb-pkg' instead of make-kpkg these days.

Debian handbook [1] mentions it as "the good old days":
 "CULTURE The good old days of kernel-package

  Before the Linux build system gained the ability to build proper
  Debian packages, the recommended way to build such packages was to
  use make-kpkg from the kernel-package package."

[1]: https://debian-handbook.info/browse/stable/sect.kernel-compilation.html

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/setlocalversion | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/setlocalversion b/scripts/setlocalversion
index 879cba956e60..f3084d6bbb22 100755
--- a/scripts/setlocalversion
+++ b/scripts/setlocalversion
@@ -88,7 +88,7 @@ scm_version()
 		if {
 			git --no-optional-locks status -uno --porcelain 2>/dev/null ||
 			git diff-index --name-only HEAD
-		} | grep -qvE '^(.. )?scripts/package'; then
+		} | read dummy; then
 			printf '%s' -dirty
 		fi
 	fi
-- 
2.27.0

[PATCH 3/5] scripts/setlocalversion: add more comments to -dirty flag detection

[Masahiro Yamada]

This script stumbled on the read-only source tree over again:

 - a2bb90a08cb3 ("kbuild: fix delay in setlocalversion on readonly
   source")

 - cdf2bc632ebc ("scripts/setlocalversion on write-protected source
   tree")

 - 8ef14c2c41d9 ("Revert "scripts/setlocalversion: git: Make -dirty
   check more robust"")

 - ff64dd485730 ("scripts/setlocalversion: Improve -dirty check with
   git-status --no-optional-locks")

Add comments to clarify that this script should never ever try to write
to the source tree.

'git describe --dirty' might look as a simple solution for appending
the -dirty string, but we cannot use it because it creates the
.git/index.lock file.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/setlocalversion | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/scripts/setlocalversion b/scripts/setlocalversion
index f3084d6bbb22..6865df6699c8 100755
--- a/scripts/setlocalversion
+++ b/scripts/setlocalversion
@@ -80,6 +80,10 @@ scm_version()
 		fi
 
 		# Check for uncommitted changes.
+		# This script must avoid any write attempt to the source tree,
+		# which might be read-only.
+		# You cannot use 'git describe --dirty' because it tries to
+		# create .git/index.lock .
 		# First, with git-status, but --no-optional-locks is only
 		# supported in git >= 2.14, so fall back to git-diff-index if
 		# it fails. Note that git-diff-index does not refresh the
-- 
2.27.0

[PATCH 4/5] scripts/setlocalversion: factor out 12-chars hash construction

[Masahiro Yamada]

Both of if and else parts append exactly 12 hex chars, but in
different ways.

Factor out the else part because we need to support it without relying
on git-describe. Remove the --abbrev=12 option since we do not use the
hash from git-describe anyway.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/setlocalversion | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/scripts/setlocalversion b/scripts/setlocalversion
index 6865df6699c8..62c0bcce1575 100755
--- a/scripts/setlocalversion
+++ b/scripts/setlocalversion
@@ -59,24 +59,12 @@ scm_version()
 			fi
 			# If we are past a tagged commit (like
 			# "v2.6.30-rc5-302-g72357d5"), we pretty print it.
-			#
-			# Ensure the abbreviated sha1 has exactly 12
-			# hex characters, to make the output
-			# independent of git version, local
-			# core.abbrev settings and/or total number of
-			# objects in the current repository - passing
-			# --abbrev=12 ensures a minimum of 12, and the
-			# awk substr() then picks the 'g' and first 12
-			# hex chars.
-			if atag="$(git describe --abbrev=12 2>/dev/null)"; then
-				echo "$atag" | awk -F- '{printf("-%05d-%s", $(NF-1),substr($(NF),0,13))}'
-
-			# If we don't have a tag at all we print -g{commitish},
-			# again using exactly 12 hex chars.
-			else
-				head="$(echo $head | cut -c1-12)"
-				printf '%s%s' -g $head
+			if atag="$(git describe 2>/dev/null)"; then
+				echo "$atag" | awk -F- '{printf("-%05d", $(NF-1))}'
 			fi
+
+			# Add -g and exactly 12 hex chars.
+			printf '%s%s' -g "$(echo $head | cut -c1-12)"
 		fi
 
 		# Check for uncommitted changes.
-- 
2.27.0

Re: [PATCH 4/5] scripts/setlocalversion: factor out 12-chars hash construction

[Nico Schottelius]

Hey Masahiro,

Masahiro Yamada <masahiroy@kernel.org> writes:
> -			if atag="$(git describe --abbrev=12 2>/dev/null)"; then
> -				echo "$atag" | awk -F- '{printf("-%05d-%s", $(NF-1),substr($(NF),0,13))}'
> -
> -			# If we don't have a tag at all we print -g{commitish},
> -			# again using exactly 12 hex chars.
> -			else
> -				head="$(echo $head | cut -c1-12)"
> -				printf '%s%s' -g $head
> +			if atag="$(git describe 2>/dev/null)"; then
> +				echo "$atag" | awk -F- '{printf("-%05d", $(NF-1))}'
>  			fi
> +
> +			# Add -g and exactly 12 hex chars.
> +			printf '%s%s' -g "$(echo $head | cut -c1-12)"
>  		fi
>
>  		# Check for uncommitted changes.

That was quite fun reviewing and wrapping my head around ~20y old code.

I had a very long mail prepared looking at all the corner cases, just to
see they are actually handled before the actual change.

Nicely simplified, for all 5 of them:

Reviewed-by: Nico Schottelius <nico-linuxsetlocalversion@schottelius.org>

--
Sustainable and modern Infrastructures by ungleich.ch

[PATCH 5/5] scripts/setlocalversion: simplify the short version part

[Masahiro Yamada]

Reduce the indentation.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/setlocalversion | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/scripts/setlocalversion b/scripts/setlocalversion
index 62c0bcce1575..151f04971faa 100755
--- a/scripts/setlocalversion
+++ b/scripts/setlocalversion
@@ -131,15 +131,13 @@ res="${res}${CONFIG_LOCALVERSION}${LOCALVERSION}"
 if test "$CONFIG_LOCALVERSION_AUTO" = "y"; then
 	# full scm version string
 	res="$res$(scm_version)"
-else
+elif [ -z "${LOCALVERSION}" ]; then
 	# append a plus sign if the repository is not in a clean
 	# annotated or signed tagged state (as git describe only
 	# looks at signed or annotated tags - git tag -a/-s) and
 	# LOCALVERSION= is not specified
-	if test "${LOCALVERSION+set}" != "set"; then
-		scm=$(scm_version --short)
-		res="$res${scm:++}"
-	fi
+	scm=$(scm_version --short)
+	res="$res${scm:++}"
 fi
 
 echo "$res"
-- 
2.27.0

Re: [PATCH 1/5] scripts/setlocalversion: remove mercurial, svn and git-svn supports

[Greg Kroah-Hartman]

On Sun, May 23, 2021 at 12:14:24PM +0900, Masahiro Yamada wrote:
> The mercurial, svn, git-svn supports were added by the following commits
> without explaining why they are useful for the kernel source tree.
> 
>  - 3dce174cfcba ("kbuild: support mercurial in setlocalversion")
> 
>  - ba3d05fb6369 ("kbuild: add svn revision information to setlocalversion")
> 
>  - ff80aa97c9b4 ("setlocalversion: add git-svn support")
> 
> Let's revert all of them, and see if somebody will complain about it.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 

For all 5 in this series:

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH 1/5] scripts/setlocalversion: remove mercurial, svn and git-svn supports

[Masahiro Yamada]

On Sun, May 23, 2021 at 4:48 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Sun, May 23, 2021 at 12:14:24PM +0900, Masahiro Yamada wrote:
> > The mercurial, svn, git-svn supports were added by the following commits
> > without explaining why they are useful for the kernel source tree.
> >
> >  - 3dce174cfcba ("kbuild: support mercurial in setlocalversion")
> >
> >  - ba3d05fb6369 ("kbuild: add svn revision information to setlocalversion")
> >
> >  - ff80aa97c9b4 ("setlocalversion: add git-svn support")
> >
> > Let's revert all of them, and see if somebody will complain about it.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
> >
>
> For all 5 in this series:
>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>


Applied to linux-kbuild.

-- 
Best Regards
Masahiro Yamada