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.

Sign up for GitHub

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 mv on windows #1457

Merged
merged 3 commits into from
Jan 27, 2017
Merged

Fix mv on windows #1457

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
92e6997
Fix mv command on windows
ferd Jan 27, 2017
bc3b53f
Rework test to work and pass on linux
ferd Jan 27, 2017
3f27d87
Re-fix the windows file movement
ferd Jan 27, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 99 additions & 18 deletions src/rebar_file_utils.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,32 +185,113 @@ mv(Source, Dest) ->
ok
end;
{win32, _} ->
Cmd = case filelib:is_dir(Source) of
true ->
?FMT("robocopy /move /e \"~s\" \"~s\" 1> nul",
[rebar_utils:escape_double_quotes(filename:nativename(Source)),
rebar_utils:escape_double_quotes(filename:nativename(Dest))]);
false ->
?FMT("robocopy /move /e \"~s\" \"~s\" \"~s\" 1> nul",
[rebar_utils:escape_double_quotes(filename:nativename(filename:dirname(Source))),
rebar_utils:escape_double_quotes(filename:nativename(Dest)),
rebar_utils:escape_double_quotes(filename:basename(Source))])
end,
Res = rebar_utils:sh(Cmd,
[{use_stdout, false}, return_on_error]),
case win32_ok(Res) of
true -> ok;
case filelib:is_dir(Source) of
true ->
SrcDir = filename:nativename(Source),
DestDir = case filelib:is_dir(Dest) of
true ->
%% to simulate unix/posix mv, we have to replicate
%% the same directory movement by moving the whole
%% top-level directory, not just the insides
SrcName = filename:basename(Source),
filename:nativename(filename:join(Dest, SrcName));
false ->
filename:nativename(Dest)
end,
robocopy_dir(SrcDir, DestDir);
false ->
{error, lists:flatten(
io_lib:format("Failed to move ~s to ~s~n",
[Source, Dest]))}
SrcDir = filename:nativename(filename:dirname(Source)),
SrcName = filename:basename(Source),
DestDir = filename:nativename(filename:dirname(Dest)),
DestName = filename:basename(Dest),
IsDestDir = filelib:is_dir(Dest),
if IsDestDir ->
%% if basename and target name are different because
%% we move to a directory, then just move there.
%% Similarly, if they are the same but we're going to
%% a directory, let's just do that directly.
FullDestDir = filename:nativename(Dest),
robocopy_file(SrcDir, FullDestDir, SrcName)
; SrcName =:= DestName ->
%% if basename and target name are the same and both are files,
%% we do a regular move with robocopy without rename.
robocopy_file(SrcDir, DestDir, DestName)
; SrcName =/= DestName->
robocopy_mv_and_rename(Source, Dest, SrcDir, SrcName, DestDir, DestName)
end

end
end.

robocopy_mv_and_rename(Source, Dest, SrcDir, SrcName, DestDir, DestName) ->
%% If we're moving a file and the origin and
%% destination names are different:
%% - mktmp
%% - robocopy source_dir tmp_dir srcname
%% - rename srcname destname (to avoid clobbering)
%% - robocopy tmp_dir dest_dir destname
%% - remove tmp_dir
case ec_file:insecure_mkdtemp() of
{error, _Reason} ->
{error, lists:flatten(
io_lib:format("Failed to move ~s to ~s (tmpdir failed)~n",
[Source, Dest]))};
TmpPath ->
case robocopy_file(SrcDir, TmpPath, SrcName) of
{error, Reason} ->
{error, Reason};
ok ->
TmpSrc = filename:join(TmpPath, SrcName),
TmpDst = filename:join(TmpPath, DestName),
case file:rename(TmpSrc, TmpDst) of
{error, _} ->
{error, lists:flatten(
io_lib:format("Failed to move ~s to ~s (via rename)~n",
[Source, Dest]))};
ok ->
case robocopy_file(TmpPath, DestDir, DestName) of
Err = {error, _} -> Err;
OK -> rm_rf(TmpPath), OK
end
end
end
end.

