-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for --rcfile argument on powershell #92
base: master
Are you sure you want to change the base?
Add support for --rcfile argument on powershell #92
Conversation
Hum, it fails for windows, specificaly in If I understand well, if the command argument is specified, the --rcfile flag should be ignored. |
(mimic the behavior of the bash shell
Ok, it still fails, I will need to investigate more. |
I'm trying to understand what the hello_world package is doing. Windows cannot interpret the header. How can this possibly work ? Is this test supposed to succeed on windows ? edit: The hello world exemple is build with rez-bind hello_world. # 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.
it triggers the windows dialog "Open with" and ask me to choose an executable. edit2: Is anyone able to confirm this ? |
…ackage has now platform variants)
Hi @HansBaldzuhn, thanks for this PR, and for investigating the cause of the problem!
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 So instead I think we could fix 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 bleeding-rez/src/rez/bind/python.py Lines 49 to 61 in 860c5b6
Would you mind have a go at implementing (basically copy/pasting) this for hello_world? I think that should do the trick! |
Hi @mottosso, I already have made a windows compatible version of hello_world. bleeding-rez/src/rez/bind/hello_world.py Lines 1 to 7 in 860c5b6
To allow a windows version without python dependency, I created a bat file that does the same thing as the python version. I'm currently checking if the --rcfile implementation is not having side effect on other arguments. |
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 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! |
Added --stdin support on windows hello_world test package is now linux and windows compatible tests in test_shells have been updated to allow testing on windows
The code has been updated to allow running tests on windows. I also added support for --stdin on windows powershellThe tests corresponding to this new feature have been udpated as well. As for pinging upstream that would be nice ! Have you ever though of porting your powershell implementation to nervegas/rez ? |
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