Skip to content

Commit de2c00b

Browse files
zed-zippy[bot]kubkoncole-miller
authored
git: Handle git pre-commit hooks separately (#43285) (cherry-pick to preview) (#43286)
Cherry-pick of #43285 to preview ---- We now run git pre-commit hooks before we commit. This ensures we don't run into timeout issues with askpass delegate and report invalid error to the user. Closes #43157 Release Notes: - Fixed long running pre-commit hooks causing committing from Zed to fail. Co-authored-by: Cole Miller <cole@zed.dev> Co-authored-by: Jakub Konka <kubkon@jakubkonka.com> Co-authored-by: Cole Miller <cole@zed.dev>
1 parent a9eab53 commit de2c00b

File tree

8 files changed

+128
-5
lines changed

8 files changed

+128
-5
lines changed

crates/collab/src/rpc.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,7 @@ impl Server {
453453
.add_request_handler(forward_mutating_project_request::<proto::StashPop>)
454454
.add_request_handler(forward_mutating_project_request::<proto::StashDrop>)
455455
.add_request_handler(forward_mutating_project_request::<proto::Commit>)
456+
.add_request_handler(forward_mutating_project_request::<proto::RunGitHook>)
456457
.add_request_handler(forward_mutating_project_request::<proto::GitInit>)
457458
.add_request_handler(forward_read_only_project_request::<proto::GetRemotes>)
458459
.add_request_handler(forward_read_only_project_request::<proto::GitShow>)

crates/fs/src/fake_git_repo.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use anyhow::{Context as _, Result, bail};
33
use collections::{HashMap, HashSet};
44
use futures::future::{self, BoxFuture, join_all};
55
use git::{
6-
Oid,
6+
Oid, RunHook,
77
blame::Blame,
88
repository::{
99
AskPassDelegate, Branch, CommitDetails, CommitOptions, FetchOptions, GitRepository,
@@ -532,6 +532,14 @@ impl GitRepository for FakeGitRepository {
532532
unimplemented!()
533533
}
534534

535+
fn run_hook(
536+
&self,
537+
_hook: RunHook,
538+
_env: Arc<HashMap<String, String>>,
539+
) -> BoxFuture<'_, Result<()>> {
540+
unimplemented!()
541+
}
542+
535543
fn push(
536544
&self,
537545
_branch: String,

crates/git/src/git.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,3 +225,28 @@ impl From<Oid> for usize {
225225
u64::from_ne_bytes(u64_bytes) as usize
226226
}
227227
}
228+
229+
#[repr(i32)]
230+
#[derive(Copy, Clone, Debug)]
231+
pub enum RunHook {
232+
PreCommit,
233+
}
234+
235+
impl RunHook {
236+
pub fn as_str(&self) -> &str {
237+
match self {
238+
Self::PreCommit => "pre-commit",
239+
}
240+
}
241+
242+
pub fn to_proto(&self) -> i32 {
243+
*self as i32
244+
}
245+
246+
pub fn from_proto(value: i32) -> Option<Self> {
247+
match value {
248+
0 => Some(Self::PreCommit),
249+
_ => None,
250+
}
251+
}
252+
}

crates/git/src/repository.rs

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use crate::commit::parse_git_diff_name_status;
22
use crate::stash::GitStash;
33
use crate::status::{DiffTreeType, GitStatus, StatusCode, TreeDiff};
4-
use crate::{Oid, SHORT_SHA_LENGTH};
4+
use crate::{Oid, RunHook, SHORT_SHA_LENGTH};
55
use anyhow::{Context as _, Result, anyhow, bail};
66
use collections::HashMap;
77
use futures::future::BoxFuture;
@@ -485,6 +485,12 @@ pub trait GitRepository: Send + Sync {
485485
env: Arc<HashMap<String, String>>,
486486
) -> BoxFuture<'_, Result<()>>;
487487

488+
fn run_hook(
489+
&self,
490+
hook: RunHook,
491+
env: Arc<HashMap<String, String>>,
492+
) -> BoxFuture<'_, Result<()>>;
493+
488494
fn commit(
489495
&self,
490496
message: SharedString,
@@ -1643,6 +1649,7 @@ impl GitRepository for RealGitRepository {
16431649
.args(["commit", "--quiet", "-m"])
16441650
.arg(&message.to_string())
16451651
.arg("--cleanup=strip")
1652+
.arg("--no-verify")
16461653
.stdout(smol::process::Stdio::piped())
16471654
.stderr(smol::process::Stdio::piped());
16481655

@@ -2037,6 +2044,26 @@ impl GitRepository for RealGitRepository {
20372044
})
20382045
.boxed()
20392046
}
2047+
2048+
fn run_hook(
2049+
&self,
2050+
hook: RunHook,
2051+
env: Arc<HashMap<String, String>>,
2052+
) -> BoxFuture<'_, Result<()>> {
2053+
let working_directory = self.working_directory();
2054+
let git_binary_path = self.any_git_binary_path.clone();
2055+
let executor = self.executor.clone();
2056+
self.executor
2057+
.spawn(async move {
2058+
let working_directory = working_directory?;
2059+
let git = GitBinary::new(git_binary_path, working_directory, executor)
2060+
.envs(HashMap::clone(&env));
2061+
git.run(&["hook", "run", "--ignore-missing", hook.as_str()])
2062+
.await?;
2063+
Ok(())
2064+
})
2065+
.boxed()
2066+
}
20402067
}
20412068

20422069
fn git_status_args(path_prefixes: &[RepoPath]) -> Vec<OsString> {

crates/project/src/git_store.rs

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use futures::{
2525
stream::FuturesOrdered,
2626
};
2727
use git::{
28-
BuildPermalinkParams, GitHostingProviderRegistry, Oid,
28+
BuildPermalinkParams, GitHostingProviderRegistry, Oid, RunHook,
2929
blame::Blame,
3030
parse_git_remote_url,
3131
repository::{
@@ -433,6 +433,7 @@ impl GitStore {
433433
client.add_entity_request_handler(Self::handle_stash_apply);
434434
client.add_entity_request_handler(Self::handle_stash_drop);
435435
client.add_entity_request_handler(Self::handle_commit);
436+
client.add_entity_request_handler(Self::handle_run_hook);
436437
client.add_entity_request_handler(Self::handle_reset);
437438
client.add_entity_request_handler(Self::handle_show);
438439
client.add_entity_request_handler(Self::handle_load_commit_diff);
@@ -1982,6 +1983,22 @@ impl GitStore {
19821983
Ok(proto::Ack {})
19831984
}
19841985

1986+
async fn handle_run_hook(
1987+
this: Entity<Self>,
1988+
envelope: TypedEnvelope<proto::RunGitHook>,
1989+
mut cx: AsyncApp,
1990+
) -> Result<proto::Ack> {
1991+
let repository_id = RepositoryId::from_proto(envelope.payload.repository_id);
1992+
let repository_handle = Self::repository_for_request(&this, repository_id, &mut cx)?;
1993+
let hook = RunHook::from_proto(envelope.payload.hook).context("invalid hook")?;
1994+
repository_handle
1995+
.update(&mut cx, |repository_handle, cx| {
1996+
repository_handle.run_hook(hook, cx)
1997+
})?
1998+
.await??;
1999+
Ok(proto::Ack {})
2000+
}
2001+
19852002
async fn handle_commit(
19862003
this: Entity<Self>,
19872004
envelope: TypedEnvelope<proto::Commit>,
@@ -4262,19 +4279,49 @@ impl Repository {
42624279
})
42634280
}
42644281

4282+
pub fn run_hook(&mut self, hook: RunHook, _cx: &mut App) -> oneshot::Receiver<Result<()>> {
4283+
let id = self.id;
4284+
self.send_job(
4285+
Some(format!("git hook {}", hook.as_str()).into()),
4286+
move |git_repo, _cx| async move {
4287+
match git_repo {
4288+
RepositoryState::Local {
4289+
backend,
4290+
environment,
4291+
} => backend.run_hook(hook, environment.clone()).await,
4292+
RepositoryState::Remote { project_id, client } => {
4293+
client
4294+
.request(proto::RunGitHook {
4295+
project_id: project_id.0,
4296+
repository_id: id.to_proto(),
4297+
hook: hook.to_proto(),
4298+
})
4299+
.await?;
4300+
4301+
Ok(())
4302+
}
4303+
}
4304+
},
4305+
)
4306+
}
4307+
42654308
pub fn commit(
42664309
&mut self,
42674310
message: SharedString,
42684311
name_and_email: Option<(SharedString, SharedString)>,
42694312
options: CommitOptions,
42704313
askpass: AskPassDelegate,
4271-
_cx: &mut App,
4314+
cx: &mut App,
42724315
) -> oneshot::Receiver<Result<()>> {
42734316
let id = self.id;
42744317
let askpass_delegates = self.askpass_delegates.clone();
42754318
let askpass_id = util::post_inc(&mut self.latest_askpass_id);
42764319

4320+
let rx = self.run_hook(RunHook::PreCommit, cx);
4321+
42774322
self.send_job(Some("git commit".into()), move |git_repo, _cx| async move {
4323+
rx.await??;
4324+
42784325
match git_repo {
42794326
RepositoryState::Local {
42804327
backend,

crates/proto/proto/git.proto

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -531,3 +531,13 @@ message GitCreateWorktree {
531531
string directory = 4;
532532
optional string commit = 5;
533533
}
534+
535+
message RunGitHook {
536+
enum GitHook {
537+
PRE_COMMIT = 0;
538+
}
539+
540+
uint64 project_id = 1;
541+
uint64 repository_id = 2;
542+
GitHook hook = 3;
543+
}

crates/proto/proto/zed.proto

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,9 @@ message Envelope {
437437
OpenImageResponse open_image_response = 392;
438438
CreateImageForPeer create_image_for_peer = 393;
439439

440-
ExternalExtensionAgentsUpdated external_extension_agents_updated = 394; // current max
440+
ExternalExtensionAgentsUpdated external_extension_agents_updated = 394;
441+
442+
RunGitHook run_git_hook = 395; // current max
441443
}
442444

443445
reserved 87 to 88;

crates/proto/src/proto.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ messages!(
4949
(ChannelMessageUpdate, Foreground),
5050
(CloseBuffer, Foreground),
5151
(Commit, Background),
52+
(RunGitHook, Background),
5253
(CopyProjectEntry, Foreground),
5354
(CreateBufferForPeer, Foreground),
5455
(CreateImageForPeer, Foreground),
@@ -349,6 +350,7 @@ request_messages!(
349350
(Call, Ack),
350351
(CancelCall, Ack),
351352
(Commit, Ack),
353+
(RunGitHook, Ack),
352354
(CopyProjectEntry, ProjectEntryResponse),
353355
(CreateChannel, CreateChannelResponse),
354356
(CreateProjectEntry, ProjectEntryResponse),
@@ -547,6 +549,7 @@ entity_messages!(
547549
BufferSaved,
548550
CloseBuffer,
549551
Commit,
552+
RunGitHook,
550553
GetColorPresentation,
551554
CopyProjectEntry,
552555
CreateBufferForPeer,

0 commit comments

Comments
 (0)