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

Restrict jsonschema #8161

Merged
merged 1 commit into from
Jul 9, 2023
Merged

Conversation

Secrus
Copy link
Member

@Secrus Secrus commented Jul 6, 2023

Pull Request Check List

Resolves: #8160

This restricts jsonschema to versions below the problematic ones.

@Secrus
Copy link
Member Author

Secrus commented Jul 6, 2023

@radoering or @neersighted could you please take a look, backport to 1.5 branch, and do a release of this as a hotfix?

@oc-ben-ellis
Copy link

Are hotfixes required for older versions? Just curious but doesn't affect me directly as I'm on 1.5.1.

@Secrus
Copy link
Member Author

Secrus commented Jul 6, 2023

Are hotfixes required for older versions? Just curious but doesn't affect me directly as I'm on 1.5.1.

The tricky part is that it's fine for already installed versions, but if you were to reinstall Poetry, it would pull jsonschema 4.18 which requires Rust toolchain. For most systems it will be fine, but as presented in the issue, MUSL based OS will have problems.

@dimbleby
Copy link
Contributor

dimbleby commented Jul 6, 2023

afaik the jsonschema that poetry requires by regular means is ignored anyway, in favour of the vendored jsonschema in poetry-core...

That vendoring seems likely to become awkward, if there's now effectively a binary-only sub-dependency.

Perhaps should be looking for a way to remove the jsonschema dependency from poetry-core altogether...

@oc-ben-ellis
Copy link

oc-ben-ellis commented Jul 6, 2023

@Secrus Looks like this version has a dependency on rust also?

error: can't find Rust compiler

@oc-ben-ellis
Copy link

Separate from this PR but maybe worth adding alpine linux to the list of distros to test?

Copy link
Member

@radoering radoering left a comment

Choose a reason for hiding this comment

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

For the BSD tests you probably can just add jsonschema<4.18.0 to

- $POETRY_HOME/bin/pip install poetry

@@ -41,7 +41,7 @@ crashtest = "^0.4.1"
dulwich = "^0.21.2"
importlib-metadata = { version = ">=4.4", python = "<3.10" }
installer = "^0.7.0"
jsonschema = "^4.10.0"
jsonschema = ">=4.10.0,<4.18.0"
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a comment, why we are restricting it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@Secrus Secrus requested review from radoering and a team July 8, 2023 10:09
@Secrus Secrus merged commit 01657dd into python-poetry:master Jul 9, 2023
@Secrus Secrus deleted the restrict-jsonschema branch July 9, 2023 09:15
@johnthagen
Copy link
Contributor

johnthagen commented Jul 10, 2023

Has anyone asked for MUSL wheels?

Last I tried to install Poetry on Alpine (year or so ago?) it also failed due to cryptography missing the Rust tool chain, but these days it looks like they publish MUSL wheels?

Perhaps jsonschema would be open to providing them too?

Copy link

github-actions bot commented Mar 3, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suddenly require Rust and Cargo installed?
5 participants