[py] Fix installing most of the data from source distributions #15128
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Change `namespaces` to true to fix installing `selenium.webdriver.common.devtools.*` packages, and fix `package-data` to fix installing `.js` and `.json` data files. The latter would specify a non-existing `selenium_package` package name -- instead, specify `*` to apply it to all Python packages found. With these changes, the result of installing the source distribution is almost identical to the result of installing the wheel. One remaining problem is that `selenium-manager` is not included -- my guess is that there is no logic copying the built Rust executable into the expected location. Another problem is that these changes do not work when installing from a VCS checkout -- since all the relevant data files are explicitly excluded by `.gitignore` and that takes precedence over `data-files`. However, this was broken before, so it should not really matter. Fixes SeleniumHQ#15125
|
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
| That linter seems to not "see" the whole file? |
| This is source dist is built from this PR https://test.pypi.org/project/selenium/4.29.0.202501211759/#files |
| Yep, I can confirm it looks good (modulo what was said above): ---- Name -selenium-4.28.0.dist-info/LICENSE -selenium-4.28.0.dist-info/METADATA -selenium-4.28.0.dist-info/RECORD -selenium-4.28.0.dist-info/WHEEL +selenium-4.29.0.202501211759.dist-info/LICENSE +selenium-4.29.0.202501211759.dist-info/METADATA +selenium-4.29.0.202501211759.dist-info/RECORD +selenium-4.29.0.202501211759.dist-info/top_level.txt +selenium-4.29.0.202501211759.dist-info/WHEEL selenium/common/exceptions.py selenium/common/__init__.py selenium/__init__.py @@ -265,9 +266,7 @@ selenium/webdriver/common/fedcm/dialog.p selenium/webdriver/common/fedcm/__init__.py selenium/webdriver/common/__init__.py selenium/webdriver/common/keys.py -selenium/webdriver/common/linux/selenium-manager selenium/webdriver/common/log.py -selenium/webdriver/common/macos/selenium-manager selenium/webdriver/common/mutation-listener.js selenium/webdriver/common/options.py selenium/webdriver/common/print_page_options.py @@ -278,7 +277,6 @@ selenium/webdriver/common/timeouts.py selenium/webdriver/common/utils.py selenium/webdriver/common/virtual_authenticator.py selenium/webdriver/common/window.py -selenium/webdriver/common/windows/selenium-manager.exe selenium/webdriver/edge/__init__.py selenium/webdriver/edge/options.py selenium/webdriver/edge/remote_connection.py |
Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
| @diemol, the fix is verified and ready for patch 4.28.1 |
Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
| @mgorny, can you also check this source dist? https://test.pypi.org/project/selenium/4.29.0.202501230540/#files |
| Looks like it works |
| @mgorny, if having time, can you do one more check in the latest source dist with new changes before merging the PR and release patch. |
Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
This one seems broken to me: |
| Oops, looks like it needs |
| @mgorny, sorry for disturbing last minute, but package 4.28.1 was already out with this error. Do you have any workaround from your end to remove those 2 lines from |
Yes, we build But I don't have a stake here, so it's up to you. |
8 tasks
any idea whether this will be fixed? i am encountering the build error as menetioned here while attempting to build |
| As the workaround, can you remove these 2 lines in The fix will be delivered in 4.29.0. |
sandeepsuryaprasad pushed a commit to sandeepsuryaprasad/selenium that referenced this pull request Mar 23, 2025
…iumHQ#15128) Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com> Co-authored-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
gryznar pushed a commit to gryznar/selenium that referenced this pull request May 17, 2025
…iumHQ#15128) Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com> Co-authored-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
I think #16074 will fix this btw. |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
User description
Description
Change
namespacesto true to fix installingselenium.webdriver.common.devtools.*packages, and fixpackage-datato fix installing.jsand.jsondata files. The latter would specify a non-existingselenium_packagepackage name -- instead, specify*to apply it to all Python packages found.With these changes, the result of installing the source distribution is almost identical to the result of installing the wheel. One remaining problem is that
selenium-manageris not included -- my guess is that there is no logic copying the built Rust executable into the expected location.Another problem is that these changes do not work when installing from a VCS checkout -- since all the relevant data files are explicitly excluded by
.gitignoreand that takes precedence overdata-files. However, this was broken before, so it should not really matter.Motivation and Context
Fixes #15125 -- install from source distribution resulting in missing data files
Types of changes
Checklist
PR Type
Bug fix
Description
Enabled
namespacesto fix package installation issues.Updated
package-datato include.jsand.jsonfiles.Resolved discrepancies between source distribution and wheel installation.
Addressed missing data files during source distribution installation.
Changes walkthrough 📝
pyproject.toml
Fix package and data file installation issuespy/pyproject.toml
namespacestotrueto fix package installation.package-datato apply to all Python packages..py,.rst,.json, and.xpifiles.