Skip to content
This repository has been archived by the owner on May 12, 2018. It is now read-only.

Use lowercase for Windows drive name to resolve issue #250 #274

Merged
merged 2 commits into from
May 19, 2014

Conversation

redpine50
Copy link
Contributor

On windows, bootstrap.bat failed with next error.
Command 'escriptize' not understood or not applicable

This happens because the drive name in path got from rebar_utils:get_cwd() and base_dir(Config) are different case.
Made the drive name the same lowercase using filename:absname().

On windows, bootstrap.bat failed with next error.
Command 'escriptize' not understood or not applicable

This happens because the drive name in path got from rebar_utils:get_cwd() and base_dir(Config) are different case.
Made the drive name the same lowercase using filename:absname().
@@ -316,7 +316,8 @@ processing_base_dir(Config) ->
processing_base_dir(Config, Cwd).

processing_base_dir(Config, Dir) ->
Dir =:= base_dir(Config).
Dir_abs = filename:absname(Dir),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AbsDir would be more consistent.

@ghost
Copy link

ghost commented May 7, 2014

Thanks a lot. I don't have access to a Windows environment, so it's great you were able to track down and fix the bug.

In addition to the code comments, can you please:

  • use a more descriptive commit message summary/title
  • reformat the commit message description following the guide

Also, feel free to add yourself to THANKS.

@ghost
Copy link

ghost commented May 8, 2014

@redpine50, out of curiosity, what exactly are the results of rebar_utils:base_dir/1 and rebar_utils:get_cwd/0 on Windows?

@redpine50
Copy link
Contributor Author

base_dir/1 should already return an absolute path, because that's how it's saved on startup
==> This seems the root cause. As you can see below, filename:absname() lowercases the drive letter, but rebar_utils:get_cwd() returns uppercase drive name.

Erlang/OTP 17 [erts-6.0] [64-bit] [smp:2:2] [async-threads:10]

Eshell V6.0  (abort with ^G)
1> rebar_utils:get_cwd().
"C:/ksj/erlang"
2> pwd().
C:/ksj/erlang
ok
3> filename:absname(rebar_utils:get_cwd()).
"c:/ksj/erlang"
4> filename:absname("foo").
"c:/ksj/erlang/foo"
5> 

@ghost
Copy link

ghost commented May 9, 2014

The patch seems like a reasonable fix, but interestingly @dgud tested and got the following results:

1> {ok, Pwd} = file:get_cwd().
{ok,"c:/Users/dgud/src"}
2> filename:absname(Pwd).
"c:/Users/dgud/src"
3> 

Nevertheless, we should merge this after @redpine50 has fixed the minor issues. If I had to make a guess, I'd say there's a difference in Windows configuration or the execution environment of escript causing the different results (seen by @redpine50 and @dgud).

@redpine50, just to be certain, in your Windows environment Pwd in {ok, Pwd} = file:get_cwd(). contains a path starting with an uppercase drive letter, doesn't it?

Also, if anyone wants to check what the underlying code does, see prim_file, efile_drv.c, and win_efile.c.

@redpine50
Copy link
Contributor Author

In my erlang R16B03-1 and 17 on Windows 7, file:get_cwd() returns drive letter as uppercase.

Anyway, filename:absname change the drive letter to lowercase as follows.

https://github.com/erlang/otp/blob/maint/lib/stdlib/src/filename.erl

fix_driveletter(Letter0) ->
    if
        Letter0 >= $A, Letter0 =< $Z ->
            Letter0+$a-$A;
        true ->
            Letter0
    end.

base_dir() returns already filename:absname()'ed path.
@dgud
Copy link
Contributor

dgud commented May 10, 2014

Since windows doesn't care about lower|upper case, I guess it's unknown
which case you are going to get.

And thus if you are going to compare the paths you will have to handle that.

On Sat, May 10, 2014 at 6:53 AM, redpine50 notifications@github.com wrote:

In my erlang R16B03-1 and 17 on Windows 7, file:get_cwd() returns drive
letter as uppercase.

Anyway, filename:absname change the drive letter to lowercase as follows.

https://github.com/erlang/otp/blob/maint/lib/stdlib/src/filename.erl

fix_driveletter(Letter0) ->
if
Letter0 >= $A, Letter0 =< $Z ->
Letter0+$a-$A;
true ->
Letter0
end.


Reply to this email directly or view it on GitHubhttps://github.com//pull/274#issuecomment-42731819
.

@ghost
Copy link

ghost commented May 10, 2014

@redpine50, you may want to squash the commits into one and add yourself to THANKS, but none of that is a (hard) merge blocker. Therefore, +1.

@ghost
Copy link

ghost commented May 15, 2014

@jaredmorrow, @Vagabond unfortunately this didn't get into 2.3.0. Can we at least get this merged to master?

jaredmorrow added a commit that referenced this pull request May 19, 2014
Use lowercase for Windows drive name to resolve issue #250
@jaredmorrow jaredmorrow merged commit 7d4d364 into rebar:master May 19, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants