Merge pull request #2731 from gonzalobf/gonzalo-dereference-symbolic-…

…links

Dereference symbolic links when copying files from ct data folder

apps/rebar/src/rebar_file_utils.erl

@@ -33,6 +33,7 @@
          symlink_or_copy/2,
          rm_rf/1,
          cp_r/2,
+         cp_r/3,
          mv/2,
          delete_each/1,
          write_file_if_contents_differ/2,
@@ -149,7 +150,7 @@ win32_symlink_or_copy(Source, Target) ->
             [{use_stdout, false}, return_on_error]),
     case win32_mklink_ok(Res, Target) of
         true -> ok;
-        false -> cp_r_win32(Source, drop_last_dir_from_path(Target))
+        false -> cp_r_win32(Source, drop_last_dir_from_path(Target), [])
     end.
 
 %% @private specifically pattern match against the output
@@ -207,9 +208,18 @@ rm_rf(Target) ->
     end.
 
 -spec cp_r(list(string()), file:filename()) -> 'ok'.
-cp_r([], _Dest) ->
-    ok;
 cp_r(Sources, Dest) ->
+    cp_r(Sources, Dest, []).
+
+%% @doc Copies files and directories.
+%% Options is a proplist with the options to be added to the copy command.
+%% It options are:
+%% - [{dereference, true|false}]: When true, if the file is a symbolic link
+%% it dereferences and copies the original content in Dest
+-spec cp_r(list(string()), file:filename(), proplists:proplist()) -> 'ok'.
+cp_r([], _Dest, _Options) ->
+    ok;
+cp_r(Sources, Dest, Options) ->
     case os:type() of
         {unix, Os} ->
             EscSources = [rebar_utils:escape_chars(Src) || Src <- Sources],
@@ -229,12 +239,19 @@ cp_r(Sources, Dest) ->
             {ok, []} = rebar_utils:sh(?FMT("mkdir -p ~ts",
                            [rebar_utils:escape_chars(Dest)]),
                       [{use_stdout, false}, abort_on_error]),
-            {ok, []} = rebar_utils:sh(?FMT("cp -Rp ~ts \"~ts\"",
-                                           [Source, rebar_utils:escape_double_quotes(Dest)]),
+
+            DefaultOptStr = "-Rp",
+            OptStr = case proplists:get_value(dereference, Options, false) of
+                true -> DefaultOptStr ++ "L";
+                false -> DefaultOptStr
+            end,
+
+            {ok, []} = rebar_utils:sh(?FMT("cp ~s ~ts \"~ts\"",
+                                           [OptStr, Source, rebar_utils:escape_double_quotes(Dest)]),
                                       [{use_stdout, true}, abort_on_error]),
             ok;
         {win32, _} ->
-            lists:foreach(fun(Src) -> ok = cp_r_win32(Src,Dest) end, Sources),
+            lists:foreach(fun(Src) -> ok = cp_r_win32(Src,Dest,Options) end, Sources),
             ok
     end.
 
@@ -531,9 +548,24 @@ delete_each_dir_win32([Dir | Rest]) ->
                               [{use_stdout, false}, return_on_error]),
     delete_each_dir_win32(Rest).
 
-xcopy_win32(Source,Dest)->
+xcopy_win32(Source,Dest, Options)->
     %% "xcopy \"~ts\" \"~ts\" /q /y /e 2> nul", Changed to robocopy to
     %% handle long names. May have issues with older windows.
+
+    CopySubdirectories = "/e",
+    DontFollow = "/sl",
+
+    Opt = [CopySubdirectories],
+    % By default Windows follows symbolic links except if the "/sl" options is given.
+    % Add "/sl" for default so it doesn't follow symbolic links and behaves more like unix
+    OptStr = case proplists:get_value(dereference, Options, false) of
+        true -> 
+            string:join(Opt, " ");
+        false -> 
+            % Default option
+            string:join([DontFollow|Opt], " ")
+    end,
+
     Cmd = case filelib:is_dir(Source) of
               true ->
                   %% For robocopy, copying /a/b/c/ to /d/e/f/ recursively does not
@@ -542,14 +574,16 @@ xcopy_win32(Source,Dest)->
                   %% must manually add the last fragment of a directory to the `Dest`
                   %% in order to properly replicate POSIX platforms
                   NewDest = filename:join([Dest, filename:basename(Source)]),
-                  ?FMT("robocopy \"~ts\" \"~ts\" /e 1> nul",
+                  ?FMT("robocopy \"~ts\" \"~ts\" ~s 1> nul",
                        [rebar_utils:escape_double_quotes(filename:nativename(Source)),
-                        rebar_utils:escape_double_quotes(filename:nativename(NewDest))]);
+                        rebar_utils:escape_double_quotes(filename:nativename(NewDest)),
+                        OptStr]);
               false ->
-                  ?FMT("robocopy \"~ts\" \"~ts\" \"~ts\" /e 1> nul",
+                  ?FMT("robocopy \"~ts\" \"~ts\" \"~ts\" ~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))])
+                        rebar_utils:escape_double_quotes(filename:basename(Source)),
+                        OptStr])
           end,
     Res = rebar_utils:sh(Cmd,
                 [{use_stdout, false}, return_on_error]),
@@ -561,21 +595,32 @@ xcopy_win32(Source,Dest)->
                                             [Source, Dest]))}
     end.
 
-cp_r_win32({true, SourceDir}, {true, DestDir}) ->
+cp_r_win32({true, SourceDir}, {true, DestDir}, Options) ->
     %% from directory to directory
      ok = case file:make_dir(DestDir) of
              {error, eexist} -> ok;
              Other -> Other
          end,
-    ok = xcopy_win32(SourceDir, DestDir);
-cp_r_win32({false, Source} = S,{true, DestDir}) ->
+    ok = xcopy_win32(SourceDir, DestDir, Options);
+cp_r_win32({false, Source} = S,{true, DestDir}, Options) ->
     %% from file to directory
-    cp_r_win32(S, {false, filename:join(DestDir, filename:basename(Source))});
-cp_r_win32({false, Source},{false, Dest}) ->
+    cp_r_win32(S, {false, filename:join(DestDir, filename:basename(Source))}, Options);
+cp_r_win32({false, Source},{false, Dest}, Options) ->
     %% from file to file
-    {ok,_} = file:copy(Source, Dest),
+    case file:read_link(Source) of
+        {ok, OriginalFile} -> case proplists:get_value(dereference, Options, false) of
+            true ->
+                {ok, _} = file:copy(Source, Dest),
+                ok;
+            false -> 
+                file:make_symlink(OriginalFile, Dest)
+            end;
+        _ -> 
+            {ok, _} = file:copy(Source, Dest),
+            ok
+    end,
     ok;
-cp_r_win32({true, SourceDir}, {false, DestDir}) ->
+cp_r_win32({true, SourceDir}, {false, DestDir}, Options) ->
     case filelib:is_regular(DestDir) of
         true ->
             %% From directory to file? This shouldn't happen
@@ -587,16 +632,16 @@ cp_r_win32({true, SourceDir}, {false, DestDir}) ->
             %% So let's attempt to create this directory
             case ensure_dir(DestDir) of
                 ok ->
-                    ok = xcopy_win32(SourceDir, DestDir);
+                    ok = xcopy_win32(SourceDir, DestDir, Options);
                 {error, Reason} ->
                     {error, lists:flatten(
                               io_lib:format("Unable to create dir ~p: ~p\n",
                                             [DestDir, Reason]))}
             end
     end;
-cp_r_win32(Source,Dest) ->
+cp_r_win32(Source, Dest, Options) ->
     Dst = {filelib:is_dir(Dest), Dest},
     lists:foreach(fun(Src) ->
-                          ok = cp_r_win32({filelib:is_dir(Src), Src}, Dst)
+                          ok = cp_r_win32({filelib:is_dir(Src), Src}, Dst, Options)
                   end, filelib:wildcard(Source)),
     ok.

