History

Merge pull request #2612 from GitoxideLabs/improvements

Sebastian Thiel committed 5 months ago

Commit 43774852 parents 87433ed 123cdaf3 files · +225 −86Browse files

Unified · Split

3 files
  1. tests/tools/src/lib.rs +180 −20
  2. tests/tools/src/main.rs +5 −66
  3. tests/tools/src/tests.rs +40 −0
tests/tools/src/lib.rs+180−20
@@ -206,18 +206,46 @@ fn wildcard_match(pattern: &str, text: &str) -> bool { · 205 unmodified lines
206206 pattern.ends_with('*') || remainder.is_empty()
207207 }
208208
209−/// A wrapper for a running git-daemon process which is killed automatically on drop.
209+/// A wrapper for a running git-daemon which is stopped automatically on drop.
210210 ///
211211 /// Note that we will swallow any errors, assuming that the test would have failed if the daemon crashed.
212212 pub struct GitDaemon {
213− child: std::process::Child,
213+ process: GitDaemonProcess,
214214 /// The base url under which all repositories are hosted, typically `git://127.0.0.1:port`.
215215 pub url: String,
216216 }
217217
218+enum GitDaemonProcess {
219+ #[cfg(not(unix))]
220+ Child(std::process::Child),
221+ #[cfg(unix)]
222+ Inetd {
223+ shutdown: std::sync::Arc<std::sync::atomic::AtomicBool>,
224+ server_addr: std::net::SocketAddr,
225+ listener_thread: Option<std::thread::JoinHandle<()>>,
226+ },
227+}
228+
218229 impl Drop for GitDaemon {
219230 fn drop(&mut self) {
220− self.child.kill().ok();
231+ match &mut self.process {
232+ #[cfg(not(unix))]
233+ GitDaemonProcess::Child(child) => {
234+ child.kill().ok();
235+ }
236+ #[cfg(unix)]
237+ GitDaemonProcess::Inetd {
238+ shutdown,
239+ server_addr,
240+ listener_thread,
241+ } => {
242+ shutdown.store(true, std::sync::atomic::Ordering::SeqCst);
243+ std::net::TcpStream::connect(*server_addr).ok();
244+ if let Some(listener_thread) = listener_thread.take() {
245+ listener_thread.join().ok();
246+ }
247+ }
248+ }
221249 }
222250 }
223251
@@ -317,6 +345,17 @@ const GIT_PROGRAM: &str = "git.exe"; · 93 unmodified lines
317345 #[cfg(not(windows))]
318346 const GIT_PROGRAM: &str = "git";
319347
348+const DISABLE_AUTO_MAINTENANCE_CONFIG: &[(&str, &str)] = &[("maintenance.auto", "false"), ("gc.auto", "0")];
349+
350+const ISOLATED_GIT_CONFIG: &[(&str, &str)] = &[
351+ ("commit.gpgsign", "false"),
352+ ("tag.gpgsign", "false"),
353+ ("init.defaultBranch", "main"),
354+ ("protocol.file.allow", "always"),
355+ ("maintenance.auto", "false"),
356+ ("gc.auto", "0"),
357+];
358+
320359 static GIT_CORE_DIR: LazyLock<PathBuf> = LazyLock::new(|| {
321360 let output = std::process::Command::new(GIT_PROGRAM)
322361 .arg("--exec-path")
@@ -421,7 +460,8 @@ impl Drop for AutoRevertToPreviousCWD { · 98 unmodified lines
421460
422461 /// Run `git` in `working_dir` with all provided `args`.
423462 pub fn run_git(working_dir: &Path, args: &[&str]) -> std::io::Result<std::process::ExitStatus> {
424− std::process::Command::new(GIT_PROGRAM)
463+ let mut cmd = std::process::Command::new(GIT_PROGRAM);
464+ apply_git_config_by_environment(&mut cmd, DISABLE_AUTO_MAINTENANCE_CONFIG)
425465 .current_dir(working_dir)
426466 .args(args)
427467 .status()
@@ -436,7 +476,8 @@ pub fn run_git(working_dir: &Path, args: &[&str]) -> std::io::Result<std::proces · 8 unmodified lines
436476 ///
437477 /// This function expects the script to succeed and will panic otherwise.
438478 pub fn invoke_bash(cwd: impl AsRef<Path>, script: &str) {
439− let status = std::process::Command::new(bash_program())
479+ let mut cmd = std::process::Command::new(bash_program());
480+ let status = apply_git_config_by_environment(&mut cmd, DISABLE_AUTO_MAINTENANCE_CONFIG)
440481 .current_dir(cwd)
441482 .arg("-c")
442483 .arg(script)
@@ -448,8 +489,22 @@ pub fn invoke_bash(cwd: impl AsRef<Path>, script: &str) { · 5 unmodified lines
448489 assert!(status.success(), "bash script failed with {status}");
449490 }
450491
451−/// Spawn a git daemon process to host all repository at or below `working_dir`.
492+/// Spawn a git daemon to host all repositories at or below `working_dir`.
493+///
494+/// It runs in the background until the [`GitDaemon`] is dropped.
452495 pub fn spawn_git_daemon(working_dir: impl AsRef<Path>) -> std::io::Result<GitDaemon> {
496+ #[cfg(unix)]
497+ {
498+ spawn_git_daemon_inetd(working_dir)
499+ }
500+ #[cfg(not(unix))]
501+ {
502+ spawn_git_daemon_process(working_dir)
503+ }
504+}
505+
506+#[cfg(not(unix))]
507+fn spawn_git_daemon_process(working_dir: impl AsRef<Path>) -> std::io::Result<GitDaemon> {
453508 let mut ports: Vec<_> = (9419u16..9419 + 100).collect();
454509 fastrand::shuffle(&mut ports);
455510 let addr_at = |port| std::net::SocketAddr::from(([127, 0, 0, 1], port));
@@ -458,12 +513,15 @@ pub fn spawn_git_daemon(working_dir: impl AsRef<Path>) -> std::io::Result<GitDae · 2 unmodified lines
458513 listener.local_addr().expect("listener address is available").port()
459514 };
460515
461− let child =
462− std::process::Command::new(GIT_CORE_DIR.join(if cfg!(windows) { "git-daemon.exe" } else { "git-daemon" }))
516+ let child = {
517+ let mut cmd =
518+ std::process::Command::new(GIT_CORE_DIR.join(if cfg!(windows) { "git-daemon.exe" } else { "git-daemon" }));
519+ apply_git_config_by_environment(&mut cmd, DISABLE_AUTO_MAINTENANCE_CONFIG)
463520 .current_dir(working_dir)
464521 .args(["--verbose", "--base-path=.", "--export-all", "--user-path"])
465522 .arg(format!("--port={free_port}"))
466− .spawn()?;
523+ .spawn()?
524+ };
467525
468526 let server_addr = addr_at(free_port);
469527 for time in gix_lock::backoff::Quadratic::default_with_random() {
@@ -473,11 +531,102 @@ pub fn spawn_git_daemon(working_dir: impl AsRef<Path>) -> std::io::Result<GitDae · 3 unmodified lines
473531 }
474532 }
475533 Ok(GitDaemon {
476− child,
534+ process: GitDaemonProcess::Child(child),
477535 url: format!("git://{server_addr}"),
478536 })
479537 }
480538
539+#[cfg(unix)]
540+fn spawn_git_daemon_inetd(working_dir: impl AsRef<Path>) -> std::io::Result<GitDaemon> {
541+ use std::{
542+ net::{TcpListener, TcpStream},
543+ os::fd::{FromRawFd, IntoRawFd},
544+ process::Stdio,
545+ sync::{
546+ Arc,
547+ atomic::{AtomicBool, Ordering},
548+ },
549+ };
550+
551+ fn stream_to_stdio(stream: TcpStream) -> Stdio {
552+ // SAFETY: `into_raw_fd()` transfers ownership of the socket fd, and `Stdio`
553+ // takes over closing it in the spawned child.
554+ unsafe { Stdio::from_raw_fd(stream.into_raw_fd()) }
555+ }
556+
557+ let working_dir = working_dir.as_ref().to_owned();
558+ let listener = TcpListener::bind(("127.0.0.1", 0))?;
559+ let server_addr = listener.local_addr()?;
560+ let shutdown = Arc::new(AtomicBool::new(false));
561+ let listener_thread = std::thread::spawn({
562+ let shutdown = shutdown.clone();
563+ move || {
564+ for incoming in listener.incoming() {
565+ let stream = match incoming {
566+ Ok(stream) => stream,
567+ Err(_) => break,
568+ };
569+ if shutdown.load(Ordering::SeqCst) {
570+ break;
571+ }
572+
573+ let peer_addr = stream.peer_addr().ok();
574+ let stdin = match stream.try_clone() {
575+ Ok(stream) => stream_to_stdio(stream),
576+ Err(_) => continue,
577+ };
578+ let stdout = stream_to_stdio(stream);
579+ let mut cmd = std::process::Command::new(GIT_PROGRAM);
580+ let Ok(mut child) = apply_git_config_by_environment(&mut cmd, DISABLE_AUTO_MAINTENANCE_CONFIG)
581+ .args([
582+ "-c",
583+ "uploadpack.allowrefinwant",
584+ "daemon",
585+ "--inetd",
586+ "--verbose",
587+ "--base-path=.",
588+ "--export-all",
589+ "--user-path",
590+ ])
591+ .current_dir(&working_dir)
592+ .stdin(stdin)
593+ .stdout(stdout)
594+ .stderr(Stdio::null())
595+ .envs(remote_env(peer_addr))
596+ .spawn()
597+ else {
598+ continue;
599+ };
600+
601+ std::thread::spawn(move || {
602+ let _ = child.wait();
603+ });
604+ }
605+ }
606+ });
607+
608+ Ok(GitDaemon {
609+ process: GitDaemonProcess::Inetd {
610+ shutdown,
611+ server_addr,
612+ listener_thread: Some(listener_thread),
613+ },
614+ url: format!("git://{server_addr}"),
615+ })
616+}
617+
618+#[cfg(unix)]
619+fn remote_env(peer_addr: Option<std::net::SocketAddr>) -> Vec<(&'static str, String)> {
620+ peer_addr
621+ .map(|addr| {
622+ vec![
623+ ("REMOTE_ADDR", addr.ip().to_string()),
624+ ("REMOTE_PORT", addr.port().to_string()),
625+ ]
626+ })
627+ .unwrap_or_default()
628+}
629+
481630 /// Don't add a suffix to the archive name as `args` are platform dependent, non-deterministic,
482631 /// or otherwise don't influence the content of the archive.
483632 /// Note that this also means that `args` won't be used to control the hash of the archive itself.
@@ -1504,16 +1653,27 @@ fn configure_command<'a, I: IntoIterator<Item = S>, S: AsRef<OsStr>>( · 1020 unmodified lines
15041653 .env("GIT_COMMITTER_DATE", "2000-01-02 00:00:00 +0000")
15051654 .env("GIT_COMMITTER_EMAIL", "committer@example.com")
15061655 .env("GIT_COMMITTER_NAME", "committer")
1507− .env("GIT_CONFIG_COUNT", "4")
1508− .env("GIT_CONFIG_KEY_0", "commit.gpgsign")
1509− .env("GIT_CONFIG_VALUE_0", "false")
1510− .env("GIT_CONFIG_KEY_1", "tag.gpgsign")
1511− .env("GIT_CONFIG_VALUE_1", "false")
1512− .env("GIT_CONFIG_KEY_2", "init.defaultBranch")
1513− .env("GIT_CONFIG_VALUE_2", "main")
1514− .env("GIT_CONFIG_KEY_3", "protocol.file.allow")
1515− .env("GIT_CONFIG_VALUE_3", "always")
1516− .env("GIT_DEFAULT_HASH", object_hash.to_string())
1656+ .env("GIT_DEFAULT_HASH", object_hash.to_string());
1657+ apply_git_config_by_environment(cmd, ISOLATED_GIT_CONFIG)
1658+}
1659+
1660+/// Apply command-scoped Git `config` to `cmd`, and return it.
1661+///
1662+/// This sets `GIT_CONFIG_COUNT` and matching `GIT_CONFIG_KEY_<n>` /
1663+/// `GIT_CONFIG_VALUE_<n>` environment variables, which Git treats like
1664+/// command-line `-c <key>=<value>` entries for the spawned process. Existing
1665+/// values for these variables on `cmd` are overwritten for the configured
1666+/// indices.
1667+pub fn apply_git_config_by_environment<'a>(
1668+ cmd: &'a mut std::process::Command,
1669+ config: &[(&str, &str)],
1670+) -> &'a mut std::process::Command {
1671+ cmd.env("GIT_CONFIG_COUNT", config.len().to_string());
1672+ for (idx, (key, value)) in config.iter().enumerate() {
1673+ cmd.env(format!("GIT_CONFIG_KEY_{idx}"), key);
1674+ cmd.env(format!("GIT_CONFIG_VALUE_{idx}"), value);
1675+ }
1676+ cmd
15171677 }
15181678
15191679 /// Get the path attempted as a `bash` interpreter, for fixture scripts having no `#!` we can use.
tests/tools/src/main.rs+5−66
@@ -23,6 +23,7 @@ fn umask() -> io::Result<()> { · 22 unmodified lines
2323 }
2424
2525 /// Run a Git protocol test daemon on an OS-assigned loopback port.
26+/// This function blocks and the process needs to be killed.
2627 ///
2728 /// Journey tests use this instead of `git daemon --port=<n>` because Git
2829 /// treats `--port=0` as "use the default port", so it can't bind an
@@ -31,73 +32,11 @@ fn umask() -> io::Result<()> { · 2 unmodified lines
3132 /// hands every accepted connection to `git daemon --inetd`.
3233 #[cfg(unix)]
3334 fn git_daemon(url_file: PathBuf) -> io::Result<()> {
34− use std::{
35− net::{TcpListener, TcpStream},
36− os::fd::{FromRawFd, IntoRawFd},
37− process::{Command, Stdio},
38− thread,
39− };
40−
41− fn stream_to_stdio(stream: TcpStream) -> Stdio {
42− // SAFETY: `into_raw_fd()` transfers ownership of the socket fd, and `Stdio`
43− // takes over closing it in the spawned child.
44− unsafe { Stdio::from_raw_fd(stream.into_raw_fd()) }
35+ let daemon = gix_testtools::spawn_git_daemon(".")?;
36+ fs::write(url_file, format!("{}/\n", daemon.url))?;
37+ loop {
38+ std::thread::park();
4539 }
46−
47− let listener = TcpListener::bind(("127.0.0.1", 0))?;
48− let addr = listener.local_addr()?;
49− fs::write(url_file, format!("git://{addr}/\n"))?;
50−
51− for incoming in listener.incoming() {
52− let stream = incoming?;
53− let peer_addr = stream.peer_addr().ok();
54− let stdin = stream_to_stdio(stream.try_clone()?);
55− let stdout = stream_to_stdio(stream);
56−
57− let mut child = Command::new("git")
58− .args([
59− "-c",
60− "uploadpack.allowrefinwant",
61− "daemon",
62− "--inetd",
63− "--verbose",
64− "--base-path=.",
65− "--export-all",
66− "--user-path",
67− ])
68− .stdin(stdin)
69− .stdout(stdout)
70− .stderr(Stdio::null())
71− .envs(remote_env(peer_addr))
72− .spawn()?;
73−
74− thread::spawn(move || {
75− // Make sure we don't create zombies but wait for all children to finish.
76− // Also needed to have our process has handle to all possibly running Git invocations.
77− let _ = child.wait();
78− });
79− }
80−
81− Ok(())
82−}
83−
84−/// Return the client-address environment Git's own daemon loop would set.
85−///
86−/// In `--inetd` mode Git doesn't accept the socket itself, so it can't derive
87−/// `REMOTE_ADDR` and `REMOTE_PORT`. The wrapper owns `accept()`, and passes
88−/// these values along for code paths that log or inspect peer information.
89−///
90−/// As such, it's optional and not needed for it to function.
91−#[cfg(unix)]
92−fn remote_env(peer_addr: Option<std::net::SocketAddr>) -> Vec<(&'static str, String)> {
93− peer_addr
94− .map(|addr| {
95− vec![
96− ("REMOTE_ADDR", addr.ip().to_string()),
97− ("REMOTE_PORT", addr.port().to_string()),
98− ]
99− })
100− .unwrap_or_default()
10140 }
10241
10342 #[cfg(not(unix))]
tests/tools/src/tests.rs+40−0
@@ -154,6 +154,46 @@ fn invoke_bash_runs_in_given_working_directory() { · 153 unmodified lines
154154 );
155155 }
156156
157+#[test]
158+fn invoke_bash_disables_auto_maintenance_for_git_commands() {
159+ let dir = tempfile::TempDir::new().expect("can create temp dir");
160+ invoke_bash(
161+ dir.path(),
162+ "git config --get maintenance.auto > out && git config --get gc.auto >> out",
163+ );
164+ assert_eq!(
165+ std::fs::read_to_string(dir.path().join("out")).expect("script wrote output"),
166+ "false\n0\n",
167+ "Git commands run from the shell should not run automatic maintenance"
168+ );
169+}
170+
171+#[test]
172+fn run_git_disables_auto_maintenance() -> Result {
173+ let dir = tempfile::TempDir::new().expect("can create temp dir");
174+ let status = run_git(dir.path(), &["config", "--get", "maintenance.auto"])?;
175+ assert!(status.success(), "command-scope maintenance.auto should be visible");
176+ let status = run_git(dir.path(), &["config", "--get", "gc.auto"])?;
177+ assert!(status.success(), "command-scope gc.auto should be visible");
178+ Ok(())
179+}
180+
181+#[test]
182+fn git_helper_disables_auto_maintenance() -> Result {
183+ let dir = tempfile::TempDir::new().expect("can create temp dir");
184+ assert_eq!(
185+ git(dir.path(), "config --get maintenance.auto")?,
186+ "false\n",
187+ "Git commands run through gix-testtools should not run automatic maintenance"
188+ );
189+ assert_eq!(
190+ git(dir.path(), "config --get gc.auto")?,
191+ "0\n",
192+ "Auto-gc should be disabled for Git commands run through gix-testtools"
193+ );
194+ Ok(())
195+}
196+
157197 #[test]
158198 fn split_git_arguments_handles_multiline_whitespace() {
159199 assert_eq!(

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t