Skip to content

Conversation

soapy1
Copy link
Contributor

@soapy1 soapy1 commented Jul 2, 2025

Description

fixes: conda/conda-planning#66

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation?

@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Jul 2, 2025
@github-project-automation github-project-automation bot moved this to 🆕 New in 🔎 Review Jul 2, 2025
@@ -269,6 +274,14 @@ def merge(cls, *environments):
)
)

virtual_packages = list(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are there any restrictions when merging virtual packages for an environment? Like, should we ensure that there are not colliding packages with the same name but different version/build? Or any other rules?

Copy link
Contributor

Choose a reason for hiding this comment

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

Virtual packages are PackageRecords, so they only take a scalar version (not a range). They are usually work as a lower bound in the solver. So I guess the strictest merge in the event of two different versions for the same package would be to keep the largest version (strictest lower bound); e.g. given __linux=2.0 and __linux=2.1, we would keep __linux=2.1.

However other virtual packages, like archspec are not so easily comparable and it might not make sense to solve a conflict between archspec=0=x64 and archspec=0=m1 because they won't be satisfiable ever.

I don't know what you want to do here, but these are the options:

  • Merge lists by keeping the latest version of repeated packages, when applicable. This will require checking all known virtual packages and some custom logic for exceptions such as archspec. They re technically plugins so we might find others in the wild, but in practice these are unlikely.
  • Always error out if there are different packages for the same virtual name.

I think that a logic for the first option might be something like this. Given two package records for the same virtual package name:

  • If all build strings are the same, keep the latest version.
  • If the build strings differ, raise error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sync'd offline about this, summarizing the findings here:

  • we maybe don't want to be doing any merging of any type of package in the model. Doing operations against packages, like comparing versions, ect. should all be handled by the solver layer
    • For example consider a similar case of merging environment models that have multiple requested specs with the same name but different versions. I think we would want the merged environment to have the whole list of requested packages, and then the solver will work out the rest.
    • Also, consider how virtual packages are handled currently. They get slurped up into the solve, similar to a package. So currently, the responsibility of reporting errors having to do with virtual packages is in the solver layer. This makes sense, because this layer is dealing with understanding the relationship between packages and encodes all the rules around how packages (and their versions) should relate to each other. I think we wouldn't want to duplicate logic related to this in other areas of the code.
  • In the most ideal situation we maybe want to be dealing with this in conda_libmamba_solver.state - but we dont' need to implement that here/just yet.

@soapy1
Copy link
Contributor Author

soapy1 commented Jul 2, 2025

pre-commit.ci autofix

@soapy1 soapy1 marked this pull request as ready for review July 2, 2025 00:54
@soapy1 soapy1 requested a review from a team as a code owner July 2, 2025 00:54
Copy link

codspeed-hq bot commented Jul 2, 2025

CodSpeed Instrumentation Performance Report

Merging #14979 will not alter performance

Comparing soapy1:env-virtual-packages (c286614) with main (8665808)

Summary

✅ 21 untouched benchmarks

@github-project-automation github-project-automation bot moved this from 🆕 New to 🏗️ In Progress in 🔎 Review Jul 2, 2025
@soapy1 soapy1 force-pushed the env-virtual-packages branch from 7206b48 to 2ca7255 Compare July 2, 2025 15:05
@soapy1 soapy1 force-pushed the env-virtual-packages branch from 2ca7255 to 7255252 Compare July 2, 2025 15:22
@jaimergp
Copy link
Contributor

jaimergp commented Jul 8, 2025

Can you add a news item? Adding this PR number to this line https://github.com/soapy1/conda/blob/7255252fbe6289eeaa358ae0b37d92b735b125fc/news/14870-add-environment-data-model#L3 will be enough.

jezdez
jezdez previously approved these changes Jul 8, 2025
@github-project-automation github-project-automation bot moved this from 🏗️ In Progress to ✅ Approved in 🔎 Review Jul 8, 2025
@jezdez jezdez enabled auto-merge (squash) July 8, 2025 10:58
@jezdez
Copy link
Member

jezdez commented Jul 8, 2025

seems like one test so far is failing with a transient networking issue, will restart the failing test when the others have run

@jezdez jezdez merged commit 927e11f into conda:main Jul 8, 2025
75 checks passed
@github-project-automation github-project-automation bot moved this from ✅ Approved to 🏁 Done in 🔎 Review Jul 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed [bot] added once the contributor has signed the CLA
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Environment dataclass should record the virtual packages used or overridden
5 participants