Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- The `source.filter` field is now also supported for `url` and `git` sources (it was previously only available for `path` sources). The filter is applied to the files copied into the work directory: the contents of the extracted archive for `url` sources (and `path` sources pointing to an archive) and the checked-out tree for `git` sources. This allows large sources to be trimmed down to the parts needed for the build.
- An empty `sha256` or `md5` (e.g. `sha256: ""`) is now accepted and treated as an all-zeros placeholder (`0000...0000`). This makes it easier to scaffold a recipe before the real checksum is known: the build downloads the source and reports the actual checksum in the resulting mismatch. (#2524)
- The build summary now prints a section for each `requirements.extras` group (optional dependency group) in the run dependencies table, so the resolved contents of each extra are visible. The section is omitted when the recipe defines no extras.
- Line endings in UTF-8 text files are now normalized to LF when building `noarch` packages (both `generic` and `python`). This makes `noarch` packages reproducible regardless of whether they are built on Windows or Unix. Binary files and other encodings (e.g. UTF-16) are left untouched. (#837)

### Changed

Expand Down
125 changes: 124 additions & 1 deletion crates/rattler_build_core/src/packaging/file_mapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,69 @@

use crate::metadata::Output;
use fs_err as fs;
use line_ending::LineEnding;
use rattler_conda_types::Platform;
use std::{
collections::HashSet,
path::{Component, Path, PathBuf},
};

use super::PackagingError;

/// Copy `src` to `dst`, normalizing line endings to LF for UTF-8 text files.
///
/// `noarch` packages must be reproducible regardless of the operating system they are
/// built on. Text files that are checked out or generated on Windows often carry CRLF
/// (or bare CR) line endings, which would otherwise leak into the package and make the
/// resulting archive differ from one built on Unix (see
/// <https://github.com/prefix-dev/rattler-build/issues/837>).
///
/// To keep the transformation safe (and cheap) we only rewrite UTF-8 text files that
/// actually contain a carriage return. Everything else is streamed straight through
/// `fs::copy` without being read into memory:
///
/// * Binary files and other text encodings (e.g. UTF-16/UTF-32, where a naive byte-level
/// CRLF rewrite could corrupt multi-byte code units such as `U+0A0D`) are classified
/// from the first kilobyte and copied verbatim.
/// * UTF-8 files that already use LF exclusively are copied verbatim too, which preserves
/// the fast copy path and the original file metadata.
fn copy_normalizing_line_endings(src: &Path, dst: &Path) -> Result<(), PackagingError> {
use std::io::Read;

// Classify the file from its first kilobyte so that large binary files are never
// read fully into memory just to be copied back out unchanged.
let mut file = fs::File::open(src)?;
let mut buffer = vec![0u8; 1024];
let read = file.read(&mut buffer)?;
buffer.truncate(read);
let content_type = content_inspector::inspect(&buffer);

// Only genuine UTF-8 (with or without BOM) is a candidate for normalization.
if matches!(
content_type,
content_inspector::ContentType::UTF_8 | content_inspector::ContentType::UTF_8_BOM
) {
// Read the rest of the file, reusing the bytes we already inspected.
file.read_to_end(&mut buffer)?;

// Only touch files that actually contain a carriage return, and only if the
// whole file is valid UTF-8 (the leading kilobyte can be misleading).
if memchr::memchr(b'\r', &buffer).is_some()
&& let Ok(text) = std::str::from_utf8(&buffer)
{
fs::write(dst, LineEnding::normalize(text).as_bytes())?;
// Preserve the permissions (e.g. the executable bit) that `fs::copy`
// would otherwise carry over.
fs::set_permissions(dst, fs::metadata(src)?.permissions())?;
return Ok(());
}
}

// Binary, non-UTF-8, or already LF-only files are copied unchanged.
Comment on lines +50 to +63

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think once the buffer has the full file from src, we can directly copy from buffer to dst instead of falling through to fs::copy(src, dst). What do you say?

Side note - I tried the following change, cargo test -p rattler_build_core --lib packaging::file_mapper and it passed the tests.

Suggested change
// Only touch files that actually contain a carriage return, and only if the
// whole file is valid UTF-8 (the leading kilobyte can be misleading).
if memchr::memchr(b'\r', &buffer).is_some()
&& let Ok(text) = std::str::from_utf8(&buffer)
{
fs::write(dst, LineEnding::normalize(text).as_bytes())?;
// Preserve the permissions (e.g. the executable bit) that `fs::copy`
// would otherwise carry over.
fs::set_permissions(dst, fs::metadata(src)?.permissions())?;
return Ok(());
}
}
// Binary, non-UTF-8, or already LF-only files are copied unchanged.
// Only trust the classification once the whole file is confirmed valid
// UTF-8 (the leading kilobyte can be misleading). We've already paid the
// cost of reading the whole file into memory at this point, so write it
// back out directly instead of re-reading/re-copying it from disk via
// `fs::copy`, whether or not it actually needed normalizing.
if let Ok(text) = std::str::from_utf8(&buffer) {
if memchr::memchr(b'\r', &buffer).is_some() {
fs::write(dst, LineEnding::normalize(text).as_bytes())?;
} else {
fs::write(dst, &buffer)?;
}
// Preserve the permissions (e.g. the executable bit) that `fs::copy`
// would otherwise carry over.
fs::set_permissions(dst, fs::metadata(src)?.permissions())?;
return Ok(());
}
}
// Binary or non-UTF-8 files are copied unchanged.

fs::copy(src, dst)?;
Ok(())
}

/// We check the (new) `pyc` files against the old files from the environment.
/// This is a temporary measure to avoid packaging `pyc` files that are not
/// generated by the build process.
Expand Down Expand Up @@ -92,6 +148,8 @@ impl Output {
/// `Scripts` is replaced with `python-scripts` (on Windows only). All other files are included
/// as-is.
/// * Absolute symlinks are made relative so that they are easily relocatable.
/// * For any `noarch` package (generic or python), UTF-8 text file line endings are
/// normalized to LF so that packages are reproducible across Windows and Unix builds.
pub fn write_to_dest(
&self,
path: &Path,
Expand Down Expand Up @@ -274,7 +332,13 @@ impl Output {
Ok(None)
} else {
tracing::trace!("Copying file {:?} to {:?}", path, dest_path);
fs::copy(path, &dest_path)?;
if target_platform == &Platform::NoArch {
// Normalize text-file line endings to LF for noarch packages so that
// builds are reproducible across Windows and Unix (issue #837).
copy_normalizing_line_endings(path, &dest_path)?;
} else {
fs::copy(path, &dest_path)?;
}
Ok(Some(dest_path))
}
}
Expand All @@ -287,8 +351,67 @@ mod test {
path::{Path, PathBuf},
};

use fs_err as fs;

use crate::packaging::file_mapper::filter_pyc;

#[test]
fn test_copy_normalizing_line_endings() {
let temp_dir = tempfile::tempdir().unwrap();

// (name, input, expected output) — all line endings collapse to LF.
let cases: &[(&str, &[u8], &[u8])] = &[
("crlf.txt", b"line1\r\nline2\r\n", b"line1\nline2\n"),
("cr.txt", b"line1\rline2\r", b"line1\nline2\n"),
("lf.txt", b"line1\nline2\n", b"line1\nline2\n"),
("mixed.txt", b"a\r\nb\rc\n", b"a\nb\nc\n"),
];
for (name, input, expected) in cases {
let src = temp_dir.path().join(name);
let dst = temp_dir.path().join(format!("out_{name}"));
fs::write(&src, input).unwrap();
super::copy_normalizing_line_endings(&src, &dst).unwrap();
assert_eq!(&fs::read(&dst).unwrap(), expected, "text case {name}");
}

// Binary data (contains a NUL and a CR/LF byte pair) must be copied verbatim.
let binary: &[u8] = &[0x00, 0x01, 0x02, 0x0D, 0x0A, 0xFF];
let src = temp_dir.path().join("data.bin");
let dst = temp_dir.path().join("data_out.bin");
fs::write(&src, binary).unwrap();
super::copy_normalizing_line_endings(&src, &dst).unwrap();
assert_eq!(fs::read(&dst).unwrap(), binary, "binary preserved");

// UTF-16LE (BOM `FF FE`): `U+0A0D` encodes as bytes `0D 0A`. A naive byte-level
// CRLF rewrite would corrupt it, so non-UTF-8 encodings are copied verbatim.
let utf16: &[u8] = &[0xFF, 0xFE, 0x0D, 0x0A, 0x41, 0x00];
let src = temp_dir.path().join("u16.txt");
let dst = temp_dir.path().join("u16_out.txt");
fs::write(&src, utf16).unwrap();
super::copy_normalizing_line_endings(&src, &dst).unwrap();
assert_eq!(fs::read(&dst).unwrap(), utf16, "utf-16 preserved");
}

#[test]
#[cfg(unix)]
fn test_copy_normalizing_line_endings_preserves_mode() {
use std::os::unix::fs::PermissionsExt;

let temp_dir = tempfile::tempdir().unwrap();
let src = temp_dir.path().join("script.sh");
fs::write(&src, b"#!/bin/sh\r\necho hi\r\n").unwrap();
fs::set_permissions(&src, std::fs::Permissions::from_mode(0o755)).unwrap();

let dst = temp_dir.path().join("script_out.sh");
super::copy_normalizing_line_endings(&src, &dst).unwrap();

// The file went through the rewrite path (CRLF was normalized) ...
assert_eq!(fs::read(&dst).unwrap(), b"#!/bin/sh\necho hi\n");
// ... and the executable bit was preserved.
let mode = fs::metadata(&dst).unwrap().permissions().mode();
assert_eq!(mode & 0o777, 0o755);
}

#[test]
fn test_filter_file() {
let test_cases = vec![
Expand Down