conda / rattler

Rust crates to work with the Conda ecosystem.
BSD 3-Clause "New" or "Revised" License
252 stars 51 forks source link

feat: start adding interface to override #834

Closed SobhanMP closed 2 weeks ago

SobhanMP commented 3 weeks ago

Following 721a6c101ecc4a726ab13b4e051adfc266691474, this commit will start adding the interface for python.

SobhanMP commented 3 weeks ago

@baszalmstra Should VirtualPackageOverride be accessible from python?

iamthebot commented 3 weeks ago

@baszalmstra Should VirtualPackageOverride be accessible from python?

@SobhanMP we'd use this-- can you include it?

SobhanMP commented 3 weeks ago

@iamthebot (we'd have to ask @baszalmstra), but as things are, the overrides are enabled by default, so they work. Do you need to rename the overrides?

iamthebot commented 3 weeks ago

@SobhanMP

Sorry, I think I misunderstood. We'd like to be able to specify the overrides for eg; CUDA directly without needing to use an env var for the override. If I'm reading this correctly-- as written, the overrides would work via env var directly but without exposing VirtualPackageOverride we can't control that override directly from Python side.

SobhanMP commented 3 weeks ago

@baszalmstra where would be the right place to put the tests? and is this interface/impl ok?

❯ CONDA_OVERRIDE_CUDA=1984 pixi run rattler create jupyterlab
Pixi task (rattler in default): cargo run --bin rattler --release -- create jupyterlab
    Finished release [optimized] target(s) in 0.13s
     Running `.pixi/target/release/rattler create jupyterlab`
Target prefix: /home/psi/rattler/.prefix
Installing for platform: Linux64
Loaded 20237 records in 520.548978ms
Virtual packages:
  - __unix=0=0
  - __linux=6.6.45=0
  - __glibc=2.40=0
  - __cuda=1984=0
  - __archspec=1=x86_64_v4

 Already up to date
baszalmstra commented 3 weeks ago

I dont think we need an end-to-end test. I would place some unit tests in the lib.rs that test a few different overrides. Watch out with environment variables though because rust runs all tests in parallel.

SobhanMP commented 2 weeks ago

@baszalmstra I think this is done (minus minor ci failure), unless overrides should also override checks like platform.is_linux (i.e., should we move the conditionals inside the current function?)

SobhanMP commented 2 weeks ago

@baszalmstra, I did the renaming and the deprecation. I think that's everything. Apologies if I missed something.

right now:

baszalmstra commented 2 weeks ago

Thanks! Sorry it took so long!

SobhanMP commented 2 weeks ago

No worries, thanks for all the helpful comments!