Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
10 changes: 1 addition & 9 deletions crates/pixi_cli/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ use pixi_record::{PinnedPathSpec, PinnedSourceSpec};
use pixi_reporters::TopLevelProgress;
use pixi_utils::variants::VariantConfig;
use rattler_conda_types::{GenericVirtualPackage, Platform};
use tempfile::tempdir;

use crate::cli_config::{LockAndInstallConfig, WorkspaceConfig};

Expand Down Expand Up @@ -174,18 +173,12 @@ pub async fn execute(args: Args) -> miette::Result<()> {
)
})?;

// Create a temporary directory to hold build outputs
let temp_output_dir = tempdir()
.into_diagnostic()
.context("failed to create temporary output directory for build artifacts")?;

// Build the individual packages
for package in packages {
let built_package = command_dispatcher
.source_build(SourceBuildSpec {
package,
// Build into a temporary directory first
output_directory: Some(temp_output_dir.path().to_path_buf()),
output_directory: None,
manifest_source: manifest_source.clone(),
build_source: None,
channels: channels.clone(),
Expand Down Expand Up @@ -217,7 +210,6 @@ pub async fn execute(args: Args) -> miette::Result<()> {
// If a simple rename fails (e.g., across filesystems), fall back to copy+remove.
if let Err(_e) = fs_err::rename(&package_path, &dest_path) {
fs_err::copy(&package_path, &dest_path).into_diagnostic()?;
fs_err::remove_file(&package_path).into_diagnostic()?;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't need to remove cached artefact

}

// Print success relative to the user-requested output directory
Expand Down
4 changes: 3 additions & 1 deletion crates/pixi_cli/src/global/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,9 @@ async fn setup_environment(
}

// Installing the environment to be able to find the bin paths later
let environment_update = project.install_environment(env_name).await?;
let environment_update = project
.install_environment_with_options(env_name, args.force_reinstall)
.await?;

// Sync exposed name
sync_exposed_names(env_name, project, args).await?;
Expand Down
1 change: 1 addition & 0 deletions crates/pixi_command_dispatcher/src/install_pixi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ impl InstallPixiEnvironmentSpec {
let force = self
.force_reinstall
.contains(&source_record.package_record.name);

let built_source = command_dispatcher
.source_build(SourceBuildSpec {
manifest_source: source_record.manifest_source.clone(),
Expand Down
8 changes: 7 additions & 1 deletion crates/pixi_command_dispatcher/src/source_build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ impl SourceBuildSpec {
output = %cached_build.record.file_name,
"using cached new source build",
);
// dont matter if we forceit , we can reuse the cache entry
// dont matter if we force it , we can reuse the cache entry
return Ok(SourceBuildResult {
output_file: build_cache.cache_dir.join(&cached_build.record.file_name),
record: cached_build.record.clone(),
Expand Down Expand Up @@ -376,6 +376,12 @@ impl SourceBuildSpec {
.file_name()
.expect("the build backend did not return a file name");
let destination = output_directory.join(file_name);
tracing::debug!(
source = %source_for_logging,
from = %output_file.display(),
to = %destination.display(),
"moving built source package to output directory",
);
if let Err(err) = move_file(&output_file, &destination) {
return Err(CommandDispatcherError::Failed(SourceBuildError::Move(
output_file,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,6 @@ impl SourceBuildCacheStatusSpec {
return Ok(CachedBuildStatus::Stale(cached_build));
}
}

Ok(CachedBuildStatus::UpToDate(cached_build))
}
}
Expand Down
6 changes: 6 additions & 0 deletions crates/pixi_glob/src/glob_set/walk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,12 @@ pub fn walk_globs(
.map(|g| anchor_literal_pattern(g.to_pattern()))
.collect_vec();

tracing::debug!(
root = ?effective_walk_root,
patterns = ?glob_patterns,
"walking globs",
);

// Always add ignore hidden folders unless the user explicitly included them
// because we add patterns as overrides, which overrides any `WalkBuilder` settings.
let ignore_patterns = set_ignore_hidden_patterns(&glob_patterns);
Expand Down
63 changes: 60 additions & 3 deletions crates/pixi_glob/src/glob_set/walk_root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,15 +260,30 @@ pub fn split_path_and_glob(input: &str) -> (&str, &str) {
}

/// Normalize paths like `../.././` into paths like `../../`
/// Also resolves components with parent dir like `recipe/..` into an empty path
pub fn normalize_relative(path: &Path) -> PathBuf {
let mut out = PathBuf::new();
let mut out = Vec::new();
for comp in path.components() {
match comp {
Component::CurDir => {}
_ => out.push(comp.as_os_str()),
Component::ParentDir => {
// Pop the last normal component if present, drop if at root, otherwise keep the ParentDir
match out.last() {
Some(Component::Normal(_)) => {
out.pop();
}
Some(Component::RootDir) => {
// Can't go above root directory - ignore this ParentDir
}
_ => {
out.push(comp);
}
}
}
_ => out.push(comp),
}
}
out
out.iter().collect()
}

#[cfg(test)]
Expand Down Expand Up @@ -330,6 +345,27 @@ mod tests {
normalize_relative(Path::new("./.././.././")),
Path::new("../../")
);
// Test that recipe/.. normalizes to empty path
assert_eq!(normalize_relative(Path::new("recipe/../")), Path::new(""));
// Test that foo/bar/../baz normalizes to foo/baz
assert_eq!(
normalize_relative(Path::new("foo/bar/../baz")),
Path::new("foo/baz")
);
// Test that ../recipe/.. normalizes to ..
assert_eq!(
normalize_relative(Path::new("../recipe/..")),
Path::new("..")
);
// Test absolute paths with .. (can't go above root)
assert_eq!(normalize_relative(Path::new("/..")), Path::new("/"));
assert_eq!(normalize_relative(Path::new("/../foo")), Path::new("/foo"));
assert_eq!(normalize_relative(Path::new("/foo/..")), Path::new("/"));
assert_eq!(normalize_relative(Path::new("/.")), Path::new("/"));
assert_eq!(
normalize_relative(Path::new("/foo/bar/../..")),
Path::new("/")
);
}

// Couple of test cases to check that rebasing works as expected
Expand Down Expand Up @@ -444,4 +480,25 @@ mod tests {
"###
);
}

#[test]
fn test_recipe_parent_dir_glob() {
// This test verifies that globs like "recipe/../**" are properly normalized
// to just "**" instead of incorrectly becoming "recipe/**"
let globs = ["recipe/**", "recipe/../**"];

let walk_roots = WalkRoot::build(globs).expect("build should succeed");

assert_yaml_snapshot!(
snapshot_walk_roots(&walk_roots, Path::new("workspace")),
@r###"
root: workspace
globs:
- pattern: recipe/**
negated: false
- pattern: "**"
negated: false
"###
);
}
}
16 changes: 15 additions & 1 deletion crates/pixi_global/src/project/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,14 @@ impl Project {
pub async fn install_environment(
&self,
env_name: &EnvironmentName,
) -> miette::Result<EnvironmentUpdate> {
self.install_environment_with_options(env_name, false).await
}

pub async fn install_environment_with_options(
&self,
env_name: &EnvironmentName,
force_reinstall: bool,
) -> miette::Result<EnvironmentUpdate> {
let environment = self
.environment(env_name)
Expand Down Expand Up @@ -609,6 +617,12 @@ impl Project {

let prefix = self.environment_prefix(env_name).await?;

let force_reinstall_packages = if force_reinstall {
dependencies_names.iter().cloned().collect()
} else {
Default::default()
};

let result = command_dispatcher
.install_pixi_environment(InstallPixiEnvironmentSpec {
name: env_name.to_string(),
Expand All @@ -621,7 +635,7 @@ impl Project {
enabled_protocols: EnabledProtocols::default(),
installed: None,
ignore_packages: None,
force_reinstall: Default::default(),
force_reinstall: force_reinstall_packages,
variants: None,
variant_files: None,
})
Expand Down
81 changes: 81 additions & 0 deletions tests/integration_python/pixi_global/test_global.py
Original file line number Diff line number Diff line change
Expand Up @@ -2365,3 +2365,84 @@ def test_update_sync_conda_file(
env=env,
cwd=cwd,
)


@pytest.mark.slow
def test_install_source_package_with_force_reinstall(
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you reduce the impact of this test on our CI time, this is taking 16s where only one test is slower than that.

Some ideas would be a really basic python package

Copy link
Contributor Author

Choose a reason for hiding this comment

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

only by moving this test to the pixi-integration-tests repo

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can make this test use the in-memory backend instead of actually installing something?

pixi: Path, tmp_path: Path, test_data: Path
) -> None:
"""Test that --force-reinstall actually rebuilds source packages."""
env = {"PIXI_HOME": str(tmp_path)}

# Copy an existing source package from test data
source_package = test_data / "cpp_simple"
target_package = tmp_path / "test-source-package"
shutil.copytree(source_package, target_package)

# # Modify the pixi.toml to enable pixi-build and set a unique name
# pixi_toml = target_package / "pixi.toml"

# # Replace with our test configuration that enables pixi-build
# modified_toml = f"""
# [workspace]
# channels = []
# platforms = [{CURRENT_PLATFORM}]
# preview = ["pixi-build"]

# [package]
# name = "test-source-package"
# version = "1.0.0"

# [package.build]
# backend = {{ name = "in-memory", version = "*" }}

# [package.build.config]
# build-globs = ["TOUCH*"]
# package = "package-b-0.1.0-h4616a5c_0.conda"
# """
# pixi_toml.write_text(modified_toml)

# First installation - should build from source and show "installed"
verify_cli_command(
[
pixi,
"global",
"install",
"--path",
str(target_package),
],
env=env,
stdout_contains="installed",
)

# Second installation without force-reinstall - should show "already installed"
verify_cli_command(
[
pixi,
"global",
"install",
"--path",
str(target_package),
],
env=env,
stdout_contains="already installed",
)

# Third installation with --force-reinstall - should rebuild and show "installed"
# This is the key assertion: force-reinstall should cause a rebuild even though
# the source hasn't changed
verify_cli_command(
[
pixi,
"global",
"install",
"--path",
str(target_package),
"--force-reinstall",
],
env=env,
# To verify that we really install again
stdout_contains="installed",
# and to verify that we really build from source
stderr_contains="Running build for recipe",
)
Loading