[Feature] Add rez-pip extra args passthrough

[lambdaclan]

Description

closes #816

Enable users to customize the pip install command by explicitly defining additional settings to be passed to rez-pip in the form of extra options. Extra arguments can be defined either by using the new rez-pip cli argument [-e/--extra..] or by adding items to the new rezconfig entry pip_extra_args in the form of:

pip_extra_args: ["--no-compile", "--prefer-binary"]

Implementation is based on the ticket above and the discussion between JC, Thorsten and Allan up on slack.

Breakdown

• Add new rez-pip argument -e/--extra to pass additional options to pip install command
• Additional options can be defined in a rezconfig file entry pip_extra_args to ensure they are always applied
• The command line argument -e/--extra overrides the configuration file options (if any)
• Repeating options automatically override previous one by pip (without extra processing) only last one counts
• Invalid options are handled by pip (no extra care was taken to prevent them)

Type of change

• New feature (non-breaking change which adds functionality)

Possible issues

As discussed below regarding repeating options since through extra args the user can override the target and use pep517 options which are prerequisites for correct package install we will need to handle that scenario somehow.

The way I see it there are two methods:

a. Always ignore the target option and anything that can affect use pep517 such as no-binary etc
b. Have a list of allowed options that can be handled by extra args (filter out invalid ones)

Edit:

It has been agreed to assume user responsibility for overriding the wrong options. It needs to be made clear that extra args adds to or overrides the pre-configured args.

How Has This Been Tested?

• Tried defining multiple option combinations between CLI and rezconfig
• I will look into the test-suite and see if I can add tests if they already exist for other commands

Test Configuration:

• Python version: 2.7.17, 3.8
• OS: Linux, Windows 10
• Toolchain: rez 2.50.0, pip 19.3.1

Demo

• rez-pip cli -e --extra argument

• rez-pip cli extra args via rezconfig

Changelog

Point release

Source | Diff

Notes

Add rez-pip extra args passthrough

Merged pull requests:

• [Feature] Add rez-pip extra args passthrough #827 (lambdaclan)

Closed issues:

• rez-pip creates .pyc files by default #816

[nebukadhezer]

What happens if someone specifies --target as an extra arg ? Or --user ? Should that be filtered out ?

[lambdaclan]

Hello Johannes, as mentioned in the PR description:

Repeating options are automatically ignored by pip (without extra processing)

I also thought that some sort of filtering will be required but after a few tests I confirmed (via the --target option) that any repeating options simply override previous ones, only the last one is taken into account.

Saying that, allowing overriding target can break the installation so I guess it is better to always ignore it same goes for --no-binary since we want pep517 to be always on. That is the problem of allowing extra args, the user can break the rez package install if they override certain options.

I propose that we have a list of allowed options to be specified and only handle those.

I am waiting for Allan's feedback on this.

Thank you for pointing it out though, I will update the PR info to make it clear.

[JeanChristopheMorinPerso]

I propose that we have a list of allowed options to be specified and only handle those.

When we discussed about it, the conclusion was that if someone uses the extra args, he must know what he is doing. At least from what I remember. Instead of having a list of accepted arguments, I would more document which argument should not be specified in the extra args.

[lambdaclan]

Hell JC, thank you very much for your input. Any way is fine with me. Actually documenting it somewhere sounds like a good idea. I could even add it to the help text if it makes sense. I will see how I can make it apparent of what needs to be avoided when using extra args. A warning can also be shown if target or pep517 are overridden.

[nerdvegas]

+1 for putting onus on the user, if they're using this then they should know what they're doing. It also needs to be clear if specifying the extra args _adds_ to the configured extra args, or overwrites them (should overwrite imo). Thx A

…

On Wed, Jan 8, 2020 at 10:37 AM Ira ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/rez/pip.py <#827 (comment)>: > @@ -249,13 +251,18 @@ def pip_install_package(source_name, pip_version=None, python_version=None, context.print_info(buf) _log(buf.getvalue()) + # gather additional pip install options + pip_extra_opts = extra_args if extra_args else config.pip_extra_args + # Build pip commandline cmd = [ py_exe, "-m", "pip", "install", "--use-pep517", "--target=%s" % destpath Hell JC, thank you very much for your input. Any way is fine with me. Actually documenting it somewhere sounds like a good idea. I could even add it to the help text if it makes sense. I will see how I can make it apparent of what needs to be avoided when using extra args. A warning can also be shown if target or pep517 are overridden. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#827>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMOUSWUUT2IDJDRVY5WFATQ4UG3XANCNFSM4J4D7RXA> .

[lambdaclan]

Hello @nerdvegas when you get a chance can you please give me an update on the status of this PR? I did the changes as discussed above so if there are no other stoppers maybe we can merge?

Thank you 🙏

[nerdvegas]

Hey Ira, No problem will get this merged really soon. Thx A

…

On Mon, 3 Feb. 2020, 15:33 Ira, ***@***.***> wrote: Hello @nerdvegas <https://github.com/nerdvegas> when you get a chance can you please give me an update on the status of this PR? I did the changes as discussed above so if there are no other stoppers maybe we can merge? Thank you 🙏 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#827>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMOUSWMQIBFVIUL7QFNULTRA6NCHANCNFSM4J4D7RXA> .