Repository: prefix-dev/rattler-build (rust)

Issue:
Regression in applying patches in 0.42.0
0.42.0 has introduced a regression in applying patches on linux. Rattler-build indicates that patches were applied but the patches were not actually applied at all. This behavior only happens on linux. The patches are successfully applied on both macos and windows.

I have two instances were this behavior can be observed.

1. https://github.com/conda-forge/staged-recipes/pull/29892 . This PR adds 5 patches. The build fails on linux because the patches are not applied.
2. https://github.com/conda-forge/libmodplug-feedstock . In this feedstock a patch is applied to expose symbols for a dynamic library. This doesnt break compilation but does silently ignore the change in modification.

The second case is especially bad. It means that patches that modify logic and don't fix compiler errors are silently ignored! 

Rattler-build 0.40.0 and 0.41.0 dont have this problem. Case number 2 can be build using both older and newer versions. Running `readelf -Ws --dyn-syms <path to libmodplug.so>` should show 600+ symbols but when the package is build with 0.42.0 it only shows 40.







Pull request:
fix: `git apply` inside a subdirectory of a git repository
This PR should fix #1670 .

### Description

The problem seemed to be that (apparently) some versions of `git` use the root directory of the repository as the root of the files described in the patch. If the working directory is a subdirectory of some git repository, running `git apply` will silently fail with `Skipping patch <path in patch>` (if run with `--verbose`).

I could obverse this behavior by running rattler-build with some patches. Wait for failure because the patches arent applied and then.

1) manually try applying the patch and observing git ignoring it.
2) initialize a git repository inside the working directory with `git init`.
3) retry applying the patch, this time it works as expected.

I could only observe this behavior on ubuntu using:

```
> git --version
git version 2.34.1
```

I did not see this behavior on Windows.

### Implementation

I did not find a combination of flags for `git apply` that could fix this issue, so instead I implemented a silly workaround: simply initialize a temporary git repository in the working directory. My implementation doesn't actually invoke `git init` but adds just the files and directories needed for `git` to think it is a git repository.

### Tests

I added a unit test that initializes a git repository in a temporary directory and then tries to apply a patch inside a sub directory. As expected, this fails with the code on `main`. With the changes from this PR, this succeeds.

I also tested this on the real-world example of libmodplug. With this PR the patches are successfully applied where previously they were silently ignored.

### Extras

The code now reads the output of the `git apply` run, regardless of whether the program returned a non-zero exit code. If it finds a line with `Skipped patch`, it raises an error and outputs the result. Patches are _explicitly_ added in the recipe, I do not think we should be silently skipping them even if it's for a good reason!

I changed the behavior of `--recount` slightly. The patches in `libmodplug` require it to be gone while the patch in this repository require it to be there. I opted to just try to run `git apply` with a combination of flags until we find one that works. `git apply` is atomic, so if it fails, no changes were made.

I also improved the error messages generated by the patch function in general:

```
Error:   × Failed to apply patch: 0001-fix-non-const-stack-alloc.patch
  │ `git apply` failed. The output of the command is:
  │
  │     Checking patch src/load_abc.cpp...
  │     error: while searching for:
  │                             }
  │                             *p = '\0';
  │                             abc_substitute(h, t, s);
  │                     }
  │             }
  │             else
  │     -
  │
  │     error: patch failed: src/load_abc.cpp:2272
  │     error: src/load_abc.cpp: patch does not apply
  │
  │ The command was invoked with:
  │
  │     /usr/bin/git apply -p1 --verbose --ignore-space-change --ignore-whitespace --recount /home/zalms/Developer/
  │ libmodplug-feedstock/recipe/0001-fix-non-const-stack-alloc.patch
```

### See also

https://stackoverflow.com/a/27283285

Patch:
diff --git a/.github/actionlint.yaml b/.github/actionlint.yaml
new file mode 100644
index 00000000..9b8f65a7
--- /dev/null
+++ b/.github/actionlint.yaml
@@ -0,0 +1,5 @@
+paths:
+  .github/workflows/rust.yml:
+    # https://github.com/rhysd/actionlint/pull/542
+    ignore:
+      - 'label "windows-11-arm" is unknown.+'
diff --git a/.github/workflows/enforce-sha.yaml b/.github/workflows/enforce-sha.yaml
index fe2abe5d..2f5bdf8e 100644
--- a/.github/workflows/enforce-sha.yaml
+++ b/.github/workflows/enforce-sha.yaml
@@ -12,7 +12,7 @@ jobs:
       - name: Checkout code
         uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4
       - name: Ensure SHA pinned actions
-        uses: zgosalvez/github-actions-ensure-sha-pinned-actions@2d6823da4039243036c86d76f503c84e2ded2517 # v3
+        uses: zgosalvez/github-actions-ensure-sha-pinned-actions@fc87bb5b5a97953d987372e74478de634726b3e5 # v3
         with:
           allowlist: |
             prefix-dev/
diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
index ca82fc6c..89536a97 100644
--- a/.github/workflows/rust.yml
+++ b/.github/workflows/rust.yml
@@ -5,6 +5,7 @@ on:
       - "v*.*.*"
   pull_request:
     paths:
+      - ".github/workflows/rust.yml"
       - "src/**"
       - "Cargo.*"
   workflow_dispatch:
@@ -66,6 +67,8 @@ jobs:
             os: windows-2022
           - target: x86_64-pc-windows-msvc
             os: windows-2022
+          - target: aarch64-pc-windows-msvc
+            os: windows-11-arm
     env:
       # These are some environment variables that configure the build so that the binary size is reduced.
       # Inspiration was taken from this blog: https://arusahni.net/blog/2020/03/optimizing-rust-binary-size.html
@@ -112,10 +115,10 @@ jobs:
           fi
 
       - name: Install Rust toolchain
-        uses: dtolnay/rust-toolchain@888c2e1ea69ab0d4330cbf0af1ecc7b68f368cc1 # master
+        uses: actions-rust-lang/setup-rust-toolchain@9d7e65c320fdb52dcd45ffaa68deb6c02c8754d9 # v1
         with:
           toolchain: "1.86.0"
-          targets: ${{ matrix.target }}
+          target: ${{ matrix.target }}
 
       - name: Install musl-gcc
         if: matrix.target == 'x86_64-unknown-linux-musl'
@@ -125,11 +128,9 @@ jobs:
         if: matrix.target == 'powerpc64le-unknown-linux-gnu'
         run: sudo apt install libssl-dev
 
-      - uses: Swatinem/rust-cache@9d47c6ad4b02e050fd481d890b2ea34778fd09d6 # v2
-
       - name: Install cross
         if: matrix.use-cross
-        uses: taiki-e/install-action@83254c543806f3224380bf1001d6fac8feaf2d0b # v2
+        uses: taiki-e/install-action@941e8a4d9d7cdb696bd4f017cf54aca281f8ffff # v2
         with:
           tool: cross
 
diff --git a/README.md b/README.md
index 318f3c73..a9432ee0 100644
--- a/README.md
+++ b/README.md
@@ -86,8 +86,7 @@ self-contained.
   libraries and executables to make it relative
 * `patchelf` is required on Linux to rewrite the `rpath` and `runpath` of shared
   libraries and executables
-* `git` to checkout Git repositories (not implemented yet, but will require `git`
-  in the future)
+* `git` is required to checkout Git repositories.
 * `msvc` on Windows because we cannot ship the MSVC compiler on conda-forge
   (needs to be installed on the host machine)
 
diff --git a/docs/reference/jinja.md b/docs/reference/jinja.md
index 571212c9..8d9d48ae 100644
--- a/docs/reference/jinja.md
+++ b/docs/reference/jinja.md
@@ -199,6 +199,28 @@ You can also check for the existence of an environment variable:
 - `${{ env.exists("MY_ENV_VAR") }}` will return `true` if the environment
   variable `MY_ENV_VAR` is set and `false` otherwise.
 
+## Tests
+
+You can write tests using minijinja to check whether objects have certain properties.
+The syntax for a filter is `{{ variable is test_name }}`.
+
+- `undefined`: Check whether a variable is undefined.
+- `defined`: Check whether a variable is defined.
+- `none`: Check whether a variable is none.
+- `safe`: Check whether a variable is safe.
+- `escaped`: Check whether a variable is escaped. Same as `is safe`.
+- `odd`: Check whether a number is odd.
+- `even`: Check whether a number is even.
+- `number`: Check whether a variable is a number.
+- `integer`: Check whether a variable is an integer.
+- `int`: Check whether a variable is an integer. Same as `is integer`.
+- `float`: Check whether a variable is a float.
+- `string`: Check whether a variable is a string.
+- `sequence`: Check whether a variable is a sequence.
+- `boolean`: Check whether a variable is a boolean.
+- `startingwith`: Check whether a variable is starting with another string: `{{ python is startingwith('3.12') }}`
+- `endingwith`: Check whether a variable is starting with another string: `{{ python is endingwith('.*') }}`
+
 ## Filters
 
 A feature of `jinja` is called "filters". Filters are functions that can be
