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

CI should verify a new wheel in localwheels by rebuilding #241

Closed
kushaldas opened this issue Apr 7, 2021 · 6 comments · Fixed by #243
Closed

CI should verify a new wheel in localwheels by rebuilding #241

kushaldas opened this issue Apr 7, 2021 · 6 comments · Fixed by #243
Assignees

Comments

@kushaldas
Copy link
Contributor

When a maintainer will update the localwheels directory with newer version of wheels or source tarballs, the CI should verify those by rebuilding them.

Why do we need this?

This CI step will verify that the sha256sum of any new wheel is exactly the same if the CI rebuilds it. Right now we have checks to verify if we generate wheels twice, both are same or not. But, we are missing the check to verify that the wheel sha256sum is the same of what the maintainer pushed manually in the PR.

@kushaldas kushaldas added this to In Development in SecureDrop Team Board Apr 7, 2021
@conorsch
Copy link
Contributor

conorsch commented Apr 7, 2021

Fair point, let's add that. A job that runs:

  1. ./scripts/build-sync-wheels --clobber -p <package>, then
  2. git diff --exit-code

should be enough. We're already doing 1) in the reprotest jobs. We may be able to add a git-diff check inside that logic, or else in a new job.

@sssoleileraaa
Copy link
Contributor

sssoleileraaa commented Apr 7, 2021

This new CI job is a good idea, especially because the jobs that produces the same set of wheels (reprotest-wheels) and debs (reprotest-debs) twice are currently unable to catch #231. We will probably need to update them in order to resolve that issue, but also it seems like this new CI job could potentially catch that issue.

@conorsch
Copy link
Contributor

conorsch commented Apr 8, 2021

Seeing some variability in the CI environments, where the wheel builds don't match what we've got version-controlled. Here's an example from a wheel build for securedrop-proxy, run within the CI environment (pyyaml1.whl is vc, pyyaml2.whl was built in CI):

(.venv) circleci@3a44d5a38092:~/project$ diffoscope /tmp/compare-wheels/pyyaml{1,2}.whl
--- /tmp/compare-wheels/pyyaml1.whl
+++ /tmp/compare-wheels/pyyaml2.whl
├── zipinfo /dev/stdin
│ @@ -1,10 +1,11 @@
│ -Zip file size: 45644 bytes, number of entries: 23
│ +Zip file size: 467690 bytes, number of entries: 24
│  -rw-r--r--  2.0 unx     1402 b- defN 11-Jun-29 20:23 _yaml/__init__.py
│  -rw-r--r--  2.0 unx    13170 b- defN 11-Jun-29 20:23 yaml/__init__.py
│ +-rwxr-xr-x  2.0 unx  1441760 b- defN 11-Jun-29 20:23 yaml/_yaml.cpython-37m-x86_64-linux-gnu.so
│  -rw-r--r--  2.0 unx     4883 b- defN 11-Jun-29 20:23 yaml/composer.py
│  -rw-r--r--  2.0 unx    28639 b- defN 11-Jun-29 20:23 yaml/constructor.py
│  -rw-r--r--  2.0 unx     3851 b- defN 11-Jun-29 20:23 yaml/cyaml.py
│  -rw-r--r--  2.0 unx     2837 b- defN 11-Jun-29 20:23 yaml/dumper.py
│  -rw-r--r--  2.0 unx    43006 b- defN 11-Jun-29 20:23 yaml/emitter.py
│  -rw-r--r--  2.0 unx     2533 b- defN 11-Jun-29 20:23 yaml/error.py
│  -rw-r--r--  2.0 unx     2445 b- defN 11-Jun-29 20:23 yaml/events.py
│ @@ -17,9 +18,9 @@
│  -rw-r--r--  2.0 unx    51277 b- defN 11-Jun-29 20:23 yaml/scanner.py
│  -rw-r--r--  2.0 unx     4165 b- defN 11-Jun-29 20:23 yaml/serializer.py
│  -rw-r--r--  2.0 unx     2573 b- defN 11-Jun-29 20:23 yaml/tokens.py
│  -rw-r--r--  2.0 unx     1101 b- defN 11-Jun-29 20:23 PyYAML-5.4.1.dist-info/LICENSE
│  -rw-r--r--  2.0 unx     2087 b- defN 11-Jun-29 20:23 PyYAML-5.4.1.dist-info/METADATA
│  -rw-r--r--  2.0 unx      104 b- defN 11-Jun-29 20:23 PyYAML-5.4.1.dist-info/WHEEL
│  -rw-r--r--  2.0 unx       11 b- defN 11-Jun-29 20:23 PyYAML-5.4.1.dist-info/top_level.txt
│ -?rw-rw-r--  2.0 unx     1688 b- defN 11-Jun-29 20:23 PyYAML-5.4.1.dist-info/RECORD
│ -23 files, 224745 bytes uncompressed, 43018 bytes compressed:  80.9%
│ +?rw-rw-r--  2.0 unx     1790 b- defN 11-Jun-29 20:23 PyYAML-5.4.1.dist-info/RECORD
│ +24 files, 1666607 bytes uncompressed, 464904 bytes compressed:  72.1%
├── zipnote {}
│ @@ -1,13 +1,16 @@
│  Filename: _yaml/__init__.py
│  Comment: 
│  
│  Filename: yaml/__init__.py
│  Comment: 
│  
│ +Filename: yaml/_yaml.cpython-37m-x86_64-linux-gnu.so
│ +Comment: 
│ +
│  Filename: yaml/composer.py
│  Comment: 
│  
│  Filename: yaml/constructor.py
│  Comment: 
│  
│  Filename: yaml/cyaml.py
├── PyYAML-5.4.1.dist-info/RECORD
│ @@ -1,9 +1,10 @@
│  _yaml/__init__.py,sha256=04Ae_5osxahpJHa3XBZUAf4wi6XX32gR8D6X6p64GEA,1402
│  yaml/__init__.py,sha256=gfp2CbRVhzknghkiiJD2l6Z0pI-mv_iZHPSJ4aj0-nY,13170
│ +yaml/_yaml.cpython-37m-x86_64-linux-gnu.so,sha256=CCptPeSMY96_MeGyZdPWBQBWMf9Qq8lffcOqpYuuixY,1441760
│  yaml/composer.py,sha256=_Ko30Wr6eDWUeUpauUGT3Lcg9QPBnOPVlTnIMRGJ9FM,4883
│  yaml/constructor.py,sha256=kNgkfaeLUkwQYY_Q6Ff1Tz2XVw_pG1xVE9Ak7z-viLA,28639
│  yaml/cyaml.py,sha256=6ZrAG9fAYvdVe2FK_w0hmXoG7ZYsoYUwapG8CiC72H0,3851
│  yaml/dumper.py,sha256=PLctZlYwZLp7XmeUdwRuv4nYOZ2UBnDIUy8-lKfLF-o,2837
│  yaml/emitter.py,sha256=jghtaU7eFwg31bG0B7RZea_29Adi9CKmXq_QjgQpCkQ,43006
│  yaml/error.py,sha256=Ah9z-toHJUbE9j-M8YpxgSRM5CgLCcwVzJgLLRF2Fxo,2533
│  yaml/events.py,sha256=50_TksgQiE4up-lKo_V-nBy-tAIxkIPQxY5qDhKCeHw,2445

That is interesting. Could a system-level dependency cause that difference?

@kushaldas
Copy link
Contributor Author

That is interesting. Could a system-level dependency cause that difference?

Yes, do we have that CI based wheel somewhere? I can dig into that tomorrow (unless you already solved it).

@conorsch
Copy link
Contributor

No, I didn't save it, the diff output from above is from an interactive SSH session in the CircleCI environment. The fact that yaml/_yaml.cpython-37m-x86_64-linux-gnu.so is in the CI-built wheel, but not in our version-controlled wheel:

$ unzip -l localwheels/PyYAML-5.4.1-cp37-cp37m-linux_x86_64.whl
Archive:  localwheels/PyYAML-5.4.1-cp37-cp37m-linux_x86_64.whl
  Length      Date    Time    Name
---------  ---------- -----   ----
     1402  2011-06-29 20:23   _yaml/__init__.py
    13170  2011-06-29 20:23   yaml/__init__.py
     4883  2011-06-29 20:23   yaml/composer.py
    28639  2011-06-29 20:23   yaml/constructor.py
     3851  2011-06-29 20:23   yaml/cyaml.py
     2837  2011-06-29 20:23   yaml/dumper.py
    43006  2011-06-29 20:23   yaml/emitter.py
     2533  2011-06-29 20:23   yaml/error.py
     2445  2011-06-29 20:23   yaml/events.py
     2061  2011-06-29 20:23   yaml/loader.py
     1440  2011-06-29 20:23   yaml/nodes.py
    25495  2011-06-29 20:23   yaml/parser.py
     6794  2011-06-29 20:23   yaml/reader.py
    14184  2011-06-29 20:23   yaml/representer.py
     8999  2011-06-29 20:23   yaml/resolver.py
    51277  2011-06-29 20:23   yaml/scanner.py
     4165  2011-06-29 20:23   yaml/serializer.py
     2573  2011-06-29 20:23   yaml/tokens.py
     1101  2011-06-29 20:23   PyYAML-5.4.1.dist-info/LICENSE
     2087  2011-06-29 20:23   PyYAML-5.4.1.dist-info/METADATA
      104  2011-06-29 20:23   PyYAML-5.4.1.dist-info/WHEEL
       11  2011-06-29 20:23   PyYAML-5.4.1.dist-info/top_level.txt
     1688  2011-06-29 20:23   PyYAML-5.4.1.dist-info/RECORD
---------                     -------
   224745                     23 files

is certainly surprising. I experimented briefly with installing packages like python3-yaml but wasn't able to nail down the root cause of the variation.

@kushaldas
Copy link
Contributor Author

@conorsch this was happening as libyaml-dev package is present in the CI and the wheel is getting the proper extension built. I added a PR #246 which contains the same wheel. Also adds the libyaml-0-2 as a packaging dependency to the securedrop-proxy package.

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.

3 participants