lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <X9qFXuNR2B2fYVk3@google.com>
Date:   Wed, 16 Dec 2020 14:08:30 -0800
From:   Will McVicker <willmcvicker@...gle.com>
To:     Jessica Yu <jeyu@...nel.org>
Cc:     Masahiro Yamada <masahiroy@...nel.org>,
        Michal Marek <michal.lkml@...kovi.net>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Christoph Hellwig <hch@...radead.org>,
        Saravana Kannan <saravanak@...gle.com>,
        linux-kernel@...r.kernel.org, linux-kbuild@...r.kernel.org,
        kernel-team@...roid.com
Subject: Re: [PATCH v3 1/2] scripts/setlocalversion: allow running in a subdir

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@...il.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@...gle.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@...roid.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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