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

[BUG] "Using .* with relational operator is superfluous and deprecated" #348

Closed
maresb opened this issue Jul 3, 2022 · 27 comments · Fixed by #365
Closed

[BUG] "Using .* with relational operator is superfluous and deprecated" #348

maresb opened this issue Jul 3, 2022 · 27 comments · Fixed by #365
Labels
bug Something isn't working

Comments

@maresb
Copy link
Contributor

maresb commented Jul 3, 2022

Consider the package dbt-core. The install_requires contains

        "dbt-extractor~=0.4.1",

Running Grayskull this gets translated into

    - dbt-extractor >=0.4.1,==0.4.*

Then running boa with conda mambabuild -c conda-forge . or conda build -c conda-forge . produces the warning

WARNING:conda.models.version:Using .* with relational operator is superfluous and deprecated and will be removed in a future version of conda. Your spec was 0.4.*, but conda is ignoring the .* and treating it as 0.4

This results in the unsatisfiable dependency >=0.4.1,==0.4 since ==0.4 seems to imply 0.4.0 and excludes 0.4.1.

Interestingly, if I change it to a single equals (>=0.4.1,=0.4.*) then it doesn't complain and seems to work as intended.

@marcelotrevisani
Copy link
Member

true, it does need some work, actually this part can be ~=
we have moved that because a request came up, but I think we should follow what conda does and ~= is good enough

@maresb
Copy link
Contributor Author

maresb commented Jul 23, 2022

Is there any reason why ~= is avoided in Conda recipes? Does it not work reliably, or was it only recently implemented?

@BastianZim
Copy link
Contributor

~= is not supported by conda-build: https://docs.conda.io/projects/conda-build/en/latest/resources/package-spec.html#build-version-spec

Conda-forge just monkey patches it, iirc.

@maresb
Copy link
Contributor Author

maresb commented Aug 2, 2022

Thanks @BastianZim! Then ~= is not an option.

Then it seems like there are two possibilities for converting ~=0.4.1:

>=0.4.1,<0.5
>=0.4.1,=0.4.*

The second one looks really confusing to me. Does the first one correctly handle prereleases? In other words, is 0.5b1<0.5? If so, then we'll need some obscure incantation to correctly deal with that case, right?

@BastianZim
Copy link
Contributor

Well, python recommends >= V.N, == V.* so I guess/hope that should work?

Ref: https://peps.python.org/pep-0440/#compatible-release

@BastianZim
Copy link
Contributor

MIght be easier to discuss with core again if we just go with ~=. What do you think @marcelotrevisani since you're core and developer. 😄

@maresb
Copy link
Contributor Author

maresb commented Aug 3, 2022

Well, python recommends >= V.N, == V.* so I guess/hope that should work?

As I point out above, this unfortunately doesn't work with Conda:

==0.4 seems to imply 0.4.0 and excludes 0.4.1.

if we just go with ~=

How would this work if it's not supported by conda-build?

@BastianZim
Copy link
Contributor

As I point out above, this unfortunately doesn't work with Conda:

Ahh true sorry forgot

How would this work if it's not supported by conda-build?

From what I remember, this is monkey patched in conda-forge. Since that's the only location where the recipe would be, it should be fine.

@maresb
Copy link
Contributor Author

maresb commented Aug 3, 2022

Sometimes I'll run conda mambabuild recipe locally when testing a really problematic recipe. It seems like it would be really confusing if it failed locally but succeeded on CF.

@marcelotrevisani
Copy link
Member

yeah, as @BastianZim had mentioned, it would be better to fix it by putting >= V.N, == V.*
so, it needs a bit of work

@maresb
Copy link
Contributor Author

maresb commented Aug 5, 2022

