playwright-test 1.47.0 -> 1.47.2 plus update script

[humemm]

Description of changes

This updates playwright-test to address some regressions that were introduced in 1.47.0 and supplies an update script for automating future updates.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[humemm]

@kalekseev @teto can you please review this?

[winterqt]

At a glance, nix-update-script should work fine here; could you try that? Furthermore, even if it doesn't, please split the package update and the addition of the update script into two separate commits.

[kalekseev]

We have update.sh script for playwright-python that also updates playwright driver to the version used in python module https://github.com/microsoft/playwright-python/blob/65a86d90b6a1643b19544cfe1e2108863a84aa24/setup.py#L33

Can you update existing update.sh script to make it update playwright to latest patch version so instead of using driver version 1.47.0 it will update playwright to 1.47.2?

[humemm]

We have update.sh script for playwright-python that also updates playwright driver to the version used in python module https://github.com/microsoft/playwright-python/blob/65a86d90b6a1643b19544cfe1e2108863a84aa24/setup.py#L33

Can you update existing update.sh script to make it update playwright to latest patch version so instead of using driver version 1.47.0 it will update playwright to 1.47.2?

@kalekseev I meant to ask about this. From what I can tell the package you're referring to playwright-python which in nix is called just playwright has a dependency on playwright-test which in nix it's all bundled under playwright-driver along with core and browsers. Unfortunately the way this is setup it forces the main playwright which for some reason in nix is called playwright-test to be held back. For example at the moment playwright-python is still on 1.47.0 and the main playwright is on 1.47.2. I think a proper way to do this would be to have a separate playwright-driver under the pythonPackages namespace so that you can track whatever version is appropriate for playwright-python and we can freely update this to the latest as we wish. As far as I'm aware you can't use different version drivers/browsers even minor or patch.

I might be wrong here as there's no clear documentation and I've only been able to learn from browsing the source.

[humemm]

@winterqt split changes in two commits as requested.

[kalekseev]

@humemm

From what I can tell the package you're referring to playwright-python which in nix is called just playwright has a dependency on playwright-test which in nix it's all bundled under playwright-driver along with core and browsers.

See a discussion regarding wrong names for packages, would be great to address this before next release

the python package should not have squatted the top-level playwright attribute so I am onboard with the change but that needs mentioning in the release notes.
It's too late for breaking changes for 24.05 nixos/doc/manual/release-notes/rl-2405.section.md, and nixos/doc/manual/release-notes/rl-2411.section.md does not exist yet so looks like it will have to wait.

I think a proper way to do this would be to have a separate playwright-driver under the pythonPackages namespace so that you can track whatever version is appropriate for playwright-python and we can freely update this to the latest as we wish.

In this case nixpkgs will have two sets of playwright and related browsers.
Until we have real evidence that patch version disperancy breaks python playwright I'd keep a single playwright.
Thus in this pr I'd prefer to see update to python module update.sh script that relaxes patch version constraint allowing playwright update from 1.74.0 to 1.74.2.

[teto]

See a #302759 (comment)_ regarding wrong names for packages, would be great to address this before next release

#302759 (comment) might be a more precise link

Fully agree. I would prefer a brutal "playwright" -> playwright-test renaming with a nentry in 24.11 release notes but the sensible way might be instead to, over one release, have playwright still pointing at plawyright-test with a warning "in 25.04, this attribute will refer to 'playwright' and not the python package anymore". And in 25.05 we do the switch. I am not sure "playwright" has many consumers so it could be fine . This kind of question is why we have a release coordinator I guess :D @RossComputerGuy

[humemm]

@kalekseev I just tested if playwright python cares if it's on the same version as the main package and it seems like it doesn't. I think this is only applicable for the main driver and browsers. So if we update the package playwright-driver independently from the python package we should be fine. I can change the update script in python-packages however it almost doesn't make sense for the python package to be updating the main playwright package. Would it be better if we had separate update scripts for each package?

[kalekseev]

@humemm

Would it be better if we had separate update scripts for each package?

No. As I said we should have the same minor version in playwright and playwright python but we can move update.sh from python module into playwright module.

[kalekseev]

For reference here's a list of PRs https://github.com/microsoft/playwright-python/pulls?q=roll where playwright version updated in python module. Minor version updates such as 1.46 -> 1.47 usually have quite significant changes, while patch versions usually updated without any changes, thus we should stick with the same minor version in both packages.

[humemm]

For reference here's a list of PRs https://github.com/microsoft/playwright-python/pulls?q=roll where playwright version updated in python module. Minor version updates such as 1.46 -> 1.47 usually have quite significant changes, while patch versions usually updated without any changes, thus we should stick with the same minor version in both packages.

@kalekseev I'm really not following and I'm kind of confused. I think we should decouple the main playwright from playwright-python to ensure that we get timely updates. As I pointed out playwright introduced some pretty significant regressions in 1.47.0 that were addressed in 1.47.2. It's been almost 3 weeks since that regression was released. I personally don't use playwright-python so I don't understand why we need to be on the same minor version and wait for playwright-python to catch up? Also minor versions do not include breaking changes and since playwright-python is just an API wrapper theoretically and practically (I tested it) it should continue to work even if python version is on 1.46.0 and the main package has moved up to 1.47.0 or any minor version for that matter. Even if it didn't work and the main package has a breaking 2.0 release I don't see why we wouldn't try to figure out a way to have the two be independent?

Is there a way to override the driver version in the installation step of playwright-python, maybe via a patch of some sort?

I'm sure there's some level of naivete caused by my lack of deep understanding of nixpkgs. However my software engineering spider sense tells me that there should be a way to get the best of all worlds and that there's no need to compromise here. This is a fundamental part of wrangling dependencies in any complex software system and I'm sure nix should be able to accommodate it. I'm asking for some guidance and I'm offering help to get to that place.

[kalekseev]

@humemm playwright and playwright-python developed by microsoft, minor version updates for them usually released within days, so there's no reason to have two sets of playwright drivers and browsers.

As I pointed out playwright introduced some pretty significant regressions in 1.47.0 that were addressed in 1.47.2.

I already said that we can relax dependency of python module and upgrade playwright to latest patch version, which is 1.47.2.

[humemm]

@kalekseev OK then this should work, let me know. I just updated the PR to remove the playwright-driver steps from the update script of the python package. Now the python package would update itself only and the main package will do the same. Since the two packages are released in sync by Microsoft this should trigger both update scripts to run in nix for each package independently.

[humemm]

Or now that I think about it's probably better to keep the update steps in the python package as well just in case and only change the version pinning that was in place. Let me know I can change it back if you think it's better that way.

[kalekseev]

Or now that I think about it's probably better to keep the update steps in the python package as well just in case and only change the version pinning that was in place. Let me know I can change it back if you think it's better that way.

@humemm I think we should do something like this:

1. (optional) Move update.sh into playwright module
2. Modify update.sh to get driver_version from playwright-python but bump it to the latest patch version
3. Use latest patch version to upgrade playwright

The rest of the script should be the same, it should update both packages but use latest patch version for playwright.

[humemm]

@kalekseev changes made as requested. I also got my branch up to date with master, however I noticed that a new PR made it's way into the update script that actually causes some errors when running it locally, those are unrelated to my changes. The updated script takes in some positional arguments that are not documented at all and passes an invalid flag to nix store namely --unpack that throws errors and results in hashes for browsers to be stripped and set to an empty string.

[kalekseev]

@humemm the change doesn't handle a situation when playwright has new minor version in latest release, eg. python module uses driver version 1.47.0 and latest playwright is version 1.48.0, in this case the script must update playwright to 1.47.2 not 1.48.0

The updated script takes in some positional arguments that are not documented at all and passes an invalid flag to nix store namely --unpack that throws errors and results in hashes for browsers to be stripped and set to an empty string.

The upack was added recently, you probably use older version of nix https://nix.dev/manual/nix/2.24/command-ref/new-cli/nix3-store-prefetch-file#opt-unpack

[kalekseev]

Actually nixpkgs stuck on nix 2.18 because of regressions #343147 so it might be that update script wont work on nixpkgs-update CI, I don't know which version of nix it uses. Would be nice to replace nix store --upack with something that works on v2.18.

[humemm]

@humemm the change doesn't handle a situation when playwright has new minor version in latest release, eg. python module uses driver version 1.47.0 and latest playwright is version 1.48.0, in this case the script must update playwright to 1.47.2 not 1.48.0

@kalekseev on this point I'm not sure if you saw my comment above. I tested this scenario by moving playwright-python back to 1.46.0 and keeping playwright on 1.47.2 and it worked just fine. I know you mentioned that there were a lot of new features introduced in 1.47.0 for python but the API is backwards compatible for minor versions. I just don't see a reason to complicate the logic in the update script and potentially introduce bugs for a problem that doesn't exist. As you mentioned in your comment earlier in addition to that Microsoft coordinates those larger releases so I don't think we risk running into a problem.

[humemm]

Actually nixpkgs stuck on nix 2.18 because of regressions #343147 so it might be that update script wont work on nixpkgs-update CI, I don't know which version of nix it uses. Would be nice to replace nix store --upack with something that works on v2.18.

On that part I'm not sure I can be of much help. I'm not experienced with the internals of nix and have no idea what could be used as an alternative. Would probably be better to open a separate PR for that change.

[kalekseev]

@humemm the change doesn't handle a situation when playwright has new minor version in latest release, eg. python module uses driver version 1.47.0 and latest playwright is version 1.48.0, in this case the script must update playwright to 1.47.2 not 1.48.0

@kalekseev on this point I'm not sure if you saw my comment above. I tested this scenario by moving playwright-python back to 1.46.0 and keeping playwright on 1.47.2 and it worked just fine. I know you mentioned that there were a lot of new features introduced in 1.47.0 for python but the API is backwards compatible for minor versions. I just don't see a reason to complicate the logic in the update script and potentially introduce bugs for a problem that doesn't exist. As you mentioned in your comment earlier in addition to that Microsoft coordinates those larger releases so I don't think we risk running into a problem.

@humemm check this prs https://github.com/microsoft/playwright-python/pulls?q=roll they are prs with playwright version update in python-playwright, if you check minor versions updates you can easily see non trivial api changes. Considering that python package usually released within several days of playwright minor release we should stay on the same minor version in nixpkgs.

[humemm]

I just went over the release history for the last 10 minor version releases and it seems like the python version of playwright lags behind the main package about 7-10 days. Not an ideal situation to force playwright to wait for playwright-python there's just no reason a package should be tied to some downstream dependent especially one that gets 6MM weekly downloads.

[kalekseev]

nixpkgs is not npm, 10 days delay is quite good actually.

[humemm]

@kalekseev updated script as requested.

[kalekseev]

you should take driver version from setup.py

https://github.com/microsoft/playwright-python/blob/d9cdfbb1e178b6770625e9f857139aff77516af0/setup.py#L33

[SuperSandro2000]

Please cleanup the git history. Also please drop the merge commits.

Please follow the contributing guide when naming your commits.

[teto]

this has been bugging me for a while so I opened #346269 to prepare the "playwright" attribute change