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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REGRESSION] 1.25.0 distributes "scripts" as a package #1500

Closed
KotlinIsland opened this issue Aug 16, 2022 · 6 comments
Closed

[REGRESSION] 1.25.0 distributes "scripts" as a package #1500

KotlinIsland opened this issue Aug 16, 2022 · 6 comments
Assignees

Comments

@KotlinIsland
Copy link

pypa/pip#1490 introduced an __init__.py into the scripts folder, this in turn made setuptools include it as a package to be installed.

馃憠 pip uninstall playwright      
Found existing installation: playwright 1.25.0
Uninstalling playwright-1.25.0:
  Would remove:
    c:\users\AMONGUS\...\lib\site-packages\playwright-1.25.0.dist-info\*
    c:\users\AMONGUS\...\lib\site-packages\playwright\*
    c:\users\AMONGUS\...\lib\site-packages\scripts\*
    c:\users\AMONGUS\...\scripts\playwright.exe
Proceed (Y/n)?

c:\users\AMONGUS\...\lib\site-packages\scripts\*?
This is highly sus and should be remediated immediately, maybe even yank the release, idk, up to you.

@mxschmitt mxschmitt changed the title [BUG] (馃悶) 1.25.0 distributes "scripts" as a package [BUG] 1.25.0 distributes "scripts" as a package Aug 16, 2022
@mxschmitt mxschmitt changed the title [BUG] 1.25.0 distributes "scripts" as a package [REGRESSION] 1.25.0 distributes "scripts" as a package Aug 16, 2022
@rwoll
Copy link
Member

rwoll commented Aug 16, 2022

Thanks for the report鈥攁nd sorry for the inconvenience this has caused. We'll be looking into remediating this today.

@rwoll
Copy link
Member

rwoll commented Aug 16, 2022

Distributing scripts as a package was not intended, and it will be fixed. It was introduced by #1490 to fix #1489.

Per #1489, there was unexplained breakage in the generation code. The "fix" was only intended to affect development, but due to an oversight, impacted the released package.

We'll be working to understand #1489 today, and take the appororiate resolution.

@rwoll

This comment was marked as outdated.

@rwoll
Copy link
Member

rwoll commented Aug 16, 2022

@KotlinIsland / @DetachHead : Assuming #1505 passes, we'll be issuing a patch release (v1.25.1).

How'd we get here?

Without changing code, the Playwright Python builds started breaking overnight (relates #1489). Commits that had built clean one day, were not building clean the next. While we thought all deps were pinned (e.g. in local-requirements.txt) to ensure determinism and reproducability. It turns out they were not all pinned in the appropriate place (as evidenced by #1505 fixing the issue).

We mistakenly landed a workaround without fully understanding which part of the build was introduced the breakage, which in retrospect was a mistake. Sorry about that! We had diffed build logs from good and bad builds and determined the deps鈥攁t least those shown in the logs鈥攚ere all identical versions, but turns out the issue ran deeper. We thought we were essentially implementing a workaround allowing us time to get to the bottom of the issue in the future, but overlooked the side effect of accidentally creating a new package.

Follow Up Actions

As a follow up, we will:

  1. Publish v1.25.1
  2. audit deps to determine which other build deps need to be pinned in pyproject.toml#build-system (e.g. like fix: pin setuptools in build-system聽#1505) [internal] ensure all build-system deps are pinned properly聽#1506
  3. move to explicitly specifying include in setup.py to avoid accidentally creating new packages ([internal] move to inlcude, instead of exclude for setup.py聽#1504)
  4. bisect setuptools versions to understand what change between 60.9.3 and 65.0.0 broke us; this will also allow us to upgrade setuptools in the future
  5. and anything else the relates to the above

Thanks for your patience and understanding! Please let us know if you encounter any issues related to the above or have additional comments.

@rwoll
Copy link
Member

rwoll commented Aug 16, 2022

1.25.1 has been released. I'll close this in favor of the individual follow up tickets linked above.

@rwoll rwoll closed this as completed Aug 16, 2022
@KotlinIsland
Copy link
Author

KotlinIsland commented Aug 16, 2022

Awesome news on this @rwoll, glad to hear there was such a quick turnaround.

Are you going to yank 2.25.0? Maybe you should move to a modern dependency management and build system like Poetry.

mxschmitt pushed a commit to mxschmitt/playwright-python that referenced this issue Aug 17, 2022
sbakerdev added a commit to sbakerdev/playwright-python that referenced this issue Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants