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

[securedrop-proxy] bookworm CI is failing because of pyyaml incompatibility with cython 3 #1681

Closed
legoktm opened this issue Jul 31, 2023 · 4 comments · Fixed by #1718
Closed

Comments

@legoktm
Copy link
Member

legoktm commented Jul 31, 2023

There are no Python 3.11 wheels for pyyaml 5.4.1 so we've been building it from source in CI, but that's now broken with the release of Cython 3. There's a good amount of discussion in yaml/pyyaml#736 and yaml/pyyaml#728.

Our options forwards are:

  1. Update pyyaml to 6.0.1, which has py3.11 wheels
  2. Drop pyyaml, c.f. [securedrop-proxy] Drop PyYAML dependency somehow #1684
  3. Some hack to pin cython<3
@Uzume
Copy link

Uzume commented Aug 1, 2023

The issue is tracked at: yaml/pyyaml#601 (and yaml/pyyaml#724 for 5.x).

Instead of (option 3. above):

You could also apply a different hack that lets pyyaml work with Cython 3+ such as:

  1. Fix builds with Cython 3 yaml/pyyaml#731 (and formerly WIP keep PyYAML build working on Cython3.0.0a10+ yaml/pyyaml#602)
  2. Make setup.py work with Cython 3 and Cython 0.29 yaml/pyyaml#729.

The first seems work by finding the stuff Cython 3+ decided to hide better (in cython/cython#4498) but this might break later (as presumably Cython is hiding this better as it intends to jettison this at some point; likely during the upcoming setuptools purge). This is all part of distutils which has been deprecated in Python 3.10 and is schedule to be removed in Python 3.12 in October of this year. The second will probably work later because it directly invokes Cython, no longer relying on cython_sources (unfortunately it also always requires Cython even when a prebuilt wheel exists) however it always depends on Cython even when installing prebuilt wheels (I believe Cython documents a better version of this solution which does not require such).

Currently pyyaml has opted to not merge any of the above, instead opting to pin Cython < 3 for the yaml/pyyaml#702 applied workaround in pyyaml 6.0.1+ and the yaml/pyyaml#736 instructions which essentially has one do this same workaround oneself for pyyaml < 6.0.1.

@legoktm
Copy link
Member Author

legoktm commented Aug 1, 2023

Thanks @Uzume! We already do build our own cython + pyyaml wheels for production installs (see https://github.com/freedomofpress/securedrop-builder/tree/main/securedrop-proxy/wheels) so this is just about unbreaking CI, which I'd like to avoid too many temporary hacks for...

The one piece of context that's relevant here is that in production deployments we're still on bullseye, so it's ok if bookworm CI is failing for a while whether because we're waiting on upstream or just dropping the dependency.

@Uzume
Copy link

Uzume commented Aug 1, 2023

@legoktm: pyyaml will soon be forced to make some major changes anyway. Later this year distutils is going away in Python 3.12 and the current "pin Cython < 3" for yaml/pyyaml#702 in 6.0.1 is problematic for Python 3.13 (apparently somewhere in Cython < 3 they depend on cgi which goes away in Python 3.13).

@legoktm
Copy link
Member Author

legoktm commented Aug 1, 2023

Oooh, gotcha. Honestly then it seems like #1684 (removing pyyaml) is the only thing really worth spending time on.

@zenmonkeykstop zenmonkeykstop changed the title bookworm CI is failing because of pyyaml incompatibility with cython 3 [securedrop-proxy] bookworm CI is failing because of pyyaml incompatibility with cython 3 Dec 13, 2023
@zenmonkeykstop zenmonkeykstop transferred this issue from freedomofpress/securedrop-proxy Dec 13, 2023
legoktm added a commit that referenced this issue Feb 9, 2024
These are failing because of <#1681>.

Our plan is to just replace them entirely with proxy v2, so just
skip these jobs for now so commits can at least get a green check again.
rocodes pushed a commit that referenced this issue Feb 12, 2024
These are failing because of <#1681>.

Our plan is to just replace them entirely with proxy v2, so just
skip these jobs for now so commits can at least get a green check again.
legoktm added a commit that referenced this issue Feb 14, 2024
These are failing because of <#1681>.

Our plan is to just replace them entirely with proxy v2, so just
skip these jobs for now so commits can at least get a green check again.
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 a pull request may close this issue.

2 participants