max / alloy
- Co-Authored-By
- Claude Opus 5 (1M context) <noreply@anthropic.com>
1 file changed,
+36 insertions,
-10 deletions
| @@ -706,11 +706,29 @@ | |||
| 706 | 706 | /// against the same list, and it has to be the same expansion or the two | |
| 707 | 707 | /// disagree about what `~/code` means. | |
| 708 | 708 | fn mount_path(mount: &str) -> Result<String> { | |
| 709 | + | mount_path_in(mount, std::env::var("HOME").ok().as_deref()) | |
| 710 | + | } | |
| 711 | + | ||
| 712 | + | /// [`mount_path`] against an explicit home. | |
| 713 | + | /// | |
| 714 | + | /// Split out for the same reason [`crate::settings::expand`] is, and its comment | |
| 715 | + | /// there already said why: `set_var` is unsafe in a threaded test binary, and a | |
| 716 | + | /// test that changes `HOME` under the other tests is a flake waiting for a slow | |
| 717 | + | /// machine. This module did not follow that and got the flake. | |
| 718 | + | /// | |
| 719 | + | /// It was `a_mount_expands_a_leading_tilde` setting `HOME=/home/tester` | |
| 720 | + | /// process-wide under a comment reading "single-threaded test", which is not what | |
| 721 | + | /// a Rust test binary is: the harness runs tests on a thread pool in one process. | |
| 722 | + | /// Any test reading `HOME` concurrently saw the fixture's value. | |
| 723 | + | /// `a_host_export_calls_distrobox_export_once_per_binary` is the one that | |
| 724 | + | /// noticed, because it compares an argv built from [`export_dir`] against a | |
| 725 | + | /// second call to it, and failed roughly one run in five. | |
| 726 | + | fn mount_path_in(mount: &str, home: Option<&str>) -> Result<String> { | |
| 709 | 727 | let path = mount.strip_suffix(":ro").unwrap_or(mount); | |
| 710 | 728 | ||
| 711 | 729 | let path = match path.strip_prefix("~/") { | |
| 712 | 730 | Some(rest) => { | |
| 713 | - | let home = std::env::var("HOME").context("`~` in a mount needs HOME set")?; | |
| 731 | + | let home = home.context("`~` in a mount needs HOME set")?; | |
| 714 | 732 | format!("{home}/{rest}") | |
| 715 | 733 | } | |
| 716 | 734 | None => path.to_string(), | |
| @@ -2499,17 +2517,25 @@ | |||
| 2499 | 2517 | ||
| 2500 | 2518 | // Podman does no tilde expansion, so an unexpanded `~` would create a | |
| 2501 | 2519 | // directory named `~` rather than mounting the home path meant. | |
| 2520 | + | // | |
| 2521 | + | // Against an explicit home rather than by setting one. The previous version | |
| 2522 | + | // of this test set `HOME` process-wide and restored it, under a comment | |
| 2523 | + | // claiming it was single-threaded; the harness runs tests on a thread pool in | |
| 2524 | + | // one process, so for the length of this test every other test saw | |
| 2525 | + | // `/home/tester`. `a_host_export_calls_distrobox_export_once_per_binary` | |
| 2526 | + | // reads `HOME` through `export_dir` and failed about one run in five. | |
| 2502 | 2527 | #[test] | |
| 2503 | 2528 | fn a_mount_expands_a_leading_tilde() { | |
| 2504 | - | // SAFETY: single-threaded test, and the value is restored below. | |
| 2505 | - | let previous = std::env::var_os("HOME"); | |
| 2506 | - | unsafe { std::env::set_var("HOME", "/home/tester") }; | |
| 2507 | - | let bound = bind("~/code/thing").unwrap(); | |
| 2508 | - | match previous { | |
| 2509 | - | Some(home) => unsafe { std::env::set_var("HOME", home) }, | |
| 2510 | - | None => unsafe { std::env::remove_var("HOME") }, | |
| 2511 | - | } | |
| 2512 | - | assert_eq!(bound, "/home/tester/code/thing:/home/tester/code/thing"); | |
| 2529 | + | let path = mount_path_in("~/code/thing", Some("/home/tester")).unwrap(); | |
| 2530 | + | assert_eq!(path, "/home/tester/code/thing"); | |
| 2531 | + | } | |
| 2532 | + | ||
| 2533 | + | // The error names the variable rather than the mount, because a `~` with no | |
| 2534 | + | // HOME is an environment problem and nothing about the spec would fix it. | |
| 2535 | + | #[test] | |
| 2536 | + | fn a_tilde_with_no_home_says_which_is_missing() { | |
| 2537 | + | let err = mount_path_in("~/code/thing", None).unwrap_err().to_string(); | |
| 2538 | + | assert!(err.contains("HOME"), "got: {err}"); | |
| 2513 | 2539 | } | |
| 2514 | 2540 | ||
| 2515 | 2541 | // Podman rejects a relative source too, but only once the command runs, and |