Skip to content

git: Fix add safe directory button doing nothing#58705

Merged
cole-miller merged 2 commits into
mainfrom
fix-nested-detach
Jun 5, 2026
Merged

git: Fix add safe directory button doing nothing#58705
cole-miller merged 2 commits into
mainfrom
fix-nested-detach

Conversation

@cole-miller

Copy link
Copy Markdown
Member

We had a nested spawn that was disguising the fact that the actual git_config task was being dropped immediately instead of detached. This only seems to cause problems in release builds due to timing issues.

Self-Review Checklist:

  • I've reviewed my own diff for quality, security, and reliability
  • Unsafe blocks (if any) have justifying comments
  • The content is consistent with the UI/UX checklist
  • Tests cover the new/changed behavior
  • Performance impact has been considered and is acceptable

Release Notes:

  • Fixed a bug where clicking the button to trust a git repository would do nothing.

@cla-bot cla-bot Bot added the cla-signed The user has signed the Contributor License Agreement label Jun 5, 2026
@zed-community-bot zed-community-bot Bot added the staff Pull requests authored by a current member of Zed staff label Jun 5, 2026
@cole-miller cole-miller enabled auto-merge June 5, 2026 23:08
@cole-miller cole-miller added this pull request to the merge queue Jun 5, 2026
Merged via the queue into main with commit e5c370b Jun 5, 2026
32 checks passed
@cole-miller cole-miller deleted the fix-nested-detach branch June 5, 2026 23:17
@zed-zippy zed-zippy Bot added the PR state:needs review Used to label PRs that are in need of a post-merge approval label Jun 5, 2026
@cole-miller

Copy link
Copy Markdown
Member Author

/cherry-pick preview
/cherry-pick stable

@probably-neb

Copy link
Copy Markdown
Collaborator

@zed-zippy approve

@zed-zippy zed-zippy Bot removed the PR state:needs review Used to label PRs that are in need of a post-merge approval label Jun 6, 2026
zed-zippy Bot added a commit that referenced this pull request Jun 6, 2026
…k to preview) (#58715)

Cherry-pick of #58705 to preview

----
We had a nested spawn that was disguising the fact that the actual
`git_config` task was being dropped immediately instead of detached.
This only seems to cause problems in release builds due to timing
issues.

Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX

checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Release Notes:

- Fixed a bug where clicking the button to trust a git repository would
do nothing.

Co-authored-by: Cole Miller <cole@zed.dev>
Comment on lines -3196 to -3198
git_panel.update(cx, |git_panel, cx| {
git_panel.project.read(cx).git_config(path, args, cx)
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if dylint could've caught that we needed a .detach() after the .git_config here. Ok we immediately dropped this task without doing anything with it

@miguelraz

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! We have must_use on Task but it doesn't penetrate generics :( :( :(

zed-zippy Bot added a commit that referenced this pull request Jun 6, 2026
…k to stable) (#58716)

Cherry-pick of #58705 to stable

----
We had a nested spawn that was disguising the fact that the actual
`git_config` task was being dropped immediately instead of detached.
This only seems to cause problems in release builds due to timing
issues.

Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX

checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Release Notes:

- Fixed a bug where clicking the button to trust a git repository would
do nothing.

Co-authored-by: Cole Miller <cole@zed.dev>
TomPlanche pushed a commit to TomPlanche/zed that referenced this pull request Jun 8, 2026
We had a nested spawn that was disguising the fact that the actual
`git_config` task was being dropped immediately instead of detached.
This only seems to cause problems in release builds due to timing
issues.

Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Release Notes:

- Fixed a bug where clicking the button to trust a git repository would
do nothing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed The user has signed the Contributor License Agreement staff Pull requests authored by a current member of Zed staff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants