Time: Implement BSD extensions to time

[envp]

Provides:

• [X] -f: Format stats with an arbitrary printf-like %<spec> formatters
• [X] -t: Format stats using the tcsh(1) format string
  • See: https://man.netbsd.org/time.1
• [X] -c: Write time stats in the csh(1) format string
  • See: https://man.netbsd.org/time.1
• [ ] -l: Dump the contents of the rusage struct (⚠️ Unimplemented as of this PR)
• [X] -a: If -o was provided, append to the target file instead of truncating
• [X] -o: Create, truncate a file to write time output to.

Part of: #53

[envp]

@GrayJack How do I get clap to not escape control characters such as "\t" in the CLI input?

I was trying out the format strings with some control chars in them, but seems they are escaped by default, and I'm not sure how to turn that off for a particular option (and unescaping after-the-fact seems a rather odd thing to do as well)

$ cargo run --bin time -- -f "%Uu %Ss %E %P\t%X+%Dk %I+%Oio %Fpf+%Ww" sleep 1
0.76u 0.24s 1.01 0.99\t0+0k 0+85954560io 1115pf+0w

[marcospb19]

I took some time for trying understanding this.

It seems that "Escaping" is the default behavior, and treating '\n' and '\t' as special characters (line break and tab, as an example) will always require extra logic, that is, searching for '\' and checking the next character (unescaping is extra work).

However, some shells have their built-in versions of some coreutils with this feature, I assume that your shell is running a built-in version of time that does it.

@GrayJack correct me if I'm mistaken, but currently there is no util in this repo that unescapes arguments, not even echo (built-in version of zsh does).

clap does not support this operation.

[GrayJack]

Sorry, I was looking in the cellphone app and by mistake I marked this pull request as ready for review.

I think @marcospb19 is correct. At least the version of clap we're using doesn't support unescaping arguments. Maybe the version 3.0 have this feature?

[GrayJack]

CI is failing because of "if in const fn" in the clap app, inside a bitflags! macro.

Looks like clap set bitflags dependency to "1.0", but 1 on 1.3.0 a function was turned to const fn

[envp]

Seems this is blocked on:

error[A001]: Potential segfault in the time crate
    ┌─ /github/workspace/Cargo.lock:106:1
    │
106 │ time 0.2.22 registry+https://github.com/rust-lang/crates.io-index

And this is time, the external command:

$ git grep '0.2.22'
coreutils_core/Cargo.toml:36:time = "= 0.2.22"
touch/Cargo.toml:18:time = "= 0.2.22"

I think it would make grepping the error messages easier if the time sub-crate was renamed to something else.

[envp]

It is really hard to implement the -l flag without some FFI. I think it will be good to have that come through another PR.

I'll leave info here for follow-up in case me or someone else is interested at a later time:

• Apple's time.c: https://opensource.apple.com/source/shell_cmds/shell_cmds-216.60.1/time/time.c.auto.html

The region of interest would be to look at:

if (rusage_ret >= 0) {
    if (ruinfo.ri_instructions > 0) {
        fprintf(stderr, "%20" PRIu64 "  %s\n", ruinfo.ri_instructions,
            "instructions retired");
    }
    if (ruinfo.ri_cycles > 0) {
        fprintf(stderr, "%20" PRIu64 "  %s\n", ruinfo.ri_cycles,
            "cycles elapsed");
    }
    if (ruinfo.ri_lifetime_max_phys_footprint > 0) {
        fprintf(stderr, "%20" PRIu64 "  %s\n",
            ruinfo.ri_lifetime_max_phys_footprint,
            "peak memory footprint");
    }
}

This uses a struct rusage_info_v4 ruinfo, that is returned by proc_pid_rusage(pid, RUSAGE_INFO_V4, (void **)&ruinfo). time should call this somehow to fill up some platform dependent / Optional fields in the RUsage struct.

[envp]

@GrayJack This is ready for review, sorry for the commit spam :)

[envp]

ping!

Any reviewers open to take a look at this?

[GrayJack]

Sorry for the late reply @envp , I'm getting used to my new job, but I'll review this PR either today or in the next weekend

[envp]

Sorry for the late reply @envp , I'm getting used to my new job, but I'll review this PR either today or in the next weekend

No worries :)

I understand it can be hard to make time with (especially new) full-time responsibilities!

[envp]

One thing I noticed is that if the code fails to write to the output, the error message doesn't have the name of the program and the name of the output, just the error message. I don't know where is the code of this error printing, I'll need more time to find it, but if you know where it is, can you point it out to me? No need to fix in this PR, I think the PR is in a great state and this specific problem should be part of another PR

Sure! I'd be happy to fix it in another PR :). I wrote the .expect to silence rustc about unused results, but now that they're there, I may as well improve them enough that they're good assertions.

I also wanted to ask, in the function custom_formatter there is some expect, this is intentional, correctly?

Yep, I'm not sure how to handle those exactly. Would it be better to wrap the errors in the individual formatters, instead of failing eagerly? (All the expects are at locations where the error is unrecoverable, i.e. user asked for output but we failed to write to the format buffer for whatever reason.)

[bors[bot]]

Timed out.

[GrayJack]

bors r+

[bors[bot]]

Timed out.

[GrayJack]

bors r+

If it doesn't work this time, I'll consider a manual merge

[bors[bot]]

Timed out.

[envp]

Timed out.

:(