recently changed cd behaviour (_cd function, autoloading etc) breaks existing ksh scripts

[jghub]

Description of problem:

I tried out a recent commit (c99e9ff) which breaks my scripts that use "command cd"

Ksh version: commit c99e9ff How reproducible: always.

Steps to reproduce:

1. type "command cd" or "command cd some_dir_name"
2. error message results: "/opt/local/bin/ksh: _cd: not found"

Actual results:

error message

Expected results:

that the cd command Is executed (in the given example cd to home dir)

Additional info:

see mailing list: overruling default cd and making such convenience measures an unavoidable (no opt-in, no opt-out) part of the shell is bad. please revert the change and make the functionality optionally available to the users if you find it desirable to provide it by some strategy (for instance activate via a new environment variable which must be actively set by the user). please don't hardcode it into the shell. many users will not want it at all. it's the users choice, not yours (or it should be...)

[krader1961]

@jghub, Does this break scripts or just your interactive shells? It should not break any scripts because I explicitly noted the function behavior of always printing to stdout the new directory was annoying and likely to break scripts (or the programs that ran those scripts due to the unexpected stdout text). Which is why, as a stepping stone, this feature is only enabled for interactive shells:

https://github.com/att/ast/blob/42a580c50d55324e27cda865f2222d71e4ae6baf/src/cmd/ksh93/data/config.ksh#L32-L38

I think this basic functionality should be enabled by default for interactive shells. A new ksh user shouldn't have to install a third-party script (or write their own) to get reasonable cd or mcd behavior. How to enable this functionality for everyone without causing undue pain for sophisticated users accustomed to working around that shortcoming is an open question.

[jghub]

I have explained all this. should have been obvious from context but maybe the use of 'script' was confusing. so, to reiterate: It happens when sourcing from my .kshrc a 1000 LOC script (...) of function definitions that set up a completely different approach (more like "URL guessing from history" when you start typing some text in the browser's adress bar) to directory stack management . the whole stuff is explained here:

http://fossil.0branch.com/sd/

and even might be downloaded:

http://fossil.0branch.com/sd/artifact/a92f3c62f0a3b615 (only the file "sd.ksh" matters and needs to be sourced (manually or from within .kshrc) to activate it.

but the problem will happen to everyone that is defining his own cd function in such a way that it calls another user function which in turn wants/has to call the builtin cd. you should have understood the problem from the start: in the descriped situation you must call

"command cd"

within the user function to acess the builtin cd (rather than recursively the user cd function) with its default behaviour since your machinery relies on that.

with your meddling you have achieved that simply typing "command cd" on the command line throws an error. totally horrible idea to impose your cd overruling on everybody. please stop that. you are doing serious harm here.

and please reread the mailing list posts w.r.t. to this problem and everything said in this issue regarding the complete and utter undesirability of having any such stuff (independent of its questionable usefulness for many more than casual ksh users) enabled mandatory or even only by default.

you are wrong with your contrary opinion. you want that feature. nobody else does. and it is not even a (unwanted) feature but a bug if a user is doing his own 'cd' overruling for his own purposes.

cd has to act by default as it always did. everything else is bad.

don't interfere with established behaviour of `command cd' . that is the crucial point as far as I can see from the problems I experienced.

[0branch]

@krader1961: Given that this changes (long) established behaviour and can break users' interactive sessions, can't you include this function as an example, and document it as a possible improvement to default cd behaviour instead?

This seems less invasive than changing cd for everyone.

[krader1961]

@jghub wrote:

you want that feature. nobody else does.

And yet you still implemented your own version of this feature. Also, you wrote:

with your meddling you have achieved that simply typing "command cd" on the command line throws an error.

That is because of the alias cd=_cd in the original implementation by the AST team. From the ksh93v- branch that this is all based on:

https://github.com/att/ast/blob/e770c77e9816e156c6df4a455e71b5f9fff79310/src/cmd/ksh93/fun/dirs#L5-L7

[krader1961]

@0branch Sure, we could do that. But that is essentially the old situation. Which meant absolutely no one knew the example functions had suffered bit rot and were not 100% compatible with the builtin cd with regard to when the new directory was written to stdout. If that is the consensus we should just eliminate those files from this source repository. I expect every single bit of code in this repo to work for everyone using ksh built from it.

This is an experiment to see if we can manage to implement, via a ksh function, cd functionality that most users expect to be present. Other shells, like fish, have managed to do so. So I don't see why this project shouldn't be able to do so. Note that I do not like, and do not use, prevd/nextd or pushd/popd. But I do use dirs and mcd (albeit a much more sophisticated version than the current ksh implementation).

P.S., If you're using a ksh built from the current master branch (not the 2020.0.0 branch) try running "man alias" or "man cd". That is also part of the change that introduced the _cd function and cd alias. That change also introduces a man function that will show you the documentation for a specific builtin. Yes, I am well aware you could do alias --man | $PAGER in ksh93u+. Not to mention that man alias in ksh93u+ does not show the man page for the ksh alias command. That is not user friendly.

P.P.S., For the moment you should be able to avoid using the new cd function by simply doing unalias cd at the top of your ~/.kshrc. What I'd like to see is a civil, constructive, discussion about whether it is possible to introduce ksh functions enabled by default that augment builtins of the same name. If every builtin improvement has to be done by modifying the C source code that makes me a sad puppy.

[0branch]

@0branch Sure, we could do that. But that is essentially the old situation. Which meant absolutely no one knew the example functions had suffered bit rot and were not 100% compatible with the builtin cd with regard to when the new directory was written to stdout.

Understood.

It seems that multiple users have indicated that this new cd behaviour is an unexpected (and unwelcome) surprise, though. Perhaps that should be treated as user feedback to this experiment of enabling these functions?

If that is the consensus we should just eliminate those files from this source repository. I expect every single bit of code in this repo to work for everyone using ksh built from it.

This is not for me to comment on, though I don't see why optional functions can't be included in the distribution, for sourcing/loading as the user sees fit. In other words, the options don't seem to be limited to enabling functionality for everyone, or deleting because not everyone likes it.

Perhaps I've misunderstood you here?

This is an experiment to see if we can manage to implement, via a ksh function, cd functionality that most users expect to be present.

Two questions:

• Is this experiment intended to to make it into the next release, or stay on a branch for now?
• How will feedback to the experiment be collected?

(The second question is more general than this issue, really: for new/experimental features that change behaviour, is there a process for obtaining user feedback and addressing concerns?)

Other shells, like fish, have managed to do so.

To be honest, I'm not sure why this fish comparison is relevant. Fish (from what I can tell) doesn't have the same legacy, user expectations, etc.

So I don't see why this project shouldn't be able to do so.

…and I don't see why ksh development ought to be driven by expectations from another shell. This isn't to say that new functionality is bad, just that the justification here seem specious to me.

Note that I do not like, and do not use, prevd/nextd or pushd/popd. But I do use dirs and mcd (albeit a much more sophisticated version than the current ksh implementation).

I think the point is that individual user preferences aren't a problem (yours, mine, @jghub's above)—but if new defaults negatively affect the experience of long term ksh users, that feedback ought to be listened to.

P.S., If you're using a ksh built from the current master branch (not the 2020.0.0 branch) try running "man alias" or "man cd". That is also part of the change that introduced the _cd function and cd alias. That change also introduces a man function that will show you the documentation for a specific builtin. Yes, I am well aware you could do alias --man | $PAGER in ksh93u+. Not to mention that man alias in ksh93u+ does not show the man page for the ksh alias command. That is not user friendly.

P.P.S., For the moment you should be able to avoid using the new cd function by simply doing unalias cd at the top of your ~/.kshrc. What I'd like to see is a civil, constructive, discussion about whether it is possible to introduce ksh functions enabled by default that augment builtins of the same name. If every builtin improvement has to be done by modifying the C source code that makes me a sad puppy.

Thanks for these pointers—will take a look.

[jghub]

@krader1961

@jghub wrote: you want that feature. nobody else does.

And yet you still implemented your own version of this feature.

you do not listen and do not pay attention (again). what you say is, therefore, beside the actually important point at hand (again). So, to reiterate (again):

1. I did not implement a "version of this feature". I implemented something rather different from the things you have in mind (try it: maybe you than can appreciate the difference...). and this different behaviour I, personally, like. personally... would I have your attitude, the next logical step would be to impose this utility without alternatives as on many colleagues as I can victimise ("make them happy" in my private copy of the actual universe). this latter misguided desire of yours is at the heart of the present situation: your curious believe to know what users want.

2. the resulting non-default behaviour of cd in the standard setup of my utility is totally optional for every one to activate by sourcing the function definitions. if he personally wants to do that. as opposed to being forced to use it because you believe you know what he wants.

3. my utilitiy does not interfere with command cd and never would. command cd calls the valid builtin of that command. nothing else.

you fail on the relevant points 2 and 3 above:

2. you want to impose your world view of what a "good" cd behaviour is on everybody. without choice for the user to avoid this. this is wrong. your cd utility should be opt-in only. not the default. let alone hard-coded behaviour.

3. you break "command cd". this is really bizarre.

Also, you wrote:

with your meddling you have achieved that simply typing "command cd" on the command line throws an error.

That is because of the alias cd=_cd in the original implementation by the AST team. From the ksh93v- branch that this is all based on...

Wrong. an alias does not break command cd as long as the expanded alias is on the search path. but functions (like _cd) are not...

executive summary (revisited): • meddling with behaviour of builtin cd is a wrong. always will be. • by default(!) cd has to act as it always did. everything else is bad. • don't interfere with established behaviour of command cd. when issuing this command, the established cd builtin needs to be found and executed.

[jghub]

@krader1961

Yes, I am well aware you could do alias --man | $PAGER in ksh93u+. Not to mention that man alias in ksh93u+ does not show the man page for the ksh alias command. That is not user friendly.`

1. there is absolutely nothing wrong with alias --man. it is the canonical way to get help for all ksh builtins. (although --man writes to stderr, so your pipe above would be ineffectual).

2. man alias does not have anything to do with ksh93u+ as long as man is just the system's man command, of course. and naturally man alias (or man cd or so many others "builtins" whose availabliity seems to be guaranteed by POSIX standard) shows the generic builtin(1) manpage. as it should. look it up...

3. "this is not user friendly": this is utter nonsense. man alias redirects to man builtin (it should/must). alias --man uses the dedicated ksh93 -internal help system. this is exactly how it should be.

you seem again to intentionally break required default behaviour with man if I get this right.

[krader1961]

@0branch wrote:

It seems that multiple users have indicated that this new cd behaviour is an unexpected (and unwelcome) surprise, though.

AFAIK it is a single individual. The other person who concurred has hated what we're doing since switching from the AST Nmake build system to Meson over two years ago.

To be honest, I'm not sure why this fish comparison is relevant.

For precisely the same reason a command completion system (that is undocumented, btw) was added by the old AST team: To keep the shell relevant by improving its interactive behavior. Keeping a cd history is something that a large number, probably greater than 50%, of interactive ksh users implement. Including the person complaining about the current, experimental, feature. My vote is this should be a basic feature provided by any new ksh release. New users shouldn't have to google how to implement this capability. How to introduce this with minimal impact to the people who have implemented their own solution is an open question.

[jghub]

@krader1961

AFAIK it is a single individual. The other person who concurred has hated what we're doing since switching from the AST Nmake build system to Meson over two years ago.

there are overall only a handful of people from which you get any feedback (here or on the mailing list). without checking in detail I remember/count now at least 3 people out of said handful that tell you your intentions w.r.t. cd are unwelcome. Vs. zero people out of that small group telling you they like it.

Keeping a cd history is something that a large number, probably greater than 50%, of interactive ksh users implement. Including the person complaining about the current, experimental, feature. My vote is this should be a basic feature provided by any new ksh release. New users shouldn't have to google how to implement this capability. How to introduce this with minimal impact to the people who have implemented their own solution is an open question

in our small statistical sample there are 0% wanting it the way you approach it. and 100% don't. your 50% is completely unsubstantiated by any data. but whatever the true percentage out there might be: this is not the point. the point is that you should provide whatever you like as an option, not enforce it. an offer to the user, not an order to use that stuff. with zero impact to people wanting to use ksh with established behaviour.

and your comparison to fish is very much beside the point anyway: fish is not comparable to ksh/bash/zsh and the like in any sensible way. ksh basically is "the standard shell descending from Bourne shell". bash tried to implement ksh88 initially, zsh mostly did the same I believe. now (in my view) ksh93u+ defines how such a bourne shell descandant should (no: must) behave. builtin cd has a explicitly specified task. "command cd" must "reach" that behaviour.

why do you not simply accept that you must ensure that "command cd" reaches the builtin cd command in the ksh C code. than the principle problem (that you actively break peoples work) is gone.

regarding interactive basic ksh use of inexperienced users: provide what you like as an option.

you clearly overestimate the virtues/usefulness of that small utility (w/ or w/o your modifications). believing that it is good to forcibly convert the infidels to your set of beliefs is destined to fail. what does not work in religion neither will work here.

[siteshwar]

@krader1961 As I understand, this issue tracks a regression that was introduced by fa9673c64d5e0dd043d4a2f5c19c43e8189dfd44. I believe if we want to provide dirs, pushd and popd commands, it will be better to do them with shell builtins rather than through functions. If there are functions for it, they should be either kept disabled by default or should be provided through separate repo (oh-my-ksh?). The variables you have introduced in config.ksh may clash with existing scripts and I expect more users to complain about this behavior in future. What do you think about providing shell builtins instead of functions for these commands ?

[jghub]

@siteshwar at last! a sane voice. this is very much appreciated. very obviously that would be the correct strategy. make dirs, pushd, popd real builtins -- if these commands are seen as desirable at all -- like it is done, e.g., in bash and stay out of the way of the user otherwise. so if you have any say in this please take care that cd keeps its default behaviour and, even more importantly, command cd does the expected thing (reach the unmodified cd builtin). I find it very very curious that these things (and the indisputable importance of maintaining backward compatibility in general) can become a point of debate in the first place (and that requests to that extent meet with so massive a lack of understanding and willingness to really listen). greetings from dresden to brno (it seems)...

[krader1961]

I fixed the _cd function to be compatible with the builtin cd vis-a-vis printing notable directory changes. I also enabled it for non-interactive shells when run via Meson unit tests. That eliminated all but three test failures. All of which are in src/cmd/ksh93/tests/builtins.sh and involve changing the value of $OLDPWD then doing cd -. Behavior that is undocumented AFAICT. Fixing that is trivial. This is why these functions need to be enabled by default. If they aren't they have no place in this source code and should be documented elsewhere.

That the legacy source used alias cd=_cd looks like the proximate cause of most of the issues raised by the O.P. I have no idea why the old source for this functionality (written or blessed by the old AST team) used an alias rather than just defining a cd function. The \cd in the original (and current) version of the _cd function shows that they knew an alias was in effect and were deliberately avoiding an alias recursion. But they still expected the builtin, rather than a function, to be invoked. The obvious answer is to use command cd rather than \cd.

[krader1961]

@siteshwar wrote:

it will be better to do them with shell builtins rather than through functions.

It should be possible to implement commands such as dirs via ksh functions. There is literally nothing about them that requires they be builtins written in C since they do not involve the byproducts of any syscalls not available via builtins. Imagine if every Python module writer was told to write their module in C rather than Python.

The variables you have introduced in config.ksh may clash with existing scripts

Maybe, but unlikely. Nonetheless, the fix for that is trivial: Move them into the .sh var namespace. Even if the new "commands" that are implemented by functions were rewritten as C builtins there would still need to be an array of the recently visited directories. Not to mention other attributes, best represented as vars the user can modify, that control things such as the depth of the cd history. All of which should be visible to ksh scripts.

[jghub]

and involve changing the value of $OLDPWD then doing cd -. Behavior that is undocumented AFAICT.

known to everybody who can claim to be fluent in ksh. required (useful guaranteed) behaviour. what is undocumented about it? cd - changes to dir defined in OLDPWD. easy.

Fixing that is trivial. This is why these functions need to be enabled by default. If they aren't they have no place in this source code and should be documented elsewhere.

this "either or" is a totally absurd position construed by you to enforce their activation by default.

That the legacy source used alias cd=_cd looks like the proximate cause of most of the issues raised by the O.P.

I have no idea why the old source for this functionality (written or blessed by the old AST team) used an alias rather than just defining a cd function.

right, you haven't. maybe better find out rather than changing things for the worse.

The obvious answer is to use command cd rather than \cd.

`command cd' is broken. that was my original bug report. what has changed since then?

