git: Use correct file mode when staging (#41900)
Closes #28667 Release Notes: - Fixed git not preserving file mode when committing. Now if an input file is executable it will be preserved when committed with Zed. --------- Signed-off-by: 11happy <soni5happy@gmail.com> Signed-off-by: 11happy <bhuminjaysoni@gmail.com> Co-authored-by: Jakub Konka <kubkon@jakubkonka.com>
This commit is contained in:
Bhuminjay Soni
10
Cargo.lock
generated
10
Cargo.lock
generated
@@ -6406,6 +6406,7 @@ dependencies = [
|
||||
"git",
|
||||
"gpui",
|
||||
"ignore",
|
||||
"is_executable",
|
||||
"libc",
|
||||
"log",
|
||||
"notify 8.2.0",
|
||||
@@ -8436,6 +8437,15 @@ dependencies = [
|
||||
"once_cell",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "is_executable"
|
||||
version = "1.0.5"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "baabb8b4867b26294d818bf3f651a454b6901431711abb96e296245888d6e8c4"
|
||||
dependencies = [
|
||||
"windows-sys 0.60.2",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "is_terminal_polyfill"
|
||||
version = "1.70.1"
|
||||
|
||||
@@ -33,6 +33,7 @@ tempfile.workspace = true
|
||||
text.workspace = true
|
||||
time.workspace = true
|
||||
util.workspace = true
|
||||
is_executable = "1.0.5"
|
||||
|
||||
[target.'cfg(target_os = "macos")'.dependencies]
|
||||
fsevent.workspace = true
|
||||
|
||||
@@ -138,6 +138,7 @@ impl GitRepository for FakeGitRepository {
|
||||
path: RepoPath,
|
||||
content: Option<String>,
|
||||
_env: Arc<HashMap<String, String>>,
|
||||
_is_executable: bool,
|
||||
) -> BoxFuture<'_, anyhow::Result<()>> {
|
||||
self.with_state_async(true, move |state| {
|
||||
if let Some(message) = &state.simulated_index_write_error_message {
|
||||
|
||||
@@ -32,6 +32,7 @@ use std::mem::MaybeUninit;
|
||||
use async_tar::Archive;
|
||||
use futures::{AsyncRead, Stream, StreamExt, future::BoxFuture};
|
||||
use git::repository::{GitRepository, RealGitRepository};
|
||||
use is_executable::IsExecutable;
|
||||
use rope::Rope;
|
||||
use serde::{Deserialize, Serialize};
|
||||
use smol::io::AsyncWriteExt;
|
||||
@@ -208,6 +209,7 @@ pub struct Metadata {
|
||||
pub is_dir: bool,
|
||||
pub len: u64,
|
||||
pub is_fifo: bool,
|
||||
pub is_executable: bool,
|
||||
}
|
||||
|
||||
/// Filesystem modification time. The purpose of this newtype is to discourage use of operations
|
||||
@@ -895,6 +897,12 @@ impl Fs for RealFs {
|
||||
#[cfg(unix)]
|
||||
let is_fifo = metadata.file_type().is_fifo();
|
||||
|
||||
let path_buf = path.to_path_buf();
|
||||
let is_executable = self
|
||||
.executor
|
||||
.spawn(async move { path_buf.is_executable() })
|
||||
.await;
|
||||
|
||||
Ok(Some(Metadata {
|
||||
inode,
|
||||
mtime: MTime(metadata.modified().unwrap_or(SystemTime::UNIX_EPOCH)),
|
||||
@@ -902,6 +910,7 @@ impl Fs for RealFs {
|
||||
is_symlink,
|
||||
is_dir: metadata.file_type().is_dir(),
|
||||
is_fifo,
|
||||
is_executable,
|
||||
}))
|
||||
}
|
||||
|
||||
@@ -2602,6 +2611,7 @@ impl Fs for FakeFs {
|
||||
is_dir: false,
|
||||
is_symlink,
|
||||
is_fifo: false,
|
||||
is_executable: false,
|
||||
},
|
||||
FakeFsEntry::Dir {
|
||||
inode, mtime, len, ..
|
||||
@@ -2612,6 +2622,7 @@ impl Fs for FakeFs {
|
||||
is_dir: true,
|
||||
is_symlink,
|
||||
is_fifo: false,
|
||||
is_executable: false,
|
||||
},
|
||||
FakeFsEntry::Symlink { .. } => unreachable!(),
|
||||
}))
|
||||
|
||||
@@ -400,6 +400,7 @@ pub trait GitRepository: Send + Sync {
|
||||
path: RepoPath,
|
||||
content: Option<String>,
|
||||
env: Arc<HashMap<String, String>>,
|
||||
is_executable: bool,
|
||||
) -> BoxFuture<'_, anyhow::Result<()>>;
|
||||
|
||||
/// Returns the URL of the remote with the given name.
|
||||
@@ -987,12 +988,15 @@ impl GitRepository for RealGitRepository {
|
||||
path: RepoPath,
|
||||
content: Option<String>,
|
||||
env: Arc<HashMap<String, String>>,
|
||||
is_executable: bool,
|
||||
) -> BoxFuture<'_, anyhow::Result<()>> {
|
||||
let working_directory = self.working_directory();
|
||||
let git_binary_path = self.any_git_binary_path.clone();
|
||||
self.executor
|
||||
.spawn(async move {
|
||||
let working_directory = working_directory?;
|
||||
let mode = if is_executable { "100755" } else { "100644" };
|
||||
|
||||
if let Some(content) = content {
|
||||
let mut child = new_smol_command(&git_binary_path)
|
||||
.current_dir(&working_directory)
|
||||
@@ -1013,7 +1017,7 @@ impl GitRepository for RealGitRepository {
|
||||
let output = new_smol_command(&git_binary_path)
|
||||
.current_dir(&working_directory)
|
||||
.envs(env.iter())
|
||||
.args(["update-index", "--add", "--cacheinfo", "100644", sha])
|
||||
.args(["update-index", "--add", "--cacheinfo", mode, sha])
|
||||
.arg(path.as_unix_str())
|
||||
.output()
|
||||
.await?;
|
||||
|
||||
@@ -301,6 +301,7 @@ impl std::ops::Deref for Repository {
|
||||
#[derive(Clone)]
|
||||
pub enum RepositoryState {
|
||||
Local {
|
||||
fs: Arc<dyn Fs>,
|
||||
backend: Arc<dyn GitRepository>,
|
||||
environment: Arc<HashMap<String, String>>,
|
||||
},
|
||||
@@ -4288,6 +4289,7 @@ impl Repository {
|
||||
RepositoryState::Local {
|
||||
backend,
|
||||
environment,
|
||||
..
|
||||
} => backend.run_hook(hook, environment.clone()).await,
|
||||
RepositoryState::Remote { project_id, client } => {
|
||||
client
|
||||
@@ -4580,6 +4582,7 @@ impl Repository {
|
||||
let id = self.id;
|
||||
let this = cx.weak_entity();
|
||||
let git_store = self.git_store.clone();
|
||||
let abs_path = self.snapshot.repo_path_to_abs_path(&path);
|
||||
self.send_keyed_job(
|
||||
Some(GitJobKey::WriteIndex(vec![path.clone()])),
|
||||
None,
|
||||
@@ -4588,14 +4591,21 @@ impl Repository {
|
||||
"start updating index text for buffer {}",
|
||||
path.as_unix_str()
|
||||
);
|
||||
|
||||
match git_repo {
|
||||
RepositoryState::Local {
|
||||
fs,
|
||||
backend,
|
||||
environment,
|
||||
..
|
||||
} => {
|
||||
let executable = match fs.metadata(&abs_path).await {
|
||||
Ok(Some(meta)) => meta.is_executable,
|
||||
Ok(None) => false,
|
||||
Err(_err) => false,
|
||||
};
|
||||
backend
|
||||
.set_index_text(path.clone(), content, environment.clone())
|
||||
.set_index_text(path.clone(), content, environment.clone(), executable)
|
||||
.await?;
|
||||
}
|
||||
RepositoryState::Remote { project_id, client } => {
|
||||
@@ -5164,6 +5174,7 @@ impl Repository {
|
||||
cx: &mut Context<Self>,
|
||||
) -> mpsc::UnboundedSender<GitJob> {
|
||||
let (job_tx, mut job_rx) = mpsc::unbounded::<GitJob>();
|
||||
let fs_cloned = fs.clone();
|
||||
|
||||
cx.spawn(async move |_, cx| {
|
||||
let environment = project_environment
|
||||
@@ -5195,8 +5206,8 @@ impl Repository {
|
||||
backend.clone(),
|
||||
);
|
||||
}
|
||||
|
||||
let state = RepositoryState::Local {
|
||||
fs: fs_cloned,
|
||||
backend,
|
||||
environment: Arc::new(environment),
|
||||
};
|
||||
|
||||
@@ -8174,6 +8174,91 @@ async fn test_single_file_diffs(cx: &mut gpui::TestAppContext) {
|
||||
});
|
||||
}
|
||||
|
||||
// TODO: Should we test this on Windows also?
|
||||
#[gpui::test]
|
||||
#[cfg(not(windows))]
|
||||
async fn test_staging_hunk_preserve_executable_permission(cx: &mut gpui::TestAppContext) {
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
init_test(cx);
|
||||
cx.executor().allow_parking();
|
||||
let committed_contents = "bar\n";
|
||||
let file_contents = "baz\n";
|
||||
let root = TempTree::new(json!({
|
||||
"project": {
|
||||
"foo": committed_contents
|
||||
},
|
||||
}));
|
||||
|
||||
let work_dir = root.path().join("project");
|
||||
let file_path = work_dir.join("foo");
|
||||
let repo = git_init(work_dir.as_path());
|
||||
let mut perms = std::fs::metadata(&file_path).unwrap().permissions();
|
||||
perms.set_mode(0o755);
|
||||
std::fs::set_permissions(&file_path, perms).unwrap();
|
||||
git_add("foo", &repo);
|
||||
git_commit("Initial commit", &repo);
|
||||
std::fs::write(&file_path, file_contents).unwrap();
|
||||
|
||||
let project = Project::test(
|
||||
Arc::new(RealFs::new(None, cx.executor())),
|
||||
[root.path()],
|
||||
cx,
|
||||
)
|
||||
.await;
|
||||
|
||||
let buffer = project
|
||||
.update(cx, |project, cx| {
|
||||
project.open_local_buffer(file_path.as_path(), cx)
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let snapshot = buffer.read_with(cx, |buffer, _| buffer.snapshot());
|
||||
|
||||
let uncommitted_diff = project
|
||||
.update(cx, |project, cx| {
|
||||
project.open_uncommitted_diff(buffer.clone(), cx)
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
uncommitted_diff.update(cx, |diff, cx| {
|
||||
let hunks = diff.hunks(&snapshot, cx).collect::<Vec<_>>();
|
||||
diff.stage_or_unstage_hunks(true, &hunks, &snapshot, true, cx);
|
||||
});
|
||||
|
||||
cx.run_until_parked();
|
||||
|
||||
let output = smol::process::Command::new("git")
|
||||
.current_dir(&work_dir)
|
||||
.args(["diff", "--staged"])
|
||||
.output()
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let staged_diff = String::from_utf8_lossy(&output.stdout);
|
||||
|
||||
assert!(
|
||||
!staged_diff.contains("new mode 100644"),
|
||||
"Staging should not change file mode from 755 to 644.\ngit diff --staged:\n{}",
|
||||
staged_diff
|
||||
);
|
||||
|
||||
let output = smol::process::Command::new("git")
|
||||
.current_dir(&work_dir)
|
||||
.args(["ls-files", "-s"])
|
||||
.output()
|
||||
.await
|
||||
.unwrap();
|
||||
let index_contents = String::from_utf8_lossy(&output.stdout);
|
||||
|
||||
assert!(
|
||||
index_contents.contains("100755"),
|
||||
"Index should show file as executable (100755).\ngit ls-files -s:\n{}",
|
||||
index_contents
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_repository_and_path_for_project_path(
|
||||
background_executor: BackgroundExecutor,
|
||||
|
||||
Reference in New Issue
Block a user