Skip to content

Increase askpass timeout for git operations #42946

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
Merged
yara-blue merged 3 commits into from
Dec 2, 2025
Merged

Increase askpass timeout for git operations #42946

yara-blue merged 3 commits into from
Dec 2, 2025

Conversation

@11happy
Copy link
Contributor

@11happy 11happy commented Nov 18, 2025

Closes #29903

Release Notes:

  • Increased timeout from 17->300 & improved the error message.
@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Nov 18, 2025
@MrSubidubi MrSubidubi changed the title feat: increase_timeout for git operation Nov 18, 2025
@cole-miller
Copy link
Member

cole-miller commented Nov 19, 2025

Hi @11happy and thanks!

I think a better approach here might be to run git hooks in a separate step without the askpass delegate, so that they don't eat into the timeout if they run for a while. This is how I would approach it, just talking about the pre-push hook for now:

  • pre-push is supposed to run after the local remote-tracking ref (e.g. origin/main) has been updated, but before we've actually done the network operation (ref). So Zed would need to update origin/main before it runs .git/hooks/pre-push
  • pre-push receives information about the remote being pushed to in its command-line arguments, and some additional information on stdin about the refs that are being pushed (see the same page in the git docs). So Zed would need to get that information and pass it in.
  • If pre-push is successful, we should go on to do git push --no-verify, but we'll want to put the remote-tracking ref back to its initial state if pre-push or git push --no-verify fails (I believe this is how git itself behaves, but it would be good to check).
@kubkon kubkon assigned kubkon and unassigned cole-miller Nov 24, 2025
@kubkon
Copy link
Member

kubkon commented Nov 24, 2025

FWIW @cole-miller and I have already implemented most of the machinery in #43285. All that's now left is to add PrePush variant to RunHook, register it correctly in git.proto, and make sure we run run_hook before we execute the push job in GitStore. Happy to answer any questions you might have about this!
11happy and others added 2 commits December 1, 2025 06:20
@11happy
increase_timeout
eb972fb 
Signed-off-by: 11happy <soni5happy@gmail.com>
@11happy
register pre-push in RunHook
843f7ef 
Signed-off-by: 11happy <bhuminjaysoni@gmail.com>
@11happy
Copy link
Contributor Author

11happy commented Dec 1, 2025

FWIW @cole-miller and I have already implemented most of the machinery in #43285. All that's now left is to add PrePush variant to RunHook, register it correctly in git.proto, and make sure we run run_hook before we execute the push job in GitStore. Happy to answer any questions you might have about this!

Thank you for these pointers & work on RunHook implementation I have implemented it accordingly.
Thank you : )
@11happy
revert error message
7e79dbe 
Signed-off-by: 11happy <bhuminjaysoni@gmail.com>
@11happy
Copy link
Contributor Author

11happy commented Dec 2, 2025

corrected the error message!
I think its good to go now , @dvdsk
Thank you : )
@yara-blue yara-blue merged commit a74aac8 into zed-industries:main Dec 2, 2025
23 checks passed
@yara-blue
Copy link
Member

yara-blue commented Dec 2, 2025

🥳 Thanks! Curious to see what you'll do next!
@katie-z-geer katie-z-geer moved this from Community PRs to Done in Quality Week – December 2025 Dec 2, 2025
@esthertrapadoux esthertrapadoux moved this to 🚢 Shipped by Community in Git board Dec 3, 2025
@esthertrapadoux
Copy link

esthertrapadoux commented Dec 3, 2025

Congrats on another great contribution @11happy !!
cole-miller added a commit that referenced this pull request Dec 10, 2025
@cole-miller
Revert "Increase askpass timeout for git operations (#42946)"
e785bf2 
This reverts commit a74aac8.
@cole-miller cole-miller mentioned this pull request Dec 10, 2025
Merged
cole-miller added a commit that referenced this pull request Dec 10, 2025
@cole-miller
Revert "Increase askpass timeout for git operations (#42946)" (#44578)
d21628c 
This reverts commit a74aac8.

cc @11happy, we need to do a bit more than just running `git hook
pre-push` before pushing, as described
[here](#42946 (comment)).
Right now this is also running the pre-push hook twice.

Release Notes:

- N/A
github-actions bot pushed a commit that referenced this pull request Dec 10, 2025
@cole-miller
Revert "Increase askpass timeout for git operations (#42946)" (#44578)
b59f597 
This reverts commit a74aac8.

cc @11happy, we need to do a bit more than just running `git hook
pre-push` before pushing, as described
[here](#42946 (comment)).
Right now this is also running the pre-push hook twice.

Release Notes:

- N/A
github-actions bot pushed a commit that referenced this pull request Dec 10, 2025
@cole-miller
Revert "Increase askpass timeout for git operations (#42946)" (#44578)
c133d79 
This reverts commit a74aac8.

cc @11happy, we need to do a bit more than just running `git hook
pre-push` before pushing, as described
[here](#42946 (comment)).
Right now this is also running the pre-push hook twice.

Release Notes:

- N/A
This was referenced Dec 10, 2025
Merged
Merged
zed-zippy bot added a commit that referenced this pull request Dec 10, 2025
@zed-zippy @cole-miller
Revert "Increase askpass timeout for git operations (#42946)" (#44578)…
3487d55 
… (cherry-pick to preview) (#44583)

Cherry-pick of #44578 to preview

----
This reverts commit a74aac8.

cc @11happy, we need to do a bit more than just running `git hook
pre-push` before pushing, as described

[here](#42946 (comment)).
Right now this is also running the pre-push hook twice.

Release Notes:

- N/A

Co-authored-by: Cole Miller <cole@zed.dev>
zed-zippy bot added a commit that referenced this pull request Dec 10, 2025
@zed-zippy @cole-miller
Revert "Increase askpass timeout for git operations (#42946)" (#44578)…
bdf4ede 
… (cherry-pick to stable) (#44584)

Cherry-pick of #44578 to stable

----
This reverts commit a74aac8.

cc @11happy, we need to do a bit more than just running `git hook
pre-push` before pushing, as described

[here](#42946 (comment)).
Right now this is also running the pre-push hook twice.

Release Notes:

- N/A

Co-authored-by: Cole Miller <cole@zed.dev>
nrbnlulu pushed a commit to nrbnlulu/zed that referenced this pull request Dec 14, 2025
@cole-miller @nrbnlulu
Revert "Increase askpass timeout for git operations (zed-industries#4…
bbe6b7a 
…2946)" (zed-industries#44578)

This reverts commit a74aac8.

cc @11happy, we need to do a bit more than just running `git hook
pre-push` before pushing, as described
[here](zed-industries#42946 (comment)).
Right now this is also running the pre-push hook twice.

Release Notes:

- N/A
CherryWorm pushed a commit to CherryWorm/zed that referenced this pull request Dec 16, 2025
@cole-miller @CherryWorm
Revert "Increase askpass timeout for git operations (zed-industries#4…
e577f45 
…2946)" (zed-industries#44578)

This reverts commit a74aac8.

cc @11happy, we need to do a bit more than just running `git hook
pre-push` before pushing, as described
[here](zed-industries#42946 (comment)).
Right now this is also running the pre-push hook twice.

Release Notes:

- N/A
someone13574 pushed a commit to someone13574/zed that referenced this pull request Dec 16, 2025
@11happy @someone13574
Increase askpass timeout for git operations (zed-industries#42946)
84b1330 
Closes zed-industries#29903

Release Notes:

- Increased timeout from 17->300 & improved the error message.

---------

Signed-off-by: 11happy <soni5happy@gmail.com>
Signed-off-by: 11happy <bhuminjaysoni@gmail.com>
someone13574 pushed a commit to someone13574/zed that referenced this pull request Dec 16, 2025
@cole-miller @someone13574
Revert "Increase askpass timeout for git operations (zed-industries#4…
6d4cfa1 
…2946)" (zed-industries#44578)

This reverts commit a74aac8.

cc @11happy, we need to do a bit more than just running `git hook
pre-push` before pushing, as described
[here](zed-industries#42946 (comment)).
Right now this is also running the pre-push hook twice.

Release Notes:

- N/A
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

5 participants

@11happy @cole-miller @kubkon @yara-blue @esthertrapadoux