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

Fix build process producing wheels with incorrect RECORD #2671

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

ttm56p
Copy link
Contributor

@ttm56p ttm56p commented Nov 27, 2024

Problem

shutil.move("wheelhouse", self.dist_dir) nests wheelhouse directory itself inside of dist/.tmp-XXX instead of moving its contents there.
That results in wheel with incorrect RECORD contents to end up in dist folder, which is then published.

Solution

Move wheel with drivers and incorrect RECORD to wheelhouse and make auditwheel to put corrected wheel to dist/.tmp-XXX.
Then remove wheelhouse folder.

Build process snapshots

Below are snapshots of dist and wheelhouse folder contents for comparison.

Note: hashes were shrunk and some output removed for readability sake

Old setup.py

@@@ before-auditwheel
d79a094  ./dist/.tmp-bz7ar00j/playwright-1.49.1.dev2+g3f04396.d20241126-py3-none-manylinux1_x86_64.whl

@@@ after-auditwheel
ee22f99  ./wheelhouse/playwright-1.49.1.dev2+g3f04396.d20241126-py3-none-manylinux1_x86_64.whl
d79a094  ./dist/.tmp-bz7ar00j/playwright-1.49.1.dev2+g3f04396.d20241126-py3-none-manylinux1_x86_64.whl

@@@ before-end
ee22f99  ./dist/.tmp-bz7ar00j/wheelhouse/playwright-1.49.1.dev2+g3f04396.d20241126-py3-none-manylinux1_x86_64.whl
d79a094  ./dist/.tmp-bz7ar00j/playwright-1.49.1.dev2+g3f04396.d20241126-py3-none-manylinux1_x86_64.whl

When python -mbuild --wheel finished:

$ find ./dist -type f -exec sha256sum {} \;
d79a094  ./dist/playwright-1.49.1.dev2+g3f04396.d20241126-py3-none-manylinux1_x86_64.whl

New setup.py

@@@ before-auditwheel
e593879  ./dist/.tmp-7np0n0up/playwright-1.49.1.dev2+g3f04396.d20241127-py3-none-manylinux1_x86_64.whl

@@@ after-auditwheel
72899a7  ./dist/.tmp-7np0n0up/playwright-1.49.1.dev2+g3f04396.d20241127-py3-none-manylinux1_x86_64.whl
e593879  ./wheelhouse/playwright-1.49.1.dev2+g3f04396.d20241127-py3-none-manylinux1_x86_64.whl

@@@ before-end
72899a7  ./dist/.tmp-7np0n0up/playwright-1.49.1.dev2+g3f04396.d20241127-py3-none-manylinux1_x86_64.whl

When python -mbuild --wheel finished:

$ find ./dist -type f -exec sha256sum {} \;
72899a7  ./dist/playwright-1.49.1.dev2+g3f04396.d20241127-py3-none-manylinux1_x86_64.whl

Instrumented code

        def snapshot_fs(label, *dirs):
            print('@@@', label)
            lines = subprocess.check_output(['find', *dirs, '-type', 'f', '-exec', 'sha256sum', '{}', ';'], encoding='utf-8')
            print(*sorted(lines.splitlines(False)), sep='\n')
            print()
        
        snapshot_fs('before-auditwheel', './dist')
        for whlfile in glob.glob(os.path.join(self.dist_dir, "*.whl")):
            os.makedirs("wheelhouse", exist_ok=True)
            if InWheel:
                wheel_house_whl = os.path.join("wheelhouse", os.path.basename(whlfile))
                shutil.move(whlfile, wheel_house_whl)
                with InWheel(in_wheel=wheel_house_whl, out_wheel=whlfile):
                # with InWheel(
                #     in_wheel=whlfile,
                #     out_wheel=os.path.join("wheelhouse", os.path.basename(whlfile)),
                # ):
                    print(f"Updating RECORD file of {whlfile}")
        print("Copying new wheels")
        snapshot_fs('after-auditwheel', './dist', './wheelhouse')
        # shutil.move("wheelhouse", self.dist_dir)
        shutil.rmtree("wheelhouse")
        snapshot_fs('before-end', './dist')

Fix _build_wheel producing wheels with incorrect RECORD file
@mxschmitt mxschmitt merged commit c5acc36 into microsoft:main Nov 27, 2024
38 of 40 checks passed
mxschmitt pushed a commit that referenced this pull request Dec 6, 2024
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.

2 participants