Build Rust redwood wheel during packaging process #6884
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Leaving as WIP because we need to figure out whether we want to re-use upstream maturin wheels, build it from source, or what. And whether we need to audit dependencies of that too. |
legoktm
force-pushed
the
stg-rust-packaging
branch
2 times, most recently
from
a68c6a7 to
39424d3
Compare
legoktm
force-pushed
the
stg-rust-packaging
branch
from
39424d3 to
91a7289
Compare
|
I realized the build wasn't reproducible because we weren't building with the lockfile and using |
25 tasks
The test to verify custom_logo.png was not included in the package was using the wrong path and missed that if a custom_logo.png existed in the local source tree, it would be included in the package (not an issue for releases since we always use a clean source tree). Fix the inclusion by deleting any custom logo if it exists.
dh-virtualenv is responsible for creating the virtualenv, installing dependencies from requirements.txt, and then munging the virtualenv to use the path that it will be installed to. There are a number of issues with it however: * largely unmaintained in Debian (missed focal) * incompatible with debhelper compat 12 * cannot use --require-hashes * hides deprecation warnings and error messages It is now getting in the way of Rust packaging since we want to install a wheel into the virtualenv, however it needs to happen before the munging step, which dh-virtualenv doesn't really support. Since we already want to get rid of it, let's just do it now. Fixes #6538.
legoktm
force-pushed
the
stg-rust-packaging
branch
2 times, most recently
from
c47f0fe to
00c0b6e
Compare
cfm
requested changes
Aug 8, 2023
There was a problem hiding this comment.
This looks great, Kunal. In addition to the test plan, I've left one request inline, plus:
-
make devsucceeds, including building libredwood via your scripting rather than maturin. - This branch isn't fully signed.
- Noting for the record: 8f884ea removes
build-requirements.{in,txt}, because they were added in Add "redwood" Sequoia Rust/Python bridge #6828 exclusively to pull inmaturin.
redwood/build-wheel.py
Outdated
Comment on lines
53
to
56
| # Yes the name is wrong, but it doesn't matter, this is only an intermediate step anyways | ||
| whl = Path("redwood-0.1.0-py3-none-any.whl") |
There was a problem hiding this comment.
Can you clarify this comment? I can guess which name you're referring to, how it's wrong, and why it doesn't matter; but I can't actually tell for sure from the comment. :-)
maturin is really great, but has a significant dependency tree that doesn't really make sense for us to review and figure out how to install into the builder environment right now. It is relatively straightforward to build the wheel ourselves using only setuptools, we just need to copy the Rust-built libredwood.so into the right place in the Python package (see <https://pyo3.rs/v0.19.0/building_and_distribution.html#manual-builds>). The new build-wheel.py script does that and produces a functionally equivalent wheel to what maturin would have generated. Probably we should switch back to maturin once it's packaged in Debian (https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=999850).
Use our script to build the redwood wheel and then install it into the virtualenv shipped in the Debian package. A testinfra check is added that verifies the redwood wheel is importable and is able to generate a key pair. Fixes #6817.
legoktm
force-pushed
the
stg-rust-packaging
branch
from
00c0b6e to
42ff407
Compare
cfm
approved these changes
Aug 8, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Status
Ready for review
Description of Changes
Fixes #6538.
Fixes #6817.
Testing
How should the reviewer test this PR?
Largely focusing on the dh-virtualenv change.
cp securedrop/static/i/logo.png securedrop/static/i/custom_logo.png(this should still be excluded from the build)make build-debshttps://apt.freedom.press/pool/main/s/securedrop/securedrop-app-code_2.6.0%2Bfocal_amd64.deb, then rundebdiffbetween the two files, you should see https://gist.github.com/legoktm/170f170e01437ba49671306df4425912/opt/venvs/securedrop-app-code/lib/python3.8/site-packages/securedrop_app_code-2.6.0-py3.8.egg-info/is gone, we're no longer installing an empty securedrop-app-code Python package because dh_virtualenv mandates it.For the Rust stuff:
make build-debslog from earlier, verify there's no garbage output (escape codes or progress bars) caused by Rust/cargo stuffVerify Rust reproducibility:
make build-debsto something likefirst.debmake build-debs, save the securedrop-app-code deb as something likesecond.debar x first.deb, thentar xvf data.tar.xzsha256sum opt/venvs/securedrop-app-code/lib/python3.8/site-packages/redwood/redwood.cpython-38-x86_64-linux-gnu.so- the checksum should be152d90961bf1cf44fcbc5c0275e31c68ca6a16f1b0c42895fe9272a53e2f1685for both.Deployment
Any special considerations for deployment? Just the packaging changes.
Checklist
make lint) and tests (make test) pass in the development container