conda / conda-lock

Lightweight lockfile for conda environments
https://conda.github.io/conda-lock/
Other
456 stars 101 forks source link

Don't filter in toposort #560

Closed maresb closed 6 months ago

maresb commented 7 months ago

Description

Before this PR, toposort would drop items (the virtual packages). I wouldn't expect a sort operation to do this. This is a major bug hazard, see #556 and also potentially #557. I've spit this out filtering into a separate step.

Other commits implement some minor cleanups along the way.

netlify[bot] commented 7 months ago

Deploy Preview for conda-lock ready!

Name Link
Latest commit 41ae11fc39826088f5c772546161950589189ab3
Latest deploy log https://app.netlify.com/sites/conda-lock/deploys/6575e531e4054800084abb42
Deploy Preview https://deploy-preview-560--conda-lock.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

jamesmyatt commented 6 months ago

Any progress on this?

maresb commented 6 months ago

Sorry, this stalled out as I couldn't figure out if my assertion was correct.

I suppose we could remove it and proceed blindly, or better replace it with a warning and ask the user to file a report if it's false.

@mariusvniekerk, any insight here? It would be nice to either get test coverage on L122 or remove it as unnecessary.

maresb commented 6 months ago

I'm dropping my assertion and will follow it up later.