Skip to content

Fix out-of-sync requirements#12

Merged
3 commits merged intoconda-forge:masterfrom
maresb:patch-1
Aug 26, 2021
Merged

Fix out-of-sync requirements#12
3 commits merged intoconda-forge:masterfrom
maresb:patch-1

Conversation

@maresb
Copy link
Copy Markdown
Contributor

@maresb maresb commented Aug 25, 2021

This is causing problems downstream, e.g. conda-forge/dvc-feedstock#234

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

@maresb maresb requested a review from a user August 25, 2021 16:06
@conda-forge-linter
Copy link
Copy Markdown

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@maresb
Copy link
Copy Markdown
Contributor Author

maresb commented Aug 25, 2021

@conda-forge-admin, please rerender

Comment thread recipe/meta.yaml
@ghost
Copy link
Copy Markdown

ghost commented Aug 25, 2021

Hmm, the CI builds aren't happy: Unsatisfiable dependencies for platform linux-64: {"portalocker[version='>=1.0,<1.1']"}

@maresb
Copy link
Copy Markdown
Contributor Author

maresb commented Aug 25, 2021

Ya, there's no conda-forge build for that version...

yet...

Comment thread recipe/meta.yaml Outdated
Comment on lines +24 to +25
- portalocker >=1.6,<1.7 # [win]
- portalocker >=1.0,<1.1 # [not win]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since msal-extensions should be forward-compatible with portalocker 2 we should simply remove the upper bound. That will still fail pip check though so it's probably necessary to remove the pip check test until it's fixed upstream

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh. Oops!!!!! Thanks so much for pointing this out! 🤦‍♂️

@roederja
Copy link
Copy Markdown
Contributor

Ya, there's no conda-forge build for that version...

yet...

So, you think this can be merged regardless as it doesn't work on linux anyway at the moment?

@maresb
Copy link
Copy Markdown
Contributor Author

maresb commented Aug 26, 2021

I'm inclined to obey pip check, the reason being that this would cause pip check to fail in everything downstream, not just msal-extensions. I'll fix the current pin...

@dhirschfeld
Copy link
Copy Markdown
Member

I'm inclined to obey pip check,

IME pip check often fails for conda packages, When it works it's great as it gives you an extra level of protection but sometimes it just doesn't

@conda-forge-linter
Copy link
Copy Markdown

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe) and found some lint.

Here's what I've got...

For recipe:

  • noarch: python recipes are required to have a lower bound on the python version. Typically this means putting python >=3.6 in both host and run but you should check upstream for the package's Python compatibility.

@maresb
Copy link
Copy Markdown
Contributor Author

maresb commented Aug 26, 2021

@conda-forge-admin, please rerender

conda-forge-linter and others added 2 commits August 26, 2021 13:11
@conda-forge-linter
Copy link
Copy Markdown

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

Comment thread recipe/meta.yaml
- python >=3.6
- msal >=0.4.1,<2.0
- portalocker
- portalocker >=1.6,<2.0
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I suspect that pip check may only be happy if you specify ~=1.6 for # [win] and ~=1.0 for # [not win]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

...which will require removing noarch (and rerendering)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

My understanding is that pip check looks at the currently installed packages and verifies that they are in a consistent state. Since anything with >=1.6,<2.0 is consistent in either case, it should be fine.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not so familiar with pip but that makes sense.... and the CI concurs!

Copy link
Copy Markdown
Member

@dhirschfeld dhirschfeld left a comment

Choose a reason for hiding this comment

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

This doesn't unblock poratlocker >=2.0 but it should stop all recipes depending on this from failing

@ghost ghost merged commit 625a3aa into conda-forge:master Aug 26, 2021
@maresb maresb deleted the patch-1 branch August 26, 2021 16:27
This pull request was closed.
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.

4 participants