fix: replace absolute symlinks from cache with correct version (#993)

src/cache.rs

@@ -16,7 +16,7 @@ use crate::{
     packaging::{contains_prefix_binary, contains_prefix_text, content_type, Files},
     recipe::parser::{Dependency, Requirements},
     render::resolved_dependencies::{resolve_dependencies, FinalizedDependencies},
-    source::copy_dir::{copy_file, CopyOptions},
+    source::copy_dir::{copy_file, create_symlink, CopyOptions},
 };
 
 /// Error type for cache key generation
@@ -113,6 +113,17 @@ impl Output {
             let source = &cache_dir.join("prefix").join(file);
             copy_file(source, &dest, &mut paths_created, &copy_options).into_diagnostic()?;
 
+            // check if the symlink starts with the old prefix, and if yes, make the symlink absolute
+            // with the new prefix
+            if source.is_symlink() {
+                let symlink_target = fs::read_link(source).into_diagnostic()?;
+                if let Ok(rest) = symlink_target.strip_prefix(&cache_prefix) {
+                    let new_symlink_target = self.prefix().join(rest);
+                    fs::remove_file(&dest).into_diagnostic()?;
+                    create_symlink(&new_symlink_target, &dest).into_diagnostic()?;
+                }
+            }
+
             if *has_prefix {
                 replace_prefix(&dest, &cache_prefix, self.prefix())?;
             }
@@ -199,15 +210,20 @@ impl Output {
                 let dest = &prefix_cache_dir.join(stripped);
                 copy_file(file, dest, &mut creation_cache, &copy_options).into_diagnostic()?;
 
-                // check if the file contains the prefix
-                let content_type = content_type(file).into_diagnostic()?;
-                let has_prefix = if content_type.map(|c| c.is_text()).unwrap_or(false) {
-                    contains_prefix_text(file, self.prefix(), self.target_platform())
+                // Defend against broken symlinks here!
+                if !file.is_symlink() {
+                    // check if the file contains the prefix
+                    let content_type = content_type(file).into_diagnostic()?;
+                    let has_prefix = if content_type.map(|c| c.is_text()).unwrap_or(false) {
+                        contains_prefix_text(file, self.prefix(), self.target_platform())
+                    } else {
+                        contains_prefix_binary(file, self.prefix())
+                    }
+                    .into_diagnostic()?;
+                    copied_files.push((stripped.to_path_buf(), has_prefix));
                 } else {
-                    contains_prefix_binary(file, self.prefix())
+                    copied_files.push((stripped.to_path_buf(), false));
                 }
-                .into_diagnostic()?;
-                copied_files.push((stripped.to_path_buf(), has_prefix));
             }
 
             // save the cache

src/packaging/metadata.rs

@@ -351,15 +351,43 @@ impl Output {
 
             if !p.exists() {
                 if p.is_symlink() {
-                    tracing::warn!(
-                        "Symlink target does not exist: {:?} -> {:?}",
-                        &p,
-                        fs::read_link(p)?
-                    );
+                    // check if the file is in the prefix
+                    if let Ok(link_target) = p.read_link() {
+                        if link_target.is_relative() {
+                            let Some(relative_path_parent) = relative_path.parent() else {
+                                tracing::warn!("could not get parent of symlink {:?}", &p);
+                                continue;
+                            };
+
+                            let resolved_path = temp_files
+                                .encoded_prefix
+                                .join(relative_path_parent)
+                                .join(&link_target);
+
+                            if !resolved_path.exists() {
+                                tracing::warn!(
+                                    "symlink target not part of this package: {:?} -> {:?}",
+                                    &p,
+                                    &link_target
+                                );
+
+                                // Think about continuing here or packaging broken symlinks
+                                continue;
+                            }
+                        } else {
+                            tracing::warn!(
+                                "packaging an absolute symlink to outside the prefix {:?} -> {:?}",
+                                &p,
+                                link_target
+                            );
+                        }
+                    } else {
+                        tracing::warn!("could not read symlink {:?}", &p);
+                    }
+                } else {
+                    tracing::warn!("file does not exist: {:?}", &p);
                     continue;
                 }
-                tracing::warn!("File does not exist: {:?} (TODO)", &p);
-                continue;
             }
 
             if meta.is_dir() {

src/source/copy_dir.rs

@@ -34,6 +34,34 @@ impl Default for CopyOptions {
     }
 }
 
+/// Cross platform way of creating a symlink
+/// Creates a symlink from `link` to `original`
+/// The file that is newly created is the `link` file
+pub(crate) fn create_symlink(
+    original: impl AsRef<Path>,
+    link: impl AsRef<Path>,
+) -> Result<(), SourceError> {
+    let original = original.as_ref();
+    let link = link.as_ref();
+
+    if link.exists() {
+        fs_err::remove_file(link)?;
+    }
+
+    #[cfg(unix)]
+    fs_err::os::unix::fs::symlink(original, link)?;
+    #[cfg(windows)]
+    {
+        if original.is_dir() {
+            std::os::windows::fs::symlink_dir(original, link)?;
+        } else {
+            std::os::windows::fs::symlink_file(original, link)?;
+        }
+    }
+
+    Ok(())
+}
+
 /// Copy a file or directory, or symlink to another location.
 /// Use reflink if possible.
 pub(crate) fn copy_file(
@@ -57,10 +85,7 @@ pub(crate) fn copy_file(
         // if file is a symlink, copy it as a symlink
         if path.is_symlink() {
             let link_target = fs_err::read_link(path)?;
-            #[cfg(unix)]
-            fs_err::os::unix::fs::symlink(link_target, dest_path)?;
-            #[cfg(windows)]
-            std::os::windows::fs::symlink_file(link_target, dest_path)?;
+            create_symlink(link_target, dest_path)?;
             Ok(())
         } else {
             if dest_path.exists() {
@@ -492,7 +517,7 @@ mod test {
             .use_gitignore(false)
             .run()
             .unwrap();
-        println!("{:?}", copy_dir.copied_paths());
+
         assert_eq!(copy_dir.copied_paths().len(), 2);
         let expected = [
             dest_dir_3.join("test_dir/test.md"),

test-data/recipes/cache/recipe-symlinks.yaml

@@ -0,0 +1,33 @@
+recipe:
+  name: cache-symlinks
+  version: 1.0.0
+
+cache:
+  build:
+    script: |
+      mkdir -p $PREFIX/bin
+      touch $PREFIX/bin/exe
+      ln -s $PREFIX/bin/exe $PREFIX/bin/exe-symlink
+      ln -s $PREFIX/bin/exe $PREFIX/bin/absolute-exe-symlink
+      touch $PREFIX/foo.txt
+      ln -s $PREFIX/foo.txt $PREFIX/foo-symlink.txt
+      ln -s $PREFIX/foo.txt $PREFIX/absolute-symlink.txt
+      ln -s $PREFIX/non-existent-file $PREFIX/broken-symlink.txt
+      ln -s ./foo.txt $PREFIX/relative-symlink.txt
+
+outputs:
+  - package:
+      name: cache-symlinks
+    build:
+      files:
+        include:
+          - "**/*"
+        exclude:
+          - "absolute-symlink.txt"
+          - "bin/absolute-exe-symlink"
+  - package:
+      name: absolute-cache-symlinks
+    build:
+      files:
+        - "absolute-symlink.txt"
+        - "bin/absolute-exe-symlink"

test/end-to-end/__snapshots__/test_symlinks/test_symlink_cache.json

@@ -0,0 +1,35 @@
+{
+  "paths": [
+    {
+      "_path": "bin/exe",
+      "path_type": "hardlink",
+      "sha256": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
+      "size_in_bytes": 0
+    },
+    {
+      "_path": "bin/exe-symlink",
+      "path_type": "softlink",
+      "sha256": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
+      "size_in_bytes": 3
+    },
+    {
+      "_path": "foo-symlink.txt",
+      "path_type": "softlink",
+      "sha256": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
+      "size_in_bytes": 7
+    },
+    {
+      "_path": "foo.txt",
+      "path_type": "hardlink",
+      "sha256": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
+      "size_in_bytes": 0
+    },
+    {
+      "_path": "relative-symlink.txt",
+      "path_type": "softlink",
+      "sha256": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
+      "size_in_bytes": 9
+    }
+  ],
+  "paths_version": 1
+}

test/end-to-end/conftest.py

@@ -0,0 +1,37 @@
+import os
+from pathlib import Path
+
+import pytest
+from helpers import RattlerBuild
+from syrupy.extensions.json import JSONSnapshotExtension
+
+
+@pytest.fixture
+def rattler_build():
+    if os.environ.get("RATTLER_BUILD_PATH"):
+        return RattlerBuild(os.environ["RATTLER_BUILD_PATH"])
+    else:
+        base_path = Path(__file__).parent.parent.parent
+        executable_name = "rattler-build"
+        if os.name == "nt":
+            executable_name += ".exe"
+
+        release_path = base_path / f"target/release/{executable_name}"
+        debug_path = base_path / f"target/debug/{executable_name}"
+
+        if release_path.exists():
+            return RattlerBuild(release_path)
+        elif debug_path.exists():
+            return RattlerBuild(debug_path)
+
+    raise FileNotFoundError("Could not find rattler-build executable")
+
+
+@pytest.fixture
+def snapshot_json(snapshot):
+    return snapshot.use_extension(JSONSnapshotExtension)
+
+
+@pytest.fixture
+def recipes():
+    return Path(__file__).parent.parent.parent / "test-data" / "recipes"

test/end-to-end/helpers.py

@@ -0,0 +1,82 @@
+from pathlib import Path
+from subprocess import STDOUT, CalledProcessError, check_output
+from typing import Any, Optional
+
+from conda_package_handling.api import extract
+
+
+class RattlerBuild:
+    def __init__(self, path):
+        self.path = path
+
+    def __call__(self, *args: Any, **kwds: Any) -> Any:
+        try:
+            return check_output([str(self.path), *args], **kwds).decode("utf-8")
+        except CalledProcessError as e:
+            print(e.output)
+            print(e.stderr)
+            raise e
+
+    def build_args(
+        self,
+        recipe_folder: Path,
+        output_folder: Path,
+        variant_config: Optional[Path] = None,
+        custom_channels: Optional[list[str]] = None,
+        extra_args: list[str] = None,
+    ):
+        if extra_args is None:
+            extra_args = []
+        args = ["build", "--recipe", str(recipe_folder), *extra_args]
+        if variant_config is not None:
+            args += ["--variant-config", str(variant_config)]
+        args += ["--output-dir", str(output_folder)]
+        args += ["--package-format", str("tar.bz2")]
+
+        if custom_channels:
+            for c in custom_channels:
+                args += ["--channel", c]
+
+        return args
+
+    def build(
+        self,
+        recipe_folder: Path,
+        output_folder: Path,
+        variant_config: Optional[Path] = None,
+        custom_channels: Optional[list[str]] = None,
+        extra_args: list[str] = None,
+    ):
+        args = self.build_args(
+            recipe_folder,
+            output_folder,
+            variant_config=variant_config,
+            custom_channels=custom_channels,
+            extra_args=extra_args,
+        )
+        return self(*args)
+
+    def test(self, package, *args: Any, **kwds: Any) -> Any:
+        return self("test", "--package-file", package, *args, stderr=STDOUT, **kwds)
+
+
+def get_package(folder: Path, glob="*.tar.bz2"):
+    if "tar.bz2" not in glob:
+        glob += "*.tar.bz2"
+    if "/" not in glob:
+        glob = "**/" + glob
+    package_path = next(folder.glob(glob))
+    return package_path
+
+
+def get_extracted_package(folder: Path, glob="*.tar.bz2"):
+    package_path = get_package(folder, glob)
+
+    if package_path.name.endswith(".tar.bz2"):
+        package_without_extension = package_path.name[: -len(".tar.bz2")]
+    elif package_path.name.endswith(".conda"):
+        package_without_extension = package_path.name[: -len(".conda")]
+
+    extract_path = folder / "extract" / package_without_extension
+    extract(str(package_path), dest_dir=str(extract_path))
+    return extract_path