2013-09-13 21:50:28

by afzal mohammed

[permalink] [raw]
Subject: [PATCH RFC] kbuild: prevent git private tag altering kernelrelease

If a private tag is created after the most recent kernelversion tag, a
commit after this private tag would feed kernelrelease with commits
after private tag and kernelversion tag. This may confuse user relying
on kernelrelease (mostly a developer while debugging), mainly if HEAD
has a private tag and otherwise w.r.t git distance from kernelversion
tag.

Suppose the Kernel is 18 commits after v3.11-rc1, kernelrelease would be
something like '3.11.0-rc1-00018-gdeadbeef'.

And if an annotated(or signed) tag is created at HEAD, kernelrelease
would become 3.11.0-rc1, which makes one feel that source corresponds to
v3.11-rc1 tag.

Instead if such a tag is created at say HEAD~, kernelrelease would be
v3.11.0-rc1-00001-gdeadbeef, misleading the observer w.r.t git distance
from nearest kernelversion tag.

Here an attempt is made to prevent private tag from altering
kernelrelease.

Signed-off-by: Afzal Mohammed <[email protected]>
---

Hi,

This seems to work on different scenarios that could be readily thought
of. I am shaky about this change, but acheives the purpose.

Regards
Afzal

Makefile | 4 ++--
scripts/setlocalversion | 12 ++++++++----
2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index e73f758..923e492 100644
--- a/Makefile
+++ b/Makefile
@@ -795,7 +795,7 @@ $(vmlinux-dirs): prepare scripts
$(Q)$(MAKE) $(build)=$@

define filechk_kernel.release
- echo "$(KERNELVERSION)$$($(CONFIG_SHELL) $(srctree)/scripts/setlocalversion $(srctree))"
+ echo "$(KERNELVERSION)$$($(CONFIG_SHELL) $(srctree)/scripts/setlocalversion $(srctree) $(KERNELVERSION))" > $@
endef

# Store (new) KERNELRELEASE string in include/config/kernel.release
@@ -1330,7 +1330,7 @@ checkstack:
$(PERL) $(src)/scripts/checkstack.pl $(CHECKSTACK_ARCH)

kernelrelease:
- @echo "$(KERNELVERSION)$$($(CONFIG_SHELL) $(srctree)/scripts/setlocalversion $(srctree))"
+ @echo "$(KERNELVERSION)$$($(CONFIG_SHELL) $(srctree)/scripts/setlocalversion $(srctree) $(KERNELVERSION))"

kernelversion:
@echo $(KERNELVERSION)
diff --git a/scripts/setlocalversion b/scripts/setlocalversion
index d105a44..2c4552c 100755
--- a/scripts/setlocalversion
+++ b/scripts/setlocalversion
@@ -10,7 +10,7 @@
#

usage() {
- echo "Usage: $0 [--save-scmversion] [srctree]" >&2
+ echo "Usage: $0 [--save-scmversion] [srctree] [kernelversion]" >&2
exit 1
}

@@ -24,7 +24,11 @@ if test $# -gt 0; then
srctree=$1
shift
fi
-if test $# -gt 0 -o ! -d "$srctree"; then
+if test $# -gt 0; then
+ kernelversion=`echo $1 | sed -e 's/[.]0//' -e 's/^/v/'`
+ shift
+fi
+if test $# -gt 0 -o ! -d "$srctree" -o -z "$kernelversion"; then
usage
fi

@@ -47,7 +51,7 @@ scm_version()

# 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.
- if [ -z "`git describe --exact-match 2>/dev/null`" ]; then
+ if [ -z "`git describe --exact-match --match $kernelversion 2>/dev/null`" ]; then

# If only the short version is requested, don't bother
# running further git commands
@@ -57,7 +61,7 @@ scm_version()
fi
# If we are past a tagged commit (like
# "v2.6.30-rc5-302-g72357d5"), we pretty print it.
- if atag="`git describe 2>/dev/null`"; then
+ if atag="`git describe --match $kernelversion 2>/dev/null`"; then
echo "$atag" | awk -F- '{printf("-%05d-%s", $(NF-1),$(NF))}'

