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

Build wheels with build #240

Merged
merged 4 commits into from
Apr 7, 2021
Merged

Build wheels with build #240

merged 4 commits into from
Apr 7, 2021

Conversation

sssoleileraaa
Copy link
Contributor

Description

Resolves #239

You can see there are 4 changes here:

  • We now allow all devs on the team to sign wheels. You can see that our pubkeys have been added in the first commit.
  • There are three new wheels. The versions are the same but there is a variation between packages that link to .so files that are built with build (our new packaging tool) versus pip. @conorsch plz verify or correct me if I am mistaken.
  • The updated sha256sums file with my signature this time
  • A small fix to support projects with a requirments directory. Without this a new build-requirements.txt file is created in securedrop-client when it already exists in requirements/build-requirements.txt.

Test Plan

@conorsch
Copy link
Contributor

conorsch commented Apr 5, 2021

The versions are the same but there is a variation between packages that link to .so files that are built with build (our new packaging tool) versus pip. @conorsch plz verify or correct me if I am mistaken.

That's right, we expect the wheels containing .so files to change slightly. It looks like the path that build uses has changed slightly. For those who want to inspect themselves, see below:

$ mkdir compare-wheels
$ git checkout main
Already on 'main'
Your branch is up to date with 'origin/main'.
$ cp localwheels/SQLAlchemy-1.3.3-cp37-cp37m-linux_x86_64.whl compare-wheels/a.whl
$ git checkout build-wheels-with-build
Switched to branch 'build-wheels-with-build'
Your branch is up to date with 'origin/build-wheels-with-build'.
$ cp localwheels/SQLAlchemy-1.3.3-cp37-cp37m-linux_x86_64.whl compare-wheels/b.whl
$ diffoscope compare-wheels/{a,b}.whl | grep /tmp/ | head
│ │ -    <15>   DW_AT_comp_dir    : (indirect string, offset: 0x22c6): /tmp/pip-wheel-build/sqlalchemy
│ │ +    <15>   DW_AT_comp_dir    : (indirect string, offset: 0x16af): /tmp/pip-wheel-build/SQLAlchemy-1.3.3
│ │ -/tmp/pip-wheel-build/sqlalchemy/lib/sqlalchemy/cextension/processors.c:443
│ │ +/tmp/pip-wheel-build/SQLAlchemy-1.3.3/lib/sqlalchemy/cextension/processors.c:443
│ │ -/tmp/pip-wheel-build/sqlalchemy/lib/sqlalchemy/cextension/processors.c:444
│ │ +/tmp/pip-wheel-build/SQLAlchemy-1.3.3/lib/sqlalchemy/cextension/processors.c:444
│ │ -/tmp/pip-wheel-build/sqlalchemy/lib/sqlalchemy/cextension/processors.c:444 (discriminator 1)
│ │ +/tmp/pip-wheel-build/SQLAlchemy-1.3.3/lib/sqlalchemy/cextension/processors.c:444 (discriminator 1)
│ │ -/tmp/pip-wheel-build/sqlalchemy/lib/sqlalchemy/cextension/processors.c:445
│ │ +/tmp/pip-wheel-build/SQLAlchemy-1.3.3/lib/sqlalchemy/cextension/processors.c:445

Right now CI is failing on these changes, because of the mismatch between the reqs files on the various component repos. By way of example, I've pushed a "TEMPORARY" commit to securedrop-proxy freedomofpress/securedrop-proxy#85 (comment) which uses this branch, and CI has gone green over there. 👍

@sssoleileraaa sssoleileraaa marked this pull request as ready for review April 5, 2021 23:31
Copy link
Contributor

@kushaldas kushaldas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test Plan

Next steps

After we merge those two PRs, we can push more changes to the CI steps in this PR so that it becomes green. We can then merge this PR.

@conorsch
Copy link
Contributor

conorsch commented Apr 7, 2021

I've merged the two component PRs consuming the newly rebuilt wheels:

Then re-ran CI here, and it's already entirely green.

After we merge those two PRs, we can push more changes to the CI steps in this PR so that it becomes green.

Not necessary! CI is already passing. Looks like recent changes in 18770bd have accommodated. Let's keep a close eye on nightly builds, so those job configs are slightly different, and patch as necessary.

The most problematic aspect of this workflow is that I had to override CI in the component repos in order to merge. That's not ideal, but since the CI configs in the various repos reference each other's main branches, it's the best we can do. Let's discuss marking the build jobs as optional, so maintainers can override as they see fit.

@conorsch conorsch self-requested a review April 7, 2021 00:28
@conorsch conorsch merged commit 0b904f2 into main Apr 7, 2021
@kushaldas kushaldas deleted the build-wheels-with-build branch April 7, 2021 07:03
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 this pull request may close these issues.

Regenerate wheels and build-requirements.txt files for projects
3 participants