Skip to content

Commit 669028f

Browse files
authored
fix: respect case sensitivity of filesystem when collecting new files (#1699)
1 parent 2add712 commit 669028f

File tree

4 files changed

+242
-11
lines changed

4 files changed

+242
-11
lines changed

src/packaging/file_finder.rs

Lines changed: 168 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,38 @@ use walkdir::WalkDir;
1111

1212
use crate::{metadata::Output, recipe::parser::GlobVec};
1313

14-
use super::{PackagingError, file_mapper};
14+
use super::{PackagingError, file_mapper, normalize_path_for_comparison};
15+
16+
/// A wrapper around PathBuf that implements case-insensitive hashing and equality
17+
/// when the filesystem is case-insensitive
18+
#[derive(Debug, Clone)]
19+
struct CaseInsensitivePath {
20+
path: String,
21+
}
22+
23+
impl CaseInsensitivePath {
24+
fn new(path: &Path) -> Self {
25+
Self {
26+
path: normalize_path_for_comparison(path, true).unwrap(),
27+
}
28+
}
29+
}
30+
31+
impl std::hash::Hash for CaseInsensitivePath {
32+
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
33+
// Convert to lowercase string for case-insensitive hashing
34+
self.path.hash(state);
35+
}
36+
}
37+
38+
impl PartialEq for CaseInsensitivePath {
39+
fn eq(&self, other: &Self) -> bool {
40+
// Case-insensitive comparison
41+
self.path == other.path
42+
}
43+
}
44+
45+
impl Eq for CaseInsensitivePath {}
1546

1647
/// This struct keeps a record of all the files that are new in the prefix (i.e. not present in the previous
1748
/// conda environment).
@@ -64,6 +95,52 @@ pub fn record_files(directory: &Path) -> Result<HashSet<PathBuf>, io::Error> {
6495
Ok(res)
6596
}
6697

98+
// Check if the filesystem is case-sensitive by creating a file with a different case
99+
// and checking if it exists.
100+
fn check_is_case_sensitive() -> Result<bool, io::Error> {
101+
// Check if the filesystem is case insensitive
102+
let tempdir = TempDir::new()?;
103+
let file1 = tempdir.path().join("testfile.txt");
104+
let file2 = tempdir.path().join("TESTFILE.txt");
105+
fs::File::create(&file1)?;
106+
Ok(!file2.exists() && file1.exists())
107+
}
108+
109+
/// Helper function to find files that exist in current_files but not in previous_files,
110+
/// taking into account case sensitivity
111+
fn find_new_files(
112+
current_files: &HashSet<PathBuf>,
113+
previous_files: &HashSet<PathBuf>,
114+
prefix: &Path,
115+
is_case_sensitive: bool,
116+
) -> HashSet<PathBuf> {
117+
if is_case_sensitive {
118+
// On case-sensitive filesystems, use normal set difference
119+
current_files.difference(previous_files).cloned().collect()
120+
} else {
121+
// On case-insensitive filesystems, use case-aware comparison
122+
let previous_case_aware: HashSet<CaseInsensitivePath> = previous_files
123+
.iter()
124+
.map(|p| {
125+
CaseInsensitivePath::new(p.strip_prefix(prefix).expect("File should be in prefix"))
126+
})
127+
.collect();
128+
129+
let current_files = current_files
130+
.clone()
131+
.into_iter()
132+
.filter(|p| {
133+
// Only include files that are not in the previous set
134+
!previous_case_aware.contains(&CaseInsensitivePath::new(
135+
p.strip_prefix(prefix).expect("File should be in prefix"),
136+
))
137+
})
138+
.collect::<HashSet<_>>();
139+
140+
current_files
141+
}
142+
}
143+
67144
impl Files {
68145
/// Find all files in the given (host) prefix and remove all previously installed files (based on the PrefixRecord
69146
/// of the conda environment). If always_include is Some, then all files matching the glob pattern will be included
@@ -81,6 +158,8 @@ impl Files {
81158
});
82159
}
83160

