All of lore.kernel.org
 help / color / mirror / Atom feed
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

  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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.