Re: [PATCH] ls: support --time=creation to show/sort birth time

[Pádraig Brady]

On 02/01/2020 20:29, Assaf Gordon wrote:
> Hello Pádraig and all,
>
>
> On 2020-01-02 10:48 a.m., Pádraig Brady wrote:
> > +  ls now supports the --time=birth option to display and sort by
> > +  file creation time, where available.
>
> +1
>
> Patch looks good, builds and passes the test on Debian 10/x86_64,
> OpenBSD 6.6, FreeBSD 12.1, Alpine Linux, and Cygwin-10/64bit on
> Windows7/NTFS.
>
> A suggestion:
>
> >    static char const *const time_args[] =
> >    {
> > -  "atime", "access", "use", "ctime", "status", NULL
> > +  "atime", "access", "use",
> > +  "ctime", "status",
> > +  "birth", "creation",
> > +  NULL
> >    };
> >    static enum time_type const time_types[] =
> >    {
> > -  time_atime, time_atime, time_atime, time_ctime, time_ctime
> > +  time_atime, time_atime, time_atime,
> > +  time_ctime, time_ctime,
> > +  time_btime, time_btime,
> >    };
>
> Perhaps add "btime" and "crtime" as aliases to birth time?
> "btime" is for completion with atime/ctime.
> "crtime" is used/mentioned in some contexts (e.g. in "debugfs").

Re naming I read about crtime in https://lwn.net/Articles/397442/
I was going for a higher level (more descriptive) naming
in the option, than that used in the programmatic interface.
I.E. I'm not sure "atime" or "ctime" should have been provided
as options in the first place.  I was hard pushed to
provide two options, but thought "birth" might aid
searches, while "creation" was more descriptive.
At least both are terms that don't require explanation.

> > +/* Return the platform birthtime member of the stat structure,
> > +   or fallback to the mtime member, which we have populated
> > +   from the statx structure where supported.  */

Oh good spot. That's a stale comment. I had the initial idea
of transparently falling back from birth time to mtime,
but thought better of it.

> Regarding "fall back to mtime", I'm seeing the following results
> on some systems - not necessarily a bug, but perhaps it's worth
> knowing what to expect:
>
> * Debian 10/x86_64, Linux Kernel 4.19.0, glibc 2.28-10,
> with ext2 file system (not supporting birthtime):
>
>     $ ./src/ls -l --time=birth /tmp/dummy-ext2/2
>     -rw-r--r-- 1 root root 0 Dec 31  1969 /tmp/dummy-ext2/2
>
> (I guess this is unix-epoch adjusted for my local time zone)

Hmm. That suggests that STATX_BTIME is set in the
returned statx mask, but populated with 0 in the structure
({-1,-1} would have printed as '?'). Though you say src/stat
prints '-' in all cases, and the logic should be much the same.
Could you confirm the birth time significant returns for this case.
epoch isn't a bad time to output in this case, but it would
be good to be consistent.

I should say I've not actually tested this on a statx()
supporting system, and will do that before pushing.

The other cases you tested return '?' as expected.

thanks!
Pádraig