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

2.11.x | Fix range for MarkupSafe to stay below 2.1 #1589

Closed
wants to merge 1 commit into from

Conversation

aditya-kar
Copy link

MarkupSafe 2.1.0 has a breaking change

Remove soft_unicode, which was previously deprecated. Use soft_str instead. #261

Jinja 2.11.x requires MarkupSafe >= 0.23. This allows it to pick up the latest version of MarkupSafe with the breaking change. This PR fixes the range of MarkupSafe to stay below 2.1.0.

Checklist:

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add or update relevant docs, in the docs folder and in code.
  • Add an entry in CHANGES.rst summarizing the change and linking to the issue.
  • Add .. versionchanged:: entries in any relevant code docs.
  • Run pre-commit hooks and fix any issues.
  • Run pytest and tox, no tests failed.

@davidism
Copy link
Member

davidism commented Feb 18, 2022

You are using an unsupported version of the project, please update to the latest version. Additionally, please read https://hynek.me/articles/semver-will-not-save-you/, then use a tool like pip-tools to pin your dependencies and control when you get updates. Be sure to run your tests with deprecation warnings treated as errors so that you get notified of these types of changes early.

@davidism davidism closed this Feb 18, 2022
@kochb
Copy link

kochb commented Feb 18, 2022

Could you clarify the harm in merging this pull request? It seems like an easy one-line change that allows us to avoid breaking existing Flask installations, and it seems like a better use of time than copy-pasting across the internet.

We couldn't make an upgrade to Flask 2.0 on release, due to a few bugs that weren't patched until 2.0.2. Not pinning transitive dependencies is on us, but expecting everyone to be up to date with breaking changes within just a few months seems like an unnecessary ask.

@ThiefMaster
Copy link
Member

It wouldn't help because it'd also require a new release of the older versions...

@davidism
Copy link
Member

davidism commented Feb 18, 2022

Pinning transitive dependencies is how you control when you become up to date with changes. Please see the two links I posted, they explain better than I will here.

@kochb
Copy link

kochb commented Feb 18, 2022

I fully understand those links. We should recognize that the headache is partially caused by Jinja2 not following their advice.

Let's take an example pinned against Flask 1.1.4. It requires Jinja2, and Jinja2 requires MarkupSafe.

Flask==1.1.4
- click [required: >=5.1,<8.0]
- itsdangerous [required: >=0.24,<2.0]
- Jinja2 [required: >=2.10.1,<3.0]
  - MarkupSafe [required: >=0.23]

Releasing a Jinja2 2.11.4 would result in the following dependency tree:

Flask==1.1.4
- click [required: >=5.1,<8.0]
- itsdangerous [required: >=0.24,<2.0]
- Jinja2 [required: >=2.10.1,<3.0]
  - MarkupSafe [required: >=0.23,<2.1]

So anyone not pinning transitive dependencies on flask would automatically pick up the change, saving a large number of developers some headaches while everyone gets their act together.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 5, 2022
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.

4 participants