mottosso / bleeding-rez

Rez - Reproducible software environments for Windows, Linux and MacOS
GNU Lesser General Public License v3.0
72 stars 10 forks source link

Bound Python shell expansion doens't work properly on Linux #90

Closed jasperges closed 5 years ago

jasperges commented 5 years ago

This is a continuation of this rez-pipz issue. As it turns out, the actual issue is with bleeding-rez, not rez-pipz.

What fails? Take this for example:

rez env python
> python -c "assert True"
  File "<string>", line 1
    assert
         ^
SyntaxError: invalid syntax

This particular Python command (assert True) is just an example. The important thing is that there is a space in the command. import sys would also fail for example. The root cause seems to be with how bleeding-rez runs the commands, specifically this part which was introduced with this commit.

nerdvegas/rez uses a different approach, which works fine on Linux. I am curious what the reason was to deviate from that approach. @mottosso Can you shed any light on this? If it had anything to do with Windows, would it be an option to revert to the 'old' behaviour for Linux?

mottosso commented 5 years ago

I am curious what the reason was to deviate from that approach

The original approach wasn't working on Windows. You can see if this has changed, by replacing this file with the original. It'll make a copy of the system Python and put it in a package, as opposed to creating this #! file.

mottosso commented 5 years ago

Oh, and once you've replaced it, you can go ahead and delete the ~/packages/python folder, and re-run rez bind python to re-generate it.

jasperges commented 5 years ago

Okay, thanks. I will check later if that works. By the way I also seem to have a fix. Replacing

{python} $*

with

{python} "$@"

seems to work. So adding double quotes and replacing the * with @.

Once I have done a bit more of testing, I might make a PR. :)

mottosso commented 5 years ago

Oo that is good. This would be preferable in a PR.

The thing about bind, is..

  1. It's only meant for getting started; it creates packages automatically for you, that you will likely want more control over later. Especially Python. On Windows, you've got rez-scoopz to install a proper Python as a package, on Linux however you will need to do that manually.
  2. As you'll find, bind is deprecated and frowned upon. It just doesn't have a viable alternative today for getting started quickly. Unless you had something like scoopz on all platforms.

With that in mind, having #! file reference your system Python takes about 2ms to finish, whereas copying the local install takes longer, and that's really all that matters in terms of which to prefer. So if that fix works, that would be fantastic.