Skip to content
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

Fix rebar_file_utils module on Windows with MSYS #142

Closed
wants to merge 4 commits into from
Closed

Fix rebar_file_utils module on Windows with MSYS #142

wants to merge 4 commits into from

Conversation

jkloetzke
Copy link
Contributor

Currently rebar_utils:sh/2 will invoke all commands through bash.exe if found. Otherwise the command will be executed directly. Despite the fact that the caller cannot know if the command is executed with Unix or Windows semantics it leads to the following problem:

The rebar_file_utils module (namely rm_rf/1, cp_r/2 and mv/2) is not working as expected on Windows when MSYS is installed. These functions call cmd.exe with some options (e.g. /c) and native path names. There are two problems with this: first any backslashes would have to be escaped (easy to fix) and options like '/c' are expanded to 'C:' by the MSYS automatic path name translation.

Therefore stop using bash on Windows to get a consistent behavior and to avoid the peculiarities of MSYS's automatic path conversion.

This will impact all calls to rebar_utils:sh/2 on Windows. I did test most of rebars commands (clean, compile, create, *-deps, generate) and the rebar_file_utils eunit test suite. But because this touches a very core part of rebar I'd love to get some testing feedback first before the pull request gets merged...

Currently rebar_utils:sh/2 will invoke all commands through bash.exe if
found. Otherwise the command will be executed directly. Despite the fact
that the caller cannot know if the command is executed with Unix or
Windows semantics it leads to problems with MSYS's automatic path name
translation.

Therefore remove bash usage on Windows to get a consistent behavior and
to avoid the peculiarities of MSYS's automatic path conversion. Instead
use cmd.exe as its typically needed by most commands.
@nrdufour
Copy link

nrdufour commented Oct 3, 2011

Very interesting. I was actually observing the same issue of using rebar on windows. Whatever environment you are using, you were loosing because of that mix.

I will try you patch for an integration I need to do on windows.

Thank you.

@juranki
Copy link
Contributor

juranki commented Oct 15, 2011

Hi,

I added windows support to file_utils and some to port_compiler. I no longer have a setup where I could quickly test with the various unixy environments on windows. But I read the diff and agree with most, if not all of it. One potential problem is that you cannot use sh/bash scripts in pre/post operations after the change. (Edit: it's still possible, if you specify it as 'bash script arg ...')

That said, I remember that I contemplated a lot which combination of the variants (plain win, msysgit, msys/mingw, cygwin, mozillabuild,...) rebar should try to support, without really coming to a conclusion. Or, the conclusion was that it'll be hard to properly cover more than one at a time. So, I think targeting plain windows first would be a good strategy.

cheers,
-juhani

@ghost
Copy link

ghost commented Oct 20, 2011

Thanks, merged.

@ghost ghost closed this Oct 20, 2011
jaredmorrow pushed a commit that referenced this pull request Nov 5, 2013
Make update-deps traverse deps breadth-first, top-down
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants