scripts/setlocalversion: also consider annotated tags of the form vx.y.z-${file_localversion}
PATCH v2 1/1 scripts/setlocalversion: also consider annotated tags of the form vx.y.z-${file_localversion}
Commit 6ab7e1f95e96 ("setlocalversion: use only the correct release tag for git-describe") was absolutely correct to limit which annotated tags would be used to compute the -01234-gabcdef suffix. Otherwise, if some random annotated tag exists closer to HEAD than the vX.Y.Z one, the commit count would be too low.
However, since the version string always includes the ${file_localversion} part, now the problem is that the count can be too high. For example, building an 6.4.6-rt8 kernel with a few patches on top, I currently get
$ make -s kernelrelease 6.4.6-rt8-00128-gd78b7f406397
But those 128 commits include the 100 commits that are in v6.4.6..v6.4.6-rt8, so this is somewhat misleading.
Amend the logic so that, in addition to the linux-next consideration, the script also looks for a tag corresponding to the 6.4.6-rt8 part of what will become the uname -r string. With this patch (so 29 patches on top of v6.4.6-rt8), one instead gets
$ make -s kernelrelease 6.4.6-rt8-00029-gd533209291a2
While there, note that the line
git describe --exact-match --match=$tag $tag 2>/dev/null
obviously asks if $tag is an annotated tag, but it does not actually tell if the commit pointed to has any relation to HEAD. So remove both uses of --exact-match, and instead just ask if the description generated is identical to the tag we provided. Since we then already have the result of
git describe --match=$tag
we also end up reducing the number of times we invoke "git describe".
---
v2: explicitly initialize desc to empty [Masahiro]
scripts/setlocalversion | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-)
scripts/setlocalversion +26 -10
@@ -57,21 +57,37 @@ scm_version() return fi - # If a localversion*' file and the corresponding annotated tag exist, - # use it. This is the case in linux-next. + # mainline kernel: 6.2.0-rc5 -> v6.2-rc5 + # stable kernel: 6.1.7 -> v6.1.7 + version_tag=v$(echo "${KERNELVERSION}" | sed -E 's/^([0-9]+\.[0-9]+)\.0(.*)$/\1\2/') + + # If a localversion* file exists, and the corresponding + # annotated tag exists and is an ancestor of HEAD, use + # it. This is the case in linux-next. tag=${file_localversion#-} - tag=$(git describe --exact-match --match=$tag $tag 2>/dev/null) + desc= + if [ -n "${tag}" ]; then + desc=$(git describe --match=$tag 2>/dev/null) + fi + + # Otherwise, if a localversion* file exists, and the tag + # obtained by appending it to the tag derived from + # KERNELVERSION exists and is an ancestor of HEAD, use + # it. This is e.g. the case in linux-rt. + if [ -z "${desc}" ] && [ -n "${file_localversion}" ]; then + tag="${version_tag}${file_localversion}" + desc=$(git describe --match=$tag 2>/dev/null) + fi # Otherwise, default to the annotated tag derived from KERNELVERSION. - # mainline kernel: 6.2.0-rc5 -> v6.2-rc5 - # stable kernel: 6.1.7 -> v6.1.7 - if [ -z "${tag}" ]; then - tag=v$(echo "${KERNELVERSION}" | sed -E 's/^([0-9]+\.[0-9]+)\.0(.*)$/\1\2/') + if [ -z "${desc}" ]; then + tag="${version_tag}" + desc=$(git describe --match=$tag 2>/dev/null) fi # If we are at the tagged commit, we ignore it because the version is # well-defined. - if [ -z "$(git describe --exact-match --match=$tag 2>/dev/null)" ]; then + if [ "${tag}" != "${desc}" ]; then # If only the short version is requested, don't bother # running further git commands
@@ -81,8 +97,8 @@ scm_version() fi # If we are past the tagged commit, we pretty print it. # (like 6.1.0-14595-g292a089d78d3) - if atag="$(git describe --match=$tag 2>/dev/null)"; then - echo "$atag" | awk -F- '{printf("-%05d", $(NF-1))}' + if [ -n "${desc}" ]; then + echo "${desc}" | awk -F- '{printf("-%05d", $(NF-1))}' fi # Add -g and exactly 12 hex chars.
Signature · 1 line
2.37.2
Quoted 36 lines · Rasmus Villemoes
Commit
6ab7e1f95e96("setlocalversion: use only the correct release tag for git-describe") was absolutely correct to limit which annotated tags would be used to compute the -01234-gabcdef suffix. Otherwise, if some random annotated tag exists closer to HEAD than the vX.Y.Z one, the commit count would be too low.However, since the version string always includes the ${file_localversion} part, now the problem is that the count can be too high. For example, building an 6.4.6-rt8 kernel with a few patches on top, I currently get
$ make -s kernelrelease 6.4.6-rt8-00128-gd78b7f406397But those 128 commits include the 100 commits that are in v6.4.6..v6.4.6-rt8, so this is somewhat misleading.
Amend the logic so that, in addition to the linux-next consideration, the script also looks for a tag corresponding to the 6.4.6-rt8 part of what will become the
uname -rstring. With this patch (so 29 patches on top of v6.4.6-rt8), one instead gets$ make -s kernelrelease 6.4.6-rt8-00029-gd533209291a2While there, note that the line
git describe --exact-match --match=$tag $tag 2>/dev/nullobviously asks if $tag is an annotated tag, but it does not actually tell if the commit pointed to has any relation to HEAD. So remove both uses of --exact-match, and instead just ask if the description generated is identical to the tag we provided. Since we then already have the result of
Quoted 7 lines
git describe --match=$tagwe also end up reducing the number of times we invoke "git describe".
Signed-off-by Rasmus Villemoes <[email protected]>---
v2: explicitly initialize desc to empty [Masahiro]
Applied to linux-kbuild.
Thanks.
Quoted 68 lines
scripts/setlocalversion | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-)scripts/setlocalversion +26
-11@@ -57,21 +57,37 @@ scm_version() return fi - # If a localversion*' file and the corresponding annotated tag exist, - # use it. This is the case in linux-next. + # mainline kernel: 6.2.0-rc5 -> v6.2-rc5 + # stable kernel: 6.1.7 -> v6.1.7 + version_tag=v$(echo "${KERNELVERSION}" | sed -E 's/^([0-9]+\.[0-9]+)\.0(.*)$/\1\2/') + + # If a localversion* file exists, and the corresponding + # annotated tag exists and is an ancestor of HEAD, use + # it. This is the case in linux-next. tag=${file_localversion#-} - tag=$(git describe --exact-match --match=$tag $tag 2>/dev/null) + desc= + if [ -n "${tag}" ]; then + desc=$(git describe --match=$tag 2>/dev/null) + fi + + # Otherwise, if a localversion* file exists, and the tag + # obtained by appending it to the tag derived from + # KERNELVERSION exists and is an ancestor of HEAD, use + # it. This is e.g. the case in linux-rt. + if [ -z "${desc}" ] && [ -n "${file_localversion}" ]; then + tag="${version_tag}${file_localversion}" + desc=$(git describe --match=$tag 2>/dev/null) + fi # Otherwise, default to the annotated tag derived from KERNELVERSION. - # mainline kernel: 6.2.0-rc5 -> v6.2-rc5 - # stable kernel: 6.1.7 -> v6.1.7 - if [ -z "${tag}" ]; then - tag=v$(echo "${KERNELVERSION}" | sed -E 's/^([0-9]+\.[0-9]+)\.0(.*)$/\1\2/') + if [ -z "${desc}" ]; then + tag="${version_tag}" + desc=$(git describe --match=$tag 2>/dev/null) fi # If we are at the tagged commit, we ignore it because the version is # well-defined. - if [ -z "$(git describe --exact-match --match=$tag 2>/dev/null)" ]; then + if [ "${tag}" != "${desc}" ]; then # If only the short version is requested, don't bother # running further git commands@@ -81,8 +97,8 @@ scm_version() fi # If we are past the tagged commit, we pretty print it. # (like 6.1.0-14595-g292a089d78d3) - if atag="$(git describe --match=$tag 2>/dev/null)"; then - echo "$atag" | awk -F- '{printf("-%05d", $(NF-1))}' + if [ -n "${desc}" ]; then + echo "${desc}" | awk -F- '{printf("-%05d", $(NF-1))}' fi # Add -g and exactly 12 hex chars. --2.37.2
Signature · 2 lines
Best Regards Masahiro Yamada
Quoted 35 lines · Rasmus Villemoes
Commit
6ab7e1f95e96("setlocalversion: use only the correct release tag for git-describe") was absolutely correct to limit which annotated tags would be used to compute the -01234-gabcdef suffix. Otherwise, if some random annotated tag exists closer to HEAD than the vX.Y.Z one, the commit count would be too low.However, since the version string always includes the ${file_localversion} part, now the problem is that the count can be too high. For example, building an 6.4.6-rt8 kernel with a few patches on top, I currently get
$ make -s kernelrelease 6.4.6-rt8-00128-gd78b7f406397But those 128 commits include the 100 commits that are in v6.4.6..v6.4.6-rt8, so this is somewhat misleading.
Amend the logic so that, in addition to the linux-next consideration, the script also looks for a tag corresponding to the 6.4.6-rt8 part of what will become the
uname -rstring. With this patch (so 29 patches on top of v6.4.6-rt8), one instead gets$ make -s kernelrelease 6.4.6-rt8-00029-gd533209291a2While there, note that the line
git describe --exact-match --match=$tag $tag 2>/dev/nullobviously asks if $tag is an annotated tag, but it does not actually tell if the commit pointed to has any relation to HEAD. So remove both uses of --exact-match, and instead just ask if the description generated is identical to the tag we provided. Since we then already have the result of
Quoted 3 lines
git describe --match=$tagwe also end up reducing the number of times we invoke "git describe".
Dropping "--exact-match" is resulting in unnacceptable latencies for me. I don't understand what this is trying to do well enough to make a suggestion, but something has to change.
E.g. on my build box, a single git describe --match=v6.5 takes ~8.5 seconds, whereas a complete from-scratch kernel build takes <30 seconds, and an incremental build takes <2 seconds. When build testing to-be-applied changes, I compile each commit ~15 times (different x86 configs plus one for each other KVM architecture), which makes the ~8.5 second delay beyond painful.
And for actual testing, I can do an incremental build and boot into a VM in under 20 seconds, a multi-second delay is extremely painful there as well.
Quoted 36 lines · Sean Christopherson
Quoted 35 lines · Rasmus Villemoes
Commit
6ab7e1f95e96("setlocalversion: use only the correct release tag for git-describe") was absolutely correct to limit which annotated tags would be used to compute the -01234-gabcdef suffix. Otherwise, if some random annotated tag exists closer to HEAD than the vX.Y.Z one, the commit count would be too low.However, since the version string always includes the ${file_localversion} part, now the problem is that the count can be too high. For example, building an 6.4.6-rt8 kernel with a few patches on top, I currently get
$ make -s kernelrelease 6.4.6-rt8-00128-gd78b7f406397But those 128 commits include the 100 commits that are in v6.4.6..v6.4.6-rt8, so this is somewhat misleading.
Amend the logic so that, in addition to the linux-next consideration, the script also looks for a tag corresponding to the 6.4.6-rt8 part of what will become the
uname -rstring. With this patch (so 29 patches on top of v6.4.6-rt8), one instead gets$ make -s kernelrelease 6.4.6-rt8-00029-gd533209291a2While there, note that the line
git describe --exact-match --match=$tag $tag 2>/dev/nullobviously asks if $tag is an annotated tag, but it does not actually tell if the commit pointed to has any relation to HEAD. So remove both uses of --exact-match, and instead just ask if the description generated is identical to the tag we provided. Since we then already have the result of
Quoted 7 lines
Quoted 3 lines
git describe --match=$tagwe also end up reducing the number of times we invoke "git describe".
Dropping "--exact-match" is resulting in unnacceptable latencies for me. I don't understand what this is trying to do well enough to make a suggestion, but something has to change.
Hm, that's quite unexpected. I mean, before that commit, I think that setlocalversion, especially when run on some dev branch, would _also_ end up doing at least one 'git describe --match=v6.5'. <goes digging>
Ah, so I assume that in your case you always end up in the
# If only the short version is requested, don't bother # running further git commands
if $short; then
echo "+"
return
fi
case, i.e. you do not have CONFIG_LOCALVERSION_AUTO=y. Can you confirm that?
Quoted 1 line
E.g. on my build box, a single
git describe --match=v6.5takes ~8.5 seconds,
That's a long time indeed. I don't really know why it can take that long.
Quoted 4 lines
whereas a complete from-scratch kernel build takes <30 seconds, and an incremental build takes <2 seconds. When build testing to-be-applied changes, I compile each commit ~15 times (different x86 configs plus one for each other KVM architecture), which makes the ~8.5 second delay beyond painful.
Sorry about that. I agree something needs to be done, but the commit above fixed a real problem with -rt kernels, and the refactoring just made it much easier to follow the logic IMO - and, for the CONFIG_LOCALVERSION_AUTO=y case, did reduce the number of times git describe gets invoked.
Until I have a bit more time to think, could you try setting the env var LOCALVERSION when building - it can be set to anything, including empty. Then if I'm reading the script right, the scm_version() function won't be called at all.
Rasmus
Quoted 22 lines · Rasmus Villemoes
Quoted 7 lines · Sean Christopherson
Quoted 6 lines · Rasmus Villemoes
obviously asks if $tag is an annotated tag, but it does not actually tell if the commit pointed to has any relation to HEAD. So remove both uses of --exact-match, and instead just ask if the description generated is identical to the tag we provided. Since we then already have the result of
Quoted 7 lines
Quoted 3 lines
git describe --match=$tagwe also end up reducing the number of times we invoke "git describe".
Dropping "--exact-match" is resulting in unnacceptable latencies for me. I don't understand what this is trying to do well enough to make a suggestion, but something has to change.
Hm, that's quite unexpected. I mean, before that commit, I think that setlocalversion, especially when run on some dev branch, would _also_ end up doing at least one 'git describe --match=v6.5'. <goes digging>
Ah, so I assume that in your case you always end up in the
Quoted 8 lines
# If only the short version is requested, don't bother # running further git commands
if $short; then
echo "+"
return
ficase, i.e. you do not have
CONFIG_LOCALVERSION_AUTO=y. Can you confirm that?
Yep, the configs I build most frequently have CONFIG_LOCALVERSION_AUTO=n, and I can induce the bad behavior before this patch by toggling that on.
Quoted 3 lines
Quoted 1 line
E.g. on my build box, a single
git describe --match=v6.5takes ~8.5 seconds,That's a long time indeed. I don't really know why it can take that long.
I can't figure that either. It's not always slow, just almost always slow. I assume it has something to do with the number of commits/objects/merges git has to trawl through, but I don't fully understand the behavior.
E.g. if I do git describe --match=v6.5-rc2 on a branch based on v6.5-rc2, it's basically instant. But if take that same branch and merge it on top of v6.5, git describe --match=v6.5 takes 8.5 seconds even though the number of delta commits is only one higher.
Quoted 11 lines
Quoted 4 lines
whereas a complete from-scratch kernel build takes <30 seconds, and an incremental build takes <2 seconds. When build testing to-be-applied changes, I compile each commit ~15 times (different x86 configs plus one for each other KVM architecture), which makes the ~8.5 second delay beyond painful.
Sorry about that. I agree something needs to be done, but the commit above fixed a real problem with -rt kernels, and the refactoring just made it much easier to follow the logic IMO - and, for the
CONFIG_LOCALVERSION_AUTO=ycase, did reduce the number of times git describe gets invoked.
Quoted 4 lines
Until I have a bit more time to think, could you try setting the env var LOCALVERSION when building - it can be set to anything, including empty. Then if I'm reading the script right, the
scm_version()function won't be called at all.
Nice! That does the trick. Adding a useful LOCALVERSION is trivially easy for all my builds, and as a bonus it's useful info for my test kernels.
So unless other people complain, I'm totally ok to leave this alone and not spend any more time on it.
Thanks much!