# If we don't have a tag at all we print -g{commitish}.
--
1.8.2.135.g7b592fa


2013-10-23 13:21:08

by Michal Marek

[permalink] [raw]
Subject: Re: [PATCH RFC] kbuild: prevent git private tag altering kernelrelease

Hi Afzal,

sorry for the late feedback.

On 13.9.2013 23:49, Afzal Mohammed wrote:
> If a private tag is created after the most recent kernelversion tag, a
> commit after this private tag would feed kernelrelease with commits
> after private tag and kernelversion tag. This may confuse user relying
> on kernelrelease (mostly a developer while debugging), mainly if HEAD
> has a private tag and otherwise w.r.t git distance from kernelversion
> tag.

The solution is simple: Do not use private annotated tags. Or rather, if
you are creating an annotated tag, modify EXTRAVERSION accordingly. Any
automagic based on the tag name is going to fail in some way.


> Instead if such a tag is created at say HEAD~, kernelrelease would be
> v3.11.0-rc1-00001-gdeadbeef, misleading the observer w.r.t git distance
> from nearest kernelversion tag.
>
> Here an attempt is made to prevent private tag from altering
> kernelrelease.
>
> Signed-off-by: Afzal Mohammed <[email protected]>
> ---
>
> Hi,
>
> This seems to work on different scenarios that could be readily thought
> of. I am shaky about this change, but acheives the purpose.

With your change, the script considers e.g. the next-YYYYMMDD tags
"private."


> define filechk_kernel.release
> - echo "$(KERNELVERSION)$$($(CONFIG_SHELL) $(srctree)/scripts/setlocalversion $(srctree))"
> + echo "$(KERNELVERSION)$$($(CONFIG_SHELL) $(srctree)/scripts/setlocalversion $(srctree) $(KERNELVERSION))" > $@
> endef

The >$@ should not be there.

Michal

2013-10-27 19:39:25

by afzal mohammed

[permalink] [raw]
Subject: Re: [PATCH RFC] kbuild: prevent git private tag altering kernelrelease

Hi Michal,

On Wed, Oct 23, 2013 at 03:21:00PM +0200, Michal Marek wrote:
> On 13.9.2013 23:49, Afzal Mohammed wrote:

> > If a private tag is created after the most recent kernelversion tag, a
> > commit after this private tag would feed kernelrelease with commits
> > after private tag and kernelversion tag. This may confuse user relying
> > on kernelrelease (mostly a developer while debugging), mainly if HEAD
> > has a private tag and otherwise w.r.t git distance from kernelversion
> > tag.
>
> The solution is simple: Do not use private annotated tags. Or rather, if
> you are creating an annotated tag, modify EXTRAVERSION accordingly. Any
> automagic based on the tag name is going to fail in some way.

Here private tags, although not a proper term, (my description wasn't
clear) was referring to any annotated/signed tag that does not match
kernelversion.

Reason for this patch was to determine the exact source from kernel
logs easily while working over subystem maintainer trees (especially
useful when checking older logs and testing). More than my private tags,
it is the tags of subystem maintainers which never is a kernelversion
that made it difficult to determine the source from logs. Presence of
those tags misleads one about the source.

Modifying EXTRAVERSION would make it dirty, else commiting only for
the sake of avoiding dirty would be required to track the git source
and chances are high that git gc may eat those commits.

> With your change, the script considers e.g. the next-YYYYMMDD tags
> "private."

Although next tags were not causing much problem (due to
localversion-next file and as no direct development over next),
maintainer tags like this without localversion-* files were the ones
that was being mostly targeted by this change.

> > define filechk_kernel.release
> > - echo "$(KERNELVERSION)$$($(CONFIG_SHELL) $(srctree)/scripts/setlocalversion $(srctree))"
> > + echo "$(KERNELVERSION)$$($(CONFIG_SHELL) $(srctree)/scripts/setlocalversion $(srctree) $(KERNELVERSION))" > $@
> > endef
>
> The >$@ should not be there.

Yes, crap, happened while rebasing - punishment for coding without
proper understanding.

Regards
Afzal