fix: don't strip the path on out of source paths.

[ruben-arts]

Description

Potential fix for #5125

Unfortunately, I struggle to understand what the goal of the function is. Should it return relative or absolute paths? The custom path mangling feels like a solution to some other problem in the pipeline.

The main issue is that it didn't work for out-of-tree paths.

How Has This Been Tested?

It has been tested against the issue.

AI Disclosure

• This PR contains AI-generated content.
  • I have tested any AI-generated content in my PR.
  • I take responsibility for any AI-generated content in my PR.

Tools: Claude

Checklist:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added sufficient tests to cover my changes.

[tdejager]

Note that claude is extremely bad at this part of the code, and has been my worst experience so far. So yeah the code is a bit tricky, and reading it after a month it can definitely be more clear, one of the reasons bas has introduced pixi_path. But matter of fact is that function should return relative paths if possible only if we are dealing with absolute path as input then we keep them absolute. At least that's what I thought:

But when reading this: https://github.com/tdejager/pixi/blob/7cff5ea608e64c03dda2e86c9737534e5934a1c8/crates/pixi_record/src/source_record.rs#L103 It seems to say something differently. But I think that comment is actually incorrect or talks about the part in parentheses? Not sure, because we also discussed about having everything as absolute to get rid of this behavior. I'm also confused.

Background

The reason these functions exist is when talking about them internally we use a different representation, relative to workspace root, then when locked, at that point they are relative to the manifest. So that's why the path mangling is here when reading from and to the lock.

This part that was removed when commented:

let base_absolute = base_path.resolve(workspace_root);
// We know that possible_relative_path is relative here
let relative_std_path = Path::new(build_source_path.as_str());
// Join base with relative path to get the target absolute path
let target_path_abs = base_absolute.join(relative_std_path);

// Normalize the path (resolve . and ..)
let normalized = crate::path_utils::normalize_path(&target_path_abs);
// Convert back to a path that's either absolute or relative to workspace
let path_spec = normalized.strip_prefix(workspace_root).expect(
    "the workspace_root should be part of the source build path at this point",
);

1. First we resolve base_path to workspace root, this gives us an absolute path.
2. We join the relative build_source_path which is the location of the source and should be relative to the manifest, the main use this is seperate is for out-of-tree sources. This is an absolute path as its joined with the base_absolute.
3. Then we do simple normalizations.
4. Because we joined the target_path_abs with the workspace, it must reside in this string. But it is hitting the expect. From reading the code it seems like this should not happen. Did the test fail on main before?

I think we want to return a path relative to the workspace and not the absolute. Would be great for @baszalmstra to chime in quickly here. Answering: does the function from_conda_source_data which goes from lock data-structures to pixi-record, need to return all absolute (resolved) paths or relative to workspace root, in this case specifically for the build-source :)

[ruben-arts]

@tdejager Thank you for the extended reply!

The test would indeed fail; it would always fail with out-of-tree packages, as the normalization would delete the workspace root from the path so it would not be able to remove that part from the path, and thus the expect is thrown.

[tdejager]

@tdejager Thank you for the extended reply!

The test would indeed fail; it would always fail with out-of-tree packages, as the normalization would delete the workspace root from the path so it would not be able to remove that part from the path, and thus the expect is thrown.

Yeah, I think that join actually does normalization so it can move out of the workspace when it's joined, I did not account for that. So that would be the error.

[tdejager]

Just fyi, I did a more involved investigation with claude if we can always return absolute paths.

Summary: Path Handling in from_relative_to

The Original Bug (Issue #5125)

When using an out-of-tree build_source like path = "../scikit-learn" (pointing to a sibling directory outside the workspace), pixi panics:

thread 'main2' panicked at crates/pixi_record/src/pinned_source.rs:266:73:
the workspace_root should be part of the source build path at this point: StripPrefixError(())

Root cause: The code assumed all paths could be made relative to workspace_root using strip_prefix(). But when build_source contains ../ that escapes the workspace, after normalization the path is outside the workspace, so strip_prefix() fails.

Example:

• workspace_root: /workspace/project
• manifest_source: . (in workspace root)
• build_source in lock file: ../scikit-learn (relative to manifest)
• After joining and normalizing: /workspace/scikit-learn ← outside /workspace/project
• strip_prefix("/workspace/project") fails because scikit-learn is a sibling, not a child

Data Flow Context

There are two transformations:

Writing to lock file (into_conda_source_data → make_relative_to):

• build_source is stored relative to manifest_source (e.g., ../scikit-learn)

Reading from lock file (from_conda_source_data → from_relative_to):

• build_source is converted back to PinnedSourceSpec
• The path should be stored relative to workspace_root

The from_relative_to function only affects the internal representation after reading from the lock file. When writing back, make_relative_to recomputes the relative path for the lock file.

The Current Fix

The PR changes from_relative_to to handle out-of-workspace paths by storing them as absolute instead of panicking:

let path = if let Ok(relative) = normalized.strip_prefix(workspace_root) {
    // Inside workspace → relative to workspace
    Utf8TypedPathBuf::from(relative.to_string_lossy().as_ref())
} else {
    // Outside workspace → absolute
    Utf8TypedPathBuf::from(normalized.to_string_lossy().as_ref())
};

Potential Concern: Path Format Consistency

In satisfiability/mod.rs:1162, there's a comparison:

if current_record.build_source \!= source_record.build_source {
    return Err(Box::new(PlatformUnsat::SourceBuildLocationChanged(...)));
}

Where:

• source_record comes from the lock file (via from_relative_to)
• current_record comes from fresh metadata computation

If these use different path formats (one absolute, one relative), the equality check will fail even if they point to the same location.

However, tracing the code shows that current_record is computed using source_record.build_source as preferred_build_source input. The question is whether this value is passed through unchanged or transformed during the fresh computation in build_backend_metadata.

Alternative Approach: Always Use Relative Paths

Instead of storing absolute paths for out-of-workspace, we could use pathdiff::diff_paths to always produce relative paths (with .. components):

let path = pathdiff::diff_paths(&normalized, workspace_root)
    .map(|p| Utf8TypedPathBuf::from(p.to_string_lossy().as_ref()))
    .expect("should always be able to compute relative path");

This would produce ../scikit-learn instead of /workspace/scikit-learn.

Problem: PinnedPathSpec::resolve() does not normalize paths:

pub fn resolve(&self, workspace_root: &Path) -> PathBuf {
    if self.path.is_absolute() {
        PathBuf::from(native_path)
    } else {
        workspace_root.join(native_path)  // Returns /workspace/project/../scikit-learn (unnormalized\!)
    }
}

This causes issues in make_relative_to which uses pathdiff::diff_paths on the resolved paths - unnormalized paths with .. components produce incorrect relative paths.

Options

1. Current fix (absolute for out-of-workspace): Works, but need to verify the equality comparison in satisfiability doesn't break
2. Always relative + fix resolve() to normalize: More consistent, portable lock files, but requires changing resolve() behavior
3. Fix the comparison: Change the equality check to compare resolved/normalized paths instead of raw PinnedSourceSpec equality

[baszalmstra]

I want to propose a different solution. The problem is that the SourceRecord loses information about whether the original build source spec provided by the user is lost. We could also consider storing the "original" (but possibly resolved in case of git) spec alongside the "workspace relative" path. Then we can simply store that in the lock-file.

[ruben-arts]

Closing in favor of #5247