search

sorin-ionescu / prezto

The configuration framework for Zsh
MIT License
14k stars 4.49k forks source link

BusyBox v1.31.1: ln: invalid option -- 'i' #1821

Open turboMaCk opened 4 years ago

turboMaCk commented 4 years ago

Description

On a system with BusyBox v1.31.1 ln alias to ln -i is not working.

Expected behavior

prezto should not break functionality

Actual behavior

ln is not usable on system with BusyBox v1.31.1

Steps to Reproduce

On a system with BusyBoyx 1.31.1

  1. cd /tmp
  2. touch foo
  3. ln -s foo bar

Versions

srijanshetty commented 4 years ago

I was able to reproduce the issue in a busybox/nixos based system, -i does not appear to be an option for ln which comes with busybox.

The problem we have here is that the OS checking code is limited to is-darwin, is-linux, is-bsd of is-cygwin which is dependent on OSTYPE variable. On checking with docker images for nixos/nix and busybox, the OSTYPE is empty which cannot be used as an affirmative way to assert that the os is based on busybox.

For the time being you could disable utility module or unset the defined alias in your .zshrc, as the fix might take some time and thinking.

@belak, @romkatv what do you think should be the way forward here?

romkatv commented 4 years ago

I do it like this: https://github.com/romkatv/zsh4humans/blob/aa625cc1511286e1a31ee3df56d6f7526cfc918a/z4h.zsh#L933

romkatv commented 4 years ago

This check has to be done separately for every command. It's common to install select GNU tools on a busybox system. For example, you start with Alpine, which has busybox, and then install GNU grep with apk add grep. ls, ln and all the rest are still from busybox but grep is now from GNU.

belak commented 4 years ago

Oof, that's unfortunate. Thanks for the background @romkatv, I'll have to look at including that.

turboMaCk commented 4 years ago

In case this is difficult to fix we can explicitly state that some functionality or even prezto as a whole doesn't support busy box based environments.

Anyway I fixed it simply by uninstalling busybox. I originally installed it just as single package installation for tools like which and others. Now I simply removed busybox and installed them one by one. This issue is definitely not NixOS specific but rather busybox specific.

If there is performance concern around checking presence of busybox at runtime I think setting explicit variable in zshrc or loading busybox specific module would be fine solution as well.

In my opinion best in class performance is number one reason to use prezto over oh-my-zsh. I assume users won't mind to do extra work at setup.

romkatv commented 4 years ago

If there is performance concern around checking presence of busybox at runtime

There isn't.

srijanshetty commented 4 years ago

A simpler solution could be to have a busybox module which can be loaded at last which cleans up the busybox related issues. This is a hack and adds a dependency in the ordering of modules but it's probably the easiest to maintain and provides opt-in for the users..

romkatv commented 4 years ago

A simpler solution could be to have a busybox module which can be loaded at last which cleans up the busybox related issues. This is a hack and adds a dependency in the ordering of modules but it's probably the easiest to maintain and provides opt-in for the users..

This doesn't look simpler from the perspective of a user or the maintainer.

There are few aliases for POSIX commands in Prezto. Updating them all to check whether they point to busybox (and thus must be restricted to POSIX) isn't very time-consuming and not technically difficult. A perfect task for someone learning the ropes of zsh and Prezto.

Disclaimer: I'm not a maintainer of Prezto. My opinion carries no weight.

turboMaCk commented 4 years ago

This doesn't look simpler from the perspective of a user or the maintainer.

Implicit change of behavior can be confusing to users. (Why it does A on one machine and B on the other?)

For maintainers it's also simpler in a sense that maintainer who doesn't care about busybox can ignore it while those who know where and how to make a change. If there was prezto:module:busybox already, I would submit patch to that rather than opening this issue.

That's being said I know what you mean. I just see it more like a trade-off.

Disclaimer: I'm also not a maintainer.

belak commented 4 years ago

This doesn't look simpler from the perspective of a user or the maintainer.

This is accurate. We want the simplest experience out of the box - enable prezto and you should be able to take advantage of the improvements without enabling different modules depending on your setup.

There are few aliases for POSIX commands in Prezto. Updating them all to check whether they point to busybox (and thus must be restricted to POSIX) isn't very time-consuming and not technically difficult. A perfect task for someone learning the ropes of zsh and Prezto.

This is also accurate. PRs would be welcome.

Disclaimer: I'm not a maintainer of Prezto. My opinion carries no weight.

You're not a maintainer of Prezto, but your opinion definitely caries weight. I'm really appreciative that you continue to stick around. You've been able to provide expertise when integrating p10k and have been able to provide useful context around some zsh internals/oddities.

romkatv commented 4 years ago

Implicit change of behavior can be confusing to users. (Why it does A on one machine and B on the other?)

Prezto already has conditional aliases. This is a feature.

srijanshetty commented 4 years ago

This doesn't look simpler from the perspective of a user or the maintainer.

I agree, and I'll keep the design goal of keeping things as simple as possible in the future. I recall how simple it was to setup and leverage prezto the first time I used it, and that memory should be cherished.

Disclaimer: I'm not a maintainer of Prezto. My opinion carries no weight.

I've looked at you commits to understand how to write better zsh code and hence your feedback is very valuable.

Thanks for your views @belak and @romkatv, I'll try to make the changes if I find some time this week.

turboMaCk commented 1 year ago

I wonder if there is something I can do to help get this resolved. This is no longer an issue for me since I simply stopped using busybox utilities which in my case is not a problem to do.

That's being said I would still like to help to prevent other users from hitting this or made prezto usable in setting when one doesn't have a choice in using busybox or simply to give back to the project.

Do we have a clear agreement of hot to fix this?

  1. Should we just detect busy box and change the alias(es)? (sounds like this is the prezto way from comments)
  2. Should we expose flag in configuration for compatibility (seems like this is not the prezto way from the comments)