freedomofpress / securedrop-builder

Packaging logic for building SecureDrop-related Debian packages
GNU General Public License v3.0
7 stars 11 forks source link

Add support for building packages that use Poetry #467

Closed eloquence closed 10 months ago

eloquence commented 1 year ago

Fixes #459

This PR adds optional support for building packages that use Poetry. It includes some other minor changes:

I've used black code formatting in anticipation of making this part of our CI tooling for this repo. It pulls in @legoktm's work in the wip/poetry branch.

Test plan

General prep

Because you'll be hopping back and forth between securedrop-proxy and securedrop-builder, I recommend using two separate terminals during this testing process.

You'll need Poetry itself. Install the latest version (1.6.1 as of this writing). It offers several installation methods. I favor the pipx method (https://python-poetry.org/docs/#installing-with-pipx), because pipx itself is useful to have around and serves a different purpose than Poetry (it lets you safely install CLI tools written in Python). I've tested these changes with Python 3.9 in a Debian 11 (Bullseye) VM.

You'll be building debs repeatedly during this process. Every time you successfully build a package, I recommend stashing it away under a descriptive name like with-poetry.deb. That way, you can use a tool like diffoscope later to compare any changes between packages.

Verify no regressions for projects using requirements.txt

  1. Ensure you have local checkouts of securedrop-proxy at main and securedrop-builder at poetry-support (this PR). The instructions below assume that they share a parent directory.
  2. Add a dependency of your choosing to securedrop-proxy locally. For example, in the securedrop-proxy checkout:
  1. In the securedrop-builder directory (ideally in a separate terminal), activate its venv (make install-deps && source .venv/bin/activate).
  2. Build the new wheel using PKG_DIR=../securedrop-proxy/ make build-wheels
    • [x] Confirm no build errors
    • [x] Verify that generated sha256sums.txt contains correct and expected checksum for the newly built wheel
  3. Sign the new sha256sums.txt: gpg --armor --output securedrop-proxy/sha256sums.txt.asc --detach-sig securedrop-proxy/sha256sums.txt
  4. Update the build-requirements.txt in securedrop-proxy by running PKG_DIR=../securedrop-proxy/ make requirements
    • [x] Confirm build-requirements.txt is updated with correct checksum for the new wheel
  5. Build a new securedrop-proxy package with PKG_PATH=../securedrop-proxy/ make securedrop-proxy
    • [x] Confirm that package is built successfully

Verify expected behavior for projects using Poetry

  1. Ensure you have local checkouts of securedrop-proxy at only-poetry and securedrop-builder at poetry-support-with-proxy-changes
  2. Examine the additional commit (relative to this PR) in securedrop-builder with git show. Note that the build-requirements.txt location is adjusted to be in the root of the repository. This change is necessary to build the repository with the changes in the only-poetry branch successfully, where this file has been moved to the root for simplicity.
  3. Delete any stale .venv directory in securedrop-proxy, undo any changes from previous testing, and ensure your venv is not active.
  4. Run poetry install to install the dependencies
  5. Run make test
    • [x] Observe that tests are passing
  6. Activate the securedrop-builder venv: make install-deps && source .venv/bin/activate - in future this too will be done via Poetry: #468
  7. Run PKG_PATH=../securedrop-proxy/ make securedrop-proxy
    • [x] Observe that you can build the package
  8. Let's add a new dependency over in securedrop-proxy. For example, poetry add cowsay==6.0.
  9. Back in securedrop-builder, let's try building the new wheel: PKG_DIR=../securedrop-proxy/ make build-wheels
    • [x] Observe that the build completes successfully and new wheels exist in the securedrop-proxy/wheels directory
    • [x] Observe that sha256sums has been updated with the new wheel checksums (git diff)
  10. Sign the new shasums: gpg --armor --output securedrop-proxy/sha256sums.txt.asc --detach-sig securedrop- proxy/sha256sums.txt
  11. This wheel won't be used yet until we update build-requirements.txt over in securedrop-proxy. Let's do that with PKG_DIR=../securedrop-proxy make requirements.
    • [x] Observe that build-requirements.txt in securedrop-proxy was updated successfully.
  12. Try building a package with the new wheel back in securedrop-builder: PKG_PATH=../securedrop-proxy/ make securedrop-proxy
    • [x] Examine that the package does include the dependency you added (diffoscope is nice for it if you have a previous .deb to compare with).
eloquence commented 11 months ago

The only other comment I have is whether it makes sense to keep the requirements/ folder since it'll now contain just one item, the build-requirements.txt file. I think we could move it into the root now?

Makes sense, I'll do that in the only-poetry branch and then update here as well.

legoktm commented 11 months ago

I was able to successfully run through the whole test plan on bullseye (bookworm is busted because of the pyyaml wheels issue). I think the main issue is figuring out how we want to handle back-compat of CI pipelines that currently don't activate the venv - either have them activate the venv before this lands or write in extra back-compat in this PR so it gracefully works.

Regarding the:

-REQUIREMENTS_FILE=requirements/build-requirements.txt
+REQUIREMENTS_FILE=build-requirements.txt

change, I'm wondering if you want to just have that be a PR that we coordinate merging at the same time, or write some temporary if exists/else logic in the make file to detect the correct path that can be removed post-poetryification.

Other than that, I think we're very close!!

eloquence commented 11 months ago

Thanks @legoktm, I think I've addressed your comments, but let me know if not! I've opened a new draft PR with just the build-requirements.txt change, so we can merge this together with the securedrop-proxy changes after this main compat PR lands.

eloquence commented 10 months ago

Maximally squashed :)

legoktm commented 10 months ago

Awesome :) Let's do it!