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.

Sign up for GitHub

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

Jump to bottom

Use default shell instead of bash on Unix-like OS #2343

Merged
merged 1 commit into from
Aug 23, 2024
Merged

Use default shell instead of bash on Unix-like OS #2343

merged 1 commit into from
Aug 23, 2024

Conversation

yerke
Copy link
Contributor

@yerke yerke commented Aug 23, 2024 •
edited
Loading

This Pull Request fixes/closes #2317.

It changes the following:

  • Add find_default_unix_shell().
  • Use default shell instead of bash on Unix-like OS.

Why we need it? macOS uses zsh by default now.

I followed the checklist:

  • I added unittests
  • I ran make check without errors
  • I tested the overall application
  • I added an appropriate item to the changelog

@yerke yerke force-pushed the yerke/use-default-shell-on-unix-like-os branch 2 times, most recently from 1a3f96d to 73343ff Compare August 23, 2024 02:46
@yerke
Copy link
Contributor Author

yerke commented Aug 23, 2024

@extrawurst @mtsgrd Do you mind taking a look? Thanks.

This was referenced Aug 23, 2024
Closed
Closed
@extrawurst
Copy link
Owner

I suggest you run make check locally - it will run everything the CI runs (and currently fails)

@yerke yerke force-pushed the yerke/use-default-shell-on-unix-like-os branch from 73343ff to 524e3d8 Compare August 23, 2024 05:42
@yerke
Copy link
Contributor Author

yerke commented Aug 23, 2024 •
edited
Loading

@extrawurst thanks for letting me know. It passes now locally.
Edit: I think it needs your approval to run the checks again in CI.

Byron
Byron reviewed Aug 23, 2024
View reviewed changes
git2-hooks/src/hookspath.rs Outdated Show resolved Hide resolved
@yerke yerke force-pushed the yerke/use-default-shell-on-unix-like-os branch from 524e3d8 to e868a4f Compare August 23, 2024 06:14
@extrawurst extrawurst merged commit 4cb9500 into extrawurst:master Aug 23, 2024
21 checks passed
@extrawurst
Copy link
Owner

@yerke thank you for your contribution ❤️

@yerke yerke deleted the yerke/use-default-shell-on-unix-like-os branch August 23, 2024 15:40
@yerke yerke mentioned this pull request Aug 24, 2024
Merged
@DontEatOreo DontEatOreo mentioned this pull request Jan 13, 2025
Merged
13 tasks
@cruessler
Copy link
Collaborator

@extrawurst There’s a recent commit in gitoxide that seems somewhat related at first sight: GitoxideLabs/gitoxide@51bbb86. I don’t have enough context, though, to know whether it’s actually related.

@Byron
Copy link

Byron commented Jan 14, 2025

I think it is related. My initial PR was based on what I saw in git2-hooks, but it was rightfully criticised which led to the change linked above.
But in the absence of gix-hooks I also don't know how Git exactly is running hooks, and to what amount that would apply to making gitui more portable.

@Joshix-1
Copy link
Contributor

My login-shell isn't posix compliant. git can successfully call the hooks, while gitui fails to do so, because of this patch.
I think sh should be used to call the hooks instead of $SHELL or bash.

If I'm not mistaken (i hate reading c code and i don't know the git codebase) it seems like git uses /bin/sh per default.
https://github.com/git/git/blob/757161efcca150a9a96b312d9e780a071e601a03/run-command.c#L411

$ git var GIT_SHELL_PATH
/bin/sh

@Joshix-1 Joshix-1 mentioned this pull request Jan 15, 2025
Open
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Jan 28, 2025
@tmeijn
build(deps): update extrawurst/gitui to v0.27.0
1a04e71 
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [extrawurst/gitui](https://github.com/extrawurst/gitui) | minor | `v0.26.3` -> `v0.27.0` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>extrawurst/gitui (extrawurst/gitui)</summary>

### [`v0.27.0`](https://github.com/extrawurst/gitui/releases/tag/v0.27.0)

[Compare Source](extrawurst/gitui@v0.26.3...v0.27.0)

**new: manage remotes**

![add-remote](assets/add-remote.png)

##### Breaking Changes

-   use default shell instead of bash on Unix-like OS \[[@&#8203;yerke](https://github.com/yerke)] ([#&#8203;2343](extrawurst/gitui#2343))

##### Added

-   add popups for viewing, adding, updating and removing remotes \[[@&#8203;robin-thoene](https://github.com/robin-thoene)] ([#&#8203;2172](extrawurst/gitui#2172))
-   support for `Copy Path` action in WSL \[[@&#8203;johnDeSilencio](https://github.com/johnDeSilencio)] ([#&#8203;2413](extrawurst/gitui#2413))
-   help popup scrollbar \[[@&#8203;wugeer](https://github.com/wugeer)] ([#&#8203;2388](extrawurst/gitui#2388))

##### Fixes

-   respect env vars like `GIT_CONFIG_GLOBAL` ([#&#8203;2298](extrawurst/gitui#2298))
-   Set `CREATE_NO_WINDOW` flag when executing Git hooks on Windows ([#&#8203;2371](extrawurst/gitui#2371))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS4xMzcuMSIsInVwZGF0ZWRJblZlciI6IjM5LjEzNy4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Byron Byron Byron left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use default shell for git-hooks instead of bash
5 participants
@yerke @extrawurst @cruessler @Byron @Joshix-1