Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 11 Dec 2020 16:33:59 +0100
From:   Jessica Yu <jeyu@kernel.org>
To:     Will McVicker <willmcvicker@google.com>
Cc:     Masahiro Yamada <masahiroy@kernel.org>,
        Michal Marek <michal.lkml@markovi.net>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Christoph Hellwig <hch@infradead.org>,
        Saravana Kannan <saravanak@google.com>,
        linux-kernel@vger.kernel.org, linux-kbuild@vger.kernel.org,
        kernel-team@android.com
Subject: Re: [PATCH v3 1/2] scripts/setlocalversion: allow running in a subdir
Message-ID: <20201211153359.GA19348@linux-8ccs>
References: <20201207153116.GA15772@linux-8ccs>
 <20201208200508.4107399-1-willmcvicker@google.com>
 <20201208200508.4107399-2-willmcvicker@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <20201208200508.4107399-2-willmcvicker@google.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

Hi Will,

+++ Will McVicker [08/12/20 20:05 +0000]:
>Getting the scmversion using scripts/setlocalversion currently only
>works when run at the root of a git or mecurial project. This was
>introduced in commit 8558f59edf93 ("setlocalversion: Ignote SCMs above
>the linux source tree") so that if one is building within a subdir of
>a git tree that isn't the kernel git project, then the vermagic wouldn't
>include that git sha1. However, the proper solution to that is to just
>set this config in your defconfig:
>
>  # CONFIG_LOCALVERSION_AUTO is not set
>
>which is already the default in many defconfigs:
>
>  $ grep -r "CONFIG_LOCALVERSION_AUTO is not set" arch/* | wc -l
>  89
>
>So let's bring back this functionality so that we can use
>scripts/setlocalversion to capture the SCM version of external modules
>that reside within subdirectories of an SCM project.

Hm, this seems to essentially be a revert of commit 8558f59edf93.
AFAICT from light testing it also reintroduces the issue it was
originally trying to fix, no?

From the reporter:

    Dan McGee <dpmcgee@gmail.com> writes:
    > Note that when in git, you get the appended "+" sign. If
    > LOCALVERSION_AUTO is set, you will get something like
    > "eee-gb01b08c-dirty" (whereas the copy of the tree in /tmp still
    > returns "eee"). It doesn't matter whether the working tree is dirty or
    > clean.
    >
    > Is there a way to disable this? I'm building from a clean tarball that
    > just happens to be unpacked inside a git repository. One would think
    > setting LOCALVERSION_AUTO to false would do it, but no such luck...

Correct me if I'm wrong, but what I'm understanding is that the
original reporter was having trouble with setlocalversion appending
unwanted strings ("+" or "gXXXXXXX-dirty" etc) when building from a
clean tarball that happens to live inside a git repo. Even if
LOCALVERSION_AUTO is disabled it still appends the "+" string if the
SCM above the linux source tree is not at an annotated tag. Since
setlocalversion is getting confused by the presence of a different scm
that commit fixed this by confining the checks to the root of the
(possibly git managed) kernel source tree. Masahiro can probably
better comment since he maintains scripts/*.

In any case, this patch isn't of interest to in-tree modules, since we
can generate the scmversion perfectly fine without it, so I doubt it's
going to get any support here. Would you be fine with dropping the
first patch or would that pose issues?

>Signed-off-by: Will McVicker <willmcvicker@google.com>
>---
> scripts/setlocalversion | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
>diff --git a/scripts/setlocalversion b/scripts/setlocalversion
>index bb709eda96cd..cd42009e675b 100755
>--- a/scripts/setlocalversion
>+++ b/scripts/setlocalversion
>@@ -44,8 +44,7 @@ scm_version()
> 	fi
>
> 	# Check for git and a git repo.
>-	if test -z "$(git rev-parse --show-cdup 2>/dev/null)" &&
>-	   head=$(git rev-parse --verify HEAD 2>/dev/null); then
>+	if head=$(git rev-parse --verify HEAD 2>/dev/null); then
>
> 		# If we are at a tagged commit (like "v2.6.30-rc6"), we ignore
> 		# it, because this version is defined in the top level Makefile.
>@@ -102,7 +101,7 @@ scm_version()
> 	fi
>
> 	# Check for mercurial and a mercurial repo.
>-	if test -d .hg && hgid=$(hg id 2>/dev/null); then
>+	if 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}')
>-- 
>2.29.2.576.ga3fc446d84-goog
>