regro / conda-forge-feedstock-check-solvable

A mamba- or rattler-based package to check if a conda-forge feedstock is solvable.
BSD 3-Clause "New" or "Revised" License
1 stars 7 forks source link

handle run_constrained and constraints in run_exports #31

Closed minrk closed 4 months ago

minrk commented 4 months ago

follow-up to #30, this actually implements applications of constraints to the solve

beckermr commented 4 months ago

@jaimergp do you understand the solver errors here? I don't get the error that arrow-cpp does not exist.

jaimergp commented 4 months ago

Hm, I haven't used that method. What about add_pin? Would that work?

beckermr commented 4 months ago

I don't see why that makes sense. Care to explain more?

minrk commented 4 months ago

I opened https://github.com/mamba-org/mamba/issues/3293 because add_constraint seems to add the package to the install specs. I think add_pin works, and I added a test to ensure that constraints don't get added to the solution. Maybe I misunderstood what add_constraint means.

beckermr commented 4 months ago

Ohhhhh wow. I think add_pin <-> add_constraint need to have their names swapped. If add_pin does not install, then we're fine.

minrk commented 4 months ago

@beckermr I don't know if it's a terminology problem or what, but that's what I expected, too. But it's not how it behaves, despite explicitly stating it should, I think. add_constraint adds the package to the install list, while add_pin only constrains the version and works whether it's installed or not. add_pin required installed_repo to be set, which I also don't understand, but 🤷 it seems to have the effect I expected constraint to have.

beckermr commented 4 months ago

If this passes, I think we'll want a tighter pin on libmambapy in the feedstock so that if this bug gets fixed, things still work.

minrk commented 4 months ago

2.0 is in beta already which appears to significantly change the Python API, so <2 might be enough.

beckermr commented 4 months ago

We'll need to patch old release of this anyways then.

minrk commented 4 months ago

I added a few tests that run in a matter of seconds for me, but CI runtime has more than doubled in this PR. I don't know if that's a fluke or a consequence of the pinning changes.

beckermr commented 4 months ago

It is probably related, but TBH we don't get a choice on these things. The solver takes how long the solver takes. I am going to merge and we can add options to turn this off later if we need it.