wheel: support for 'plugin' type entry_points - #349
Conversation
| self.add_string(self.distinfo_path('METADATA'), metadata) | ||
|
|
||
| def add_entry_points(self, console_scripts): | ||
| def add_entry_points(self, console_scripts, plugins): |
There was a problem hiding this comment.
I'd change it so that the .bzl file just merges console_scripts and plugins, and wheelmaker just always sees only entry_points.
| match = pattern.fullmatch(line) | ||
| if not match: | ||
| raise ValueError('{line} is not a valid entry point'.format(line=line)) | ||
| plugin_type = match.group(1).strip() |
There was a problem hiding this comment.
This can produce multiple groups with the same name. Even if that's allowed by the spec (I'm not sure?) it would be better to order the entries so that each group appears only once.
There was a problem hiding this comment.
This did not happen before (at least from wheel.bzl) because there was only one entry allowed per group. As part of switching that to allow multiple entry points per group, rewrite this code from scratch. I believe all such concerns are now handled.
* deprecate console_scripts, update docs * rename plugins to entry_points, update docs * switch to string_list_dict to support multiple entries per group * sorting of groups and entries within groups for idempotence
This reverts commit 5d5a06d.
| self.add_string(self.distinfo_path('METADATA'), metadata) | ||
|
|
||
| def add_entry_points(self, console_scripts): | ||
| def add_entry_points(self, console_scripts, plugins): |
| match = pattern.fullmatch(line) | ||
| if not match: | ||
| raise ValueError('{line} is not a valid entry point'.format(line=line)) | ||
| plugin_type = match.group(1).strip() |
There was a problem hiding this comment.
This did not happen before (at least from wheel.bzl) because there was only one entry allowed per group. As part of switching that to allow multiple entry points per group, rewrite this code from scratch. I believe all such concerns are now handled.
| name = match.group(2).strip() | ||
| object_reference = match.group(3).strip() | ||
| if name in group_dict: | ||
| raise ValueError("Duplicate entry for name {name} in group {group}".format(name=name, group=group)) |
There was a problem hiding this comment.
Is there a stock formatter for this repo? I tried yapf but it changed way more than just the modified code.
pstradomski
left a comment
There was a problem hiding this comment.
@andyscott @thundergolfer : This change looks good to me (except for the comments I've left). Should I merge it?
I authored the initial implementation of py_wheel for this repo and kept maintianing it, but with you as new maintainers I'd like a clarification if I should continue merging changes here or would you prefer to review/merge all new PRs?
pstradomski
left a comment
There was a problem hiding this comment.
Presubmit complains about the print() function.
Let's remove it.
PR Checklist
.parfiles. See CONTRIBUTING.md for infoPR Type
What kind of change does this PR introduce?
What is the current behavior?
Python wheels have a rich concept called
entry_points. The most common use of these is the existingconsole_scriptssupport: command-line wrappers for Python code.However, there are many more uses. Another common one is
plugins-- the Python version of, say, Java@AutoService. This is howpytestfinds modules on thePYTHONPATH.This PR is something we've been using for a while to add our own plugins to the
entry_pointsfile; but the change is worth discussing.In one sense, this is stupid. There's some interference between
console_scriptsandplugins-- in reality, the underlying structure is:So maybe this would be best as a single argument merging
console_scriptsin:{group: {k1: v1, k2: v2, ...}, ...}... if we wanted to make a breaking change.But since
console_scriptsalready existed, I addedpluginsseparately. (I did not call itentry_pointssince it doesn't handleconsole_scripts).(There are of course, other backwards-compat options like adding an
entry_pointsarg that throws ifconsole_scriptsis present.)Issue Number: not yet, but happy to file one if needed
What is the new behavior?
Wheels built by
rules_pythonsupportentry_pointsother thanconsole_scripts, hopefully without breaking existingconsole_scriptssupport.Does this PR introduce a breaking change?
Other information
See https://packaging.python.org/specifications/entry-points/ and specifically https://packaging.python.org/guides/creating-and-discovering-plugins/#using-package-metadata
Sorry for not ticking all the boxes from jump - this has been sitting around a while and when I realized PRs are being reviewed again I thought it might be worth starting the discussion.
This change is