erlware / relx

Sane, simple release creation for Erlang
http://erlware.github.io/relx
Apache License 2.0
697 stars 232 forks source link

[BUG] Tar includes a corrupt version of itself if you use overlay on the release root folder #674

Closed elbrujohalcon closed 5 years ago

elbrujohalcon commented 5 years ago

I created a repository where I describe this issue: https://github.com/elbrujohalcon/relx-sample

TL;DR

If you have an overlay like {copy, "extra_folder/", "."}, the tar generation will include the actual tar file being generated within the generated tar file.

lrascao commented 5 years ago

you came across this while trying to accomplish something else or this was what you intended to do?

elbrujohalcon commented 5 years ago

In one of my projects at @adroll, I have a scripts folder with contents that are dynamically generated based on some configuration. I need to include that folder in the root of the release so that I can find it in the target machines as $ROOT/scripts and run the scripts in it. That's what triggered all this.

lrascao commented 5 years ago

cool, i use that as well, i do it like this though (example):

{mkdir, "scripts"} {copy, "scripts/lib.link.sh", "bin/lib.link.sh"}

elbrujohalcon commented 5 years ago

That's similar but it has 2 main differences with my approach:

  1. You have a fixed list of scripts. The contents of my scripts folder are determined just before building the release. And they're also a lot, with subfolders and what-not. I can't easily list them all in the relx.config file. That's why I used {copy, "scripts/", "."} instead.
  2. You're dumping your scripts in your bin folder. I don't want my scripts there, I need them in a folder called scripts/ right there on the root of the release. 🤷‍♂️
ferd commented 5 years ago

to be clear, this is also a bit of a problem because outside of tarballs, the paths are relative to the current project, but if you put in a path like /a/b/c the path will be handled as absolute. However in the tarball, the path will be handled as relative to the root of the tarball.

So to be safe, you must use a relative path, but here . seems to be an unsafe value as well.

elbrujohalcon commented 5 years ago

Not exactly, @ferd … even if you replace . by any other way to reference that same path (as long as it doesn't start with bin/) the same thing will happen. relx_prv_archive is just asking erl_tar to compress all the paths on the right side of overlays and . (or /the/absolute/path/to/_build/default/rel/your_app/) is included in that list. Since the generated tar.gz file lands on that same path… while it's being generated, it finds itself and tries to compress it[self]. What I think would be a relatively cheap (although not thorough) solution would be to generate the new tar.gz in a temp folder and then move it to its final destination once it's done. That will prevent the inclusion of the intermediate tar file, although I think a more careful analysis of why are we just adding everything that is on the right side of the overlays in the tar file instead of using a more precise approach is due. Another approach would be to better handle copying folders… maybe with a new overlay (copy_folder or copy_recursive) that allows us to write stuff like {copy_folder, "this_folder", "this_folder"} (like we currently do with files). That will effectively remove the need of adding . on the right side of overlays.

tsloughter commented 5 years ago

I just ran into this myself :).

I think it can be fixed by improving these functions:

overlay_files(_, undefined, _) ->
    [];
overlay_files(OverlayVars, Overlay, OutputDir) ->
    [begin
         To = to(O),
         File = rlx_prv_overlay:render_string(OverlayVars, To),
         {ec_cnv:to_list(File), ec_cnv:to_list(filename:join(OutputDir, File))}
     end || O <- Overlay, filter(O)].

to({link, _, To}) ->
    To;
to({copy, _, To}) ->
    To;
to({mkdir, To}) ->
    To;
to({template, _, To}) ->
    To.

It is treating To like it must be a file or directory name and not possibly . and that is why the whole damn thing is being copied.

I might just filter on "."...