[jghub]

@siteshwar: I have become well aware that krader does ignore comments he does not like. what happens to me has obviously happened to multiple people before. he is wearing them down with his unbelievable rigidness and self-righteousness.

I have now reiterated not only my personal position but also very principle problems in "culture" and "politics" of this project a couple of times, basically as documentation what is happening here for others to see. going on is senseless. if you, personally, care about ksh and how it will fare I would appreciate if you intervene at relevant points (cd behaviour is sure one of these) and prevent that krader demolishes ksh completely.

what is going on here is sort of a lesson of what happens when good software is hijacked by some semi-competent zealous guy whose grasp does not reach beyond the purely technical level without any real clue what the software is achieving but who wishes to leave his mark on the software.

consequences: bugs, absurd decisions for "mandatory" new features, performance breakdown.

I propose to ask the AST guys to revoke commit access to krader. he should fork and do what he wants for his private hobby. this "semi-official" repo should not provide a basis and a blessing for kraders ill-guided (if well-intentioned I really cannot tell) modifications to the ksh code any longer.

I am done here as far as any attempt at a discussion with and feedback to krader is concerned. bye.

[siteshwar]

It should be possible to implement commands such as dirs via ksh functions. There is literally nothing about them that requires they be builtins written in C since they do not involve the byproducts of any syscalls not available via builtins. Imagine if every Python module writer was told to write their module in C rather than Python.

Afaik all other POSIX shells (bash, zsh etc.) implement these commands through builtins. This allows these commands to be available during troubleshooting (for e.g. when root filesystems is not mounted). Your comment about python is correct, but not comparable with respect to POSIX shells.

I am aware these commands are implemented as functions in fish shell, but primary goals of fish are different from POSIX shells. I believe it would be safer to do what other POSIX shells do here and implement these commands as builtins.

[siteshwar]

what is going on here is sort of a lesson of what happens when good software is hijacked by some semi-competent zealous guy whose grasp does not reach beyond the purely technical level without any real clue what the software is achieving but who wishes to leave his mark on the software.

While I agree that at times @krader1961 has ignored feedback from community and followed his own intuition, mostly his influence on ksh has been positive. This codebase has a legacy that goes back to more than 3 decades and it takes extreme patience to work on it. He has helped in improving test coverage by writing new tests, framework for interactive tests, fixing coverity defect rate and compiler warnings etc. This has only helped in bringing this codebase to more stable state. There have been several regressions introduced by our changes, but these regressions are minor as compared to amount of changes we have made (almost 80% of the code was dropped, changed build system etc.).

The regerssions tracked by this issue were caused by an interesting experiment (introducing pushd, popd commands in ksh) and we need to continue experimenting to keep ksh relevant to present times. I personally don't like the implementation and hopefully we will be able to agree on a more backward compatible solution.

[krader1961]

Note that the core bug this issue brings to our attention is present in the ksh93u+ release:

KSH PROMPT:1: function _cd { command cd "$@"; }
KSH PROMPT:2: alias cd=_cd
KSH PROMPT:3: cd /
KSH PROMPT:4: pwd
/
KSH PROMPT:5: cd /usr/local
KSH PROMPT:6: pwd
/usr/local
KSH PROMPT:7: command cd ~/tmp
/bin/ksh: _cd: not found
KSH PROMPT:8: pwd
/usr/local

In case the above isn't obvious the problem is that an alias affects the behavior of command. This looks like a bug to me since command cd in this context is supposed to evaluate to a builtin, if one exists, otherwise an external command. I have no idea why the old AST team implemented the augmented cd behavior as an alias to a _cd function. Fixing this is obvious and trivial: simply define an autoloaded cd function and don't use an alias. I've already written the fix and verified that using the augmented cd function every unit test passes.

Regarding the behavior of cd - honoring changes to $OLDPWD @jghub wrote:

known to everybody who can claim to be fluent in ksh. required (useful guaranteed) behaviour. what is undocumented about it? cd - changes to dir defined in OLDPWD. easy.

Then why wasn't it known to the AST team who included this broken function in the code base? I'm not the one who wrote the _cd function that ignores $OLDPWD. Also, if someone asked me to review a ksh script which did

OLDPWD=/some/path
cd -

Rather than the straightforward

cd /some/path

I would question whether they should be writing code. I would argue that $OLDPWD should be a read-only var. But that ship sailed long ago and we're stuck with the current semantics. The reason for my previous comment is that it would never occur to me to manipulate $OLDPWD as a way to affect cd - and I didn't notice the single sentence in the ksh man page describing this implementation artifact. The fix for this bug is also straightforward and part of the change I'll push for review in a few minutes.

These two fixes (to code written by the old AST team) allow your sd suite of functions to work, AFAICT.

The regerssions tracked by this issue were caused by an interesting experiment (introducing pushd, popd commands in ksh) and we need to continue experimenting to keep ksh relevant to present times. I personally don't like the implementation and hopefully we will be able to agree on a more backward compatible solution.

Please note, as I've already stated numerous times, that I do not like pushd/popd or prevd/nextd. Long ago, (in the 1990's) I tried to get comfortable with both idioms and concluded that their usefulness is limited to highly stylized situations and therefore are not generally useful. I do, however, think that having the shell keep track of recent cd behavior and exposing it via commands like dirs and mcd is worthwhile. Enabling the latter two commands is my motivation for enabling this behavior by default. New ksh users shouldn't have to install a third-party plugin, or write their own, for such basic functionality.

[krader1961]

Afaik all other POSIX shells (bash, zsh etc.) implement these commands through builtins.

That is a very weak argument for ksh implementing them as builtins. Especially since these features, as I've already discussed, should be implemented as functions unless doing so is impossible; e.g., due to the need for a syscall. If the shell script language is inadequate for these features what does that say about the language?

