diff options
| -rw-r--r-- | CONTRIBUTING.md | 6 | ||||
| -rw-r--r-- | THANKS | 1 | ||||
| -rw-r--r-- | src/rebar_file_utils.erl | 117 | ||||
| -rw-r--r-- | src/rebar_prv_new.erl | 3 | ||||
| -rw-r--r-- | src/rebar_templater.erl | 20 | ||||
| -rw-r--r-- | test/rebar_file_utils_SUITE.erl | 190 |
6 files changed, 307 insertions, 30 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8077813..16de1e5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -162,7 +162,7 @@ $ ./rebar3 ct ``` Most tests are named according to their module name followed by the `_SUITE` -suffi. Providers are made shorter, such that `rebar_prv_new` is tested in +suffix. Providers are made shorter, such that `rebar_prv_new` is tested in `rebar_new_SUITE`. Most tests in the test suite will rely on calling Rebar3 in its API form, @@ -285,7 +285,7 @@ The reviewer may ask you to later squash the commits together to provide a clean commit history before merging in the feature. It's important to write a proper commit title and description. The commit title -should fir around 50 characters; it is the first line of the commit text. The +should be no more than 50 characters; it is the first line of the commit text. The second line of the commit text must be left blank. The third line and beyond is the commit message. You should write a commit message. If you do, wrap all lines at 72 characters. You should explain what the commit does, what @@ -299,7 +299,7 @@ maintainers. When opening a pull request, explain what the patch is doing and if it makes sense, why the proposed implementation was chosen. Try to use well-defined commits (one feature per commit) so that reading -them and testing them is easier for reviewers and while bissecting the code +them and testing them is easier for reviewers and while bisecting the code base for issues. During the review process, you may be asked to correct or edit a few things @@ -138,3 +138,4 @@ Carlos Eduardo de Paula Derek Brown Heinz N. Gies Roberto Aloi +Andrew McRobb diff --git a/src/rebar_file_utils.erl b/src/rebar_file_utils.erl index 8645641..8158312 100644 --- a/src/rebar_file_utils.erl +++ b/src/rebar_file_utils.erl @@ -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]) -> diff --git a/src/rebar_prv_new.erl b/src/rebar_prv_new.erl index 064315e..152a56e 100644 --- a/src/rebar_prv_new.erl +++ b/src/rebar_prv_new.erl @@ -132,11 +132,14 @@ show_template({Name, Type, Location, Description, Vars}) -> format_vars(Vars)]). format_type(escript) -> "built-in"; +format_type(builtin) -> "built-in"; format_type(plugin) -> "plugin"; format_type(file) -> "custom". format_type(escript, _) -> "built-in template"; +format_type(builtin, _) -> + "built-in template"; format_type(plugin, Loc) -> io_lib:format("plugin template (~s)", [Loc]); format_type(file, Loc) -> diff --git a/src/rebar_templater.erl b/src/rebar_templater.erl index e64ce71..9b33ec5 100644 --- a/src/rebar_templater.erl +++ b/src/rebar_templater.erl @@ -267,8 +267,8 @@ find_templates(State) -> PluginTemplates = find_plugin_templates(State), {MainTemplates, Files} = case rebar_state:escript_path(State) of - undefined -> - {find_priv_templates(State), []}; + undefined -> % running in local install + {find_localinstall_templates(State), []}; _ -> %% Cache the files since we'll potentially need to walk it several times %% over the course of a run. @@ -307,11 +307,10 @@ find_escript_templates(Files) -> || {Name, _Bin} <- Files, re:run(Name, ?TEMPLATE_RE, [{capture, none}]) == match]. -find_priv_templates(State) -> - OtherTemplates = rebar_utils:find_files(code:priv_dir(rebar), ?TEMPLATE_RE), - HomeFiles = rebar_utils:find_files(rebar_dir:template_dir(State), - ?TEMPLATE_RE, true), % recursive - [{file, F} || F <- OtherTemplates ++ HomeFiles]. +find_localinstall_templates(_State) -> + Templates = rebar_utils:find_files(code:priv_dir(rebar), ?TEMPLATE_RE), + %% Pretend we're still running escripts; should work transparently. + [{builtin, F} || F <- Templates]. %% Fetch template indexes that sit on disk in the user's HOME find_disk_templates(State) -> @@ -354,6 +353,10 @@ prioritize_templates([{Name, Type, File} | Rest], Valid) -> ?DEBUG("Skipping template ~p, due to presence of a built-in " "template with the same name", [Name]), prioritize_templates(Rest, Valid); + {_, builtin, _} -> + ?DEBUG("Skipping template ~p, due to presence of a built-in " + "template with the same name", [Name]), + prioritize_templates(Rest, Valid); {_, plugin, _} -> ?DEBUG("Skipping template ~p, due to presence of a plugin " "template with the same name", [Name]), @@ -369,6 +372,9 @@ prioritize_templates([{Name, Type, File} | Rest], Valid) -> load_file(Files, escript, Name) -> {Name, Bin} = lists:keyfind(Name, 1, Files), Bin; +load_file(_Files, builtin, Name) -> + {ok, Bin} = file:read_file(Name), + Bin; load_file(_Files, plugin, Name) -> {ok, Bin} = file:read_file(Name), Bin; diff --git a/test/rebar_file_utils_SUITE.erl b/test/rebar_file_utils_SUITE.erl index 7285e13..4cc6a93 100644 --- a/test/rebar_file_utils_SUITE.erl +++ b/test/rebar_file_utils_SUITE.erl @@ -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, @@ -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"). @@ -25,6 +33,7 @@ all() -> [{group, tmpdir}, {group, reset_dir}, + {group, mv}, path_from_ancestor, canonical_path, resolve_link, @@ -33,7 +42,9 @@ 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"]), @@ -41,6 +52,20 @@ init_per_group(reset_dir, 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; @@ -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. |