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

⚠ CRITICAL: Copier 9.1.1 broken by too loose pyyaml-include requirement ⚠ #1568

Closed
bswck opened this issue Apr 1, 2024 · 22 comments
Closed
Labels
bug triage Trying to make sure if this is valid or not
Milestone

Comments

@bswck
Copy link

bswck commented Apr 1, 2024

Describe the problem

Yesterday, pyyaml-include 2.0 was released. Unsurprisingly for a non-CalVer major release, it stabilized a breaking change: the pyyaml-include's package namespace yamlinclude was moved to yaml_include (ref).

The latest Copier release, 9.1.1, was released prior to removing the pyyaml-include dependency in 037d7f0.
Back then, the pyproject.toml version specification for pyyaml-include was ">=1.2":

pyyaml-include = ">=1.2"

which has been welcoming Copier-incompatible pyyaml-include==2.0 with open arms in every installation since the yesterday's pyyaml-include release.

Copier is now unusable in every environment that installed Copier upon the pyyaml-include 2.0 release (including my today's CI environment that adopts CTT for testing my Copier template), unless one pins the legacy pyyaml-include<2 explicitly as a direct, non-transitive dependency.

Template

Doesn't apply.

To Reproduce

  1. pip install copier
  2. copier

Logs

~/dev/skeleton git:(main|)❯❯❯ copier
Traceback (most recent call last):
  File "/home/bswck/.local/bin/copier", line 5, in <module>
    from copier.__main__ import copier_app_run
  File "/home/bswck/.local/pipx/venvs/copier/lib/python3.10/site-packages/copier/__init__.py", line 6, in <module>
    from .main import *  # noqa: F401,F403
  File "/home/bswck/.local/pipx/venvs/copier/lib/python3.10/site-packages/copier/main.py", line 46, in <module>
    from .subproject import Subproject
  File "/home/bswck/.local/pipx/venvs/copier/lib/python3.10/site-packages/copier/subproject.py", line 15, in <module>
    from .template import Template
  File "/home/bswck/.local/pipx/venvs/copier/lib/python3.10/site-packages/copier/template.py", line 20, in <module>
    from yamlinclude import YamlIncludeConstructor
ModuleNotFoundError: No module named 'yamlinclude'

Expected behavior

No errors.

Screenshots/screencasts/logs

No response

Operating system

Linux

Operating system distribution and version

Ubuntu 23.04

Copier version

9.1.1

Python version

3.10

Installation method

pipx+pypi

Additional context

Workaround: Pinning the legacy pyyaml-include<2 directly.

I strongly suggest pinning all versions properly to prevent unexpected breaking changes in the future and make Copier more reliable.
The current pyproject.toml file might expose Copier to many, many more identical pitfalls.

@bswck bswck added bug triage Trying to make sure if this is valid or not labels Apr 1, 2024
@pawamoy
Copy link
Contributor

pawamoy commented Apr 1, 2024

Upper bounds were explicitly removed before IIRC, because they usually cause conflicts in the Python ecosystem. But since Copier uses Dependabot/Renovatebot, I suppose we could add back upper bounds on major versions. I'd strongly recommend adding some CI jobs to test early on unaccepted versions if we go this route again!

@bswck
Copy link
Author

bswck commented Apr 1, 2024

Upper bounds were explicitly removed before IIRC, because they usually cause conflicts in the Python ecosystem. But since Copier uses Dependabot/Renovatebot, I suppose we could add back upper bounds on major versions. I'd strongly recommend adding some CI jobs to test early on unaccepted versions if we go this route again!

I'd only like to note here that I provided a temporary workaround for troubled users, not a solution for Copier.
The best solution would definitely be either cutting a new release compatible with 2 or, if we're sure that a post-2 version introduces some backward compatibility (which I doubt), specifying !=2.0 in a new Copier release.

@bswck
Copy link
Author

bswck commented Apr 1, 2024

When it comes to my suggestion, when I suggested pinning versions properly, I meant checking for future similar risks and pinning where necessary. This doesn't necessarily mean bringing back conservative and troublesome upper bounds, but rather checking for potential, analogous pitfalls in the future, due to this issue having been filed.

@sisp
Copy link
Member

sisp commented Apr 1, 2024

I'd keep only lower bounds because upper bounds can cause unresolvable dependency trees. Major releases in SemVer-versioned projects may break dependants but often don't. A missing upper bound like in this case can be worked around in userland by adding a constraint – see, e.g., #1225 (comment).

Ideally, we should temporarily upper-bound pyyaml-include, make a patch release to fix the incompatibility that exists now, and remove the upper bound again along with a proper fix afterwards (see, e.g., #1244 and #1229 for a similar situation with Pydantic). But like many open-source projects do we not backport fixes, and the current master branch does not depend on pyyaml-include anymore. I think master is stable. Should we release v9.2.0 ASAP, @yajo?

@sisp
Copy link
Member

sisp commented Apr 1, 2024

@bswck Regarding upper bounds and pinning in Python (libraries): https://iscinumpy.dev/post/bound-version-constraints/

@bswck
Copy link
Author

bswck commented Apr 1, 2024

I'd been fully aware of troubles that come from upper bounds and slowly adapt my projects to best practices.
Again, I don't insist on bringing them back in long term.
Regardless, thanks for recommending a great article though!

@pawamoy
Copy link
Contributor

pawamoy commented Apr 1, 2024

I'd vote for a quick release from master if possible indeed :)

@edgarrmondragon
Copy link
Contributor

edgarrmondragon commented Apr 1, 2024

I'd vote for a quick release from master if possible indeed :)

Second that. The dependency on pyyaml-include was dropped in 037d7f0, but the latest copier release is older than that. So indeed a release from master should fix this issue.

@andrew-glenn
Copy link

Adding that we're pinned to 0.8.30 for other reasons and this affects us as well

blakeNaccarato added a commit to softboiler/boilercv that referenced this issue Apr 1, 2024
@trymzet
Copy link

trymzet commented Apr 1, 2024

To add to #1225 (comment) regarding userland workarounds, with Rye, you can do rye install copier --extra-requirement "pyyaml-include<2"

@krassowski
Copy link

8.x branch is broken too. Would you consider a patch on 8.x for those who cannot upgrade to 9.x yet?

@sisp
Copy link
Member

sisp commented Apr 2, 2024

@krassowski Like many open-source projects, I don't think we have the capacity to backport fixes. But the ultimate decision lies with @yajo.

If I may ask, what's your reason for not being able to upgrade to 9.x?

Edit: Ah, I see you're using it for a Jupyter extension template and upgrading.

@sschrijver-pon
Copy link

sschrijver-pon commented Apr 2, 2024

For those of you who installed copier using pipx, running pipx install copier --force --pip-args "pyyaml-include<2" seems to work.

Edit: in #1225 (comment) I see the pipx inject ... command is used to specify a particular version of a sub-dependency. Is inject preferred? If so, it should be pipx inject copier 'pyyaml-include<2' instead of my suggestion!

@bswck
Copy link
Author

bswck commented Apr 2, 2024

Edit: in #1225 (comment) I see the pipx inject ... command is used to specify a particular version of a sub-dependency. Is inject preferred? If so, it should be pipx inject copier 'pyyaml-include<2' instead of my suggestion!

Yeah, pipx inject copier 'pyyaml-include<2' would ensure that Copier's autonomous pipx venv sees pyyaml-include<2, that's the best approach for pipx IMO.

@FCamborda
Copy link

Any timeline on when we can expect the next release without pyyaml-include?

pwwang added a commit to pwwang/immunopipe that referenced this issue Apr 2, 2024
* tests: init tests

* deps: bump biopipen to 0.27.2

* deps: temporary fix copier breaks with pyyaml-include v2 (copier-org/copier#1568)

* docs: update FAQ.md with instructions for running pipeline on a cluster

* 1.3.3
@henryiii
Copy link
Contributor

henryiii commented Apr 3, 2024

FWIW, pyyaml-include should have had a deprecation period where both namespaces were provided and the one that was to be removed produced a warning. Making a sudden breaking change without warning is not a good idea in the Python ecosystem. There was a beta release, but unless you pass --prerelease, you don't see it (not a bad idea for CI).

@grbaiges
Copy link

grbaiges commented Apr 3, 2024

Hi, we're experiencing this issue with Copier 8.1.0 as well, which was working well until last week. Is there anyway that old Copier versions were modified when doing the new release? Or is it that pyyaml-include was not pinned in old versions of Copier? Is there any Copier version safe from this bug that we can use straight away? This issue is quite blocking

@FCamborda
Copy link

Hi, we're experiencing this issue with Copier 8.1.0 as well, which was working well until last week. Is there anyway that old Copier versions were modified when doing the new release? Or is it that pyyaml-include was not pinned in old versions of Copier? Is there any Copier version safe from this bug that we can use straight away? This issue is quite blocking

You can just install the latest Copier version with a compatible pyyaml-include for now
pip install copier 'pyyaml-include<2'

@grbaiges
Copy link

grbaiges commented Apr 3, 2024

Thanks! I was aware of this, but we're maintaining cross-company Github Actions which use Copier, which we'll already have to upgrade to Copier 9.X once the fix is released (and hope for the best), so I'd rather avoid this workaround in-between.

In other words, I was also asking for a timeline for this fix from Copier's side to make a decision on this (I assume this fix can be quickly done but I'm not sure which are Copier's plans after reading this thread).

@pawamoy
Copy link
Contributor

pawamoy commented Apr 3, 2024

I agree with @henryiii, deprecation periods with warnings are the most reliable way to inform downstream users that something is about to break.

Regarding new releases, @sisp and I agree on releasing a 9.x release quickly, and I believe we have enough privileges to do that (pushing a tag), but @yajo is our captain and we'd like to hear from him first 🙂
For a 8.x release, I'm ready to consider it myself, exceptionally. Again, needs blessing from @yajo.

pwwang added a commit to pwwang/biopipen that referenced this issue Apr 3, 2024
* deps: temporary fix copier breaks with pyyaml-include v2 (copier-org/copier#1568)

* deps: bump pipen-poplog to 0.1.1

* deps: bump pipen-poplog to 0.1.2

* 0.27.3

* choir(scrna.ScFGSEA): Skip cases when no cells found (pwwang/immunopipe#50)

* choir(scrna.MarkersFinder): Skip cases when no cells found (pwwang/immunopipe#50)

* choir(scrna.MetaMarkers): Skip cases when no cells found (pwwang/immunopipe#50)

* docs: update CHANGELOG

* feat(scrna.SeuratPreparing): support DoubletFinder

* docs: update CHANGELOG
teddygroves added a commit to teddygroves/bibat that referenced this issue Apr 4, 2024
See <copier-org/copier#1568>.

This should be fixed in a new version of copier in the next few days.
@yajo
Copy link
Member

yajo commented Apr 4, 2024

Hello! Sorry everybody. Since some weeks ago, gmail decided that copier issues were spam and I lost every message 😆

FWIW one advantage of not pinning versions is that we let you pin them downstream. Whatever packaging system you use, it should have a way to downgrade a package. Just do it and it will serve as a workaround for older releases.

Let me push a new release without pyyaml-include, which we don't require in master anymore.

@yajo
Copy link
Member

yajo commented Apr 4, 2024

Fixed in https://github.com/copier-org/copier/releases/tag/v9.2.0.

@yajo yajo closed this as completed Apr 4, 2024
@sisp sisp added this to the Soon milestone Apr 18, 2024
xaf added a commit to xaf/qolsysgw that referenced this issue Apr 27, 2024
End-to-end tests were broken because of the release of pyyaml-include 2.0 and a too loose dependency in copier. This was fixed in copier 9.2.0. See copier-org/copier#1568
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug triage Trying to make sure if this is valid or not
Projects
None yet
Development

No branches or pull requests