* [Buildroot] [PATCH 1/1] setlocalversion: fix detection of hg revision for untagged versions
@ 2017-04-26 12:43 Thomas De Schampheleire
2017-04-26 13:13 ` Peter Korsgaard
0 siblings, 1 reply; 6+ messages in thread
From: Thomas De Schampheleire @ 2017-04-26 12:43 UTC (permalink / raw)
To: buildroot
From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
By default, cut prints the entire line if the specified delimiter is not
present at all:
$ printf "foo bar" | cut -d' ' -f2
bar
$ printf "foobar" | cut -d' ' -f2
foobar
In setlocalversion, cut is presented with the output of 'hg id' which has
the format:
"<revision> <tags-if-any>"
If the current revision is not tagged, the output of 'hg id' does not
contain the delimiter (space), cut prints the entire string, and
setlocalversion thinks the version is the tag.
As setlocalversion does not print anything for tagged versions, there is no
output overall, and no correct indication of the mercurial revision.
Fix by passing the extra cut option '--only-delimited', which suppresses
output if no delimiter is found.
This problem likely went unnoticed for so long, because the tag 'tip' (i.e.
most recent revision of the branch) is treated specially: in this case the
mercurial revision _is_ printed, i.e. the situation is treated as
'untagged'.
The problem is only seen when you are _not_ at the most recent revision in
your branch.
Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
support/scripts/setlocalversion | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/support/scripts/setlocalversion b/support/scripts/setlocalversion
index adeeb78..33cd605 100755
--- a/support/scripts/setlocalversion
+++ b/support/scripts/setlocalversion
@@ -54,7 +54,7 @@ fi
# Check for mercurial and a mercurial repo.
if hgid=`hg id 2>/dev/null`; then
- tag=`printf '%s' "$hgid" | cut -d' ' -f2`
+ tag=`printf '%s' "$hgid" | cut -d' ' -f2 --only-delimited`
# Do we have an untagged version?
if [ -z "$tag" -o "$tag" = tip ]; then
--
2.10.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 1/1] setlocalversion: fix detection of hg revision for untagged versions
2017-04-26 12:43 [Buildroot] [PATCH 1/1] setlocalversion: fix detection of hg revision for untagged versions Thomas De Schampheleire
@ 2017-04-26 13:13 ` Peter Korsgaard
2017-04-26 13:56 ` Thomas De Schampheleire
0 siblings, 1 reply; 6+ messages in thread
From: Peter Korsgaard @ 2017-04-26 13:13 UTC (permalink / raw)
To: buildroot
>>>>> "Thomas" == Thomas De Schampheleire <patrickdepinguin@gmail.com> writes:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> By default, cut prints the entire line if the specified delimiter is not
> present at all:
> $ printf "foo bar" | cut -d' ' -f2
> bar
> $ printf "foobar" | cut -d' ' -f2
> foobar
> In setlocalversion, cut is presented with the output of 'hg id' which has
> the format:
> "<revision> <tags-if-any>"
> If the current revision is not tagged, the output of 'hg id' does not
> contain the delimiter (space), cut prints the entire string, and
> setlocalversion thinks the version is the tag.
> As setlocalversion does not print anything for tagged versions, there is no
> output overall, and no correct indication of the mercurial revision.
> Fix by passing the extra cut option '--only-delimited', which suppresses
> output if no delimiter is found.
> This problem likely went unnoticed for so long, because the tag 'tip' (i.e.
> most recent revision of the branch) is treated specially: in this case the
> mercurial revision _is_ printed, i.e. the situation is treated as
> 'untagged'.
> The problem is only seen when you are _not_ at the most recent revision in
> your branch.
setlocalversion comes from the kernel. Do you have the same problem
there?
I see this particular line changed back in 2010 in the kernel repo:
commit 8558f59edf935cf5ee5ffc29a9e9458fd9a71be1
Author: Michal Marek <mmarek@suse.cz>
Date: Mon Aug 16 17:09:52 2010 +0200
setlocalversion: Ignote SCMs above the linux source tree
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...
Fix this by checking if the kernel source tree is the root of the git or
hg repository. No fix for svn: If the kernel source is not tracked in
the svn repository, it works as expected, otherwise determining the
'repository root' is not really a defined task.
Reported-and-tested-by: Dan McGee <dpmcgee@gmail.com>
Signed-off-by: Michal Marek <mmarek@suse.cz>
Perhaps synching with the kernel would be a better way forward than
adding local modifications?
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 1/1] setlocalversion: fix detection of hg revision for untagged versions
2017-04-26 13:13 ` Peter Korsgaard
@ 2017-04-26 13:56 ` Thomas De Schampheleire
2017-06-13 19:04 ` Thomas De Schampheleire
0 siblings, 1 reply; 6+ messages in thread
From: Thomas De Schampheleire @ 2017-04-26 13:56 UTC (permalink / raw)
To: buildroot
Hi Peter,
2017-04-26 15:13 GMT+02:00 Peter Korsgaard <peter@korsgaard.com>:
>>>>>> "Thomas" == Thomas De Schampheleire <patrickdepinguin@gmail.com> writes:
>
> > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> > By default, cut prints the entire line if the specified delimiter is not
> > present at all:
>
> > $ printf "foo bar" | cut -d' ' -f2
> > bar
> > $ printf "foobar" | cut -d' ' -f2
> > foobar
>
> > In setlocalversion, cut is presented with the output of 'hg id' which has
> > the format:
>
> > "<revision> <tags-if-any>"
>
> > If the current revision is not tagged, the output of 'hg id' does not
> > contain the delimiter (space), cut prints the entire string, and
> > setlocalversion thinks the version is the tag.
> > As setlocalversion does not print anything for tagged versions, there is no
> > output overall, and no correct indication of the mercurial revision.
>
> > Fix by passing the extra cut option '--only-delimited', which suppresses
> > output if no delimiter is found.
>
> > This problem likely went unnoticed for so long, because the tag 'tip' (i.e.
> > most recent revision of the branch) is treated specially: in this case the
> > mercurial revision _is_ printed, i.e. the situation is treated as
> > 'untagged'.
> > The problem is only seen when you are _not_ at the most recent revision in
> > your branch.
>
> setlocalversion comes from the kernel. Do you have the same problem
> there?
>
> I see this particular line changed back in 2010 in the kernel repo:
>
>
> commit 8558f59edf935cf5ee5ffc29a9e9458fd9a71be1
> Author: Michal Marek <mmarek@suse.cz>
> Date: Mon Aug 16 17:09:52 2010 +0200
>
> setlocalversion: Ignote SCMs above the linux source tree
>
> 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...
>
> Fix this by checking if the kernel source tree is the root of the git or
> hg repository. No fix for svn: If the kernel source is not tracked in
> the svn repository, it works as expected, otherwise determining the
> 'repository root' is not really a defined task.
>
> Reported-and-tested-by: Dan McGee <dpmcgee@gmail.com>
> Signed-off-by: Michal Marek <mmarek@suse.cz>
>
>
> Perhaps synching with the kernel would be a better way forward than
> adding local modifications?
I tried using the latest version from the kernel in Buildroot and
notice the following:
- the version from the kernel now expects to find
include/config/auto.conf and bails out otherwise. Commenting out this
test...
- the output of that script is now just a + i.e. the actual output
of the scm version does not matter. This is due to following code:
scm=$(scm_version --short)
res="$res${scm:++}"
which means: if scm is empty, don't change res -- if scm is not
empty, then the literal '+' is appended to res.
- if I ignore that point and check the actual content of the 'scm'
variable, I notice that this script suffers from the same problem as
the buildroot one. My fix solves that aspect also in the kernel
script.
The kernel script has evolved a lot from the version that Buildroot
once took, I don't know if the changes have real value for us.
I also assume that the kernel developers will not really want to
customize their version of the script to add more configuration
switches to tweak the behavior.
https://github.com/torvalds/linux/blob/master/scripts/setlocalversion
Based on this feedback, how do you think we should proceed?
Thanks,
Thomas
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 1/1] setlocalversion: fix detection of hg revision for untagged versions
2017-04-26 13:56 ` Thomas De Schampheleire
@ 2017-06-13 19:04 ` Thomas De Schampheleire
2017-07-03 9:40 ` Peter Korsgaard
0 siblings, 1 reply; 6+ messages in thread
From: Thomas De Schampheleire @ 2017-06-13 19:04 UTC (permalink / raw)
To: buildroot
On Wed, Apr 26, 2017 at 03:56:09PM +0200, Thomas De Schampheleire wrote:
> Hi Peter,
>
> 2017-04-26 15:13 GMT+02:00 Peter Korsgaard <peter@korsgaard.com>:
> >>>>>> "Thomas" == Thomas De Schampheleire <patrickdepinguin@gmail.com> writes:
> >
> > > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> > > By default, cut prints the entire line if the specified delimiter is not
> > > present at all:
> >
> > > $ printf "foo bar" | cut -d' ' -f2
> > > bar
> > > $ printf "foobar" | cut -d' ' -f2
> > > foobar
> >
> > > In setlocalversion, cut is presented with the output of 'hg id' which has
> > > the format:
> >
> > > "<revision> <tags-if-any>"
> >
> > > If the current revision is not tagged, the output of 'hg id' does not
> > > contain the delimiter (space), cut prints the entire string, and
> > > setlocalversion thinks the version is the tag.
> > > As setlocalversion does not print anything for tagged versions, there is no
> > > output overall, and no correct indication of the mercurial revision.
> >
> > > Fix by passing the extra cut option '--only-delimited', which suppresses
> > > output if no delimiter is found.
> >
> > > This problem likely went unnoticed for so long, because the tag 'tip' (i.e.
> > > most recent revision of the branch) is treated specially: in this case the
> > > mercurial revision _is_ printed, i.e. the situation is treated as
> > > 'untagged'.
> > > The problem is only seen when you are _not_ at the most recent revision in
> > > your branch.
> >
> > setlocalversion comes from the kernel. Do you have the same problem
> > there?
> >
> > I see this particular line changed back in 2010 in the kernel repo:
> >
> >
> > commit 8558f59edf935cf5ee5ffc29a9e9458fd9a71be1
> > Author: Michal Marek <mmarek@suse.cz>
> > Date: Mon Aug 16 17:09:52 2010 +0200
> >
> > setlocalversion: Ignote SCMs above the linux source tree
> >
> > 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...
> >
> > Fix this by checking if the kernel source tree is the root of the git or
> > hg repository. No fix for svn: If the kernel source is not tracked in
> > the svn repository, it works as expected, otherwise determining the
> > 'repository root' is not really a defined task.
> >
> > Reported-and-tested-by: Dan McGee <dpmcgee@gmail.com>
> > Signed-off-by: Michal Marek <mmarek@suse.cz>
> >
> >
> > Perhaps synching with the kernel would be a better way forward than
> > adding local modifications?
>
>
> I tried using the latest version from the kernel in Buildroot and
> notice the following:
>
> - the version from the kernel now expects to find
> include/config/auto.conf and bails out otherwise. Commenting out this
> test...
> - the output of that script is now just a + i.e. the actual output
> of the scm version does not matter. This is due to following code:
> scm=$(scm_version --short)
> res="$res${scm:++}"
>
> which means: if scm is empty, don't change res -- if scm is not
> empty, then the literal '+' is appended to res.
> - if I ignore that point and check the actual content of the 'scm'
> variable, I notice that this script suffers from the same problem as
> the buildroot one. My fix solves that aspect also in the kernel
> script.
>
>
> The kernel script has evolved a lot from the version that Buildroot
> once took, I don't know if the changes have real value for us.
> I also assume that the kernel developers will not really want to
> customize their version of the script to add more configuration
> switches to tweak the behavior.
> https://github.com/torvalds/linux/blob/master/scripts/setlocalversion
>
> Based on this feedback, how do you think we should proceed?
ping?
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 1/1] setlocalversion: fix detection of hg revision for untagged versions
2017-06-13 19:04 ` Thomas De Schampheleire
@ 2017-07-03 9:40 ` Peter Korsgaard
2017-07-04 15:36 ` Peter Korsgaard
0 siblings, 1 reply; 6+ messages in thread
From: Peter Korsgaard @ 2017-07-03 9:40 UTC (permalink / raw)
To: buildroot
>>>>> "Thomas" == Thomas De Schampheleire <thomas.de_schampheleire@nokia.com> writes:
Hi,
>> > Perhaps synching with the kernel would be a better way forward than
>> > adding local modifications?
>> I tried using the latest version from the kernel in Buildroot and
>> notice the following:
>>
>> - the version from the kernel now expects to find
>> include/config/auto.conf and bails out otherwise. Commenting out this
>> test...
>> - the output of that script is now just a + i.e. the actual output
>> of the scm version does not matter. This is due to following code:
>> scm=$(scm_version --short)
>> res="$res${scm:++}"
>>
>> which means: if scm is empty, don't change res -- if scm is not
>> empty, then the literal '+' is appended to res.
>> - if I ignore that point and check the actual content of the 'scm'
>> variable, I notice that this script suffers from the same problem as
>> the buildroot one. My fix solves that aspect also in the kernel
>> script.
>>
>>
>> The kernel script has evolved a lot from the version that Buildroot
>> once took, I don't know if the changes have real value for us.
>> I also assume that the kernel developers will not really want to
>> customize their version of the script to add more configuration
>> switches to tweak the behavior.
>> https://github.com/torvalds/linux/blob/master/scripts/setlocalversion
>>
>> Based on this feedback, how do you think we should proceed?
Sorry for the slow response. I was afraid that the cut option wouldn't
be supported by older versions, but it was apparently added ~20 years
go.
Committed, thanks.
It might still be interesting to submit a similar patch to the Linux
kernel (as you presumably have the same issue there).
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 1/1] setlocalversion: fix detection of hg revision for untagged versions
2017-07-03 9:40 ` Peter Korsgaard
@ 2017-07-04 15:36 ` Peter Korsgaard
0 siblings, 0 replies; 6+ messages in thread
From: Peter Korsgaard @ 2017-07-04 15:36 UTC (permalink / raw)
To: buildroot
>>>>> "Peter" == Peter Korsgaard <peter@korsgaard.com> writes:
>>> Based on this feedback, how do you think we should proceed?
> Sorry for the slow response. I was afraid that the cut option wouldn't
> be supported by older versions, but it was apparently added ~20 years
> go.
> Committed, thanks.
Committed to 2017.02.x and 2017.05.x, thanks.
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-07-04 15:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-26 12:43 [Buildroot] [PATCH 1/1] setlocalversion: fix detection of hg revision for untagged versions Thomas De Schampheleire
2017-04-26 13:13 ` Peter Korsgaard
2017-04-26 13:56 ` Thomas De Schampheleire
2017-06-13 19:04 ` Thomas De Schampheleire
2017-07-03 9:40 ` Peter Korsgaard
2017-07-04 15:36 ` Peter Korsgaard
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.