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

feat: support scheme 'file' to process local conda files #71

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

amamiya-len
Copy link

Motivation

In my situations, I made some private packages locally and add dependencies with 'file' URLs in pixi.toml(actually pixi support 'file' URLs), but it seems that pixi-pack doesn't support 'file' URLs. So I add support for scheme 'file'.

Changes

Just changed function download_package.

Copy link
Member

@delsner delsner left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! Could you please add an integration test for this feature? You can add a new example for this.


// Ensure target directory exists
let target_path = output_dir.join(file_name);
create_dir_all(target_path.parent().unwrap_or_else(|| Path::new(".")))
Copy link
Member

Choose a reason for hiding this comment

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

This seems fishy. Why do we need it and why using the current working directory as default? Shouldn't we rather fail in these cases?

@amamiya-len
Copy link
Author

Thanks for your contribution! Could you please add an integration test for this feature? You can add a new example for this.

I ran integration tests but only 3 tests passed(main branch and my PR are the same). I searched for the environment about integration tests but didn't find it. Could you please provide me with detailed instructions?

@pavelzw
Copy link
Member

pavelzw commented Nov 22, 2024

in https://github.com/Quantco/pixi-pack/blob/19c4b336f37c8c11de467ce058cf846de589d536/tests/integration_test.rs you can find some integration tests, just adding another one that covers your use case should be fine

@amamiya-len
Copy link
Author

in https://github.com/Quantco/pixi-pack/blob/19c4b336f37c8c11de467ce058cf846de589d536/tests/integration_test.rs you can find some integration tests, just adding another one that covers your use case should be fine

Alright, I will add a new integration test and rethink the implementation of download_package function

@amamiya-len
Copy link
Author

amamiya-len commented Nov 25, 2024

I encountered a problem: file URLs only supports absolute paths, and I need to dynamically generate pixi.toml and pixi.lock according to the project path. I don't know how to gracefully do it. Do you have any idea?

@pavelzw
Copy link
Member

pavelzw commented Nov 25, 2024

@amamiya-len i pushed an integration test that just templates the pixi.toml

@amamiya-len
Copy link
Author

OK,I see. I was a little busy days ago.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants