-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[webkitcorepy] Packaging requires pyparsing #18232
[webkitcorepy] Packaging requires pyparsing #18232
Conversation
EWS run on previous version of this PR (hash 43354a5) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one nit but otherwise non-reviewer r+
@@ -277,10 +277,11 @@ def install(self): | |||
return | |||
|
|||
# Make sure that setuptools, tomli, setuptools_scm, wheel and packaging are installed, since setup.py relies on them | |||
if self.name not in ['wheel', 'packaging', 'setuptools', 'tomli', 'setuptools_scm']: | |||
if self.name not in ['setuptools', 'wheel', 'pyparsing', 'packaging', 'tomli', 'setuptools_scm']: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes the comment above incorrect.
43354a5
to
16e4e7a
Compare
@@ -433,6 +429,11 @@ class AutoInstall(object): | |||
DISABLE_ENV_VAR = 'DISABLE_WEBKITCOREPY_AUTOINSTALLER' | |||
CA_CERT_PATH_ENV_VAR = 'AUTOINSTALL_CA_CERT_PATH' | |||
|
|||
# This list of libraries is required to install other libraries, and must be installed first | |||
BASE_LIBRARIES = ['setuptools', 'wheel', 'pyparsing', 'packaging', 'setuptools_scm'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Install order is changing from wheel
, packaging
, setuptools
to setuptools
, wheel
, packaging
. Is that intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is intentional. wheel
needs setuptools
, packaging
needs pyparsing
BASE_LIBRARIES = ['setuptools', 'wheel', 'pyparsing', 'packaging', 'setuptools_scm'] | ||
if sys.version_info >= (3, 0): | ||
BASE_LIBRARIES.insert(-1, 'tomli') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since (IIUC) tomli is needed to install setuptools_scm, I think it's clearer and less error-prone to say:
BASE_LIBRARIES = ['setuptools', 'wheel', 'pyparsing', 'packaging', 'setuptools_scm'] | |
if sys.version_info >= (3, 0): | |
BASE_LIBRARIES.insert(-1, 'tomli') | |
BASE_LIBRARIES = ['setuptools', 'wheel', 'pyparsing', 'packaging'] | |
if sys.version_info >= (3, 0): | |
BASE_LIBRARIES += ['tomli', 'setuptools_scm'] | |
else: | |
BASE_LIBRARIES += ['setuptools_scm'] |
(And you could of course collapse this into one or two statements as desired)
16e4e7a
to
7858c2e
Compare
EWS run on current version of this PR (hash 7858c2e) |
https://bugs.webkit.org/show_bug.cgi?id=262124 rdar://116062896 Reviewed by Elliott Williams. * Tools/Scripts/libraries/webkitcorepy/setup.py: Bump version. * Tools/Scripts/libraries/webkitcorepy/webkitcorepy/__init__.py: Ditto. * Tools/Scripts/libraries/webkitcorepy/webkitcorepy/autoinstall.py: (Package.install): Install pyparsing before packaging. Canonical link: https://commits.webkit.org/268523@main
7858c2e
to
5bae443
Compare
Committed 268523@main (5bae443): https://commits.webkit.org/268523@main Reviewed commits have been landed. Closing PR #18232 and removing active labels. |
Ugh, wrong version of this change landed. A moment, let me re-land correctly.... |
Landed correctly in #18311 |
5bae443
7858c2e
π§ͺ styleπ iosπ macπ wpeπ wincairoπ§ͺ bindingsπ ios-simπ mac-AS-debugπ§ͺ wpe-wk2π§ͺ webkitperlπ§ͺ ios-wk2π§ͺ api-macπ gtkπ§ͺ webkitpyπ§ͺ ios-wk2-wptπ§ͺ mac-wk1π§ͺ gtk-wk2π§ͺ api-iosπ§ͺ mac-wk2π§ͺ api-gtkπ tvπ§ͺ mac-AS-debug-wk2π§ͺ servicesπ tv-simπ§ͺ mac-wk2-stressπ watchπ watch-sim