Add support for "packages" install key in v2 config

[NiklasRosenstein]

This adds support for a packages install option in the v2 config file. Example:

version: 3.7
install:
  - packages:
    - mkdocs-material
    - Pygments

The motivation for this change is that you don't have to have a requirements.txt in your project when you only need those requirements for the Read the Docs build environment.

Testing

• Add unit test
• Actually test the package installation (install_packages_list())

[stsewd]

Thanks for doing this! We need to see how we should check for valid packages names before passing them down to pip. I have made some suggestions in the meantime.

[stsewd]

We should be careful here, users could set a package name like --option and we will execute that. Not sure the best way to prevent that. We could probably validate this in the config module and raise an exception for invalid names, or maybe escape those characters here. /cc @readthedocs/core

Invalid names would be those that start with -, not sure what else we should validate here.

[NiklasRosenstein]

How much of a concern is this really? Pip also reads options from requirements.txt files, or at least a subset of options that are valid for the pip install command like

--extra-index-url https://example.org/pypi/simple
some-requirement >=1.0.0, <2.0.0

[stsewd]

yeah, you are right, maybe allow the same subset of options?

[humitos]

what about using the same parser that pip uses and check for a valid name?

On the other hand, @davidfischer created this package that could be useful if we can validate each line separately: https://requirements-parser.readthedocs.io/en/latest/

[NiklasRosenstein]

Does RTD have a separate validation step? Otherwise Pip will just complain if the requirement is invalid.

[humitos]

Right, but we need to verify it before sending to pip in some way because otherwise you can inject random commands into the pip by calling pip install package1 "echo test > /tmp/test.txt" (quotes should be `, but I can escape them here)

[davidfischer]

On the other hand, @davidfischer created this package that could be useful if we can validate each line separately: https://requirements-parser.readthedocs.io/en/latest/

This package is super old and outdated and probably would need some love before it's useful. That package doesn't directly parse the lines by itself but instead uses the parser built-in to setuptools.

[humitos]

We should consider supporting private packages https://docs.readthedocs.io/en/stable/guides/private-python-packages.html for Read the Docs for Business, probably.

[NiklasRosenstein]

Unless I am missing something, this should already be supported implicitly. You can just use a Git repository URL instead of a package name, e.g.:

- packages:
  - mkdocs-material
  - git+https://${GITHUB_TOKEN}@github.com/user/project.git@{version}

[humitos]

Have you tested that the environment variable is properly replaced with its value?

[NiklasRosenstein]

Nope and I doubt that it will be at this stage. Good point that we should probably support the substitution of environment variables, otherwise while it technically works to hard code the token, that's probably not what anyone wants 😄

Does Pip support the substitution in requirements.txt?

[stsewd]

Does Pip support the substitution in requirements.txt?

Yes, pip supports expanding variables

[NiklasRosenstein]

Maybe it'll be easier to just write the arguments into a temporary requirements file and re-use the existing logic for installing from that

[agjohnson]

In order to push this along and finish this work, I'd be fine opening an issue to add private repository support to the packages config key later. This PR seems close otherwise and it looks to be blocked on this additional feature.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stsewd]

With our future direction to allow executing arbitrary commands, do we still want this feature?

[humitos]

We may want to talk a little more about this now that we are deciding to make more changes to the builder. However, at first sight, I still think this is a useful feature for noob users that don't need to understand how pip/requirements.txt file works (probably, this is more interesting in particular for the commercial site --cc @agjohnson) or just when your project is simple enough to require a small number of packages that are different from the requirements of the package itself.