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

Replace command with which in tool_exists #192

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cocco111
Copy link

command is a built-in shell command in linux (at least in debian , ubuntu and similars)
If executed in ruby system it return nil (inspecting: 127 command not found)
This not happens with which, that is a system command.

I 've done a PR for fast, but if you prefer I can open a simple issue. 🙂
TY

@dhh
Copy link
Member

dhh commented Jul 29, 2024

It's fine for it to return nil when the command is not found. It's falsy.

@dhh dhh closed this Jul 29, 2024
@cocco111
Copy link
Author

Sorry but It seems not ok to me
If 'command' is not found returns falsy, but in this situation routine is unable to find 'tool'

@dhh
Copy link
Member

dhh commented Jul 29, 2024

I don't understand the concern you're raising:

irb(main):001> system "command -v yarn > /dev/null"
=> true
irb(main):002> system "command -v yorn > /dev/null"
=> false

@cocco111
Copy link
Author

'command' is a shell (sh) command
If you execute irb not in a shell but in windows CMD or kind of different terminals, the result will be always falsy, for error 'command not found' referred to 'command' itself

Instead 'which' is an external command, often installed at system level (is an executable in PATH) in both Linux, Windows etc

@dhh
Copy link
Member

dhh commented Jul 30, 2024

Ah, I see. So you're hitting this problem running it in a Windows shell? Does the rest of everything work otherwise? We typically don't spend much/any time on compatibility with Windows shell, since WSL offers a much easier road to compatibility. Or is there another shell where this is manifesting itself?

@cocco111
Copy link
Author

cocco111 commented Aug 2, 2024

Also in linux
See these screens
WSL2 - Ubuntu
wsl
Real linux - Debian
linux
Both have same problem
My opinion: IRB does not spawn on the sh shell who launched it (it's a different process? standalone executable? don't know),

@dhh dhh reopened this Aug 2, 2024
@4lllex
Copy link

4lllex commented Sep 17, 2024

> /dev/null part is kind of required for system to run it as a shell command:

>> system "command -v yarn > /dev/null"
=> true

as opposed to trying to find a file to execute:

>> system "command -v yarn", exception: true
(irb):1:in `system': No such file or directory - command (Errno::ENOENT)
  • command_line if it is a string, and if it begins with a shell reserved word or special built-in, or if it contains one or more meta characters.
  • exe_path otherwise.

https://rubyapi.org/3.3/o/kernel#method-i-system


I think command -v should stay:

https://unix.stackexchange.com/questions/85249/why-not-use-which-what-to-use-then

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

Successfully merging this pull request may close these issues.

3 participants