apps/rebar/src/rebar_prv_common_test.erl

@@ -621,7 +621,7 @@ copy_bare_suites(From, To) ->
     DataDirs = lists:filter(fun filelib:is_dir/1,
                             filelib:wildcard(filename:join([From, "*_SUITE_data"]))),
     ok = rebar_file_utils:cp_r(SrcFiles, To),
-    rebar_file_utils:cp_r(DataDirs, To).
+    rebar_file_utils:cp_r(DataDirs, To, [{dereference, true}]).
 
 maybe_copy_spec(State, [App|Apps], Spec) ->
     case rebar_file_utils:path_from_ancestor(filename:dirname(Spec), rebar_app_info:dir(App)) of

apps/rebar/test/rebar_file_utils_SUITE.erl

@@ -25,7 +25,9 @@
          mv_file_diff/1,
          mv_file_dir_same/1,
          mv_file_dir_diff/1,
-         mv_no_clobber/1]).
+         mv_no_clobber/1,
+         cp_r_copies_files/1,
+         cp_r_dereferences_symbolic_links/1]).
 
 -include_lib("common_test/include/ct.hrl").
 -include_lib("eunit/include/eunit.hrl").
@@ -38,6 +40,7 @@ all() ->
     [{group, tmpdir},
      {group, reset_dir},
      {group, mv},
+     {group, cp},
      path_from_ancestor,
      canonical_path,
      absolute_path,
@@ -50,7 +53,8 @@ groups() ->
     [{tmpdir, [], [raw_tmpdir, empty_tmpdir, simple_tmpdir, multi_tmpdir]},
      {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]}].
+               mv_file_dir_same, mv_file_dir_diff, mv_no_clobber]},
+     {cp, [], [cp_r_copies_files, cp_r_dereferences_symbolic_links]}].
 
 init_per_group(reset_dir, Config) ->
     TmpDir = rebar_file_utils:system_tmpdir(["rebar_file_utils_SUITE", "resettable"]),
@@ -381,3 +385,104 @@ mk_base_dir(BasePath, Name) ->
     Path = filename:join(BasePath, atom_to_list(Name) ++ Index),
     ec_file:mkdir_p(Path),
     Path.
