GNU bug report logs -
#5958
Sort-8.4 bug
Previous Next
Reported by: srodri <at> datsi.fi.upm.es
Date: Fri, 16 Apr 2010 12:14:02 UTC
Severity: normal
Merged with 5991
Fixed in version 8.5
Done: Eric Blake <eblake <at> redhat.com>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 5958 in the body.
You can then email your comments to 5958 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
owner <at> debbugs.gnu.org, bug-coreutils <at> gnu.org:
bug#5958; Package
coreutils.
(Fri, 16 Apr 2010 12:14:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
srodri <at> datsi.fi.upm.es:
New bug report received and forwarded. Copy sent to
bug-coreutils <at> gnu.org.
(Fri, 16 Apr 2010 12:14:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
Dear sirs,
I think I have found a bug in sort coreutils command. When I type
sort -T /tmp +1 -2 +2rn -3 +0 -1<<EOF
perra/S perra 2.200000
perro/PS perra 4.400000
EOF
The result is:
perra/S perra 2.200000
perro/PS perra 4.400000
If I type
sort -T /tmp +1 -2 +2n -3 +0 -1<<EOF
perra/S perra 2.200000
perro/PS perra 4.400000
EOF
The result is the same. If I make the same executions with sort 5.0 it
works properly.
Setting or unsetting the LC_ALL=POSIX environment variable has no effect.
If you need more information, do not hesitate to ask.
Best regards,
Santiago Rodríguez.
Information forwarded
to
owner <at> debbugs.gnu.org, bug-coreutils <at> gnu.org:
bug#5958; Package
coreutils.
(Fri, 16 Apr 2010 14:13:01 GMT)
Full text and
rfc822 format available.
Message #8 received at 5958 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On 04/16/2010 04:58 AM, Santiago Rodriguez wrote:
> Dear sirs,
>
> I think I have found a bug in sort coreutils command. When I type
>
> sort -T /tmp +1 -2 +2rn -3 +0 -1<<EOF
> perra/S perra 2.200000
> perro/PS perra 4.400000
> EOF
Thanks for the report; however, this is not a bug.
The syntax 'sort +1' is obsolete. You are better off rewriting your
scripts to conform to POSIX:
sort -T /tmp -k2,3 -k3,4rn -k1,2
And in doing so, you've just made it apparent why sort behaved
correctly, but differently than you expected. Basically, you have
requested that your first sort key be the combination of the second and
third field. And since 'perra 2.200000' sorts before 'perra 4.400000',
there is no need for sort to fall back on the second and third key
specifications.
You can get the desired results with:
sort -T /tmp -k2,2 -k3,3rn -k1,1
or the obsolete:
sort -T /tmp +1 -1 +2rn -2 +0 -0
Meanwhile, we have a patch brewing (but not in 8.4) that allows sort to
output some debug hints, to actually show which portions of each line
were used in the various comparisons. I'm hoping we can get that patch
polished soon, because it would have been very helpful in demonstrating
my reply.
>
> The result is the same. If I make the same executions with sort 5.0 it
> works properly.
Actually, sort 5.0 was buggy in this area. Sort 8.4 has a number of bug
fixes for bad behavior in sort 5.0.
--
Eric Blake eblake <at> redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
[signature.asc (application/pgp-signature, attachment)]
Information forwarded
to
owner <at> debbugs.gnu.org, bug-coreutils <at> gnu.org:
bug#5958; Package
coreutils.
(Sat, 17 Apr 2010 00:10:03 GMT)
Full text and
rfc822 format available.
Message #11 received at 5958 <at> debbugs.gnu.org (full text, mbox):
Eric Blake writes:
>
> On 04/16/2010 04:58 AM, Santiago Rodriguez wrote:
> > Dear sirs,
> >=20
> > I think I have found a bug in sort coreutils command. When I type
> >=20
> > sort -T /tmp +1 -2 +2rn -3 +0 -1<<EOF
> > perra/S perra 2.200000
> > perro/PS perra 4.400000
> > EOF
>
> Thanks for the report; however, this is not a bug.
>
> The syntax 'sort +1' is obsolete. You are better off rewriting your
When you pry it from my cold dead hands...
> scripts to conform to POSIX:
>
> sort -T /tmp -k2,3 -k3,4rn -k1,2
I don't think that's a correct equivalence. Traditional options +1 -2 should
mean the same as -k2,2 (i.e. the -2 means the key ends *before* field 2,
counting from 0).
Instead of comparing new coreutils to old coreutils, how about reading some
documentation that actually specifies the +pos1 -pos2 syntax, and is not
written from the "why won't those old people die off already" point of view?
For example the V7 man page:
http://www.freebsd.org/cgi/man.cgi?query=sort&apropos=0&sektion=0&manpath=Unix+Seventh+Edition&format=ascii
Or something more recent, from Solaris, that provides a precise formula for
translating +pos1 -pos2 into -k options:
http://www.freebsd.org/cgi/man.cgi?query=sort&apropos=0&sektion=0&manpath=SunOS+5.10&format=ascii
--
Alan Curry
Reply sent
to
Jim Meyering <jim <at> meyering.net>:
You have taken responsibility.
(Sun, 18 Apr 2010 08:48:01 GMT)
Full text and
rfc822 format available.
Notification sent
to
srodri <at> datsi.fi.upm.es:
bug acknowledged by developer.
(Sun, 18 Apr 2010 08:48:01 GMT)
Full text and
rfc822 format available.
Message #16 received at 5958-done <at> debbugs.gnu.org (full text, mbox):
pacman <at> kosh.dhis.org wrote:
> Eric Blake writes:
>> On 04/16/2010 04:58 AM, Santiago Rodriguez wrote:
>> > I think I have found a bug in sort coreutils command. When I type
>> >
>> > sort -T /tmp +1 -2 +2rn -3 +0 -1<<EOF
>> > perra/S perra 2.200000
>> > perro/PS perra 4.400000
>> > EOF
>>
>> Thanks for the report; however, this is not a bug.
>>
>> The syntax 'sort +1' is obsolete. You are better off rewriting your
>
> When you pry it from my cold dead hands...
It has been declared "obsolete" for a good reason: it is ambiguous.
If you require that syntax, use some other sort program.
Message #17 received at 5958-done <at> debbugs.gnu.org (full text, mbox):
Jim Meyering writes:
>
> pacman <at> kosh.dhis.org wrote:
> > Eric Blake writes:
> >> The syntax 'sort +1' is obsolete. You are better off rewriting your
> >
> > When you pry it from my cold dead hands...
>
> It has been declared "obsolete" for a good reason: it is ambiguous.
> If you require that syntax, use some other sort program.
>
What a crock.
POSIX abdicated its responsibility to fully document the sort command. This
makes the traditional documentation (i.e. the V7 man page) the most
authoritative specification for what "sort +1 -2" means. If you won't make
GNU sort behave correctly, it is time to remove it from general distribution
and let people go find a working sort command elsewhere.
Message #18 received at 5958-done <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On 04/18/2010 03:00 AM, pacman <at> kosh.dhis.org wrote:
> Jim Meyering writes:
>>
>> pacman <at> kosh.dhis.org wrote:
>>> Eric Blake writes:
>>>> The syntax 'sort +1' is obsolete. You are better off rewriting your
>>>
> POSIX abdicated its responsibility to fully document the sort command.
No, sort +1 was properly documented in POSIX 1992; unfortunately, that
old version of POSIX is not available online, and I don't have handy
access to a hardcopy or pdf version. It was POSIX 2001 that withdrew
documentation for sort +1 as part of deprecating the syntax as obsolete,
while still allowing implementations to support it as a non-POSIX extension.
> This
> makes the traditional documentation (i.e. the V7 man page) the most
> authoritative specification for what "sort +1 -2" means.
I agree that the coreutils documentation (info sort) could do a better
job of documenting the translation from the obsolete syntax to the
current syntax. If it weren't for copyright questions, I would even
agree that blind copy-and-paste from the link you gave:
http://www.freebsd.org/cgi/man.cgi?query=sort&apropos=0&sektion=0&manpath=SunOS+5.10&format=ascii
would make sense. But to be on the safe side, the best approach would
be to write the rules by scratch, referring to the coreutils
implementation and nothing external. Would you care to submit the patch?
> If you won't make
> GNU sort behave correctly, it is time to remove it from general distribution
> and let people go find a working sort command elsewhere.
First, you have to prove that sort is not behaving according to a
particular standards document. And since POSIX 1992 _did_ document
'sort +1', that means quoting a relevant portion of that document along
with a simple test case demonstrating why you think coreutils does not
comply with that document. If you can prove that, then we will gladly
fix the bug.
--
Eric Blake eblake <at> redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
[signature.asc (application/pgp-signature, attachment)]
Message #19 received at 5958-done <at> debbugs.gnu.org (full text, mbox):
Eric Blake writes:
>
> First, you have to prove that sort is not behaving according to a
> particular standards document. And since POSIX 1992 _did_ document
> 'sort +1', that means quoting a relevant portion of that document along
> with a simple test case demonstrating why you think coreutils does not
> comply with that document. If you can prove that, then we will gladly
> fix the bug.
So your plan is to ignore the many easily available documents that explain
exactly what sort +1 -2 means, in favor of a document that you don't have,
and then blow off bug reports by sending people on a wild goose chase.
Please just take a current BSD or Solaris man page as definitive.
Compatibility with other actually-existing implementations is what matters
here, not some former standard.
--
Alan Curry
Message #20 received at 5958-done <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On 04/19/2010 03:51 PM, pacman <at> kosh.dhis.org wrote:
> Eric Blake writes:
>>
>> First, you have to prove that sort is not behaving according to a
>> particular standards document. And since POSIX 1992 _did_ document
>> 'sort +1', that means quoting a relevant portion of that document along
>> with a simple test case demonstrating why you think coreutils does not
>> comply with that document. If you can prove that, then we will gladly
>> fix the bug.
>
> So your plan is to ignore the many easily available documents that explain
> exactly what sort +1 -2 means, in favor of a document that you don't have,
> and then blow off bug reports by sending people on a wild goose chase.
I am not blowing off this bug report - I agree with you that we need
better documentation. However, I don't have the resources to do all the
legwork myself (in particular, access to the POSIX 1992 standard would
be a big help), so in open source fashion, I'm asking for help. And
since it seems to be your itch, the best you can do is scratch it by
providing that help, rather than criticizing the fact that we are trying
to spread the work load.
>
> Please just take a current BSD or Solaris man page as definitive.
It may be definitive for Solaris, but it is not definitive for POSIX,
and it does not good to copy someone else's documentation if the
implementation is subtly different. That's why I'm insisting that we go
to the sources - somewhere where the behavior is standardized, then
contrast that against our implementation. At that point, if the two
agree (which I hope they do), then we are done; while if the two
disagree, then it is much easier to justify a change to coreutils to
come in line with the standards.
> Compatibility with other actually-existing implementations is what matters
> here, not some former standard.
The POSIX standard was written with the goal of being compatible with
existing implementations; it did not always achieve that goal, but the
hope is that it did in the case of sort. However, these days, it is
better to comply to POSIX than it is to be bugwards compatible to subtle
bugs in older implementations.
--
Eric Blake eblake <at> redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
[signature.asc (application/pgp-signature, attachment)]
Information forwarded
to
owner <at> debbugs.gnu.org, bug-coreutils <at> gnu.org:
bug#5958; Package
coreutils.
(Tue, 20 Apr 2010 00:45:02 GMT)
Full text and
rfc822 format available.
Message #23 received at 5958 <at> debbugs.gnu.org (full text, mbox):
Eric Blake writes:
>
> I am not blowing off this bug report - I agree with you that we need
> better documentation. However, I don't have the resources to do all the
It's not just a documentation problem. The behavior in older versions of
coreutils was compatible with other members of the unix family; the current
version is not. That's a regression and should be recognized as such, even if
no version of POSIX ever specified the behavior.
> legwork myself (in particular, access to the POSIX 1992 standard would
> be a big help), so in open source fashion, I'm asking for help. And
> since it seems to be your itch, the best you can do is scratch it by
> providing that help, rather than criticizing the fact that we are trying
> to spread the work load.
I didn't send the initial bug report, but it did catch my interest. "sort +1"
is for me a finger-macro that I'll never stop using no matter how many people
declare it obsolete. "sort +1 -2" not so much.
The hunt for old POSIX versions is a dead end. Instead, I'll try to find
exactly when this stopped working. Maybe it was an unintended side effect of
a change elsewhere.
--
Alan Curry
Information forwarded
to
owner <at> debbugs.gnu.org, bug-coreutils <at> gnu.org:
bug#5958; Package
coreutils.
(Tue, 20 Apr 2010 08:38:02 GMT)
Full text and
rfc822 format available.
Message #26 received at 5958 <at> debbugs.gnu.org (full text, mbox):
A full investigation has revealed:
This bug was introduced between coreutils 7.1 and 7.2, here:
>commit 224a69b56b716f57e3a018af5a9b9379f32da3fc
>Author: Pádraig Brady <P <at> draigBrady.com>
>Date: Tue Feb 24 08:37:18 2009 +0000
>
> sort: Fix two bugs with determining the end of field
>
> * src/sort.c: When no specific number of chars to skip
> is specified for the end field, always skip the whole field.
> Also never include leading spaces from next field.
> * tests/misc/sort: Add 2 new tests for these cases.
> * NEWS: Mention this bug fix.
> * THANKS: Add bug reporter.
> Reported by Davide Canova.
In the diff of that commit, an eword++ was removed from the case 'k' section
of option parsing, where it did not affect traditional options, and added to
the limfield() function, where it takes effect regardless of how fields were
specified.
So it fixed a -k option parsing bug and added a traditional option parsing
bug. And on the way, it removed a comment describing the correct
correspondence between the two!
The following patch moves the eword++ back to its old location (under the
case 'k') but keeps the new test for when it should be applied (echar==0,
whether by explicit .0 on the field end specifier or by omission of the field
end specifier). This allows the -k bug that was fixed to stay fixed, while
undoing the damage to the traditional options.
With this patch applied, all the sort tests in make check still pass,
including the tests added in the above commit, which I take as a sign that I
got it right. And the traditional options are back to working again.
I'd suggest the following new test case:
printf "a b c\na c b\n" | sort +0 -1 +2
should output "a c b\na b c\n"
I'd put that in the diff too, but the organization of tests/misc/sort is
baffling.
--- coreutils-8.4.orig/src/sort.c 2010-04-20 02:45:35.000000000 -0500
+++ coreutils-8.4/src/sort.c 2010-04-20 03:12:57.000000000 -0500
@@ -1460,9 +1460,6 @@
char *ptr = line->text, *lim = ptr + line->length - 1;
size_t eword = key->eword, echar = key->echar;
- if (echar == 0)
- eword++; /* Skip all of end field. */
-
/* Move PTR past EWORD fields or to one past the last byte on LINE,
whichever comes first. If there are more than EWORD fields, leave
PTR pointing at the beginning of the field having zero-based index,
@@ -3424,6 +3421,8 @@
s = parse_field_count (s + 1, &key->echar,
N_("invalid number after `.'"));
}
+ if (key->echar == 0)
+ key->eword++; /* Skip all of end field. */
s = set_ordering (s, key, bl_end);
}
if (*s)
OK now let's not say I haven't done any legwork.
--
Alan Curry
Information forwarded
to
owner <at> debbugs.gnu.org, bug-coreutils <at> gnu.org:
bug#5958; Package
coreutils.
(Tue, 20 Apr 2010 12:11:02 GMT)
Full text and
rfc822 format available.
Message #29 received at 5958 <at> debbugs.gnu.org (full text, mbox):
On 20/04/10 09:37, Alan Curry wrote:
> A full investigation has revealed:
>
> This bug was introduced between coreutils 7.1 and 7.2, here:
>
>> commit 224a69b56b716f57e3a018af5a9b9379f32da3fc
>> Author: Pádraig Brady <P <at> draigBrady.com>
>> Date: Tue Feb 24 08:37:18 2009 +0000
>>
>> sort: Fix two bugs with determining the end of field
>>
>> * src/sort.c: When no specific number of chars to skip
>> is specified for the end field, always skip the whole field.
>> Also never include leading spaces from next field.
>> * tests/misc/sort: Add 2 new tests for these cases.
>> * NEWS: Mention this bug fix.
>> * THANKS: Add bug reporter.
>> Reported by Davide Canova.
>
> In the diff of that commit, an eword++ was removed from the case 'k' section
> of option parsing, where it did not affect traditional options, and added to
> the limfield() function, where it takes effect regardless of how fields were
> specified.
>
> So it fixed a -k option parsing bug and added a traditional option parsing
> bug.
Sigh. I didn't fully understand/consider the obsolete syntax when doing that.
I'll look at you patch tonight and push it in.
cheers,
Pádraig.
Information forwarded
to
owner <at> debbugs.gnu.org, bug-coreutils <at> gnu.org:
bug#5958; Package
coreutils.
(Tue, 20 Apr 2010 16:37:02 GMT)
Full text and
rfc822 format available.
Message #32 received at 5958 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On 20/04/10 09:37, Alan Curry wrote:
> --- coreutils-8.4.orig/src/sort.c 2010-04-20 02:45:35.000000000 -0500
> +++ coreutils-8.4/src/sort.c 2010-04-20 03:12:57.000000000 -0500
> @@ -1460,9 +1460,6 @@
> char *ptr = line->text, *lim = ptr + line->length - 1;
> size_t eword = key->eword, echar = key->echar;
>
> - if (echar == 0)
> - eword++; /* Skip all of end field. */
> -
> /* Move PTR past EWORD fields or to one past the last byte on LINE,
> whichever comes first. If there are more than EWORD fields, leave
> PTR pointing at the beginning of the field having zero-based index,
> @@ -3424,6 +3421,8 @@
> s = parse_field_count (s + 1, &key->echar,
> N_("invalid number after `.'"));
> }
> + if (key->echar == 0)
> + key->eword++; /* Skip all of end field. */
> s = set_ordering (s, key, bl_end);
> }
> if (*s)
I've changed it around in the attached patch so that
we consistently use zero based limits throughout the code.
I'll push this later on tonight unless there are objections.
cheers,
Pádraig.
[sort-key-limit.diff (text/x-patch, attachment)]
Information forwarded
to
owner <at> debbugs.gnu.org, bug-coreutils <at> gnu.org:
bug#5958; Package
coreutils.
(Tue, 20 Apr 2010 16:56:01 GMT)
Full text and
rfc822 format available.
Message #35 received at 5958 <at> debbugs.gnu.org (full text, mbox):
P��draig Brady wrote:
> I've changed it around in the attached patch so that
> we consistently use zero based limits throughout the code.
> I'll push this later on tonight unless there are objections.
...
> This regression was introduced in commit 224a69b5, 2009-02-24,
> "sort: Fix two bugs with determining the end of field".
> The specific regression being that we include 1 field too many when
> an end field is specified using obsolescent key syntax (+POS -POS).
>
> * src/sort.c (main): When processing obsolescent format key specifications,
> normalize eword to a zero based count when no specific end char is given
> for an end field. This matches what's done when keys are specified with -k.
> * tests/misc/sort: Add a few more tests for the obsolescent key formats,
> with test 07i being the particular failure addressed by this change.
> * THANKS: Add Alan Curry who precisely identified the issue.
> * NEWS: Mention the fix.
> Reported by Santiago Rodr��guez
...
> +#ensure obsolescent key limits are handled correctly
Nicely done. Thanks!
s/en/ en/
Information forwarded
to
owner <at> debbugs.gnu.org, bug-coreutils <at> gnu.org:
bug#5958; Package
coreutils.
(Tue, 20 Apr 2010 21:07:01 GMT)
Full text and
rfc822 format available.
Message #38 received at 5958 <at> debbugs.gnu.org (full text, mbox):
>
> I've changed it around in the attached patch so that
> we consistently use zero based limits throughout the code.
> I'll push this later on tonight unless there are objections.
In the definition of struct keyfield, is the comment by the "eword" member
still correct? Or was it ever?
size_t eword; /* Zero-origin first word after field. */
A shared understanding of what exactly the field represents would reduce the
chance of future mishaps.
--
Alan Curry
Message #39 received at 5958-done <at> debbugs.gnu.org (full text, mbox):
On 20/04/10 22:05, Alan Curry wrote:
>>
>> I've changed it around in the attached patch so that
>> we consistently use zero based limits throughout the code.
>> I'll push this later on tonight unless there are objections.
>
> In the definition of struct keyfield, is the comment by the "eword" member
> still correct? Or was it ever?
>
> size_t eword; /* Zero-origin first word after field. */
>
> A shared understanding of what exactly the field represents would reduce the
> chance of future mishaps.
Good point. I think it was correct initially but
became out of sync with the logic about 10 years ago:
http://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=d92f4ac8
I've clarified the comment in what I've just pushed:
http://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=8fc12909
cheers,
Pádraig.
Forcibly Merged 5958 5991.
Request was from
Eric Blake <eblake <at> redhat.com>
to
control <at> debbugs.gnu.org.
(Wed, 21 Apr 2010 00:00:04 GMT)
Full text and
rfc822 format available.
bug marked as fixed in version 8.5, send any further explanations to Eric Blake <eblake <at> redhat.com>
Request was from
Eric Blake <eblake <at> redhat.com>
to
control <at> debbugs.gnu.org.
(Wed, 21 Apr 2010 15:03:02 GMT)
Full text and
rfc822 format available.
bug marked as fixed in version 8.5, send any further explanations to srodri <at> datsi.fi.upm.es
Request was from
Eric Blake <eblake <at> redhat.com>
to
control <at> debbugs.gnu.org.
(Wed, 21 Apr 2010 15:03:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
owner <at> debbugs.gnu.org, bug-coreutils <at> gnu.org:
bug#5958; Package
coreutils.
(Fri, 23 Apr 2010 14:27:02 GMT)
Full text and
rfc822 format available.
Message #48 received at 5958 <at> debbugs.gnu.org (full text, mbox):
Alan Curry wrote:
> A full investigation has revealed:
> This bug was introduced between coreutils 7.1 and 7.2, here:
Thanks for digging.
I closed this ticket a day or two prematurely.
Sorry about that.
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Sat, 22 May 2010 11:24:03 GMT)
Full text and
rfc822 format available.
This bug report was last modified 15 years and 34 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.