feat: Add support for SOURCE packages#244
Conversation
feat: try to add intergation tests
e214536 to
faf6c1d
Compare
fix: remove unused fixture fix: fix typo fix: fix typo fix: fix typo fix: fix test on arm-64, osx and win builds fix: fix test on arm-64, osx and win builds fix: add valid checksum for other architectures fix: fix test using correct checksum fix: fix typo error fix: fix typo
6abfcac to
a9b0110
Compare
fix: fix typo fix: fix typo
abf424a to
0847e0d
Compare
pavelzw
left a comment
There was a problem hiding this comment.
Looks very solid already, thanks!
I have a couple of comments
| let pkg_dir = base_dir.join(match &source_data.location { | ||
| UrlOrPath::Path(p) => p.to_path().to_string(), | ||
| UrlOrPath::Url(u) => { | ||
| anyhow::bail!("Unexpected URL for local package source: {}", u) |
There was a problem hiding this comment.
Is this a thing that will be supported eventually @Hofer-Julian? Depending on other pixi projects from git is also not supported yet, right?
fix: fix typo for ci precommit fix: fix typo for ci precommit fix: fix typo for ci precommit
a8e2dbc to
1377324
Compare
|
Thanks @pavelzw ! So I made the requested changes, could you review the new version and tell me if there are some changes that are not relevant or things I could have done better ? |
src/pack.rs
Outdated
| .arg(output_dir) | ||
| .arg("--build-platform") | ||
| .arg(platform) | ||
| .current_dir(manifest_dir); |
There was a problem hiding this comment.
with things like PIXI_PROJECT_ROOT (set by pixi shell) i think it's safer to explicitly use pixi build --path
There was a problem hiding this comment.
also, another thing i noticed:
❯ cd examples/local-build/
❯ cargo run --bin pixi-pack --
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.99s
Running `/Users/pavel/projects/pixi-pack/target/debug/pixi-pack`
⠁ Building local-build-main-pkg-0.1.0-hbf21a9e_0.conda
❌ Build failed with exit code: Some(1)
Command: pixi build --output-dir /var/folders/rz/q1r4tv0n6ll38q18fbk419mc0000gn/T/.tmpZQnitN --build-platform osx-arm64 (in /Users/pavel/projects/pixi-pack/examples)
📤 stderr:
Error: × the source directory '/Users/pavel/projects/pixi-pack/examples', does not contain a supported manifest
help: Ensure that the source directory contains a valid pixi.toml, pyproject.toml, recipe.yaml, package.xml or mojoproject.toml file.
Error: Failed to build package in /Users/pavel/projects/pixi-pack/examples
❯ cargo run --bin pixi-pack -- pixi.toml
Compiling pixi-pack v0.7.5 (/Users/pavel/projects/pixi-pack)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 8.78s
Running `/Users/pavel/projects/pixi-pack/target/debug/pixi-pack pixi.toml`
✅ Built local-build-main-pkg-0.1.0-hbf21a9e_0.conda
✅ Built local-build-local-pkg-0.1.0-hbf21a9e_0.conda
✅ Built local-build-local-pkg-2-0.1.0-hbf21a9e_0.conda
⏳ Downloading 14 packages...
📦 Created pack at /Users/pavel/projects/pixi-pack/examples/local-build/environment.tar with size 7.70 MiB.
something is still wrong with the manifest path...
There was a problem hiding this comment.
I updated the base_dir logic to correctly handle manifest paths when building source packages. Previously, when options.manifest_path was a directory (not a file), the code incorrectly used its parent directory as the base, causing build failures with paths like /examples/./pixi.toml. Now it properly uses the manifest directory itself as the base when it's a directory. Also added canonicalize() to resolve relative paths (like ./) and ensure the manifest file exists before attempting to build.
oprinsen@lappc-p1138 local-build ⌥ (add_source_pkg_handling) cargo run --bin pixi-pack --
Compiling pixi-pack v0.7.5 (/home/oprinsen/pixi-pack)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 3.62s
Running `/home/oprinsen/pixi-pack/target/debug/pixi-pack`
DEBUG: options.manifest_path = /home/oprinsen/pixi-pack/examples/local-build
DEBUG: source_data.location = Path(Unix(Utf8PathBuf { _encoding: "unix", inner: "." }))
DEBUG: base_dir = /home/oprinsen/pixi-pack/examples/local-build
✅ Built local-build-main-pkg-0.1.0-hbf21a9e_0.conda DEBUG: options.manifest_path = /home/oprinsen/pixi-pack/examples/local-build
DEBUG: source_data.location = Path(Unix(Utf8PathBuf { _encoding: "unix", inner: "local-pkg" }))
DEBUG: base_dir = /home/oprinsen/pixi-pack/examples/local-build
✅ Built local-build-local-pkg-0.1.0-hbf21a9e_0.conda DEBUG: options.manifest_path = /home/oprinsen/pixi-pack/examples/local-build
DEBUG: source_data.location = Path(Unix(Utf8PathBuf { _encoding: "unix", inner: "local-pkg2" }))
DEBUG: base_dir = /home/oprinsen/pixi-pack/examples/local-build
✅ Built local-build-local-pkg-2-0.1.0-hbf21a9e_0.conda ⏳ Downloading 21 packages...
📦 Created pack at /home/oprinsen/pixi-pack/examples/local-build/environment.tar with size 15.28 MiB.
oprinsen@lappc-p1138 local-build ⌥ (add_source_pkg_handling)
|
pushed some adjustments to your branch; after the manifest dir fixes, this is good to merge 👍🏻 |
fix: fix typo
6a65aed to
177e276
Compare
|
Hello @pavelzw, I saw that the tests have failed after merging main into my branch. Is the issue linked to the |
|
yeah this looks really weird to me. sounds like a bug in pixi tbh. |
|
Hello @pavelzw. So I looked into the issue of the regenerated hashes and it seems that recently the cmake build backend used in the tests has been updated and the hash generation might have been impacted. I'm not sure about how it impacted the tests and it's still unclear to me but for now I added the new generated hashes using the cmake build backend 0.3.5 (maybe it could be intersting to pin the build backend version in the test to avoid new strange behaviors). Another idea could be to use match patterns instead of the exact version of the package containing the hashes. Instead of looking for I also updated the pixi version to use 0.61.0 so the API of |
|
Sorry for not coming back to you, lost track of this pr. One thing I'm wondering is why the build string from the pixi.lock didn't match the build string of the built package 🤔 theoretically the lockfile should always match the package exactly and thus represent the correct build string already? Maybe I'm missing something, will investigate later In general, i think pinning the build backend makes sense in this case 👍🏻 I think it would be nice if we even persisted the build backends and host+build environments used in pixi.lock (optionally): prefix-dev/pixi#5161 |
No problem, I was also really busy! I can pin the build backend version in the tests to the latest version (0.3.5). Regarding the build string mismatch issue, I'm not sure what approach you'd prefer: should we wait for pixi to provide a clear fix for this issue, or would you prefer to merge the PR as-is and address the build string inconsistency once pixi implements the changes mentioned in the issue you linked? |
sounds good, maybe also link
i would like to understand the issue a bit better and at least have an issue to point to. i'll look into creating a minimal reproducer for this problem and let's see what the prefix-dev team has to say |
Sounds good! I will pin the version and follow the issue tracking on pixi side. I also tried to do a small reproducer but I did not manage to do something showing the exact same issue. |
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
regenerate lock file to use pixi 0.61.0 fix: pin the build backend issue
9301789 to
bea3f62
Compare
|
Hello! May I kindly ask what is the status about this PR? Anything that we could help with? |
Motivation
This PR adds support for building packages from source, a feature that is currently missing. See issue #80 for background and motivation.
When a local package is found in the pixi.lock of the main package, start a
pixi-buildcommand to build the local dependencies and add them as injected packages in theenvironment.tarChanges
pixi-buildcommand to automatically build local packages from the mainpixi.lock