+
+cp_r_copies_files(Config) ->
+    % Checks that the files in src/ are copied to dest/
+    PrivDir = ?config(priv_dir, Config),
+    BaseDir = mk_base_dir(PrivDir, cp_r_copies_files),
+    {SrcList, DestDir} = create_dir_tree_helper(BaseDir),
+
+    rebar_file_utils:cp_r(SrcList, DestDir, []),
+
+    % dest/file
+    DestFile = filename:join(DestDir, "file"),
+    ?assert(filelib:is_file(DestFile)),
+    ?assertEqual({ok, <<"hello">>}, file:read_file(DestFile)),
+    % dest/symbolic
+    DestSym = filename:join(DestDir, "symbolic"),
+    {ok, #file_info{type = symlink}} = file:read_link_info(DestSym),
+    % dest/sub_dir
+    DestSubDir = filename:join(DestDir, "sub_dir"),
+    ?assert(filelib:is_dir(DestSubDir)),
+    % dest/sub_dir/sub_file
+    DestSubFile = filename:join(DestSubDir, "sub_file"),
+    ?assert(filelib:is_file(DestSubFile)),
+    ?assertEqual({ok, <<"hello">>}, file:read_file(DestSubFile)),
+    % dest/sub_dir/sub_symbolic
+    DestSubSymbolic = filename:join(DestSubDir, "sub_symbolic"),
+    {ok, #file_info{type = symlink}} = file:read_link_info(DestSubSymbolic),
+    ?assertEqual({ok, <<"hello">>}, file:read_file(DestSubSymbolic)).
+
+cp_r_dereferences_symbolic_links(Config) ->
+    % Checks that the files in src/ are copied to dest/ AND symbolic file has been dereference
+    PrivDir = ?config(priv_dir, Config),
+    BaseDir = mk_base_dir(PrivDir, cp_r_dereferences_symbolic_links),
+    {SrcList, DestDir} = create_dir_tree_helper(BaseDir),
+
+    rebar_file_utils:cp_r(SrcList, DestDir, [{dereference, true}]),
+
+    % dest/file
+    DestFile = filename:join(DestDir, "file"),
+    ?assert(filelib:is_file(DestFile)),
+    ?assertEqual({ok, <<"hello">>}, file:read_file(DestFile)),
+    % dest/symbolic
+    % At this point symbolic is not anymore a symbolic link but a regular file
+    DestSym = filename:join(DestDir, "symbolic"),
+    {ok, #file_info{type = regular}} = file:read_link_info(DestSym),
+    ?assertEqual({ok, <<"hello">>}, file:read_file(DestSym)),
+    % dest/sub_dir
+    DestSubDir = filename:join(DestDir, "sub_dir"),
+    ?assert(filelib:is_dir(DestSubDir)),
+    % dest/sub_dir/sub_file
+    DestSubFile = filename:join(DestSubDir, "sub_file"),
+    ?assert(filelib:is_file(DestSubFile)),
+    ?assertEqual({ok, <<"hello">>}, file:read_file(DestSubFile)),
+    % dest/sub_dir/sub_symbolic
+    % At this point sub_symbolic is not anymore a symbolic link but a regular file
+    DestSubSymbolic = filename:join(DestSubDir, "sub_symbolic"),
+    {ok, #file_info{type = regular}} = file:read_link_info(DestSubSymbolic),
+    ?assertEqual({ok, <<"hello">>}, file:read_file(DestSubSymbolic)).
+
+create_dir_tree_helper(BaseDir) ->
+    % Give a directory path, it creates the tree directory below and returns a map
+    % with the expected paths in dest/ aftercopying them.
+    %
+    % sub_dir is it needed so try all the win32 copy options  
+    %
+    % |-- src
+    %   |-- file
+    %   |-- symbolic -> file
+    %   |-- sub_dir
+    %       |-- sub_file
+    %       |-- sub_symbolic
+    % |-- dest
+    Dir = [
+        {folder, ["src"]},
+        {folder, ["dest"]},
+        {file, ["src", "file"], <<"hello">>},
+        {symbolic, ["src", "symbolic"], ["src", "file"]},
+        {folder, ["src", "sub_dir"]},
+        {file, ["src", "sub_dir", "sub_file"], <<"hello">>},
+        {symbolic, ["src", "sub_dir", "sub_symbolic"], ["src", "file"]}
+    ],
+    ok = create_by_type(Dir, BaseDir),
+    SrcList = [
+        filename:join([BaseDir,"src", File])
+        || File <- ["file", "symbolic", "sub_dir"]
+    ],
+    DestDir = filename:join(BaseDir, "dest"),
+    {SrcList, DestDir}.
+
+create_by_type([], _) ->
+    ok;
+create_by_type([{folder, Path} |Rest], BaseDir) ->
+    ok = ec_file:mkdir_p(filename:join([BaseDir | Path])),
+    create_by_type(Rest, BaseDir);
+create_by_type([{file, Path, Content} |Rest], BaseDir) ->
+    ok = file:write_file(filename:join([BaseDir | Path]), Content),
+    create_by_type(Rest, BaseDir);
+create_by_type([{symbolic, Path, TargetFile} |Rest], BaseDir) ->
+    Target = filename:join([BaseDir | TargetFile]),
+    Symbolic = filename:join([BaseDir | Path]),
+    ok = file:make_symlink(Target, Symbolic),
+    create_by_type(Rest, BaseDir).