Skip to content

Commit f57bfab

Browse files
committed
fix: Resolve Windows test failures - local path detection and Git path separators
This commit fixes two critical cross-platform issues causing test failures on Windows: ## Problem 1: Local Path Detection Fails on Windows The preflight health check logic in split.rs and sync.rs only recognized Unix-style paths (/, ./, ../), causing Windows paths like C:\Users\... to be incorrectly identified as remote URLs. This triggered unnecessary SSH preflight checks that failed in test environments. ### Solution: - Added is_local_path() helper function that properly detects local paths on both Unix and Windows - Uses Path::is_absolute() which works cross-platform - Handles Windows-specific cases: drive letters (C:\), UNC paths (\\server\share) - Applied to both split.rs:114 and sync.rs:156 ## Problem 2: Git Commands Receive Windows Backslashes Git always expects forward slashes in path specifications (e.g., "HEAD:src/file.rs"), even on Windows. However, Path::display() outputs backslashes on Windows, causing git cat-file and git ls-tree commands to fail with "missing" file errors. ### Solution: - Added path_to_git_format() helper in system_git_ops.rs that converts backslashes to forward slashes on Windows - Applied to all Git tree spec operations: - read_files_bulk() at line 766 (bulk file reading) - get_file_at_commit() at line 175 (single file reads) - list_files_at_commit() at line 356 (tree listing) ## Impact: - Fixes 16 failing integration tests on Windows (all test_split::*, test_sync::*, test_workflow::* tests) - Fixes 1 failing unit test (test_collect_tree_files_with_bulk) - No behavior change on Unix/Linux/macOS - Tests pass locally on macOS, should now pass on Windows CI runners Closes the Windows test failure blocking issue.
1 parent 98f7d3c commit f57bfab

File tree

3 files changed

+115
-9
lines changed

3 files changed

+115
-9
lines changed

src/commands/split.rs

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use std::env;
22
use std::io::{self, Write};
3+
use std::path::Path;
34

45
use crate::commands::doctor;
56
use crate::core::config::RailConfig;
@@ -9,6 +10,50 @@ use crate::core::split::{SplitConfig, Splitter};
910
use crate::ui::progress::{FileProgress, MultiProgress};
1011
use rayon::prelude::*;
1112

