linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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
Date: Fri, 11 Dec 2020 16:33:59 +0100	[thread overview]
Message-ID: <20201211153359.GA19348@linux-8ccs> (raw)
In-Reply-To: <20201208200508.4107399-2-willmcvicker@google.com>

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
>

  reply	other threads:[~2020-12-11 16:31 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-21  1:16 [PATCH v1 0/2] Add support to capture external module's SCM version Will McVicker
2020-11-21  1:16 ` [PATCH v1 1/2] scripts/setlocalversion: allow running in a subdir Will McVicker
2020-11-21  1:16 ` [PATCH v1 2/2] modules: add scmversion field Will McVicker
2020-11-23  9:30   ` Greg KH
2020-11-23  9:32   ` Greg KH
2020-11-23  9:02 ` [PATCH v1 0/2] Add support to capture external module's SCM version Christoph Hellwig
2020-11-23 22:13   ` William Mcvicker
2020-11-24  9:31     ` Jessica Yu
2020-11-24 18:05       ` William Mcvicker
2020-11-24 18:12         ` Greg Kroah-Hartman
2020-11-24 18:31           ` William Mcvicker
2020-11-24 20:24             ` Greg Kroah-Hartman
2020-11-24 20:40               ` William Mcvicker
2020-11-24 20:45                 ` Saravana Kannan
2020-11-25  1:05                   ` [PATCH v2 0/2] Adds support to capture " Will McVicker
2020-11-25  1:05                     ` [PATCH v2 1/2] scripts/setlocalversion: allow running in a subdir Will McVicker
2020-11-25  1:05                     ` [PATCH v2 2/2] modules: add scmversion field Will McVicker
2020-12-07 15:31                       ` Jessica Yu
2020-12-08 20:05                         ` [PATCH v3 0/2] " Will McVicker
2020-12-08 20:05                           ` [PATCH v3 1/2] scripts/setlocalversion: allow running in a subdir Will McVicker
2020-12-11 15:33                             ` Jessica Yu [this message]
2020-12-16 22:08                               ` Will McVicker
2020-12-08 20:05                           ` [PATCH v3 2/2] modules: introduce the MODULE_SCMVERSION config Will McVicker
2020-12-04  0:36                     ` [PATCH v2 0/2] Adds support to capture module's SCM version William Mcvicker
2020-12-04  7:51                       ` Christoph Hellwig
2020-12-04 18:13                         ` Will McVicker
2020-12-04 18:18                           ` Christoph Hellwig
2020-12-04 18:20                             ` Will McVicker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201211153359.GA19348@linux-8ccs \
    --to=jeyu@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hch@infradead.org \
    --cc=kernel-team@android.com \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masahiroy@kernel.org \
    --cc=michal.lkml@markovi.net \
    --cc=saravanak@google.com \
    --cc=willmcvicker@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).