robocopy_file(SrcPath, DestPath, FileName) ->
Cmd = ?FMT("robocopy /move /e \"~s\" \"~s\" \"~s\"",
[rebar_utils:escape_double_quotes(SrcPath),
rebar_utils:escape_double_quotes(DestPath),
rebar_utils:escape_double_quotes(FileName)]),
Res = rebar_utils:sh(Cmd, [{use_stdout, false}, return_on_error]),
case win32_ok(Res) of
false ->
{error, lists:flatten(
io_lib:format("Failed to move ~s to ~s~n",
[filename:join(SrcPath, FileName),
filename:join(DestPath, FileName)]))};
true ->
ok
end.

robocopy_dir(Source, Dest) ->
Cmd = ?FMT("robocopy /move /e \"~s\" \"~s\"",
[rebar_utils:escape_double_quotes(Source),
rebar_utils:escape_double_quotes(Dest)]),
Res = rebar_utils:sh(Cmd,
[{use_stdout, false}, return_on_error]),
case win32_ok(Res) of
true -> ok;
false ->
{error, lists:flatten(
io_lib:format("Failed to move ~s to ~s~n",
[Source, Dest]))}
end.

win32_ok({ok, _}) -> true;
win32_ok({error, {Rc, _}}) when Rc<9; Rc=:=16 -> true;
win32_ok(_) -> false.


delete_each([]) ->
ok;
delete_each([File | Rest]) ->
Expand Down
190 changes: 188 additions & 2 deletions test/rebar_file_utils_SUITE.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
groups/0,
init_per_group/2,
end_per_group/2,
init_per_testcase/2,
end_per_testcase/2,
raw_tmpdir/1,
empty_tmpdir/1,
simple_tmpdir/1,
Expand All @@ -15,7 +17,13 @@
canonical_path/1,
resolve_link/1,
split_dirname/1,
mv_warning_is_ignored/1]).
mv_warning_is_ignored/1,
mv_dir/1,
mv_file_same/1,
mv_file_diff/1,
mv_file_dir_same/1,
mv_file_dir_diff/1,
mv_no_clobber/1]).

-include_lib("common_test/include/ct.hrl").
-include_lib("eunit/include/eunit.hrl").
Expand All @@ -25,6 +33,7 @@
all() ->
[{group, tmpdir},
{group, reset_dir},
{group, mv},
path_from_ancestor,
canonical_path,
resolve_link,
Expand All @@ -33,14 +42,30 @@ all() ->

groups() ->
[{tmpdir, [], [raw_tmpdir, empty_tmpdir, simple_tmpdir, multi_tmpdir]},
{reset_dir, [], [reset_nonexistent_dir, reset_empty_dir, reset_dir]}].
{reset_dir, [], [reset_nonexistent_dir, reset_empty_dir, reset_dir]},
{mv, [], [mv_dir, mv_file_same, mv_file_diff,
mv_file_dir_same, mv_file_dir_diff, mv_no_clobber]}].

init_per_group(reset_dir, Config) ->
TmpDir = rebar_file_utils:system_tmpdir(["rebar_file_utils_SUITE", "resetable"]),
[{tmpdir, TmpDir}|Config];
init_per_group(_, Config) -> Config.
end_per_group(_, Config) -> Config.

init_per_testcase(Test, Config) ->
case os:type() of
{win32, _} ->
case lists:member(Test, [resolve_link, mv_warning_is_ignored]) of
true -> {skip, "broken in windows"};
false -> Config
end;
_ ->
Config
end.

end_per_testcase(_Test, Config) ->
Config.