Also, notice that while bash and zsh implements pushd/popd as builtins they do not implement nextd/prevd. And zsh implements numerous global options that affect the behavior of pushd/popd; e.g., PUSHD_MINUS and PUSHD_SILENT, AUTOPUSHD, PUSHDIGNOREDUPS, etc. Which is precisely the sort of thing that caused me to abandon zsh. Not to mention that in zsh using pushd/popd is independent of cd. We should not be using zsh as an example of desirable behavior.

[jghub]

@krader1961

I have stopped paying intention (gave up since pointless to argue with you) but since you mentioned me, I received email from github and since you are so totally wrong: for the record this correction.

you say:

In case the above isn't obvious the problem is that an alias affects the behavior of command. This looks like a bug to me since command cd in this context is supposed to evaluate to a builtin, if one exists, otherwise an external command.

the manpage says:

     " command [ -pvxV ] name [ arg ... ]
              Without  the  -v  or  -V options, command executes name with the
              arguments given by arg.  The -p option causes a default path  to
              be  searched  rather  than the one defined by the value of PATH.
              Functions will not be searched for when finding name...."

you might consider to looking up defined behaviour in the manpage yourself instead of declaring it a bug (took me 30 seconds to do so).

in case the above isn't obvious (...): "Functions will not be searched for when finding name...." states that what you see is the defined behaviour of ksh93u+. it is not an alias problem at all, contrary to what you state. the alias just expands to a function and those are not searched. full stop. easy and obvious. again. if you still don't get it:

alias cd=ls
command cd /

will succeed but it will not execute cd/ but ls / since

a) ls is not a function but a command that is found on the search path

b) the manpage says:

  Aliasing.
       The  first  word of each command is replaced by the text of an alias if
       an alias for this word has been defined.

which seems easy enough to understand, no? so take home message for you: alias expansion is done first and no matter what. this has to be so. after the expansion command expanded_name is tried and functions are not searched at this point.

you don't observe a bug here. you observe your limited understanding of ksh but you yourself believe otherwise (the latter is the problem, not the former: the former is sure true for many people including myself). which makes the whole situation of this project so bleak.

to reiterate: the observed behaviour is the defined behaviour and there will be scripts out there relying on it explicitly or unwittingly and will break if you insist on breaking ksh for silly perceived "advantages" of changed cd behaviour or because you don't like the existing behaviour.

ps: as with all things ksh I believe D. Korn must have had good reasons to make functions not be searched when trying command name. but bash and mksh do this differently I believe (whether their behaviour is in accord with their manpage/documentation you might check yourself. I don't know...). so this is not a question of "right" or "wrong" behaviour but one of "different" behaviour. don't touch ksh behaviour here and maintain backward compatibility.

EDIT: regarding bash: it is not the case that bash does search functions when issuing command name. if you use alias foo=_cd rather than alias cd=_cd, than command foo will not work either -- just like in ksh93. but bash does some deeper magic (basically introducing inconsistent behaviour) by treating alias foo=_cd different from alias cd=_cd in your example and makes command cd work (by ignoring the alias altogether ....)

------------START EDIT 2------------- update regarding bash/mksh behaviour, in case somebody comes across this thread and does care for the difference in behaviour between ksh and bash etc here. the above remark regarding bash is not really correct. actually, cd is not treated specially by bash. rather, bash simply does no alias expansion at all on name when calling command name (where in the presently relevant case name is cd). it only performs alias substitution on command itself, so something like

function foo { echo foo;  }
alias cd=foo
alias cmd=command
cmd cd

would expand to command cd and, therefore, reach the builtin while ksh would expand cmd cd to command foo. I looked up the explanation in the bolsky/korn KORNSHELL book (p156/157 if someone has it at hand): ksh has a set of predefined aliases (@krader1961: the authors warning explicitly against unsetting these to avoid confusion but I am confident your first reflex is to do exactly that) among which we have

alias command='command ' # note trailing space in alias value

otoh, alias expansion is explained as acting on first word of command only, usually. but "if alias value ends with Space or Tab, then alias substitution also applies to next word and so on".

the consequence is that in bash, given the above definitions, by default

cd                    # calls function foo
command cd   # calls builtin

while in ksh by default

cd                    # calls function foo
command cd   # fails with "foo not found"

if one unsets the command alias in ksh, it behaves like bash, if one defines the command alias in bash like it is predefined in ksh, bash behaves like ksh.

while krader thus expected bash-behaviour this here is the Korn shell and ksh has behaved in this way literally for decades now. my previous remark still applies: first and foremost this is just different default behaviour not "wrong" or "right". I prefer the ksh default (since it leads to more consistent behaviour I'd say by ensuring that alias expansion always occurs on name irrespective of whether preceded by command or not) but that is a matter of taste. while without any doubt, the default should not be changed (by removing the predefined command alias) since this definitely would raise havoc in a multitude of ksh scripts out there.

PS: all of the above can also be deduced from the "Aliasing" section in the ksh manpage. one only has to pay attention to the details... ------------STOP EDIT 2-------------

so bottom line: functions are never searched when issuing command name. ksh just behaves more consistent than bash.

[jghub]

@siteshwar please monitor what is going on here (see the above abstruse claims regarding what command name should do and what not in krader's world) and don't let ksh be further messed up beyond repair or, since this has very probably already happened (at the very very least performance-wise) revert to a sane stable state. maintain new build system, maybe.

but it is totally obvious that the overall quality of ksh2020 after two years of interference by krader is inferior to ksh93u+ measured by all user-visible metrices (bugs, performance, stability). I don't care for 100 fixed (alleged or real) bugs (like potential memory leaks or "statement never reached" stuff) that never hit me in 20 years of ksh93 usage if I get hit by 5 new ones at the first day of using ksh2020.

ksh will remain relevant if its superiority (stability, predictable well defined backward compatible behaviour, speed) over competing shells (oksh, mksh, bash, zsh, maybe more) is maintained -- for the time being a stable, compilable ksh93u+ would do much more good in this respect than ksh2020 "releases" which are highly deficient in comparison in every respect.

I am sure you at least understand (but maybe currently do not share) my concerns. please reassess the situation yourself...

[jghub]

@krader1961

I would question whether they should be writing code. I would argue that $OLDPWD should be a read-only var. But that ship sailed long ago and we're stuck with the current semantics. The reason for my previous comment is that it would never occur to me to manipulate $OLDPWD as a way to affect cd - and I didn't notice the single sentence in the ksh man page describing this implementation artifact. The fix for this bug is also straightforward and part of the change I'll push for review in a few minutes.

you are not fixing bugs. you are breaking backward compatibility regarding behaviour of cd - if I understand the above correctly. factually you are plain wrong. why and how people rely on the defined behaviour should be none of your concern. your "aesthetic" judgement is irrelevant.

you break backward compatibility on purpose for no reason. this is not good engineering. this is vandalism.

[jelmd]

Exactly. Since krader1961 got access to this repo by mistake ~2 years ago, he has demonstrated several times, that he seems to know C and some tooling around it, but has neither a clue about the language itself (i.e. ksh93) nor its user base nor the "frameworks" ksh93 uses. The work and comments seen so far imply that he doesn't even know ksh93 basics (still, after 2 years!). So no wonder, why one "fix" introduces several other problems he is obviously not able to catch. Being absolutely narrow minded, the tendency to just make arbitrary, completely unfounded assumptions instead of doing the required homework contributes to the bad quality of his work. And of course, telling lies to complaining users does not help either. IMHO one should read and try to understand books/source code instead of burning it blindly ...

Not sure, what Redhat's intention here is. Perhaps getting "fixes" for free, or earning the biggest shitstorm in its history when shipping the thing made from this repo and trying to sell it as ksh93 to its customers. Who knows ... Anyway, siteshwar may talk on several conferences, workshops, etc. and ask for contribution, but when people see this disaster, they probably stay not very long - they are not as dumb as krader1961 might think ...

So if you are looking for alternatives: https://github.com/oracle/solaris-userland/tree/master/components/ksh93 (that's at least, what I prefer/use day by day - not perfect but sufficient/does the job).

[jghub]

@krader1961

I have no idea why the old AST team implemented the augmented cd behavior as an alias to a _cd function.

no, you haven't...

how about: this stuff is an exercise (and by no means an integral part of ksh93) from the appendix of the KornShell book for the reader/user to study and, possibly try out?

how about: they didn't want command cd to pick up overruled cd behaviour because they were sane people and had a real understanding of what the shell should do and what it should avoid to do by all means (namely completely mask builtins by a shell function layer)?

the fact remains that command cd should be guaranteed to yield the standard behaviour of cd or nothing at all (i.e it should fail as a signal that the user did something stupid: attempting to overrule cd with a function in a context where he actually needs to reach the builtin.

it continues to amaze me that you are not able/willing to appreciate this requirement. if it were not so sad it would be funny.

[jghub]

@jelmd

Exactly. Since krader1961 got access to this repo by mistake ~2 years ago, he has demonstrated several times, that he seems to know C and some tooling around it, but has neither a clue about the language itself (i.e. ksh93) nor its user base nor the "frameworks" ksh93 uses. The work and comments seen so far imply that he doesn't even know ksh93 basics (still, after 2 years!). So no wonder, why one "fix" introduces several other problems he is obviously not able to catch. Being absolutely narrow minded, the tendency to just make arbitrary, completely unfounded assumptions instead of doing the required homework contributes to the bad quality of his work. And of course, telling lies to complaining users does not help either. IMHO one should read and try to understand books/source code instead of burning it blindly ...

thanks for this support/feedback. I, of course, completely agree. and I wonder who made that mistake and why the situation is not reviewed to revoke his commit access? the man is very obviously not up to the task of maintaining ksh. the harmful thing here is not what he does to his private copy of the ast sources but that this disaster unfolds under the official umbrella of the ast/att project implying that this here really is the official ksh93 from now on.

Not sure, what Redhat's intention here is. Perhaps getting "fixes" for free, or earning the biggest shitstorm in its history when shipping the thing made from this repo and trying to sell it as ksh93 to its customers. Who knows ... Anyway, siteshwar may talk on several conferences, workshops, etc. and ask for contribution, but when people see this disaster, they probably stay not very long - they are not as dumb as krader1961 might think ...

that part of the story I do not understand at all: if it has been Redhat's (via @siteshwar) responsibility to obtain commit access for krader here, they/he quickly should do something about it. it his hardly imaginable that a capable engineer would not come to the unescapable conclusion that way (way!) more harm is done here than good.

So if you are looking for alternatives: https://github.com/oracle/solaris-userland/tree/master/components/ksh93 (that's at least, what I prefer/use day by day - not perfect but sufficient/does the job).

I most definitely am! I would never dream a second of using this ksh2020 thing here in production/every day use. there are more entertaining text adventure games for sure...

so thanks a lot for the pointer. I was not aware of that repo. will have to check this out. question: this is ksh93u+ patched by Sun for solaris? or pure ksh93u+ with its known hickups in tab completion etc?

[siteshwar]

Let's compare the behavior of different shells when cd is aliased:

bash:

$ function _cd { command cd "$@"; }
$ alias cd=_cd
$ command cd /tmp
$ echo $?
0

zsh:

% function _cd { command cd "$@"; }
% alias cd=_cd
% command cd /tmp
% echo $?
0

ksh:

$ function _cd { command cd "$@"; }
$ alias cd=_cd
$ command cd /tmp
ksh: _cd: not found
$ echo $?
127

As I understand both bash and zsh do not execute _cd alias when command is used.

@krader1961 Have you checked why ksh behaves differently than other shells ? Quoting command from POSIX specification:

In every other respect, if command_name is not the name of a function, the effect of command (with no options) shall be the same as omitting command.

Note that the core bug this issue brings to our attention is present in the ksh93u+ release:

Yes, but this behavior is now visible by default to every user that executes command cd and breaks user experience. If we can not change behavior of command builtin then we will have to revert the commit that introduced cd alias.

[jghub]

@siteshwar:

please read through the lower part of https://github.com/att/ast/issues/1445#issuecomment-562838000 -- notably the section labeld EDIT2 -- where i have explained to krader the reason for the differences between the shells. than explain it to him again if you are able to make him listen.

it is NOT A BUG of ksh. it is doing what it explains in its manpage....

and then prevent him from unsetting the predefined command alias in ksh that is responsible for the difference to bash and please(!) keep him from overruling cd by default with these toy functions by other means as well. nobody wants to be exposed to this just so. give the users the choice to activtate it if they want it.

[jghub]

@siteshwar:

If we can not change behavior of command builtin then we will have to revert the commit that introduced cd alias.

see previous post: you easily can change the behaviour of command by deleting the pre-defined command alias (command = 'command '). but Korn explicitly warned against doing this for obvious reasons: it completely would alter so far established behaviour of ksh at this point (which is simply different from what bash is doing by default). for the sake of backward compatibility alone this must not be altered. one also could argue that, actually, ksh, does behave saner here overall, but this aspect is of secondary importance.

[jghub]

@siteshwar, I'd appreciate an update (waited 3 weeks for something to happen ;): has there a consensus or decision been reached on your side how to proceed regarding

not to interfere with POSIX defined cd behaviour as defined here https://pubs.opengroup.org/onlinepubs/9699919799/utilities/cd.html. i.e. issuing `cd' by default is guaranteed to exhibit this behaviour and nothing else, w/o side effects. as every bourne shell descandent can be expected to do.

to ensure that command cd does not reach an alias out of the box (i.e. w/o the user himself explicitly defining (or activating) such an alias for cd) but the builtin. in view of point 3 below this means: no default alias for cd.

to leave the long-established ksh default behaviour in place (guaranteed by the builtin alias alias command='command ') that alias expansion is also applied to word in command word.