Skip to content

Add --seed-packages. #2015

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Add --seed-packages. #2015

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 25 additions & 5 deletions src/virtualenv/seed/embed/base_embed.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@
PERIODIC_UPDATE_ON_BY_DEFAULT = True


def _parse_basic_req(line):
parts = line.split("==")
if len(parts) == 1:
return (line, None)
return (parts[0], parts[1])


@add_metaclass(ABCMeta)
class BaseEmbed(Seeder):
def __init__(self, options):
Expand All @@ -30,6 +37,7 @@ def __init__(self, options):
self.no_wheel = options.no_wheel
self.app_data = options.app_data
self.periodic_update = not options.no_periodic_update
self.seed_package = options.seed_package

if not self.distribution_to_versions():
self.enabled = False
Expand All @@ -43,11 +51,14 @@ def distributions(cls):
}

def distribution_to_versions(self):
return {
distribution: getattr(self, "{}_version".format(distribution))
for distribution in self.distributions()
if getattr(self, "no_{}".format(distribution)) is False
}
distributions = dict(self.seed_package)
for distribution in self.distributions():
if getattr(self, "no_{}".format(distribution)):
distributions.pop(distribution, None)
else:
version = getattr(self, "{}_version".format(distribution))
distributions[distribution] = version
return distributions

@classmethod
def add_parser_arguments(cls, parser, interpreter, app_data):
Expand Down Expand Up @@ -98,6 +109,15 @@ def add_parser_arguments(cls, parser, interpreter, app_data):
help="disable the periodic (once every 14 days) update of the embedded wheels",
default=not PERIODIC_UPDATE_ON_BY_DEFAULT,
)
parser.add_argument(
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO this should be a separate plugin. This needs a lot more thought, on e.g. how this interacts with periodic update feature, transitive dependencies and where do we get these custom wheels you're injecting here. Perhaps you're after a pip functionality here, not virtualenv one. In general would be nice to open an issue to discuss such proposal before you lead on with a solution.

Copy link
Author

Choose a reason for hiding this comment

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

IMHO this should be a separate plugin. This needs a lot more thought, on e.g. how this interacts with periodic update feature, transitive dependencies and where do we get these custom wheels you're injecting here. Perhaps you're after a pip functionality here, not virtualenv one. In general would be nice to open an issue to discuss such proposal before you lead on with a solution.

Should the discussion be here? I find it's often useful to propose a problem and solution together.

The wheels are supplied externally, hence my VIRTUALENV_EXTRA_SEARCH_DIR.

It shouldn't interact with periodic update; AFAICT the intent of that is only for the embedded wheels (maybe I should ensure that code is explicitly restricted to those).

Based on #1923 it seems like virtualenv won't support traversing dependencies for now. Until it does, you need to specify your whole dependency set in --seed-package, but this seems fine for now.

Updating pip would solve my problem as well; I thought that --seed-package was a smaller ask than pip vendoring more code. I thought they would want to be as lightweight as possible. If you think I'm more likely to succeed with pip, I can try there instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

Based on #1923 it seems like virtualenv won't support traversing dependencies for now.

This is not true, just waiting for implementation. I'd consider this a blocking functionality request for this PR.

Until it does, you need to specify your whole dependency set in --seed-package, but this seems fine for now.

I disagree.

Updating pip would solve my problem as well; I thought that --seed-package was a smaller ask than pip vendoring more code.

Wishful thinking. 😊

If you want to go ahead with the current works for your half-baked solution I recommend using a plugin, but for acceptance in the core, we'd need at the very least traversing and handling dependencies too.

Copy link
Author

Choose a reason for hiding this comment

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

If you want to go ahead with the current works for your half-baked solution I recommend using a plugin, but for acceptance in the core, we'd need at the very least traversing and handling dependencies too.

Fair enough! thanks for your time.

"--seed-package",
metavar="package[==version]",
dest="seed_package",
nargs="+",
type=_parse_basic_req,
help="a list of extra packages to install",
default=[],
)

def __unicode__(self):
result = self.__class__.__name__
Expand Down
6 changes: 4 additions & 2 deletions src/virtualenv/seed/wheels/embed/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,10 @@


def get_embed_wheel(distribution, for_py_version):
path = BUNDLE_FOLDER / (BUNDLE_SUPPORT.get(for_py_version, {}) or BUNDLE_SUPPORT[MAX]).get(distribution)
return Wheel.from_path(path)
wheel_name = (BUNDLE_SUPPORT.get(for_py_version, {}) or BUNDLE_SUPPORT[MAX]).get(distribution)
if wheel_name is None:
return None
return Wheel.from_path(BUNDLE_FOLDER / wheel_name)


__all__ = (
Expand Down