From: Will McVicker <willmcvicker@google.com>
To: Jessica Yu <jeyu@kernel.org>
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: Wed, 16 Dec 2020 14:08:30 -0800 [thread overview]
Message-ID: <X9qFXuNR2B2fYVk3@google.com> (raw)
In-Reply-To: <20201211153359.GA19348@linux-8ccs>
On Fri, Dec 11, 2020 at 04:33:59PM +0100, Jessica Yu wrote:
> 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
> >
>
> --
> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
>
Hi Jessica,
I'm okay with dropped this first patch since it does only related to external
modules. I'll upload v4 now with just the bits that related to in-tree modules.
Thanks,
Will
next prev parent reply other threads:[~2020-12-16 22:09 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
2020-12-16 22:08 ` Will McVicker [this message]
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=X9qFXuNR2B2fYVk3@google.com \
--to=willmcvicker@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=hch@infradead.org \
--cc=jeyu@kernel.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 \
/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).