Re: patch: contrib/compare_tests

[Paul Eggert]

Zack Weinberg <address@hidden> writes:

> we might consider a patch that did something clever like
> figure out which command line syntax this system's sort actually
> supports.

That's what GCC already does in lots of places, so let's do that.

I thought this issue was done, but I guess not, so here's a brief
recent history to bring people up to date.

In January 2005 the Open Group agreed that POSIX implementations can
support usages like "head -1" as an extension.  As a result, coreutils
version 5.90 and later support these usages, even when they are
executed in conform-to-recent-POSIX mode.  GCC maintainers and
builders can either use these newer versions of coreutils, or, if they
want to use older coreutils versions that have been compiled to be in
conform-to-recent-POSIX mode, can override the default by setting
_POSIX2_VERSION=199209 in their environment.

The remaining problem in this area is usages with leading "+", e.g.,
"tail +16c" and "sort +1", where POSIX says that the "+16c" and "+1"
are file names, not options.  Here, the standard-required behavior is
useful, since it supports file names with leading "+" and leads to
less-surprising behavior for commands like "sort *.txt".  So I'm
asking other developers to not depend on the old-fashioned and
incompatible behavior in this relatively-small area.

GCC was swept for these leading-"+" usages a while ago and they were
fixed, which is why I thought the issue was closed.  But I just now
swept GCC's sources again and found four instances of the problem.

The first was the problem that started this thread.  I omitted the
'contrib' directory in my earlier sweep, which is why I missed this.
(I didn't omit 'contrib' in today's sweep....)

Two are from ltmain implementations derived from old versions of
Libtool.  Libtool was fixed years ago but I guess GCC forked.  I had
omitted Libtool updates in my earlier sweep since I incorrectly
assumed GCC would pick up fixes from Libtool.

The last was the implementation of the slowcompare option.  I omitted
this in my earlier sweep, since I assumed nobody would want to use
slowcompare (or, at least, slowcompare with coreutils), and since the
old code is clearly buggy anyway (it skips 15 bytes, not 16).
However, just so's we can put this baby to bed, let's fix this too.

Here's a proposed patch that should address all the remaining issues
in this area.

2006-05-03  Paul Eggert  <address@hidden>

        Port to hosts whose 'sort' and 'tail' implementations
        treat operands with leading '+' as file names, as POSIX
        has required since 2001.  However, make sure the code still
        works on pre-POSIX hosts.
        * gcc/Makefile.in (slowcompare): Port to POSIX 1003.1-2001,
        which says you should use "tail -c +N" rather than "tail +Nc".
        Fix a bug: the old code incorrectly skipped 15 bytes, not 16.
        * ltmain.sh: Don't assume "sort +2" is equivalent to
        "sort -k 3", since POSIX 1003.1-2001 no longer requires this.
        This uses the same fix that is already in
        libjava/libltdl/ltmain.sh.
        * libjava/classpath/ltmain.sh: Likewise.
        * contrib/compare_tests: Likewise.

Index: gcc/Makefile.in
===================================================================
--- gcc/Makefile.in     (revision 113500)
+++ gcc/Makefile.in     (working copy)
@@ -4325,8 +4325,13 @@
            for file in $$dir/*$(objext); do \
              case "$@" in \
                slowcompare* ) \
-                 tail +16c ./$$file > tmp-foo1; \
-                 tail +16c stage$$stage/$$file > tmp-foo2; \
+                 if tail -c +1 </dev/null >/dev/null 2>&1; then \
+                   skip16='-c +17'; \
+                 else \
+                   skip16='+17c'; \
+                 fi; \
+                 tail $$skip16 ./$$file > tmp-foo1; \
+                 tail $$skip16 stage$$stage/$$file > tmp-foo2; \
                  cmp tmp-foo1 tmp-foo2 > /dev/null 2>&1; \
                  cmpret=$$?; \
                  ;; \
Index: ltmain.sh
===================================================================
--- ltmain.sh   (revision 113500)
+++ ltmain.sh   (working copy)
@@ -3839,7 +3839,13 @@
            fi
 
            # Try sorting and uniquifying the output.
-           if grep -v "^: " < "$nlist" | sort +2 | uniq > "$nlist"S; then
+           if grep -v "^: " < "$nlist" |
+               if sort -k 3 </dev/null >/dev/null 2>&1; then
+                 sort -k 3
+               else
+                 sort +2
+               fi |
+               uniq > "$nlist"S; then
              :
            else
              grep -v "^: " < "$nlist" > "$nlist"S
Index: libjava/classpath/ltmain.sh
===================================================================
--- libjava/classpath/ltmain.sh (revision 113500)
+++ libjava/classpath/ltmain.sh (working copy)
@@ -3839,7 +3839,13 @@
            fi
 
            # Try sorting and uniquifying the output.
-           if grep -v "^: " < "$nlist" | sort +2 | uniq > "$nlist"S; then
+           if grep -v "^: " < "$nlist" |
+               if sort -k 3 </dev/null >/dev/null 2>&1; then
+                 sort -k 3
+               else
+                 sort +2
+               fi |
+               uniq > "$nlist"S; then
              :
            else
              grep -v "^: " < "$nlist" > "$nlist"S
Index: contrib/compare_tests
===================================================================
--- contrib/compare_tests       (revision 113500)
+++ contrib/compare_tests       (working copy)
@@ -31,9 +31,15 @@
 exit_status=0
 trap "rm -f $tmp1 $tmp2 $now_s $before_s" 0 1 2 3 5 9 13 15
 
-sort -t ':' +1 "$now" > "$now_s"
-sort -t ':' +1 "$before" > "$before_s"
+if sort -k 2 </dev/null >/dev/null 2>&1; then
+  skip1='-k 2'
+else
+  skip1='+1'
+fi
 
+sort -t ':' $skip1 "$now" > "$now_s"
+sort -t ':' $skip1 "$before" > "$before_s"
+
 grep '^FAIL:' "$now_s" | sed 's/^[^:]*:[       ]//' >$tmp1
 grep '^PASS' "$before_s" | sed 's/^[^:]*:[     ]//' | comm -12 $tmp1 - >$tmp2