Skip to content

Commit c2c8c2f

Browse files
authored
Merge pull request #2115 from EliahKagan/run-ci/arm-windows
Fix ARM64 Windows null device and Git alternative locations
2 parents 20b7c20 + ba17f4b commit c2c8c2f

File tree

6 files changed

+317
-93
lines changed

6 files changed

+317
-93
lines changed

.github/workflows/ci.yml

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -208,9 +208,14 @@ jobs:
208208
matrix:
209209
os:
210210
- windows-latest
211+
- windows-11-arm
211212
- macos-latest
212213
- ubuntu-latest
213214
- ubuntu-24.04-arm
215+
include:
216+
- test-args: ''
217+
- os: windows-11-arm
218+
test-args: '--skip fuzzed_timeout --skip performance --skip speed'
214219

215220
runs-on: ${{ matrix.os }}
216221

@@ -227,12 +232,39 @@ jobs:
227232
- name: Test (nextest)
228233
env:
229234
GIX_TEST_CREATE_ARCHIVES_EVEN_ON_CI: '1'
230-
run: cargo nextest run --workspace --no-fail-fast
235+
run: cargo nextest run --workspace --no-fail-fast -- ${{ matrix.test-args }}
231236
- name: Check that tracked archives are up to date
232237
run: git diff --exit-code # If this fails, the fix is usually to commit a regenerated archive.
238+
- name: Remove Git for Windows directories from PATH
239+
if: startsWith(matrix.os, 'windows')
240+
run: |
241+
$prefix = 'C:\Program Files\Git'
242+
$filtered = ($Env:PATH -split ';' | Where-Object { $_ -notlike "$prefix\*" }) -join ';'
243+
Add-Content -Value "PATH=$filtered" -Path $Env:GITHUB_ENV
244+
- name: Check that `git` is no longer found
245+
if: startsWith(matrix.os, 'windows')
246+
run: |
247+
$git = Get-Command git -ErrorAction SilentlyContinue
248+
if ($null -eq $git) { exit 0 } else { exit 1 }
249+
- name: Check that `EXEPATH` is unset
250+
if: startsWith(matrix.os, 'windows')
251+
run: if ($null -eq $Env:EXEPATH) { exit 0 } else { exit 1 }
252+
- name: Retest gix-path without `git` in `PATH` (nextest)
253+
if: startsWith(matrix.os, 'windows')
254+
run: cargo nextest run -p gix-path --no-fail-fast -- ${{ matrix.test-args }}
233255

234256
test-fixtures-windows:
235-
runs-on: windows-latest
257+
strategy:
258+
matrix:
259+
os:
260+
- windows-latest
261+
- windows-11-arm
262+
include:
263+
- test-args: ''
264+
- os: windows-11-arm
265+
test-args: '--skip fuzzed-timeout --skip performance --skip speed'
266+
267+
runs-on: ${{ matrix.os }}
236268

