erlware / relx

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

Crash while creating tar file #312

Closed arjan closed 8 years ago

arjan commented 9 years ago

This happens when generating a release using exrm, which calls relx using the following command line:

/home/arjan/devel/example/exampleapp/_build/prod/lib/exrm/priv/bin/relx tar -V 1 --root /home/arjan/devel/example/exampleapp --config /home/arjan/devel/example/exampleapp/rel/files/relx.config --relname exampleapp --relvsn 1.0.3 --output-dir /home/arjan/devel/example/exampleapp/rel
{{case_clause,
     {'EXIT',
         {{badmatch,539},
          [{erl_tar,create_header,3,[{file,"erl_tar.erl"},{line,367}]},
           {erl_tar,add1,4,[{file,"erl_tar.erl"},{line,305}]},
           {erl_tar,foreach_while_ok,2,[{file,"erl_tar.erl"},{line,912}]},
           {systools_make,add_to_tar,3,
               [{file,"systools_make.erl"},{line,1878}]},
           {systools_make,add_appl,7,[{file,"systools_make.erl"},{line,1749}]},
           {systools_make,'-add_applications/5-fun-0-',6,
               [{file,"systools_make.erl"},{line,1568}]},
           {lists,foldl,3,[{file,"lists.erl"},{line,1261}]},
           {systools_make,add_applications,5,
               [{file,"systools_make.erl"},{line,1567}]}]}}},
 [{systools_make,'-add_applications/5-fun-0-',6,
      [{file,"systools_make.erl"},{line,1568}]},
  {lists,foldl,3,[{file,"lists.erl"},{line,1261}]},
  {systools_make,add_applications,5,[{file,"systools_make.erl"},{line,1567}]},
  {systools_make,mk_tar,6,[{file,"systools_make.erl"},{line,1561}]},
  {systools_make,mk_tar,5,[{file,"systools_make.erl"},{line,1537}]},
  {systools_make,make_tar,2,[{file,"systools_make.erl"},{line,335}]},
  {rlx_prv_archive,make_tar,3,[{file,"src/rlx_prv_archive.erl"},{line,74}]},
  {relx,run_provider,2,[{file,"src/relx.erl"},{line,280}]}]}

The weird thing is that it crashes inside erl_tar on the header creation:

https://github.com/erlang/otp/blob/maint/lib/stdlib/src/erl_tar.erl#L414

tsloughter commented 9 years ago

Damn, that looks hairy. Any way you can get me a project that reproduces this issue?

arjan commented 9 years ago

Yeah, that's what I thought. :) I'll try to create a project.

relx version is 1.0.1+build.35.reff8737c8, btw

arjan commented 9 years ago

Cannot reproduce it yet. Actually when I do a clean checkout of my own project in a separate directory, and perform the release, it works just fine, I just noticed. So the urgency is not very high :)

tsloughter commented 9 years ago

Ok. Someone in #elixir-lang just had the same problem. But I haven't got a response yet from them.

zipme commented 9 years ago

Got a similar issue here:

{case_clause,
     {'EXIT',
         {{badmatch,514},
          [{erl_tar,create_header,3,[{file,"erl_tar.erl"},{line,414}]},
           {erl_tar,add1,4,[{file,"erl_tar.erl"},{line,322}]},
           {erl_tar,foreach_while_ok,2,[{file,"erl_tar.erl"},{line,999}]},
           {systools_make,add_to_tar,3,
               [{file,"systools_make.erl"},{line,1878}]},
           {systools_make,add_appl,7,[{file,"systools_make.erl"},{line,1749}]},
           {systools_make,'-add_applications/5-fun-0-',6,
               [{file,"systools_make.erl"},{line,1568}]},
           {lists,foldl,3,[{file,"lists.erl"},{line,1261}]},
           {systools_make,add_applications,5,
               [{file,"systools_make.erl"},{line,1567}]}]}}},
 [{systools_make,'-add_applications/5-fun-0-',6,
      [{file,"systools_make.erl"},{line,1568}]},
  {lists,foldl,3,[{file,"lists.erl"},{line,1261}]},
  {systools_make,add_applications,5,[{file,"systools_make.erl"},{line,1567}]},
  {systools_make,mk_tar,6,[{file,"systools_make.erl"},{line,1561}]},
  {systools_make,mk_tar,5,[{file,"systools_make.erl"},{line,1537}]},
  {systools_make,make_tar,2,[{file,"systools_make.erl"},{line,335}]},
  {rlx_prv_archive,make_tar,3,[{file,"src/rlx_prv_archive.erl"},{line,82}]},
  {relx,run_provider,2,[{file,"src/relx.erl"},{line,286}]}]}
zipme commented 9 years ago

And indeed, it works with a clean checkout.

tsloughter commented 8 years ago

Has anyone seen this happen recently and can help reproduce?

arjan commented 8 years ago

I haven't seen it in a long while now..

tsloughter commented 8 years ago

Cool, then I'm going to close this. Please reopen if you see it again.

arjan commented 8 years ago

:+1:

bitwalker commented 8 years ago

@tsloughter I think I know what may have caused this - filenames longer than what erl_tar supports, see bitwalker/exrm#299. I'm not sure what to do about this honestly - if there even is anything we can do beyond providing a better error message.

noizu commented 8 years ago

Does erl_tar not support tar creation with ustar or the newer posix format? It seems like things are crashing at the 100 char limit. Which will be an increasing problem for any elixir users relying on this release manager.

If it is possible to enable ustar or posix when invoking erl_tar adding an optional backwards compatible config option somewhere might make everyone's life easier and avoid the need for the elixir people to spin their own solution to accommodate more verbose beam file names caused by how protocols are generated and naming conventions.

If not, exrm might have to eventually fork out relx to change how builds are packaged which seems like a waste of opensource developer time.

bitwalker commented 8 years ago

@noizu The problem is not relx or exrm, but rather in Erlang's stdlib. There is no ustar option for erl_tar, as it writes ustar tarballs by default based on my reading of the code. There appear to be two specific issues though:

  1. There appears to be a bug here in erl_tar, where if the filename is too long, the last clause of split_filename is hit when Suffix is empty, which results in filename:join([]) being called, which is an invalid argument - thus an exception is thrown due to a long filename.
  2. There is something going on here in erl_tar when it writes the tar header, where the byte size is off by two bytes (hence the error in one of the stacktraces earlier in the thread. Given that two-byte discrepancy, it makes me question the presence of the "00" on line 411, but I don't know enough about the tarball format to know what is going on there exactly.

So as I mentioned above, based on my reading of erl_tar, it does indeed use the ustar option when writing the tar header, but the failure is occurring due to, what I'm guessing anyway, are flawed assumptions about filenames. It would be nice if someone more familiar with erl_tar or the tarball format could review the above issues and confirm what I'm seeing. At this point, I would need to find some time to hack on erl_tar and try to fix the issue. Sadly, I can't seem to reproduce the second issue, only the first, which blocks me from getting to the point where the second one can occur.

@tsloughter Any thoughts on the above?

martin-langhoff commented 7 years ago

We are hitting this on a project. Can we have this relx bug reopened?

bitwalker commented 7 years ago

The problem is that this isn't a relx bug - the problem is in systools (because we must use systools and because it requires the use of erl_tar and doesn't let you plug in your own module for tar'ing up the release) and erl_tar (where the actual bug occurs), and needs to be fixed by the OTP team (or someone who submits a PR to them). I did some initial digging (as you can see above), but have not had time to dig further, or even determine if what I was thinking the problem was, is actually it.

tsloughter commented 7 years ago

So it wouldn't be fixed by relx use relative instead of absolute paths?

bitwalker commented 7 years ago

@tsloughter For Erlang applications, that might effectively solve the problem - but for Elixir applications the problem tends to arise with module names with deep namespacing. Even with relative paths the names can reach the limit easily.

tsloughter commented 7 years ago

Ah. Is @martin-langhoff using Elixir?

Does that mean this is also an issue in distillery?

bitwalker commented 7 years ago

Yeah this basically affects any tool which builds on top of systools, so Distillery has this problem too. Elixir bears the brunt of it, because there is no namespacing to speak of in Erlang, so it just hasn't been a problem up until now.

slashdotdash commented 7 years ago

I wrote up some more detail on this problem in an issue for Distillery.

The root cause is a bug in OTP's erl_tar that mistakenly restricts the maximum filename in a tar file to 100 characters. It should allow up to 256 characters.

martin-langhoff commented 7 years ago

Yes, Elixir + Distillery.

martin-langhoff commented 7 years ago

@slashdotdash - have you tested it? you are saying the patch is

diff --git a/lib/stdlib/src/erl_tar.erl b/lib/stdlib/src/erl_tar.erl
index a383a0f..96b2ea8 100644
--- a/lib/stdlib/src/erl_tar.erl
+++ b/lib/stdlib/src/erl_tar.erl
@@ -263,7 +263,7 @@ format_error(Term) ->

 %% Length of these fields.

--define(th_name_len, 100).
+-define(th_name_len, 254).
 -define(th_mode_len, 8).
 -define(th_uid_len, 8).
 -define(th_gid_len, 8).
slashdotdash commented 7 years ago

@martin-langhoff I haven't tried it, but it looks reasonable that would extend the restriction.

martin-langhoff commented 7 years ago

Well, there's quite a few kinks. Which implementation is erl_tar header shooting for? The 255 limit is set in the ustar format, and that's not a straightforward field -

"The maximum length of a file name is limited to 256 characters, provided that the file name can be split at a directory separator in two parts, first of them being at most 155 bytes long. So, in most cases the maximum file name length will be shorter than 256 characters."

So we'd have to validate that erl_tar will DTRT. According to git blame, this section erl_tar of erl_tar has not changed since 2009 :-/

bitwalker commented 7 years ago

For those interested, I've opened a PR to OTP which updates erl_tar with support for modern tar formats, one of the primary benefits being that this issue will no longer be a problem. You can check it out here.

arjan commented 7 years ago

Nice! Impressive PR :)

bitwalker commented 7 years ago

PR was merged, not sure when it will be released though.