[py] Fix installing most of the data from source distributions #15128
Conversation
This file contains 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 <[email protected]>
|
@diemol, the fix is verified and ready for patch 4.28.1 |
Signed-off-by: Viet Nguyen Duc <[email protected]>
|
@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 <[email protected]>
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 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.