Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'times' should be a special builtin #16

Closed
stephane-chazelas opened this issue May 13, 2016 · 2 comments
Closed

'times' should be a special builtin #16

stephane-chazelas opened this issue May 13, 2016 · 2 comments
Assignees

Comments

@stephane-chazelas
Copy link

In ksh93, times is implemented as an alias to "{ { time;} 2>&1;}" and "command" as an alias to "command " (which means aliases are expanded after it).

That means that things like:

$ ksh93 -c 'LC_ALL=C times'
ksh93: syntax error at line 1: `{' unexpected
$ ksh93 -c 'command times'
ksh93: syntax error at line 1: `{' unexpected

Don't work, so the times utility is not POSIX compliant.

"times" should be implemented as a special builtin.

More generally, even though allowed by POSIX, implementing builtin utilities as aliases is not a good idea IMO if only for the reasons detailed at:
http://thread.gmane.org/gmane.comp.standards.posix.austin.general/12485/focus=12568

@McDutchie
Copy link
Contributor

As Stéphane has pointed out on austin-group-l, the output of times (being actually the output of time) on ksh93 is not POSIX compliant either. It should follow this format. So that's another reason to implement a proper special builtin.

@dannyweldon dannyweldon added the bug label Oct 7, 2017
@krader1961 krader1961 added enhancement and removed bug labels Nov 7, 2017
@siteshwar siteshwar changed the title ksh93: "LC_ALL=C times" or "command times" don't work 'times' should be a special builtin Jul 24, 2018
@krader1961
Copy link
Contributor

krader1961 commented Jun 11, 2019

I'm laughing out loud (LOL!) because so many responses to my proposed changes have stated that adherence to POSIX is paramount. Yet the old ksh development team couldn't be bothered to spend what should take no more than a day (total time for all involved) to implement a POSIX compliant times command. This came to my attention due to PR #1327 and PR #1329. At first glance it looks like the path through sh_exec() that used the uninitialized var tb exists solely to allow the times alias to work. As well as produce output reflecting zero CPU time used when no pipeline follows the time keyword. The latter case being an error that should behave as an error; e.g., a non-zero status and an error message written to stderr rather than meaningless times written to stdout.

@krader1961 krader1961 self-assigned this Jun 12, 2019
krader1961 added a commit that referenced this issue Jun 19, 2019
krader1961 added a commit that referenced this issue Jun 19, 2019
Add support for using `getrusage()` if available.

Change the default precision of the `times` and `time` output to be
milliseconds. This exposes a bug in the formating of `time` output. See
issue #1333. I'll fix that in a subsequent change.

Related #16
krader1961 added a commit that referenced this issue Jun 19, 2019
Verify that real and user mode times that are in the range [1.0, 2.0)
are correctly reported.

Related #16
citrus-it pushed a commit to citrus-it/ast that referenced this issue Apr 15, 2021
Remove a buggy optimization for variables in subshells

Fixes att#15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants