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

fix(shim): Use bash executable directly #5433

Merged
merged 2 commits into from
Mar 16, 2023
Merged

fix(shim): Use bash executable directly #5433

merged 2 commits into from
Mar 16, 2023

Conversation

rashil2000
Copy link
Member

Description

I just realized that when installing Git through the installer, it puts the bin directory in PATH, which contains the bash executable.

Likewise, when installing Git through Scoop, we create a shim for bash executable: https://github.com/ScoopInstaller/Main/blob/master/bucket/git.json#L40

Motivation and Context

Closes ScoopInstaller/Main#4564

Checklist:

  • I have read the Contributing Guide.
  • I have ensured that I am targeting the develop branch.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.
  • I have added an entry in the CHANGELOG.

@niheaven
Copy link
Member

What if no git installed?

@rashil2000
Copy link
Member Author

What if no git installed?

That's precisely the advantage of this PR. Even if git is not installed, the installation of programs like Neofetch will not fail. It will succeed. Users will just see an error while calling neofetch, saying 'bash not found', which will indicate that they need to install something that provides bash (i.e. Git, MSYS2 etc.)

Currently the installation of neofetch fails if git isn't found (see linked issue).

@rashil2000 rashil2000 merged commit a20bb4f into develop Mar 16, 2023
@rashil2000 rashil2000 deleted the bash-shim branch March 16, 2023 14:39
@chawyehsu
Copy link
Member

chawyehsu commented Mar 27, 2024

I'm sure after this change, shell shim calls WSL's bash.exe stub. I don't if this would be expected but it might cause unexpected behaviors. (comment from reviewing 0.4.0 release changes)

@rashil2000
Copy link
Member Author

You're right, a range of Windows 10 versions does have WSL's bash executable on PATH, but it was removed by Microsoft a long time ago. If the issue arises, we can tell people what's going on...

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.

None yet

3 participants