237269
steps:
238270
- uses: actions/checkout@v4
@@ -245,7 +277,8 @@ jobs:
245277
id: nextest
246278
env:
247279
GIX_TEST_IGNORE_ARCHIVES: '1'
248-
run: cargo nextest --profile=with-xml run --workspace --no-fail-fast
280+
run: |
281+
cargo nextest --profile=with-xml run --workspace --no-fail-fast -- ${{ matrix.test-args }}
249282
continue-on-error: true
250283
- name: Check for errors
251284
run: |
@@ -264,7 +297,8 @@ jobs:
264297
- name: Compare expected and actual failures
265298
run: |
266299
# Fail on any differences, even unexpectedly passing tests, so they can be investigated.
267-
git --no-pager diff --no-index --exit-code --unified=1000000 --color=always -- `
300+
git --no-pager -c diff.color.old='magenta bold' -c diff.color.new='blue bold' `
301+
diff --no-index --exit-code --unified=1000000 --color=always -- `
268302
etc/test-fixtures-windows-expected-failures-see-issue-1358.txt actual-failures.txt
269303
270304
test-32bit:
@@ -339,7 +373,7 @@ jobs:
339373
with:
340374
tool: nextest
341375
- name: Test data structure sizes (nextest)
342-
run: cargo nextest run --target $env:TARGET --workspace --no-fail-fast size
376+
run: cargo nextest run --target $Env:TARGET --workspace --no-fail-fast size
343377
- name: Doctest
344378
run: cargo test --workspace --doc --no-fail-fast
345379

gix-path/src/env/git/mod.rs

Lines changed: 39 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -27,47 +27,50 @@ where
2727
let varname_x86 = "ProgramFiles(x86)";
2828

2929
// Should give a 32-bit program files path on a 32-bit system. We also check this on a 64-bit
30-
// system, even though it *should* equal the process's architecture specific variable, so that
30+
// system, even though it *should* equal the process's architecture-specific variable, so that
3131
// we cover the case of a parent process that passes down an overly sanitized environment that
3232
// lacks the architecture-specific variable. On a 64-bit system, because parent and child
33-
// processes' architectures can be different, Windows sets the child's ProgramFiles variable
34-
// from the ProgramW6432 or ProgramFiles(x86) variable applicable to the child's architecture.
35-
// Only if the parent does not pass that down is the passed-down ProgramFiles variable even
36-
// used. But this behavior is not well known, so that situation does sometimes happen.
33+
// processes' architectures can be different, Windows sets the child's `ProgramFiles` variable
34+
// from whichever of the `ProgramW6432` or `ProgramFiles(x86)` variable corresponds to the
35+
// child's architecture. Only if the parent does not pass down the architecture-specific
36+
// variable corresponding to the child's architecture does the child receive its `ProgramFiles`
37+
// variable from `ProgramFiles` as passed down by the parent. But this behavior is not well
38+
// known. So the situation where a process only passes down `ProgramFiles` sometimes happens.
3739
let varname_current = "ProgramFiles";
3840

39-
// 64-bit relative bin dir. So far, this is always mingw64, not ucrt64, clang64, or clangarm64.
40-
let suffix_64 = Path::new(r"Git\mingw64\bin");
41+
// 64-bit relative bin dirs. So far, this is always `mingw64` or `clangarm64`, not `urct64` or
42+
// `clang64`. We check `clangarm64` before `mingw64`, because in the strage case that both are
43+
// available, we don't want to skip over a native ARM64 executable for an emulated x86_64 one.
44+
let suffixes_64 = [r"Git\clangarm64\bin", r"Git\mingw64\bin"].as_slice();
4145

42-
// 32-bit relative bin dir. So far, this is always mingw32, not clang32.
43-
let suffix_32 = Path::new(r"Git\mingw32\bin");
46+
// 32-bit relative bin dirs. So far, this is only ever `mingw32`, not `clang32`.
47+
let suffixes_32 = [r"Git\mingw32\bin"].as_slice();
4448

4549
// Whichever of the 64-bit or 32-bit relative bin better matches this process's architecture.
4650
// Unlike the system architecture, the process architecture is always known at compile time.
4751
#[cfg(target_pointer_width = "64")]
48-
let suffix_current = suffix_64;
52+
let suffixes_current = suffixes_64;
4953
#[cfg(target_pointer_width = "32")]
50-
let suffix_current = suffix_32;
54+
let suffixes_current = suffixes_32;
5155

5256
let rules = [
53-
(varname_64bit, suffix_64),
54-
(varname_x86, suffix_32),
55-
(varname_current, suffix_current),
57+
(varname_64bit, suffixes_64),
58+
(varname_x86, suffixes_32),
59+
(varname_current, suffixes_current),
5660
];
5761

5862
let mut locations = vec![];
5963

60-
for (name, suffix) in rules {
61-
let Some(pf) = var_os_func(name) else { continue };
62-
let pf = Path::new(&pf);
63-
if pf.is_relative() {
64-
// This shouldn't happen, but if it does then don't use the path. This is mainly in
65-
// case we are accidentally invoked with the environment variable set but empty.
64+
for (varname, suffixes) in rules {
65+
let Some(program_files_dir) = var_os_func(varname).map(PathBuf::from).filter(|p| p.is_absolute()) else {
66+
// The environment variable is unset or somehow not an absolute path (e.g. an empty string).
6667
continue;
67-
}
68-
let location = pf.join(suffix);
69-
if !locations.contains(&location) {
70-
locations.push(location);
68+
};
69+
for suffix in suffixes {
70+
let location = program_files_dir.join(suffix);
71+
if !locations.contains(&location) {
72+
locations.push(location);
73+
}
7174
}
7275
}
7376

@@ -81,11 +84,15 @@ pub(super) const EXE_NAME: &str = "git";
8184

8285
/// Invoke the git executable to obtain the origin configuration, which is cached and returned.
8386
///
84-
/// The git executable is the one found in PATH or an alternative location.
87+
/// The git executable is the one found in `PATH` or an alternative location.
8588
pub(super) static GIT_HIGHEST_SCOPE_CONFIG_PATH: Lazy<Option<BString>> = Lazy::new(exe_info);
8689

90+
// There are a number of ways to refer to the null device on Windows, but they are not all equally
91+
// well supported. Git for Windows rejects `\\.\NUL` and `\\.\nul`. On Windows 11 ARM64 (and maybe
92+
// some others), it rejects even the legacy name `NUL`, when capitalized. But it always accepts the
93+
// lower-case `nul`, handling it in various path checks, some of which are done case-sensitively.
8794
#[cfg(windows)]
88-
const NULL_DEVICE: &str = "NUL";
95+
const NULL_DEVICE: &str = "nul";
8996
#[cfg(not(windows))]
9097
const NULL_DEVICE: &str = "/dev/null";
9198

@@ -130,17 +137,17 @@ fn git_cmd(executable: PathBuf) -> Command {
130137
} else {
131138
"/".into()
132139
};
133-
// Git 2.8.0 and higher support --show-origin. The -l, -z, and --name-only options were
134-
// supported even before that. In contrast, --show-scope was introduced later, in Git 2.26.0.
140+
// Git 2.8.0 and higher support `--show-origin`. The `-l`, `-z`, and `--name-only` options were
141+
// supported even before that. In contrast, `--show-scope` was introduced later, in Git 2.26.0.
135142
// Low versions of Git are still sometimes used, and this is sometimes reasonable because
136143
// downstream distributions often backport security patches without adding most new features.
137-
// So for now, we forgo the convenience of --show-scope for greater backward compatibility.
144+
// So for now, we forgo the convenience of `--show-scope` for greater backward compatibility.
138145
//
139-
// Separately from that, we can't use --system here, because scopes treated higher than the
146+
// Separately from that, we can't use `--system` here, because scopes treated higher than the
140147
// system scope are possible. This commonly happens on macOS with Apple Git, where the config
141148
// file under `/Library` or `/Applications` is shown as an "unknown" scope but takes precedence
142149
// over the system scope. Although `GIT_CONFIG_NOSYSTEM` suppresses this scope along with the
143-
// system scope, passing --system selects only the system scope and omit this "unknown" scope.
150+
// system scope, passing `--system` selects only the system scope and not this "unknown" scope.
144151
cmd.args(["config", "-lz", "--show-origin", "--name-only"])
145152
.current_dir(cwd)
146153
.env_remove("GIT_CONFIG")
@@ -161,7 +168,7 @@ fn first_file_from_config_with_origin(source: &BStr) -> Option<&BStr> {
161168
file[..end_pos].as_bstr().into()
162169
}
163170

164-
/// Try to find the file that contains git configuration coming with the git installation.
171+
/// Try to find the file that contains Git configuration coming with the Git installation.
165172
///
166173
/// This returns the configuration associated with the `git` executable found in the current `PATH`
167174
/// or an alternative location, or `None` if no `git` executable was found or there were other

0 commit comments

Comments
 (0)