13+
/// Check if a path is a local filesystem path (not a remote URL)
14+
///
15+
/// Returns true for:
16+
/// - Absolute paths on Unix: /path/to/repo
17+
/// - Absolute paths on Windows: C:\path\to\repo or C:/path/to/repo
18+
/// - Relative paths: ./path or ../path
19+
/// - UNC paths on Windows: \\server\share
20+
///
21+
/// Returns false for:
22+
/// - SSH URLs: [email protected]:user/repo.git
23+
/// - HTTPS URLs: <https://github.com/user/repo.git>
24+
fn is_local_path(path: &str) -> bool {
25+
let p = Path::new(path);
26+
27+
// Check for relative paths
28+
if path.starts_with("./") || path.starts_with("../") {
29+
return true;
30+
}
31+
32+
// Check for absolute paths (works on both Unix and Windows)
33+
if p.is_absolute() {
34+
return true;
35+
}
36+
37+
// Check for Windows UNC paths (\\server\share)
38+
#[cfg(target_os = "windows")]
39+
if path.starts_with("\\\\") {
40+
return true;
41+
}
42+
43+
// If it contains :// it's a URL
44+
if path.contains("://") {
45+
return false;
46+
}
47+
48+
// If it contains @ it's likely an SSH URL ([email protected]:user/repo.git)
49+
if path.contains('@') {
50+
return false;
51+
}
52+
53+
// Default to false for safety (require preflight checks)
54+
false
55+
}
56+
1257
/// Prompt user for confirmation
1358
fn prompt_for_confirmation(message: &str) -> RailResult<bool> {
1459
print!("\n{}: ", message);
@@ -66,9 +111,7 @@ pub fn run_split(
66111
}
67112

68113
// Check if all remotes are local paths (skip SSH checks for local testing)
69-
let all_local = crates_to_split_check
70-
.iter()
71-
.all(|s| s.remote.starts_with('/') || s.remote.starts_with("./") || s.remote.starts_with("../"));
114+
let all_local = crates_to_split_check.iter().all(|s| is_local_path(&s.remote));
72115

73116
// Run preflight health checks before proceeding (skip for local-only operations)
74117
if !json && apply && !all_local {

src/commands/sync.rs

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use std::env;
2+
use std::path::Path;
23

34
use crate::commands::doctor;
45
use crate::core::config::RailConfig;
@@ -9,6 +10,50 @@ use crate::core::sync::{SyncConfig, SyncDirection, SyncEngine, SyncResult};
910
use crate::ui::progress::{FileProgress, MultiProgress};
1011
use rayon::prelude::*;
1112

13+
/// Check if a path is a local filesystem path (not a remote URL)
14+
///
15+
/// Returns true for:
16+
/// - Absolute paths on Unix: /path/to/repo
17+
/// - Absolute paths on Windows: C:\path\to\repo or C:/path/to/repo
18+
/// - Relative paths: ./path or ../path
19+
/// - UNC paths on Windows: \\server\share
20+
///
21+
/// Returns false for:
22+
/// - SSH URLs: [email protected]:user/repo.git
23+
/// - HTTPS URLs: <https://github.com/user/repo.git>
24+
fn is_local_path(path: &str) -> bool {
25+
let p = Path::new(path);
26+
27+
// Check for relative paths
28+
if path.starts_with("./") || path.starts_with("../") {
29+
return true;
30+
}
31+
32+
// Check for absolute paths (works on both Unix and Windows)
33+
if p.is_absolute() {
34+
return true;
35+
}
36+
37+
// Check for Windows UNC paths (\\server\share)
38+
#[cfg(target_os = "windows")]
39+
if path.starts_with("\\\\") {
40+
return true;
41+
}
42+
43+
// If it contains :// it's a URL
44+
if path.contains("://") {
45+
return false;
46+
}
47+
48+
// If it contains @ it's likely an SSH URL ([email protected]:user/repo.git)
49+
if path.contains('@') {
50+
return false;
51+
}
52+
53+
// Default to false for safety (require preflight checks)
54+
false
55+
}
56+
1257
/// Sync command parameters
1358
pub struct SyncParams {
1459
pub crate_name: Option<String>,
@@ -108,9 +153,7 @@ fn run_sync_impl(params: SyncParams) -> RailResult<()> {
108153
}
109154

110155
// Check if all remotes are local paths (skip SSH checks for local testing)
111-
let all_local = crates_to_sync_check
112-
.iter()
113-
.all(|s| s.remote.starts_with('/') || s.remote.starts_with("./") || s.remote.starts_with("../"));
156+
let all_local = crates_to_sync_check.iter().all(|s| is_local_path(&s.remote));
114157

115158
// Run preflight health checks before proceeding (skip for local-only operations)
116159
if !json && apply && !all_local {

src/core/vcs/system_git_ops.rs

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,23 @@ impl SystemGit {
1818
}
1919
}
2020

21+
/// Convert a path to Git format (always forward slashes)
22+
///
23+
/// Git expects paths with forward slashes, even on Windows.
24+
/// This function converts backslashes to forward slashes for use in Git commands.
25+
fn path_to_git_format(&self, path: &Path) -> String {
26+
// On Windows, convert backslashes to forward slashes
27+
// On Unix, this is a no-op since paths already use forward slashes
28+
#[cfg(target_os = "windows")]
29+
{
30+
path.to_string_lossy().replace('\\', "/")
31+
}
32+
#[cfg(not(target_os = "windows"))]
33+
{
34+
path.to_string_lossy().to_string()
35+
}
36+
}
37+
2138
/// Get commit history from HEAD with optional limit
2239
///
2340
/// Returns commits in reverse chronological order (newest first).
@@ -153,8 +170,9 @@ impl SystemGit {
153170
#[allow(dead_code)]
154171
pub fn get_file_at_commit(&self, commit_sha: &str, path: &Path) -> RailResult<Option<Vec<u8>>> {
155172
let relative_path = self.normalize_path(path);
173+
let git_path = self.path_to_git_format(relative_path);
156174

157-
let spec = format!("{}:{}", commit_sha, relative_path.display());
175+
let spec = format!("{}:{}", commit_sha, git_path);
158176

159177
let output = self
160178
.git_cmd()
@@ -334,7 +352,8 @@ impl SystemGit {
334352
let spec = if path.as_os_str().is_empty() {
335353
commit_sha.to_string()
336354
} else {
337-
format!("{}:{}", commit_sha, path.display())
355+
let git_path = self.path_to_git_format(path);
356+
format!("{}:{}", commit_sha, git_path)
338357
};
339358

340359
let output = self
@@ -746,7 +765,8 @@ impl SystemGit {
746765
// Write all requests to stdin
747766
for (commit_sha, path) in items {
748767
let relative_path = self.normalize_path(path);
749-
let spec = format!("{}:{}\n", commit_sha, relative_path.display());
768+
let git_path = self.path_to_git_format(relative_path);
769+
let spec = format!("{}:{}\n", commit_sha, git_path);
750770
stdin
751771
.write_all(spec.as_bytes())
752772
.context("Failed to write to git cat-file stdin")?;

0 commit comments

Comments
 (0)