Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: matchspec build / version from brackets and string serialization #917

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

wolfv
Copy link
Contributor

@wolfv wolfv commented Oct 31, 2024

We were parsing brackets correctly, but we overrode it with None.

This fixes matchspecs like foo * [build="*bla*"]

For now this is no error when the build string is set twice (e.g. foo * bar*[build="baz"]). I am not sure if we should treat that as an error (at least in strict mode?). Same goes for the version.

cc @jaimergp thanks for discovering this today!

@wolfv wolfv changed the title Fix matchspec build / version from brackets fix: matchspec build / version from brackets and string serialization Oct 31, 2024
@wolfv
Copy link
Contributor Author

wolfv commented Nov 1, 2024

Interestingly in conda, it looks like brackets are always preferred (which is the inverse of what I did here).

For example:

>>> a = MatchSpec("foo [version=5.5.5] 1.2.3 foo")
>>> a
MatchSpec("foo==5.5.5=foo")

We can easily match conda behavior here. We could also add the option to fail on this in strict mode and allow version / build only once in strict mode (or only allow duplication if the value is exactly the same?).

@baszalmstra
Copy link
Collaborator

That seems like a good approach!

@wolfv
Copy link
Contributor Author

wolfv commented Nov 1, 2024

I implemented that strict parsing fails when encountering a key multiple times (in brackets, or outside brackets & inside brackets).
IMO this is good to go! :)

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.

2 participants