Do you mean >= V.N, = V.*? (As I have mentioned above, >= V.N, == V.* doesn't seem to work properly with conda.)

@marcelotrevisani
Copy link
Member

marcelotrevisani commented Aug 6, 2022

Do you mean >= V.N, = V.*? (As I have mentioned above, >= V.N, == V.* doesn't seem to work properly with conda.)

Sorry, that should be >= V.N, < (V+1).0 then

@maresb
Copy link
Contributor Author

maresb commented Aug 6, 2022

I strongly suspect that >= V.N, < (V+1).0 is still incorrect due to prerelease versions.

I just found this where it seems to be quite the logic puzzle to deduce the minimal version string which is greater than V.*. I think we need something like >= V.N, < (V+1)dev, but I'm not entirely satisfied by the examples given in that link.

Maybe I can find the relevant classes in Conda's source so that I can test these things myself...

@marcelotrevisani
Copy link
Member

To be honest, I am not taking care much of pre-release versions because they shouldn't be in conda-forge in general.

The dev can be added, it will need more tests.

@maresb
Copy link
Contributor Author

maresb commented Aug 6, 2022

Makes sense. Thinking about it from that perspective, adding this obscure dev suffix certainly makes the recipe more confusing and less readable. That's a steep price to pay for handling edge cases which are virtually nonexistent. Thanks for the explanation!

And the root of this issue seems like a bug in conda-build...

@BastianZim
Copy link
Contributor

And the root of this issue seems like a bug in conda-build...

That’s what I was thinking. Maybe it’s easier to add support to conda-build for ~=

@maresb
Copy link
Contributor Author

maresb commented Aug 6, 2022

Ya, it seems that there was this attempt, but for some subtle reason it's not working properly.

If we fix it upstream, then it'll only work properly for people who have up-to-date versions of conda-build. But I suppose we care primarily about conda-forge, where this isn't a problem.

@BastianZim
Copy link
Contributor

But I suppose we care primarily about conda-forge, where this isn't a problem.

Yes. The only ones that would have to update are people like you who use it for testing but I guess you create fresh environments for that anyways?

@maresb
Copy link
Contributor Author

maresb commented Aug 6, 2022

but I guess you create fresh environments for that anyways

Honestly not so often. I just run conda activate forge where I have Grayskull, Boa, and a few other tools. There's not much point to updating when things are working. And then if it does break, I'm going to try to update Grayskull, which doesn't depend on conda-build. I've only ever updated conda-build when I've already spent an hour trying to figure out why a recipe is succeeding on CF but failing locally.

From this perspective, if we fix ~= in conda-build, it may make sense to update the Grayskull recipe to add a minimal version of conda-build to run_constrained requirements so that in case conda-build is installed it'll be updated.

@BastianZim
Copy link
Contributor

Oh yeah that’s maybe not a bad idea at all! @marcelotrevisani What do you think?

@maresb
Copy link
Contributor Author

maresb commented Aug 6, 2022

Hold on, perhaps the situation has changed over the past few years...

~= is not supported by conda-build: https://docs.conda.io/projects/conda-build/en/latest/resources/package-spec.html#build-version-spec

I just checked this, and it does appear to work, even though it's not documented. 😕 Presumably it was added for v3.20.5 in the PR linked above (the first bullet point under "Bug fixes:".)

Therefore, I think we can solve this by using ~= in the recipe and adding run_constrained: conda-build >=3.20.5 to the Grayskull recipe (and adding tests...).

maresb added a commit to maresb/grayskull that referenced this issue Aug 6, 2022
Building such recipes requires conda-build >=3.20.5
See conda#348
@maresb
Copy link
Contributor Author

maresb commented Aug 6, 2022

PR to switch to simply using ~=: #365 ✔️
PR to add run_constrained: conda-forge/grayskull-feedstock#55 ✔️

marcelotrevisani pushed a commit that referenced this issue Aug 9, 2022
Building such recipes requires conda-build >=3.20.5
See #348
@jaimergp
Copy link
Contributor

The problem with ~= is that it shouldn't make it to the repodata.json files, because older conda clients won't be able to parse it and error out. IIRC, this operator is patched out during the CDN cloning (when repodata patches are applied).

There's also one problem with roundtripped MatchSpec objects. name=1.2 is parsed correctly, but when converted back to string, it can appear as name==1.2, so it's not the same anymore. Unfortunately, conda does this to sanitize inputs, so one need to be careful.

@maresb
Copy link
Contributor Author

maresb commented Aug 10, 2022

@jaimergp thanks for the info! Yikes, that sounds like a mess. Does the repodata patching work well enough that it's safe to use ~= in recipes?

If not, do you have any suggestions on how we could better solve this problem?

@jaimergp
Copy link
Contributor

I don't think we have audited every possibility, but that's the main issue: back-compatibility. The repodata patch only covers the channel repodata.json, but the metadata in the tarball will be still the same, which might leak into the PrefixData object used to parse the installed packages (although in some circumstances the data might come from the repodata, I don't have a very clear idea on when this happens).

When we discussed this in a core call some months ago I recall that the consensus was to avoid its use whenever possible, and use the more verbose but back compatible syntax: ~=1.4 gets translated to >=1.4,<2.0a, iirc.

With respect to the round trip issues in MatchSpec, Wolf started some work on conda/conda#9980, and I'd like to resume it in the next weeks, so hopefully this will become less of an issue.

@BastianZim
Copy link
Contributor

I think, I just bumped into this (or at least it's related).

One of my feedstocks has the requirements ~=0.9.0 which gets translated to >=0.9.0,==0.9.*.
This makes the solves throw an error despite v0.9.6 being available.

PR: conda-forge/aiofile-feedstock#3
Logs: https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=548936&view=logs&j=656edd35-690f-5c53-9ba3-09c10d0bea97&t=e5c8ab1d-8ff9-5cae-b332-e15ae582ed2d

maresb added a commit to maresb/grayskull that referenced this issue Aug 10, 2022
@maresb
Copy link
Contributor Author

maresb commented Aug 10, 2022

First attempt at new fix: #368

marcelotrevisani pushed a commit that referenced this issue Aug 12, 2022
* Revert "Use "compatible release" operator ~= (#365)"

This reverts commit ac33392.

* Fix "compatible release" operator

See #348

* Use dev0 instead of a0 as suffix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants