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

chromedriver: 91.0.4472.101 -> 92.0.4515.43 and aarch64-darwin support #128979

Closed
wants to merge 1 commit into from

Conversation

hlolli
Copy link
Member

@hlolli hlolli commented Jul 2, 2021

Motivation for this change

I need to develop selenium tests, and the chrome team are already release aarch64-darwin binaries.

Things done
  1. bumped the version, called nix-prefetch-url to get the sha256 for linux and x86_64-darwin.
  2. Added aarch64-darwin
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.05 Relase 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.

@cole-h
Copy link
Member

cole-h commented Jul 2, 2021

@ofborg eval

(Sorry, ofborg's certs recently had an issue, meaning that it was unable to
react to PRs. This has been fixed, but all PRs filed in this period will need
ofborg to be manually kicked off.)

@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Jul 2, 2021
Copy link
Member

@primeos primeos left a comment

Choose a reason for hiding this comment

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

aarch64-darwin support sounds good to me but there are two additional requirements here:

  1. The chromedriver version must be compatible with the stable Chrom{e,ium} release: https://chromedriver.chromium.org/downloads/version-selection (I guess we could consider extending the Nix expressions to support the beta channel as well if required).
  2. upstream-info.json must be generated automatically via pkgs/applications/networking/browsers/chromium/update.py

@kclejeune
Copy link

aarch64-darwin support sounds good to me but there are two additional requirements here:

1. The `chromedriver` version must be compatible with the stable Chrom{e,ium} release: https://chromedriver.chromium.org/downloads/version-selection (I guess we could consider extending the Nix expressions to support the beta channel as well if required).

2. `upstream-info.json` must be generated automatically via `pkgs/applications/networking/browsers/chromium/update.py`

Besides resolving the conflicts, is this all that's needed to get this merged?

@hlolli
Copy link
Member Author

hlolli commented Nov 10, 2021

I totally forgot this was even still open. Hasn't even newer version been pushed long ago? If so, then I'd close it.

@hlolli
Copy link
Member Author

hlolli commented Nov 10, 2021

I'm forgetting aarch64 support in this too. Feel free to make a new PR based on the ideas here @kclejeune (or PR onto my fork). I fear I'll forget again, since I don't need this right now.

@primeos
Copy link
Member

primeos commented Nov 10, 2021

is this all that's needed to get this merged?

Yes, adding M1 support should be good to go as long as everything in upstream-info.json is generated automatically by the update script.

@hlolli
Copy link
Member Author

hlolli commented Jan 1, 2022

@primeos turns out I need this again :) fixed the connflict and added an entry for generating the shasums for aarch64-darwin as is done with the other targers.

@kclejeune
Copy link

@primeos turns out I need this again :) fixed the connflict and added an entry for generating the shasums for aarch64-darwin as is done with the other targers.

Thanks for taking care of this! I've been so busy during my last semester of school that I completely forgot.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux and removed 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Jan 1, 2022
@hlolli
Copy link
Member Author

hlolli commented Jan 1, 2022

putting a grain of salt, not sure if it's a result of my setup but it seems it segfaults
this could be entirely my side. Would make sense if someone tests to confirm.

  remoteStacktrace: '0   chromedriver                        0x0000000104ed1408 __gxx_personality_v0 + 543364\n' +
    '1   chromedriver                        0x0000000104e67138 __gxx_personality_v0 + 108468\n' +
    '2   chromedriver                        0x0000000104a667d8 chromedriver + 157656\n' +
    '3   chromedriver                        0x0000000104a6137c chromedriver + 136060\n' +
    '4   chromedriver                        0x0000000104a543f4 chromedriver + 82932\n' +
    '5   chromedriver                        0x0000000104a5509c chromedriver + 86172\n' +
    '6   chromedriver                        0x0000000104a54650 chromedriver + 83536\n' +
    '7   chromedriver                        0x0000000104a53d04 chromedriver + 81156\n' +
    '8   chromedriver                        0x0000000104a52f1c chromedriver + 77596\n' +
    '9   chromedriver                        0x0000000104a531d0 chromedriver + 78288\n' +
    '10  chromedriver                        0x0000000104a68038 chromedriver + 163896\n' +
    '11  chromedriver                        0x0000000104abfe34 chromedriver + 523828\n' +
    '12  chromedriver                        0x0000000104abf7d4 chromedriver + 522196\n' +
    '13  chromedriver                        0x0000000104a8b238 chromedriver + 307768\n' +
    '14  chromedriver                        0x0000000104e94c7c __gxx_personality_v0 + 295672\n' +
    '15  chromedriver                        0x0000000104ea8bd8 __gxx_personality_v0 + 377428\n' +
    '16  chromedriver                        0x0000000104ead2b0 __gxx_personality_v0 + 395564\n' +
    '17  chromedriver                        0x0000000104ea99f8 __gxx_personality_v0 + 381044\n' +
    '18  chromedriver                        0x0000000104e8a754 __gxx_personality_v0 + 253392\n' +
    '19  chromedriver                        0x0000000104ec2a38 __gxx_personality_v0 + 483508\n' +
    '20  chromedriver                        0x0000000104ec2bac __gxx_personality_v0 + 483880\n' +
    '21  chromedriver                        0x0000000104ed7fec __gxx_personality_v0 + 570984\n' +
    '22  libsystem_pthread.dylib             0x00000001bc59d4ec _pthread_start + 148\n' +
    '23  libsystem_pthread.dylib             0x00000001bc5982d0 thread_start + 8\n'
    ```

@primeos
Copy link
Member

primeos commented Jan 4, 2022

Someone else opened another PR that already got merged: #152800
Sorry about that and thanks for your work here @hlolli.
Also: Sorry for not responding here. I read the messages and the diff LGTM but due to #128979 (comment) I assumed this isn't ready yet.

@primeos primeos closed this Jan 4, 2022
@hlolli
Copy link
Member Author

hlolli commented Jan 5, 2022

@primeos what a timing, but the other PR is essentialy exactly the same, so is suffers the same segfault. It will be fixed one day I'm sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: darwin Running or building packages on Darwin 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants