Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 9 commits into from
Aug 1, 2024
Merged
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
32 changes: 24 additions & 8 deletions src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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())?;
}
Expand Down Expand Up @@ -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
Expand Down
42 changes: 35 additions & 7 deletions src/packaging/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
35 changes: 30 additions & 5 deletions src/source/copy_dir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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() {
Expand Down Expand Up @@ -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"),
Expand Down
33 changes: 33 additions & 0 deletions test-data/recipes/cache/recipe-symlinks.yaml
Original file line number Diff line number Diff line change
@@ -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"
Original file line number Diff line number Diff line change
@@ -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
}
37 changes: 37 additions & 0 deletions test/end-to-end/conftest.py
Original file line number Diff line number Diff line change
@@ -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"
82 changes: 82 additions & 0 deletions test/end-to-end/helpers.py
Original file line number Diff line number Diff line change
@@ -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
Loading
Loading