diff --git a/src/recipe/jinja.rs b/src/recipe/jinja.rs
index 6b43c652..facef7a8 100644
--- a/src/recipe/jinja.rs
+++ b/src/recipe/jinja.rs
@@ -309,6 +309,8 @@ fn default_tests(env: &mut Environment) {
     env.add_test("string", minijinja::tests::is_string);
     env.add_test("sequence", minijinja::tests::is_sequence);
     env.add_test("boolean", minijinja::tests::is_boolean);
+    env.add_test("startingwith", minijinja::tests::is_startingwith);
+    env.add_test("endingwith", minijinja::tests::is_endingwith);
 
     // operators
     env.add_test("eq", minijinja::tests::is_eq);
diff --git a/src/source/copy_dir.rs b/src/source/copy_dir.rs
index 75cac602..9c302bb0 100644
--- a/src/source/copy_dir.rs
+++ b/src/source/copy_dir.rs
@@ -36,8 +36,8 @@ impl Default for CopyOptions {
 }
 
 /// Copy metadata from source to destination
-/// The call to `fs::copy` already copied the data and the permissions, but
-/// we need to copy the timestamps as well.
+/// `fs::copy` handles permissions, but it won't be called if the file is reflinked
+/// We need to deal with permissions and timestamps ourselves
 fn copy_metadata(from: &Path, to: &Path) -> std::io::Result<()> {
     let metadata = fs_err::metadata(from)?;
 
@@ -48,6 +48,7 @@ fn copy_metadata(from: &Path, to: &Path) -> std::io::Result<()> {
 
     let file = std::fs::OpenOptions::new().write(true).open(to)?;
     file.set_times(file_times)?;
+    file.set_permissions(metadata.permissions())?;
 
     Ok(())
 }
diff --git a/src/source/patch.rs b/src/source/patch.rs
index 51863378..1984e498 100644
--- a/src/source/patch.rs
+++ b/src/source/patch.rs
@@ -1,14 +1,16 @@
 //! Functions for applying patches to a work directory.
+use super::SourceError;
+use crate::system_tools::{SystemTools, Tool};
+use itertools::Itertools;
+use std::process::{Command, Output};
 use std::{
     collections::HashSet,
-    io::{BufRead, BufReader, Write},
+    ffi::OsStr,
+    io::{BufRead, BufReader},
     path::{Path, PathBuf},
     process::Stdio,
 };
 
-use super::SourceError;
-use crate::system_tools::{SystemTools, Tool};
-
 fn parse_patch_file<P: AsRef<Path>>(patch_file: P) -> std::io::Result<HashSet<PathBuf>> {
     let file = fs_err::File::open(patch_file.as_ref())?;
     let reader = BufReader::new(file);
@@ -115,6 +117,19 @@ pub(crate) fn apply_patches(
     work_dir: &Path,
     recipe_dir: &Path,
 ) -> Result<(), SourceError> {
+    // Early out to avoid unnecessary work
+    if patches.is_empty() {
+        return Ok(());
+    }
+
+    // Ensure that the working directory is a valid git directory.
+    let git_dir = work_dir.join(".git");
+    let _dot_git_dir = if !git_dir.exists() {
+        Some(TempDotGit::setup(work_dir)?)
+    } else {
+        None
+    };
+
     for patch_path_relative in patches {
         let patch_file_path = recipe_dir.join(patch_path_relative);
 
@@ -124,60 +139,137 @@ pub(crate) fn apply_patches(
             return Err(SourceError::PatchNotFound(patch_file_path));
         }
 
-        // Read the patch content into a string. This also normalizes line endings to LF.
-        let patch_content_for_stdin =
-            fs_err::read_to_string(&patch_file_path).map_err(SourceError::Io)?;
-
         let strip_level = guess_strip_level(&patch_file_path, work_dir)?;
 
-        let mut cmd_builder = system_tools
-            .call(Tool::Git)
-            .map_err(SourceError::GitNotFound)?;
-
-        cmd_builder
-            .current_dir(work_dir)
-            .arg("apply")
-            .arg(format!("-p{}", strip_level))
-            .arg("--ignore-space-change")
-            .arg("--ignore-whitespace")
-            .arg("--recount")
-            .stdin(Stdio::piped())
-            .stdout(Stdio::piped())
-            .stderr(Stdio::piped());
-
-        let mut child_process = cmd_builder.spawn().map_err(SourceError::Io)?;
-
-        // Write the patch content to the child process's stdin.
-        {
-            if let Some(mut child_stdin) = child_process.stdin.take() {
-                child_stdin
-                    .write_all(patch_content_for_stdin.as_bytes())
-                    .map_err(SourceError::Io)?;
-            } else {
-                return Err(SourceError::Io(std::io::Error::new(
-                    std::io::ErrorKind::Other,
-                    "Failed to obtain stdin handle for git apply",
-                )));
+        struct GitApplyAttempt {
+            command: Command,
+            output: Output,
+        }
+
+        let mut outputs = Vec::new();
+        for try_extra_flag in [None, Some("--recount")] {
+            let mut cmd_builder = system_tools
+                .call(Tool::Git)
+                .map_err(SourceError::GitNotFound)?;
+            cmd_builder
+                .current_dir(work_dir)
+                .arg("apply")
+                .arg(format!("-p{}", strip_level))
+                .arg("--verbose")
+                .arg("--ignore-space-change")
+                .arg("--ignore-whitespace")
+                .args(try_extra_flag.into_iter())
+                .arg(patch_file_path.as_os_str())
+                .stdout(Stdio::piped())
+                .stderr(Stdio::piped());
+
+            tracing::debug!(
+                "Running: {} {}",
+                cmd_builder.get_program().to_string_lossy(),
+                cmd_builder
+                    .get_args()
+                    .map(OsStr::to_string_lossy)
+                    .format(" ")
+            );
+
+            let output = cmd_builder.output().map_err(SourceError::Io)?;
+            outputs.push(GitApplyAttempt {
+                command: cmd_builder,
+                output: output.clone(),
+            });
+
+            if outputs
+                .last()
+                .expect("we just added an entry")
+                .output
+                .status
+                .success()
+            {
+                break;
             }
         }
 
-        let output = child_process.wait_with_output().map_err(SourceError::Io)?;
+        // Check if the last output was successful, if not, we report all the errors.
+        let last_output = outputs.last().expect("we just added at least one entry");
+        if !last_output.output.status.success() {
+            return Err(SourceError::PatchFailed(format!(
+                "{}\n`git apply` failed with a combination of flags.\n\n{}",
+                patch_path_relative.display(),
+                outputs
+                    .into_iter()
+                    .map(
+                        |GitApplyAttempt {
+                             output, command, ..
+                         }| {
+                            let stderr = String::from_utf8_lossy(&output.stderr);
+                            format!(
+                                "With the che command:\n\n\t{} {}The output was:\n\n\t{}\n\n",
+                                command.get_program().to_string_lossy(),
+                                command.get_args().map(OsStr::to_string_lossy).format(" "),
+                                stderr.lines().format("\n\t")
+                            )
+                        }
+                    )
+                    .format("\n\n")
+            )));
+        }
 
-        if !output.status.success() {
-            eprintln!(
-                "Failed to apply patch: {}",
-                patch_file_path.to_string_lossy()
-            );
-            eprintln!("Stdout: {}", String::from_utf8_lossy(&output.stdout));
-            eprintln!("Stderr: {}", String::from_utf8_lossy(&output.stderr));
-            return Err(SourceError::PatchFailed(
-                patch_file_path.to_string_lossy().to_string(),
-            ));
+        // Sometimes git apply will skip the contents of a patch. This usually is *not* what we
+        // want, so we detect this behavior and return an error.
+        let stderr = String::from_utf8_lossy(&last_output.output.stderr);
+        let skipped_patch = stderr
+            .lines()
+            .any(|line| line.starts_with("Skipped patch "));
+        if skipped_patch {
+            return Err(SourceError::PatchFailed(format!(
+                "{}\n`git apply` seems to have skipped some of the contents of the patch. The output of the command is:\n\n\t{}\n\nThe command was invoked with:\n\n\t{} {}",
+                patch_path_relative.display(),
+                stderr.lines().format("\n\t"),
+                last_output.command.get_program().to_string_lossy(),
+                last_output
+                    .command
+                    .get_args()
+                    .map(OsStr::to_string_lossy)
+                    .format(" ")
+            )));
         }
     }
     Ok(())
 }
 
+/// A temporary .git directory that contains the bare minimum files and
+/// directories needed for git to function as if the directory that contains
+/// the .git directory is a proper git repository.
+struct TempDotGit {
+    path: PathBuf,
+}
+
+impl TempDotGit {
+    /// Creates a temporary .git directory in the specified root directory.
+    fn setup(root: &Path) -> std::io::Result<Self> {
+        // Initialize a temporary .git directory
+        let dot_git = root.join(".git");
+        fs_err::create_dir(&dot_git)?;
+        let dot_git = TempDotGit { path: dot_git };
+
+        // Add the minimum number of files and directories to the .git directory that are needed for
+        // git to work
+        fs_err::create_dir(dot_git.path.join("objects"))?;
+        fs_err::create_dir(dot_git.path.join("refs"))?;
+        fs_err::write(dot_git.path.join("HEAD"), "ref: refs/heads/main")?;
+
+        Ok(dot_git)
+    }
+}
+
+impl Drop for TempDotGit {
+    fn drop(&mut self) {
+        fs_err::remove_dir_all(&self.path).unwrap_or_else(|e| {
+            eprintln!("Failed to remove temporary .git directory: {}", e);
+        });
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use crate::source::copy_dir::CopyDir;
@@ -294,4 +386,27 @@ mod tests {
         let cmake_list = fs_err::read_to_string(&cmake_list).unwrap();
         assert!(cmake_list.contains("cmake_minimum_required(VERSION 3.12)"));
     }
+
+    #[test]
+    fn test_apply_git_patch_in_git_ignored() {
+        let (tempdir, _) = setup_patch_test_dir();
+
+        // Initialize a temporary .git directory at the root of the temporary directory. This makes
+        // git take the working directory is in a git repository.
+        let _temp_dot_git = TempDotGit::setup(tempdir.path()).unwrap();
+
+        // Apply the patches in the working directory
+        apply_patches(
+            &SystemTools::new(),
+            &[PathBuf::from("0001-increase-minimum-cmake-version.patch")],
+            &tempdir.path().join("workdir"),
+            &tempdir.path().join("patches"),
+        )
+        .expect("Patch 0001-increase-minimum-cmake-version.patch should apply successfully");
+
+        // Read the cmake list file and make sure that it contains `cmake_minimum_required(VERSION 3.12)`
+        let cmake_list = tempdir.path().join("workdir/CMakeLists.txt");
+        let cmake_list = fs_err::read_to_string(&cmake_list).unwrap();
+        assert!(cmake_list.contains("cmake_minimum_required(VERSION 3.12)"));
+    }
 }


Test patch:
diff --git a/test-data/recipes/jinja-types/recipe.yaml b/test-data/recipes/jinja-types/recipe.yaml
index 3350f24c..d36192ed 100644
--- a/test-data/recipes/jinja-types/recipe.yaml
+++ b/test-data/recipes/jinja-types/recipe.yaml
@@ -32,6 +32,10 @@ context:
   # quoted bool is quoted in variants.yaml and thus should be a string
   is_quoted_true_var: ${{ quoted_bool is string }}
   is_quoted_int_var: ${{ quoted_int is string }}
+  starting_with: ${{ xstring is startingwith('blah') }}
+  not_starting_with: ${{ xstring is startingwith('foo') }}
+  ending_with: ${{ xstring is startingwith('blah') }}
+  not_ending_with: ${{ xstring is startingwith('foo') }}
 
 package:
   name: testtypes
diff --git a/test/end-to-end/__snapshots__/test_simple/test_jinja_types.json b/test/end-to-end/__snapshots__/test_simple/test_jinja_types.json
index 4f085bd2..a7888756 100644
--- a/test/end-to-end/__snapshots__/test_simple/test_jinja_types.json
+++ b/test/end-to-end/__snapshots__/test_simple/test_jinja_types.json
@@ -7,6 +7,7 @@
   "cquoted": "5",
   "cquoted_bool": "true",
   "cstring": "blah",
+  "ending_with": true,
   "float": "1.23",
   "inline_list": [
     "a",
@@ -35,5 +36,8 @@
     "b",
     "c"
   ],
+  "not_ending_with": false,
+  "not_starting_with": false,
+  "starting_with": true,
   "string": "blah"
 }