diff --git a/src/cache.rs b/src/cache.rs index e8b35dba8..20b1ab86d 100644 --- a/src/cache.rs +++ b/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, ©_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, ©_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 diff --git a/src/packaging/metadata.rs b/src/packaging/metadata.rs index e9e4f490e..736244983 100644 --- a/src/packaging/metadata.rs +++ b/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() { diff --git a/src/source/copy_dir.rs b/src/source/copy_dir.rs index 7e2ecf29b..dc8df02df 100644 --- a/src/source/copy_dir.rs +++ b/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, + link: impl AsRef, +) -> 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"), diff --git a/test-data/recipes/cache/recipe-symlinks.yaml b/test-data/recipes/cache/recipe-symlinks.yaml new file mode 100644 index 000000000..8c9ec28fc --- /dev/null +++ b/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" diff --git a/test/end-to-end/__snapshots__/test_symlinks/test_symlink_cache.json b/test/end-to-end/__snapshots__/test_symlinks/test_symlink_cache.json new file mode 100644 index 000000000..d2d735b8a --- /dev/null +++ b/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 +} diff --git a/test/end-to-end/conftest.py b/test/end-to-end/conftest.py new file mode 100644 index 000000000..bce17b8ea --- /dev/null +++ b/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" diff --git a/test/end-to-end/helpers.py b/test/end-to-end/helpers.py new file mode 100644 index 000000000..88d19c111 --- /dev/null +++ b/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 diff --git a/test/end-to-end/test_simple.py b/test/end-to-end/test_simple.py index 7ec9bfa43..df0bef906 100644 --- a/test/end-to-end/test_simple.py +++ b/test/end-to-end/test_simple.py @@ -4,94 +4,11 @@ import platform from pathlib import Path from subprocess import DEVNULL, STDOUT, CalledProcessError, check_output -from typing import Any, Optional import pytest import requests import yaml -from conda_package_handling.api import extract -from syrupy.extensions.json import JSONSnapshotExtension - - -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) - - -@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) +from helpers import RattlerBuild, get_extracted_package, get_package def test_functionality(rattler_build: RattlerBuild): @@ -100,33 +17,6 @@ def test_functionality(rattler_build: RattlerBuild): assert text[0] == f"Usage: rattler-build{suffix} [OPTIONS] [COMMAND]" -@pytest.fixture -def recipes(): - return Path(__file__).parent.parent.parent / "test-data" / "recipes" - - -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 - - def test_license_glob(rattler_build: RattlerBuild, recipes: Path, tmp_path: Path): rattler_build.build(recipes / "globtest", tmp_path) pkg = get_extracted_package(tmp_path, "globtest") diff --git a/test/end-to-end/test_symlinks.py b/test/end-to-end/test_symlinks.py new file mode 100644 index 000000000..e269a37ff --- /dev/null +++ b/test/end-to-end/test_symlinks.py @@ -0,0 +1,63 @@ +import json +import os +from pathlib import Path + +import pytest +from helpers import RattlerBuild, get_extracted_package + + +@pytest.mark.skipif( + os.name == "nt", reason="recipe does not support execution on windows" +) +def test_symlink_cache( + rattler_build: RattlerBuild, recipes: Path, tmp_path: Path, snapshot_json +): + rattler_build.build( + recipes / "cache/recipe-symlinks.yaml", tmp_path, extra_args=["--experimental"] + ) + + pkg = get_extracted_package(tmp_path, "absolute-cache-symlinks") + assert pkg.exists() + link_file = pkg / "absolute-symlink.txt" + assert link_file.is_symlink() + # assure that this is a relative link + assert link_file.readlink() == Path("foo.txt") + + link_target = link_file.resolve() + assert link_target == (pkg / "foo.txt") + + link_file = pkg / "bin/absolute-exe-symlink" + assert link_file.is_symlink() + assert link_file.readlink() == Path("exe") + + pkg = get_extracted_package(tmp_path, "cache-symlinks") + + paths_json = pkg / "info/paths.json" + j = json.loads(paths_json.read_text()) + assert snapshot_json == j + + paths = j["paths"] + assert len(paths) == 5 + for p in paths: + if "symlink" in p["_path"]: + assert p["path_type"] == "softlink" + assert ( + p["sha256"] + == "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + ) + + foo_symlink = pkg / "foo-symlink.txt" + assert foo_symlink.exists() + assert foo_symlink.is_symlink() + assert not foo_symlink.readlink().is_absolute() + + broken_symlink = pkg / "broken-symlink.txt" + assert not broken_symlink.exists() + + relative_symlink = pkg / "relative-symlink.txt" + assert relative_symlink.is_symlink() + assert relative_symlink.readlink() == Path("foo.txt") + + relative_symlink = pkg / "bin/exe-symlink" + assert relative_symlink.is_symlink() + assert relative_symlink.readlink() == Path("exe")