Subject: bug#20511: closed (Re: bug#20511: split : does not account for --numeric-suffixes=FROM in calculation of suffix length?)

From: [email-masked] (GNU bug Tracking System)
Date: Wed, 13 May 2015 01:22:02 +0000

Your bug report

#20511: split : does not account for --numeric-suffixes=FROM in calculation of suffix length?

which was filed against the coreutils package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
20511: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=20511
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#20511: split : does not account for --numeric-suffixes=FROM in calculation of suffix length?
From: Pádraig Brady <[email-masked]>
To: Ben Rusholme <[email-masked]>, [email-masked]
Date: Wed, 13 May 2015 02:20:27 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

On 06/05/15 11:53, Pádraig Brady wrote:
> On 06/05/15 05:29, Ben Rusholme wrote:
>> As you say, this can always be fixed by the "--suffix-length" argument, but it’s only required for certain combinations of FROM and CHUNK, (and “split” already has all the information it needs).
>>
>>> Now you could bump the suffix length based on the start number,
>>> though I don't think we should as that would impact on future
>>> processing (ordering) of the resultant files.  I.E. specifying
>>> a FROM value to --numeric-suffixes should only impact the
>>> start value, rather than the width.
>>
>> Could you clarify this for me? Doesn’t the zero-padding ensure correct processing order?
> 
> There are two use cases supported by specifying FROM.
> 1. Setting the start for a single run (FROM is usually 1 in this case)
> 2. Setting the offset for multiple independent split runs.
> In the second case we can't infer the size of the total set
> in any particular run, and thus require that --suffix-length is specified appropriately.
> I.E. for multiple independent runs, the suffix length needs to be
> fixed width across the entire set for the total ordering to be correct.
> 
> 
> Things we could change are...
> 
> 1. Special case FROM=1 to assume a single run and thus
> enable auto suffix expansion or appropriately sized suffix with CHUNK.
> This would be a backwards incompat change and also not
> guaranteed a single run, so I'm reluctant to do that.
> 
> 2. Give an early error with specified FROM and CHUNK
> that would overflow the suffix size for CHUNK.
> This would save some processing, though doesn't add
> any protections against latent issues. I.E. you still get
> the error which is dependent on the parameters rather than the input data size.
> Therefore it's probably not worth the complication.
> 
> 3. Leave suffix length at 2 when both FROM and CHUNK are specified.
> In retrospect, this would probably have been the best option
> to avoid ambiguities like this. However now we'd be breaking
> compat with scripts with FROM=1 and CHUNK=200 etc.
> While CHUNK values > 100 would be unusual
> 
> 4. Auto set the suffix len based on FROM + CHUNK.
> That would support use case 1 (single run),
> but _silently_ break subsequent processing order
> of outputs from multiple split runs
> (as FROM is increased in multiples of CHUNK size).
> We could mitigate the _silent_ breakage though
> by limiting this change to when FROM < CHUNK.
> 
> 5. Document in man page and with more detail in info docs
> that -a is recommended when specifying FROM
> 
> So I'll do 4 and 5 I think.

Attached.

cheers,
Pádraig

>From 4d5e6c4f4a2ba8407420e56282c0d4e37b2691ee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email-masked]>
Date: Wed, 6 May 2015 01:48:40 +0100
Subject: [PATCH] split: auto set suffix len for --numeric-suffixes=<N
 --number=N

Supporting `split --numeric-suffixes=1 -n100` for example.

* doc/coreutils.texi (split invocation): Mention the two
use cases for the FROM parameter, and the consequences on
the suffix length determination.
* src/split.c (set_suffix_length): Use the --numeric-suffixes
FROM parameter in the suffix width calculation, when it's
less than the number of files specified in --number.
* tests/split/suffix-auto-length.sh: Add test cases.
Fixes http://bugs.gnu.org/20511
---
 doc/coreutils.texi                | 11 ++++++++---
 src/split.c                       | 22 ++++++++++++++++++++--
 tests/split/suffix-auto-length.sh | 21 ++++++++++++++++-----
 3 files changed, 44 insertions(+), 10 deletions(-)

diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 51d96b4..f887e04 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -3181,9 +3181,14 @@ specified, will auto increase the length by 2 as required.
 @opindex --numeric-suffixes
 Use digits in suffixes rather than lower-case letters.  The numerical
 suffix counts from @var{from} if specified, 0 otherwise.
-Note specifying a @var{from} value also disables the default
-auto suffix length expansion described above, and so you may also
-want to specify @option{-a} to allow suffixes beyond @samp{99}.
+
+@var{from} is used to either set the initial suffix for a single run,
+or to set the suffix offset for independently split inputs, and consequently
+the auto suffix length expansion described above is disabled.  Therefore
+you may also want to use option @option{-a} to allow suffixes beyond @samp{99}.
+Note if option @option{--number} is specified and the number of files is less
+than @var{from}, a single run is assumed and the minimum suffix length
+required is automatically determined.
 
 @item --additional-suffix=@var{suffix}
 @opindex --additional-suffix
diff --git a/src/split.c b/src/split.c
index 5d6043f..b6fe2dd 100644
--- a/src/split.c
+++ b/src/split.c
@@ -39,6 +39,7 @@
 #include "sig2str.h"
 #include "xfreopen.h"
 #include "xdectoint.h"
+#include "xstrtol.h"
 
 /* The official name of this program (e.g., no 'g' prefix).  */
 #define PROGRAM_NAME "split"
