Paul Eggert <[email protected]> writes:
> On 8/7/26 15:00, Walter Beschon wrote:
>> I was looking around the coreutils bug and
>> found #70586. Is it still an open issue ?
>
> Yes, I think so. Collin said he was thinking of working on it, so you
> might coordinate with him. If you make nontrivial changes, we'll need
> copyright papers signed by you and your employer; I assume that's OK.
I haven't gotten anything meaningful done here.
One word of caution, though, I don't think is a particularly good issue
to start out with in GNU coreutils. I don't say that to discourage you
from contributing, of course, this issue is just particularly tricky
even for me who is more familiar with the code. I'll share some thoughts
about it, since I spent a little bit too long thinking about it
previously.
However, before I get to that, to avoid discouraging you I should
mention that the GNU coreutils test suite is probably a good place to
get started. There are various behaviors that aren't tested, e.g., I
recently added tests for 'dirname -z' [1]. Spotting them is not the most
easy thing, but getting comfortable writing tests will certainly help
when you want to make changes to the programs themselves. Also, it will
help you learn how the commands work, if you are not very comfortable
with them. As an example, I knew very little when I started working on
gnulib and coreutils. See Pádraig helping me understand how 'join'
worked [2].
Back to why I don't think this issue in particular isn't a great
starting point. The source code for 'cp' is shared with 'install' and
'mv'. As such, it has many options and handles many different file
types. This is all quite complex, as you can see from looking at
src/copy.c. Introducing a bug here is also worse than other places, as
you can imagine, so even I am a bit hesitant to make changes.
Paul had said earlier in this thread that 'ls -R' was likely easier than
'cp' and I initially agreed. However, looking into it I realized it was
much more tricky than I originally thought. Given the following tree,
with levels on the right:
tmp | 1
/ | \ |
/ | \ |
/ | \ | 2
c b a |
/ | \ / | \ / | \ |
/ | \ / | \ / | \ |
f e d f e d f e d | 3
'ls -R tmp' will emit the directories at level 2 in sorted order, i.e.,
"a b c" and then visit each of them in that order, doing the same for
each level below recursively. The fts functions could typically handle
this easily, with something like this:
static int sort_fn (FTSENT const **, FTSENT const **);
char const *ftspath[2] = { "/tmp", NULL };
int fts_flags = ...;
FTS *fts = fts_open ((char *const *) ftspath, fts_flags,
sort_fn);
The tricky part is that 'ls' handles the case where "strcoll" fails,
falling back to "strcmp". With the sort function given to "fts_open"
there is no way to change the ordering after directory entries are
visited. I.e., if "strcoll" fails you will visit directories in the
wrong order because you cannot change the order to "strcmp".
The initial thought I had to avoid this was to buffer the current levels
directory entries in memory, as 'ls' does now anyways (in most cases,
'ls -R1 --sort=none', for example, does not). This works well for
printing them, but not for visiting them recursively. There isn't a way
to change the order used by the fts API for previously visited entries.
In other words, this when recursing into the level 2 directories you
will visit them in the order "c b a", which is incorrect since they
should be visited in the order "a b c". However, once you are in the
level 2 directories the children in level 3 will be printed in the
correct order "d e f".
I considered suggesting fts_openat because of this, which would allow
you to use fts_open on each directory relative to the current visited
level safely. You could also change the sort function per level that
way. However, I never got around to it.
Hopefully I didn't discourage you too much. :)
Collin
[1]
https://github.com/coreutils/coreutils/commit/6b8c70623329f9399ec6f313a956d9d10879bf40
[2] https://lists.gnu.org/archive/html/bug-gnulib/2024-03/msg00019.html
