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

Make targets build-debs[-notest] fails on new circular dependency between PyPI packages flit_core and tomli #6137

Closed
cfm opened this issue Oct 12, 2021 · 2 comments · Fixed by #6141

Comments

@cfm
Copy link
Member

cfm commented Oct 12, 2021

Description

Since October 10, staging-test-with-rebase fails in make build-debs-notest. This is likely to bork make build-debs for the current production release cycle too.

Expected Behavior

https://app.circleci.com/pipelines/github/freedomofpress/securedrop/3075/workflows/6c173f25-a21a-40f6-a476-7b6f9c996b2b/jobs/57150

Actual Behavior

https://app.circleci.com/pipelines/github/freedomofpress/securedrop/3076/workflows/4a156c36-7049-4645-a6a8-c5583f658e99/jobs/57152

Comments

This looks like pypa/flit#451, for which the resolution may be to update pip ≥ 21.

@cfm cfm added this to the 2.1.0 milestone Oct 12, 2021
@cfm cfm changed the title Make targets build-debs[-notest] fails on new circular dependency between PyPI packages flit and tomli Make targets build-debs[-notest] fails on new circular dependency between PyPI packages flit_core and tomli Oct 12, 2021
@cfm cfm assigned cfm and zenmonkeykstop and unassigned cfm Oct 12, 2021
@cfm cfm self-assigned this Oct 13, 2021
@cfm
Copy link
Member Author

cfm commented Oct 13, 2021

To be exact: flit/flit_core 3.4.0 switched its TOML parser from toml, which has no dependencies, to tomli, which requires flit_core.

Dependency graph (excluding version constraints):
graph TD

setuptools-rust --> setuptools
setuptools-rust --> setuptools_scm["setuptools_scm[toml]"]
setuptools_scm --> setuptools
setuptools_scm --> tomli
tomli --> flit_core
flit_core --> tomli

stg-6137-translation-requirements is now testing whether pip ≥ 21 can handle this circular dependency. We'll then have to decide whether that upgrade is acceptable (pending a diff review) at this point in the release process.

Meanwhile, stg-6137-translation-requirements-2 is testing whether pinning flit_core<3.4 in our top-level translation-requirements.in will satisfy the version constraints of the setuptools-rust dependency graph.

@conorsch
Copy link
Contributor

Summarizing synchronous discussion from today. We basically have two options here:

  1. Stop building all python reqs from source, and fetch wheels from pypi. Doing so avoids the dependency conflicts in the build-deps described above, but involves a significant shift in our dependency management policies, so we consider it a last resort.
  2. Update pip within the build environment from Focal's system version, 20.0.2-5ubuntu1.6, to PyPI version 21.3. This is also a slight departure, but it unbreaks the build, so we're happy with it for now.

Option 2, then, is our path forward. It's got some shortcomings in that we cannot pin the hash in all places, particularly in the debian/rules file used by dh-virtualenv. Punting on a larger refactor of the build logic until we're out of QA period. As part of the pip update, I will not be submitting a diff review, since pip is a foundational tool in the Python ecosystem, and re-reviewing that widely used dependency feels superfluous.

I'll aim to have a PR up by EOD, then toss to @cfm for review and backporting!

conorsch pushed a commit that referenced this issue Oct 13, 2021
Fixes #6137. Stop using distro pip from Focal (20.0.2-5ubuntu1.6)
for managing the virtualenvs used for the securedrop-app-code package
builds; instead, use 21.3 from PyPI. We can only update the hash in the
in-line pip install actions for the temporary i18n venv. For the
debian/rules invocation via dh-virtualenv, specifying a checksum for pip
is not possible. Oh, well: we were already not pinning the hash on
setuptools-scm in the debian/rules file.

Also updated the pip dependencies pinned in the various requirements
files, e.g. `{docker,develop}-requirements`, for consistency's sake,
but note those changes don't unbreak package builds.
@cfm cfm closed this as completed in #6141 Oct 14, 2021
cfm pushed a commit that referenced this issue Oct 14, 2021
Fixes #6137. Stop using distro pip from Focal (20.0.2-5ubuntu1.6)
for managing the virtualenvs used for the securedrop-app-code package
builds; instead, use 21.3 from PyPI. We can only update the hash in the
in-line pip install actions for the temporary i18n venv. For the
debian/rules invocation via dh-virtualenv, specifying a checksum for pip
is not possible. Oh, well: we were already not pinning the hash on
setuptools-scm in the debian/rules file.

Also updated the pip dependencies pinned in the various requirements
files, e.g. `{docker,develop}-requirements`, for consistency's sake,
but note those changes don't unbreak package builds.

(cherry picked from commit 89063e4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants