Skip to content

Commit 9329c3f

Browse files
committed
fix(edit): try harder on Windows to run editor
This emulates more of git's Windows-specific behavior for launching interactive editors. When GIT_WINDOWS_NATIVE is defined, git uses `mingw_spawnvpe()` to spawn the editor which, in turn, calls a `parse_interpreter()`. The proposed editor command is run as-is if it has a .exe extension. Otherwise, the command path is opened, read, and parsed to find a shebang line with an interpreter path. This plays out in run-command.c and compat/mingw.c in Git's source code. The Windows CI is updated to set SHELL_PATH to something suitable for the msys2 environment; `/bin/sh` is **not** a viable path in this environment. Refs: #407
1 parent 048f182 commit 9329c3f

File tree

2 files changed

+97
-11
lines changed

2 files changed

+97
-11
lines changed

.github/workflows/ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ jobs:
235235
STG_TEST_OPTS: "--verbose-log"
236236
STG_PROFILE: "release"
237237
run: |
238-
timeout 900s make -C t prove
238+
timeout 900s make -C t SHELL_PATH=C:/msys64/usr/bin/bash prove
239239
- name: Show Failures
240240
if: ${{ failure() }}
241241
shell: msys2 {0}

src/patch/edit/interactive.rs

Lines changed: 96 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,12 @@
55
use std::{
66
ffi::{OsStr, OsString},
77
fs::File,
8-
io::{BufWriter, Write},
8+
io::{BufWriter, Read, Write},
99
path::Path,
1010
};
1111

1212
use anyhow::{anyhow, Context, Result};
13-
use bstr::BString;
13+
use bstr::{BString, ByteSlice};
1414

1515
use super::description::{EditablePatchDescription, EditedPatchDescription};
1616

@@ -113,14 +113,25 @@ pub(crate) fn call_editor<P: AsRef<Path>>(
113113
}
114114

115115
fn run_editor<P: AsRef<Path>>(editor: &OsStr, path: P) -> Result<std::process::ExitStatus> {
116-
let mut child = std::process::Command::from(
117-
gix_command::prepare(editor)
118-
.arg(path.as_ref())
119-
.with_shell_allow_argument_splitting()
120-
.stdout(std::process::Stdio::inherit()),
121-
)
122-
.spawn()
123-
.with_context(|| format!("running editor: {}", editor.to_string_lossy()))?;
116+
let prep = gix_command::prepare(editor)
117+
.arg(path.as_ref())
118+
.with_shell_allow_argument_splitting()
119+
.stdout(std::process::Stdio::inherit());
120+
121+
let mut command = if cfg!(windows) && !prep.use_shell {
122+
if let Some(interpreter) = parse_interpreter(&prep.command) {
123+
let mut cmd = std::process::Command::new(interpreter);
124+
cmd.arg(prep.command).arg(path.as_ref());
125+
cmd
126+
} else {
127+
std::process::Command::from(prep)
128+
}
129+
} else {
130+
std::process::Command::from(prep)
131+
};
132+
let mut child = command
133+
.spawn()
134+
.with_context(|| format!("running editor: {}", editor.to_string_lossy()))?;
124135

125136
child
126137
.wait()
@@ -152,3 +163,78 @@ fn get_editor(config: &gix::config::Snapshot) -> Result<OsString> {
152163
};
153164
Ok(editor)
154165
}
166+
167+
fn parse_interpreter(command: &OsStr) -> Option<OsString> {
168+
let command_path = Path::new(command);
169+
if command_path.extension().and_then(|ext| ext.to_str()) == Some("exe") {
170+
return None;
171+
}
172+
173+
let mut buffer = [0; 128];
174+
if let Some(n) = std::fs::File::open(command_path)
175+
.ok()
176+
.and_then(|mut file| file.read(&mut buffer).ok())
177+
{
178+
parse_shebang(&buffer[..n])
179+
.and_then(|bytes| bytes.to_os_str().ok())
180+
.map(|osstr| osstr.to_os_string())
181+
} else {
182+
None
183+
}
184+
}
185+
186+
fn parse_shebang(buffer: &[u8]) -> Option<&[u8]> {
187+
buffer
188+
.as_bstr()
189+
.lines()
190+
.next()
191+
.and_then(|line| line.strip_prefix(b"#!"))
192+
.and_then(|shebang| {
193+
shebang.rfind_byteset(b"/\\").map(|index| {
194+
if let Some(space_index) = shebang[index..].find_byte(b' ') {
195+
&shebang[..index + space_index]
196+
} else {
197+
shebang
198+
}
199+
})
200+
})
201+
}
202+
203+
#[cfg(test)]
204+
mod tests {
205+
use super::*;
206+
207+
#[test]
208+
fn plain_shebang() {
209+
assert_eq!(parse_shebang(b"#!/bin/sh\nsome stuff").unwrap(), b"/bin/sh");
210+
}
211+
212+
#[test]
213+
fn shebang_with_options() {
214+
assert_eq!(
215+
parse_shebang(b"#!/bin/sh -i -o -u\nsome stuff").unwrap(),
216+
b"/bin/sh"
217+
);
218+
}
219+
220+
#[test]
221+
fn shebang_with_backslashes() {
222+
assert_eq!(
223+
parse_shebang(b"#!C:\\Program Files\\Imashell.exe\nsome stuff").unwrap(),
224+
b"C:\\Program Files\\Imashell.exe"
225+
);
226+
}
227+
228+
#[test]
229+
fn shebang_with_trailing_space() {
230+
assert_eq!(
231+
parse_shebang(b"#!/bin/sh \nsome stuff").unwrap(),
232+
b"/bin/sh"
233+
);
234+
}
235+
236+
#[test]
237+
fn not_a_shebang() {
238+
assert!(parse_shebang(b"/bin/sh\nsome stuff").is_none());
239+
}
240+
}

0 commit comments

Comments
 (0)