Skip to content

Conversation

@hameerabbasi
Copy link
Contributor

@hameerabbasi hameerabbasi commented Jan 6, 2026

Description

Fixes #1259

How Has This Been Tested?

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added sufficient tests to cover my changes.
  • I have verified that changes that would impact the JSON schema have been made in schema/model.py.

@hameerabbasi hameerabbasi changed the title Support PEP508 environment markers feat: Support PEP508 environment markers Jan 6, 2026
@hameerabbasi hameerabbasi force-pushed the support-pep508-env-markers branch 2 times, most recently from 781704a to c840936 Compare January 6, 2026 14:59
@hameerabbasi hameerabbasi force-pushed the support-pep508-env-markers branch from c840936 to 684c3da Compare January 6, 2026 15:14
@hameerabbasi hameerabbasi force-pushed the support-pep508-env-markers branch from 734fa83 to 0abb254 Compare January 6, 2026 15:41
@hameerabbasi hameerabbasi marked this pull request as ready for review January 6, 2026 16:09
Copy link
Contributor

@tdejager tdejager left a comment

Choose a reason for hiding this comment

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

Good work seems easier then expected! Can you test if satisfiability works as expected, i.e changing markers re-solves and such? Also test if adding a linux marker with windows as a platform for example creates an empty dependency set. Maybe there are more edge cases we could test and report on?

@tdejager
Copy link
Contributor

tdejager commented Jan 7, 2026

Maybe we should also have some documentation updates, not sure, could you have a look?

@hameerabbasi
Copy link
Contributor Author

hameerabbasi commented Jan 7, 2026

Also test if adding a linux marker with windows as a platform for example creates an empty dependency set.

I changed the test to do this in 2a24e4b, but I have no idea how to fix it sadly. Soo many types and conversions my grepping abilities are failing me.

Edit: Added another test in 1e824d2.

@hameerabbasi hameerabbasi force-pushed the support-pep508-env-markers branch from 688bc09 to 1737ae2 Compare January 7, 2026 08:51
@hameerabbasi hameerabbasi force-pushed the support-pep508-env-markers branch from 1737ae2 to 2bb2372 Compare January 7, 2026 09:00
@hameerabbasi hameerabbasi force-pushed the support-pep508-env-markers branch from f3004e7 to d2e632f Compare January 7, 2026 12:38
@hameerabbasi
Copy link
Contributor Author

Can you test if satisfiability works as expected, i.e changing markers re-solves and such?

Done in 06329e3

@hameerabbasi
Copy link
Contributor Author

hameerabbasi commented Jan 8, 2026

@tdejager @baszalmstra I think this is ready, review/merge appreciated.

@tdejager
Copy link
Contributor

tdejager commented Jan 9, 2026

I looked through everything and all of it makes sense! Let me do a small user-test myself.

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.

Add Environment Markers to pypi-dependencies specification

3 participants