raw_tmpdir(_Config) ->
case rebar_file_utils:system_tmpdir() of
"/tmp" -> ok;
Expand Down Expand Up @@ -143,3 +168,164 @@ mv_warning_is_ignored(_Config) ->
meck:expect(rebar_utils, sh, fun("mv ding dong", _) -> {ok, "Warning"} end),
ok = rebar_file_utils:mv("ding", "dong"),
meck:unload(rebar_utils).

%%% Ensure Windows & Unix operations to move files

mv_dir(Config) ->
%% Move a directory to another one location
PrivDir = ?config(priv_dir, Config),
BaseDir = mk_base_dir(PrivDir, mv_dir),
SrcDir = filename:join(BaseDir, "src/"),
ec_file:mkdir_p(SrcDir),
?assert(filelib:is_dir(SrcDir)),
%% empty dir movement
DstDir1 = filename:join(BaseDir, "dst1/"),
?assertNot(filelib:is_dir(DstDir1)),
?assertEqual(ok, rebar_file_utils:mv(SrcDir, DstDir1)),
?assert(filelib:is_dir(DstDir1)),

%% move files from dir to empty dir
F1 = filename:join(SrcDir, "file1"),
F2 = filename:join(SrcDir, "subdir/file2"),
filelib:ensure_dir(F2),
file:write_file(F1, "hello"),
file:write_file(F2, "world"),
DstDir2 = filename:join(BaseDir, "dst2/"),
D2F1 = filename:join(DstDir2, "file1"),
D2F2 = filename:join(DstDir2, "subdir/file2"),
?assertNot(filelib:is_dir(DstDir2)),
?assertEqual(ok, rebar_file_utils:mv(SrcDir, DstDir2)),
?assert(filelib:is_file(D2F1)),
?assert(filelib:is_file(D2F2)),

%% move files from dir to existing dir moves it to
%% a subdir
filelib:ensure_dir(F2),
file:write_file(F1, "hello"),
file:write_file(F2, "world"),
DstDir3 = filename:join(BaseDir, "dst3/"),
D3F1 = filename:join(DstDir3, "src/file1"),
D3F2 = filename:join(DstDir3, "src/subdir/file2"),
ec_file:mkdir_p(DstDir3),
?assert(filelib:is_dir(DstDir3)),
?assertEqual(ok, rebar_file_utils:mv(SrcDir, DstDir3)),
?assertNot(filelib:is_file(F1)),
?assertNot(filelib:is_file(F2)),
?assert(filelib:is_file(D3F1)),
?assert(filelib:is_file(D3F2)),
?assertNot(filelib:is_dir(SrcDir)),
ok.

mv_file_same(Config) ->
%% Move a file from a directory to the other without renaming
PrivDir = ?config(priv_dir, Config),
BaseDir = mk_base_dir(PrivDir, mv_file_same),
SrcDir = filename:join(BaseDir, "src/"),
ec_file:mkdir_p(SrcDir),
?assert(filelib:is_dir(SrcDir)),
F = filename:join(SrcDir, "file"),
file:write_file(F, "hello"),
DstDir = filename:join(BaseDir, "dst/"),
ec_file:mkdir_p(DstDir),
Dst = filename:join(DstDir, "file"),
?assertEqual(ok, rebar_file_utils:mv(F, Dst)),
?assert(filelib:is_file(Dst)),
?assertNot(filelib:is_file(F)),
ok.

mv_file_diff(Config) ->
%% Move a file from a directory to another one while renaming
%% into a pre-existing file
PrivDir = ?config(priv_dir, Config),
BaseDir = mk_base_dir(PrivDir, mv_file_diff),
SrcDir = filename:join(BaseDir, "src/"),
ec_file:mkdir_p(SrcDir),
?assert(filelib:is_dir(SrcDir)),
F = filename:join(SrcDir, "file"),
file:write_file(F, "hello"),
DstDir = filename:join(BaseDir, "dst/"),
ec_file:mkdir_p(DstDir),
Dst = filename:join(DstDir, "file-rename"),
file:write_file(Dst, "not-the-right-content"),
?assert(filelib:is_file(Dst)),
?assertEqual(ok, rebar_file_utils:mv(F, Dst)),
?assert(filelib:is_file(Dst)),
?assertEqual({ok, <<"hello">>}, file:read_file(Dst)),
?assertNot(filelib:is_file(F)),
ok.

mv_file_dir_same(Config) ->
%% Move a file to a directory without renaming
PrivDir = ?config(priv_dir, Config),
BaseDir = mk_base_dir(PrivDir, mv_file_dir_same),
SrcDir = filename:join(BaseDir, "src/"),
ec_file:mkdir_p(SrcDir),
?assert(filelib:is_dir(SrcDir)),
F = filename:join(SrcDir, "file"),
file:write_file(F, "hello"),
DstDir = filename:join(BaseDir, "dst/"),
ec_file:mkdir_p(DstDir),
Dst = filename:join(DstDir, "file"),
?assert(filelib:is_dir(DstDir)),
?assertEqual(ok, rebar_file_utils:mv(F, DstDir)),
?assert(filelib:is_file(Dst)),
?assertNot(filelib:is_file(F)),
ok.

mv_file_dir_diff(Config) ->
%% Move a file to a directory while renaming
PrivDir = ?config(priv_dir, Config),
BaseDir = mk_base_dir(PrivDir, mv_file_dir_diff),
SrcDir = filename:join(BaseDir, "src/"),
ec_file:mkdir_p(SrcDir),
?assert(filelib:is_dir(SrcDir)),
F = filename:join(SrcDir, "file"),
file:write_file(F, "hello"),
DstDir = filename:join(BaseDir, "dst/"),
ec_file:mkdir_p(DstDir),
Dst = filename:join(DstDir, "file-rename"),
?assert(filelib:is_dir(DstDir)),
?assertNot(filelib:is_file(Dst)),
?assertEqual(ok, rebar_file_utils:mv(F, Dst)),
?assert(filelib:is_file(Dst)),
?assertNot(filelib:is_file(F)),
ok.

mv_no_clobber(Config) ->
%% Moving a file while renaming does not clobber other files
PrivDir = ?config(priv_dir, Config),
BaseDir = mk_base_dir(PrivDir, mv_no_clobber),
SrcDir = filename:join(BaseDir, "src/"),
ec_file:mkdir_p(SrcDir),
?assert(filelib:is_dir(SrcDir)),
F = filename:join(SrcDir, "file"),
file:write_file(F, "hello"),
FBad = filename:join(SrcDir, "file-alt"),
file:write_file(FBad, "wrong-data"),
DstDir = filename:join(BaseDir, "dst/"),
ec_file:mkdir_p(DstDir),
Dst = filename:join(DstDir, "file-alt"),
DstBad = filename:join(DstDir, "file"),
file:write_file(DstBad, "wrong-data"),
?assert(filelib:is_file(F)),
?assert(filelib:is_file(FBad)),
?assert(filelib:is_dir(DstDir)),
?assertNot(filelib:is_file(Dst)),
?assert(filelib:is_file(DstBad)),
?assertEqual(ok, rebar_file_utils:mv(F, Dst)),
?assert(filelib:is_file(Dst)),
?assertNot(filelib:is_file(F)),
?assert(filelib:is_file(DstBad)),
?assert(filelib:is_file(FBad)),
?assertEqual({ok, <<"hello">>}, file:read_file(Dst)),
?assertEqual({ok, <<"wrong-data">>}, file:read_file(FBad)),
?assertEqual({ok, <<"wrong-data">>}, file:read_file(DstBad)),
ok.


mk_base_dir(BasePath, Name) ->
{_,_,Micro} = os:timestamp(),
Index = integer_to_list(Micro),
Path = filename:join(BasePath, atom_to_list(Name) ++ Index),
ec_file:mkdir_p(Path),
Path.