Ensure we always respect --site-packages

[techalchemy]

Ensure we always respect --site-packages

• Sometimes we fail to respect --site-packages when it is passed with
  --install
• This resolves that and ensures we always pass it to ensure_project
• Fixes Not all site-packages available when installed with --site-packages #3718

[frostming]

As far as I understand venv is ensured to be created if --site-packages is passed. This is guaranteed by the parent group parser:

pipenv/pipenv/cli/command.py

Lines 198 to 206 in 065ebdb

	if (state.python or state.three is not None) or state.site_packages:
	ensure_project(
	three=state.three,
	python=state.python,
	warn=True,
	site_packages=state.site_packages,
	pypi_mirror=state.pypi_mirror,
	clear=state.clear,
	)

I suggest not add duplicate codes in subcommand parser if the parent already handled this. If it isn't working that should be a problem

[frostming]

I remember I submitted a patch to fix the issue of options passing from parent to subcommand: 3f053a3

Let's waiting for user feedback about whether it is still an issue.

[techalchemy]

As far as I understand venv is ensured to be created if --site-packages is passed. This is guaranteed by the parent group parser: https://github.com/pypa/pipenv/blob/master/pipenv/cli/command.py#L198-L206

This only works if you specifically place the --site-packages flag before any subcommand. If you place the flag after a subcommand it gets handled in the subcommand parser instead, or at the very least is passed through to the subcommand parser as a normal, unparsed part of the command line and is not consumed.

Let's waiting for user feedback about whether it is still an issue.

As a user I tried to test out the functionality to confirm whether it still exists for the purposes of triaging the specified issue (#3718) but I couldn't because it passed --site-packages through to do_install as a package to install, which is obviously not correct

[frostming]

This only works if you specifically place the --site-packages flag before any subcommand.

I thought it works because I remember I can do pipenv install --three. But after checking the code I found the two/three option is passed to the install command, too.

So pipenv --site-packages install works on the master branch, but pipenv install --site-packages raises an error. Should we allow options at arbitrary positions? Since it is more an env specific option than an installation option.

[techalchemy]

We can't allow options in arbitrary positions, not if we are going to use the argument parser the way we are using it (i.e. allowing --two, --three or --python without any additional subcommands and also respecting some options and arguments on those).

The reason is that we parse --two and --three in the case where there is no subcommand, and in that case we parse for --site-packages, among other options, which we will include when we create the environment. Currently any option that is handled after a subcommand is explicitly passed through to the subcommand itself.

I started looking into this specific issue because I was confused about the fact that I couldn't see --site-packages when I ran pipenv install --help, which I found kind of odd. I feel it is a major UX problem that we allow the creation of an environment but only respect certain arguments in certain cases and consider this a bug.

[techalchemy]

In my mind the question is more about what else we are neglecting to parse in subcommands or what other user experience stumbling blocks there are

[frostming]

Thanks for the explanation, so the main difference between install and clean is we are expecting the former one to initialize an environment, then a --site-packages option is natural.

[techalchemy]

well you can't really enable site packages while you're removing things, from a user experience standpoint that doesn't make that much sense