@@ -173,9 +174,26 @@ set_suffix_length (uintmax_t n_units, enum Split_type split_type)
   if (split_type == type_chunk_bytes || split_type == type_chunk_lines
       || split_type == type_rr)
     {
+      uintmax_t n_units_end = n_units;
+      if (numeric_suffix_start)
+        {
+          uintmax_t n_start;
+          strtol_error e = xstrtoumax (numeric_suffix_start, NULL, 10,
+                                       &n_start, "");
+          if (e == LONGINT_OK && n_start <= UINTMAX_MAX - n_units)
+            {
+              /* Restrict auto adjustment so we don't keep
+                 incrementing a suffix size arbitrarily,
+                 as that would break sort order for files
+                 generated from multiple split runs.  */
+              if (n_start < n_units)
+                n_units_end += n_start;
+            }
+
+        }
       size_t alphabet_len = strlen (suffix_alphabet);
-      bool alphabet_slop = (n_units % alphabet_len) != 0;
-      while (n_units /= alphabet_len)
+      bool alphabet_slop = (n_units_end % alphabet_len) != 0;
+      while (n_units_end /= alphabet_len)
         suffix_needed++;
       suffix_needed += alphabet_slop;
       suffix_auto = false;
diff --git a/tests/split/suffix-auto-length.sh b/tests/split/suffix-auto-length.sh
index 6ee2200..6ef221a 100755
--- a/tests/split/suffix-auto-length.sh
+++ b/tests/split/suffix-auto-length.sh
@@ -1,5 +1,5 @@
 #!/bin/sh
-# Test the suffix auto widening functionality
+# Test the suffix auto width functionality
 
 # Copyright (C) 2012-2015 Free Software Foundation, Inc.
 
@@ -20,14 +20,14 @@
 print_ver_ split

-# ensure this feature is off when start number specified
-truncate -s12 file.in
+# ensure auto widening is off when start number specified
+truncate -s12 file.in || framework_failure_
 returns_ 1 split file.in -b1 --numeric=89 || fail=1
 test "$(ls -1 x* | wc -l)" = 11 || fail=1
 rm -f x*
 
-# ensure this feature works when no start num specified
-truncate -s91 file.in
+# ensure auto widening works when no start num specified
+truncate -s91 file.in || framework_failure_
 for prefix in 'x' 'xx' ''; do
     for add_suffix in '.txt' ''; do
       split file.in "$prefix" -b1 --numeric --additional-suffix="$add_suffix" \
@@ -39,4 +39,15 @@ for prefix in 'x' 'xx' ''; do
     done
 done
 
+# ensure auto width with --number and start num < number of files
+# That's the single run use case which is valid to adjust suffix len
+truncate -s100 file.in || framework_failure_
+split --numeric-suffixes=1 --number=r/100 file.in || fail=1
+rm -f x*
+
+# ensure no auto width with --number and start num >= number of files
+# That's the multi run use case which is invalid to adjust suffix len
+# as that would result in an incorrect order for the total output file set
+returns_ 1 split --numeric-suffixes=100 --number=r/100 file.in || fail=1
+
 Exit $fail
-- 
2.3.4

Subject: split : does not account for --numeric-suffixes=FROM in calculation of suffix length?
From: Ben Rusholme <[email-masked]>
To: [email-masked]
Date: Tue, 5 May 2015 13:42:12 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

Hi,

“split” (in the current GNU coreutils 8.23 release) does not account for the optional start index (“split --numeric-suffixes=FROM”) when calculating suffix length.

I couldn’t find any prior reference to this problem in either the bug tracker or mailing list archive.

Thanks, Ben

$ seq 100 >& input.txt
$ split --numeric-suffixes --number=l/100 input.txt
$ ls
input.txt  x06  x13  x20  x27  x34  x41  x48  x55  x62  x69  x76  x83  x90  x97
x00        x07  x14  x21  x28  x35  x42  x49  x56  x63  x70  x77  x84  x91  x98
x01        x08  x15  x22  x29  x36  x43  x50  x57  x64  x71  x78  x85  x92  x99
x02        x09  x16  x23  x30  x37  x44  x51  x58  x65  x72  x79  x86  x93
x03        x10  x17  x24  x31  x38  x45  x52  x59  x66  x73  x80  x87  x94
x04        x11  x18  x25  x32  x39  x46  x53  x60  x67  x74  x81  x88  x95
x05        x12  x19  x26  x33  x40  x47  x54  x61  x68  x75  x82  x89  x96

$ rm x*
$ split --numeric-suffixes=1 --number=l/100 input.txt
split: output file suffixes exhausted
$ ls
input.txt  x07  x14  x21  x28  x35  x42  x49  x56  x63  x70  x77  x84  x91  x98
x01        x08  x15  x22  x29  x36  x43  x50  x57  x64  x71  x78  x85  x92  x99
x02        x09  x16  x23  x30  x37  x44  x51  x58  x65  x72  x79  x86  x93
x03        x10  x17  x24  x31  x38  x45  x52  x59  x66  x73  x80  x87  x94
x04        x11  x18  x25  x32  x39  x46  x53  x60  x67  x74  x81  x88  x95
x05        x12  x19  x26  x33  x40  x47  x54  x61  x68  x75  x82  x89  x96
x06        x13  x20  x27  x34  x41  x48  x55  x62  x69  x76  x83  x90  x97
$ # Should run from x001 to x100!

$ rm x*
$ split --numeric-suffixes=1 --number=l/101 input.txt
$ ls
input.txt  x008  x016  x024  x032  x040  x048  x056  x064  x072  x080  x088  x096
x001       x009  x017  x025  x033  x041  x049  x057  x065  x073  x081  x089  x097
x002       x010  x018  x026  x034  x042  x050  x058  x066  x074  x082  x090  x098
x003       x011  x019  x027  x035  x043  x051  x059  x067  x075  x083  x091  x099
x004       x012  x020  x028  x036  x044  x052  x060  x068  x076  x084  x092  x100
x005       x013  x021  x029  x037  x045  x053  x061  x069  x077  x085  x093  x101
x006       x014  x022  x030  x038  x046  x054  x062  x070  x078  x086  x094
x007       x015  x023  x031  x039  x047  x055  x063  x071  x079  x087  x095