161+
let fs_is_case_sensitive = check_is_case_sensitive()?;
162+
84163
let previous_files = if prefix.join("conda-meta").exists() {
85164
let prefix_records: Vec<PrefixRecord> = PrefixRecord::collect_from_prefix(prefix)?;
86165
let mut previous_files =
@@ -99,16 +178,23 @@ impl Files {
99178
};
100179

101180
let current_files = record_files(prefix)?;
102-
let mut difference = current_files
103-
.difference(&previous_files)
104-
// If we have an files glob, we only include files that match the glob
105-
.filter(|f| {
106-
files.is_empty()
107-
|| files.is_match(f.strip_prefix(prefix).expect("File should be in prefix"))
108-
})
109-
.cloned()
110-
.collect::<HashSet<_>>();
111181

182+
// Use case-aware difference calculation
183+
let mut difference = find_new_files(
184+
&current_files,
185+
&previous_files,
186+
prefix,
187+
fs_is_case_sensitive,
188+
);
189+
190+
// Filter by files glob if specified
191+
if !files.is_empty() {
192+
difference.retain(|f| {
193+
files.is_match(f.strip_prefix(prefix).expect("File should be in prefix"))
194+
});
195+
}
196+
197+
// Handle always_include files
112198
if !always_include.is_empty() {
113199
for file in current_files {
114200
let file_without_prefix =
@@ -171,3 +257,75 @@ impl TempFiles {
171257
&self.content_type_map
172258
}
173259
}
260+
261+
#[cfg(test)]
262+
mod test {
263+
use std::{collections::HashSet, path::PathBuf};
264+
265+
use crate::packaging::file_finder::{check_is_case_sensitive, find_new_files};
266+
267+
#[test]
268+
fn test_find_new_files_case_sensitive() {
269+
let current_files: HashSet<PathBuf> = [
270+
PathBuf::from("/test/File.txt"),
271+
PathBuf::from("/test/file.txt"),
272+
PathBuf::from("/test/common.txt"),
273+
]
274+
.into_iter()
275+
.collect();
276+
277+
let previous_files: HashSet<PathBuf> = [
278+
PathBuf::from("/test/File.txt"),
279+
PathBuf::from("/test/common.txt"),
280+
]
281+
.into_iter()
282+
.collect();
283+
284+
let prefix = PathBuf::from("/test");
285+
let new_files = find_new_files(&current_files, &previous_files, &prefix, true);
286+
287+
// On case-sensitive filesystem, file.txt should be considered new
288+
assert_eq!(new_files.len(), 1);
289+
assert!(new_files.contains(&PathBuf::from("/test/file.txt")));
290+
}
291+
292+
#[test]
293+
fn test_find_new_files_case_insensitive() {
294+
let current_files: HashSet<PathBuf> = [
295+
PathBuf::from("/test/File.txt"),
296+
PathBuf::from("/test/file.txt"),
297+
PathBuf::from("/test/common.txt"),
298+
PathBuf::from("/test/NEW.txt"),
299+
]
300+
.into_iter()
301+
.collect();
302+
303+
let previous_files: HashSet<PathBuf> = [
304+
PathBuf::from("/test/FILE.TXT"), // Different case of File.txt
305+
PathBuf::from("/test/common.txt"),
306+
]
307+
.into_iter()
308+
.collect();
309+
310+
let prefix = PathBuf::from("/test");
311+
let new_files = find_new_files(&current_files, &previous_files, &prefix, false);
312+
313+
// On case-insensitive filesystem, only NEW.txt should be considered new
314+
// Both File.txt and file.txt should be considered as existing (matching FILE.TXT)
315+
assert_eq!(new_files.len(), 1);
316+
assert!(new_files.contains(&PathBuf::from("/test/NEW.txt")));
317+
assert!(!new_files.contains(&PathBuf::from("/test/File.txt")));
318+
assert!(!new_files.contains(&PathBuf::from("/test/file.txt")));
319+
}
320+
321+
#[test]
322+
fn test_check_is_case_sensitive() {
323+
// This test will behave differently on different filesystems
324+
let result = check_is_case_sensitive();
325+
assert!(result.is_ok());
326+
327+
// We can't assert the specific value since it depends on the filesystem,
328+
// but we can verify the function doesn't panic and returns a boolean
329+
let _is_case_sensitive = result.unwrap();
330+
}
331+
}

src/packaging/metadata.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ pub fn contains_prefix_binary(file_path: &Path, prefix: &Path) -> Result<bool, P
3333
// TODO on Windows check both ascii and utf-8 / 16?
3434
#[cfg(target_family = "windows")]
3535
{
36-
tracing::warn!("Windows is not supported yet for binary prefix checking.");
36+
tracing::debug!("Windows is not supported yet for binary prefix checking.");
3737
Ok(false)
3838
}
3939

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
recipe:
2+
name: case-insensitive-test
3+
version: "1.0.0"
4+
5+
outputs:
6+
- package:
7+
name: c1
8+
9+
build:
10+
script:
11+
- if: win
12+
then:
13+
- mkdir %PREFIX%\CMake\
14+
- echo "This is a test file for case insensitivity" > %PREFIX%\CMake\test_file.txt
15+
else:
16+
- mkdir -p $PREFIX/CMake/
17+
- echo "This is a test file for case insensitivity" > $PREFIX/CMake/test_file.txt
18+
19+
- package:
20+
name: c2
21+
22+
requirements:
23+
host:
24+
- c1
25+
26+
build:
27+
script:
28+
- if: win
29+
then:
30+
- rmdir /S /Q %PREFIX%\CMake\
31+
- mkdir %PREFIX%\cmake\
32+
- echo "foo" > %PREFIX%\TEST.txt
33+
- echo "bar" > %PREFIX%\test.txt
34+
- echo "This is a test file for case insensitivity" > %PREFIX%\cmake\test_file.txt
35+
else:
36+
- rm -rf $PREFIX/CMake/
37+
- mkdir -p $PREFIX/cmake/
38+
- echo "foo" > $PREFIX/TEST.txt
39+
- echo "bar" > $PREFIX/test.txt
40+
- echo "This is a test file for case insensitivity" > $PREFIX/cmake/test_file.txt

test/end-to-end/test_simple.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2044,3 +2044,36 @@ def test_condapackageignore(rattler_build: RattlerBuild, recipes: Path, tmp_path
20442044
assert (files_dir / "recipe.yaml").exists()
20452045
assert not (files_dir / "ignored.txt").exists()
20462046
assert not (files_dir / "test.pyc").exists()
2047+
2048+
2049+
def test_caseinsensitive(rattler_build: RattlerBuild, recipes: Path, tmp_path: Path):
2050+
"""Test that case-insensitive file systems handle files correctly."""
2051+
# Build the package with a recipe that has mixed-case filenames
2052+
rattler_build.build(
2053+
recipes / "case-insensitive/recipe.yaml",
2054+
tmp_path,
2055+
)
2056+
2057+
pkg = get_extracted_package(tmp_path, "c2")
2058+
2059+
# check if the current filesystem is case-insensitive by creating a temporary file with a mixed case name
2060+
test_file = tmp_path / "MixedCaseFile.txt"
2061+
mixed_case_file = tmp_path / "mixedcasefile.txt"
2062+
2063+
# create the mixed-case files
2064+
test_file.write_text("This is a test.")
2065+
case_insensitive = mixed_case_file.exists()
2066+
2067+
paths_json = (pkg / "info/paths.json").read_text()
2068+
paths = json.loads(paths_json)
2069+
paths = [p["_path"] for p in paths["paths"]]
2070+
2071+
if case_insensitive:
2072+
# we don't package `cmake/test_file.txt` again, because our dependency already contains `CMake/test_file.txt`
2073+
assert len(paths) == 1
2074+
assert "TEST.txt" in paths or "test.txt" in paths
2075+
else:
2076+
assert len(paths) == 3
2077+
assert "cmake/test_file.txt" in paths
2078+
assert "TEST.txt" in paths
2079+
assert "test.txt" in paths

0 commit comments

Comments
 (0)