Add support for --rcfile argument on powershell

[HansBaldzuhn]

Hello,

We are using the --rcflag on our linux systems to mannage the defaults alias and customize the prompt with more information.

To have the same behavior on windows, I changed the code to inject the value of --rcfile inside the rez-shell.ps1 It is injected just before the info message You are now in a rez-configured environment So just after the default prompt override so that the specified rcfile can override the prompt.

This has only been tested on windows

[HansBaldzuhn]

Hum, it fails for windows, specificaly in test_shell.py test_rcfile

If I understand well, if the command argument is specified, the --rcfile flag should be ignored. At least this is what happens for the bash shell. This is not happening in my modification for now

[HansBaldzuhn]

Ok, it still fails, I will need to investigate more.

[HansBaldzuhn]

I'm trying to understand what the hello_world package is doing. It contains only one "binary" that is in fact a python file with no extension but with the #!/usr/bin/python header.

Windows cannot interpret the header. How can this possibly work ? Also, if python is not requested when creating the context, is won't be available on windows. On linux, as python is often part of the os builtin it is always accessible (at least the version provided by the distribution)

Is this test supposed to succeed on windows ?

edit: The hello world exemple is build with rez-bind hello_world. It is using the rez.util.py, create_executable_script and function is commented with:

# TODO: use distlib.ScriptMaker
# TODO: or, do the work ourselves to make this cross platform
# FIXME: *nix only
# TODO: make cross platform

But is is very surprising that all tests except test_rcfile, relying on the same hello_world packages are successful on windows. When running on my windows:

rez env hello_world
> hello_world

it triggers the windows dialog "Open with" and ask me to choose an executable. And even if I manually choose python.exe, I don't have any output. The test is expecting to have Hello Rez World! in output. I don't understand how this could be successful.

edit2: Ok, is is indeed very simple. The test: test_stdin is successful because the powershell shell is ignoring the stdin parameter and thus the test is not run at all. Also, before my modification, the rc_file argument was also ignored and the test was never run. Now it tries to run and fails, but not because of my modification but because the test is not possible on windows.

Is anyone able to confirm this ?

[mottosso]

Hi @HansBaldzuhn, thanks for this PR, and for investigating the cause of the problem!

Also, before my modification, the rc_file argument was also ignored and the test was never run.

I had a quick look at this, and I think you're right. Some tests are excluded based on platform and shell, and in this case it looks like this addition highlighted a problem with one of the tests. I think the simple solution would be to exclude this test from running on PowerShell, or Windows altogether. But we should have a test for --rcfile.

So instead I think we could fix hello_world.

The problem, as you found, was that the resulting package includes a binary but that binary isn't usable on Windows. So let's make it so. If you have a look at the neighbor python.py binding, you'll find that it implements something very similar to hello_world.py, also for Windows. Basically just a .bat file if the platform is windows.

https://github.com/mottosso/bleeding-rez/blob/860c5b68014a2761349640532d14ddd5a157a04d/src/rez/bind/python.py#L49-L61

Would you mind have a go at implementing (basically copy/pasting) this for hello_world? I think that should do the trick!

[HansBaldzuhn]

Hi @mottosso,

I already have made a windows compatible version of hello_world. But I am not doing it in the same way as the exemple from the python binding. The hello_world is specifically said to contain no requirement. (even if it actually needs python to run, but it il always available on linux anyway ...) https://github.com/mottosso/bleeding-rez/blob/860c5b68014a2761349640532d14ddd5a157a04d/src/rez/bind/hello_world.py#L1-L7

To allow a windows version without python dependency, I created a bat file that does the same thing as the python version. This works fine now on my computer.

I'm currently checking if the --rcfile implementation is not having side effect on other arguments. Then I will perform a few cleanups before pushing a new commit.

[mottosso]

The hello_world is specifically said to contain no requirement. (even if it actually needs python to run, but it il always available on linux anyway ...)

Ah, yes I see what you mean.

In this case, it looks like the hello_world example needs updating to not require Python, which I'm surprised it does. We could quite easily make this a .sh script for Linux and a .bat script for Windows, that takes arguments akin to these, using the shell scripting language rather than Python. That way, it'll (1) not have any dependencies, (2) run on every platform and (3) make everyone happy.

I probably won't have time for that prior to the holidays coming up, but can try and take a look at this afterwards. As far as this feature goes, I think it looks great and that it should get included.

If you find a moment to salvage this yourself, that would be fantastic. If not then I'll try and remember merging this after I've had a look at the test. We could also try and ping upstream nerdvegas/rez, which will have the same problem, to see if maybe they're interested in solving it, as we could then port/copy that solution over.

Thanks again @HansBaldzuhn!

[HansBaldzuhn]

The code has been updated to allow running tests on windows.

I also added support for --stdin on windows powershell

The tests corresponding to this new feature have been udpated as well.

As for pinging upstream that would be nice ! But as far as I looked into the code, their implementation of windows is very rough. That's why they never needed an hello_world exemple to work on windows.

Have you ever though of porting your powershell implementation to nervegas/rez ?