Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Trying to sort through a crash related to compile_dir #146

Closed
choptastic opened this Issue Mar 9, 2014 · 3 comments

Comments

Projects
None yet
2 participants
Contributor

choptastic commented Mar 9, 2014

Hi,

I'm trying to sort through a crash related to ChicagoBoss/ChicagoBoss#449

It appears to be crashing when ChicagoBoss is passing "src/view/lib/tag_html" (a non-existent directory) to erlydtl_compiler:compile_dir, which ends up calling, the last erlydtl_compiler:do_compile clause with the (default) value of undefined for #dtl_context.bin. As far as I can tell, this is because the ParseTrail value is set to [] when called from erlydtl_compiler:process_opts when File matches {dir, _}. In the end, it's crashing because it's trying to get the md5 of the atom undefined. (https://github.com/erlydtl/erlydtl/blob/master/src/erlydtl_compiler.erl#L334)

As far as I can tell (I'm sure I'm missing something), the first do_compile clause (https://github.com/erlydtl/erlydtl/blob/master/src/erlydtl_compiler.erl#L430) will never match because is_compile_dir being set is contingent on ParseTrail being empty (https://github.com/erlydtl/erlydtl/blob/master/src/erlydtl_compiler.erl#L245).

Being new to ChicagoBoss and ErlyDTL isn't helping, as I don't have enough of an internal model of both systems to intuit myself into the right area.

The stacktrace for the error from ChicagoBoss is:

Error: error:badarg
Stacktrace: [{erlang,md5,[undefined],[]},
             {erlydtl_compiler,parse_template,2,
                               [{file,"src/erlydtl_compiler.erl"},{line,334}]},
             {erlydtl_compiler,do_compile,1,
                               [{file,"src/erlydtl_compiler.erl"},{line,436}]},
             {erlydtl_compiler,compile,1,
                               [{file,"src/erlydtl_compiler.erl"},{line,177}]},
             {boss_load,compile_view_dir_erlydtl,5,
                        [{file,"src/boss/boss_load.erl"},{line,312}]},
             {boss_load,load_view_lib,3,
                        [{file,"src/boss/boss_load.erl"},{line,376}]},
             {boss_load,'-make_all_modules/3-fun-1-',3,
                        [{file,"src/boss/boss_load.erl"},{line,84}]},
             {lists,map,2,[{file,"lists.erl"},{line,1224}]}]

Any help with this would be fantastic. Should ChicagoBoss not be passing the non-existent directory to it? Or should ErlyDTL be handling it more gracefully? Or is there an error here that's just unanticipated given the recent refactoring? What does ParseTrail even mean?

Thanks,

Owner

kaos commented Mar 9, 2014

It should be handled gracefully by erlydtl. Give me a day to look into it.

I'll get back with more details on the topic when I'm not on my phone :)

@kaos kaos added the compiler label Mar 9, 2014

@kaos kaos added this to the 0.9.2 milestone Mar 9, 2014

@kaos kaos closed this in 76736d8 Mar 10, 2014

Owner

kaos commented Mar 10, 2014

Thanks for the report @choptastic !

Apparently I messed up the compile_dir functionality (for the second time already), and I think I said last time that I really should add a test case for the compile_dir functionality, as that was missing.

Well, this time I got it in place, so I hope this won't come up again.. :p

As you were on to, the ParseTrail wasn't used properly. It keeps track of which template files we've been parsing, in case of includes etc. Previously, an empty parse trail was used to indicate a dir compile, but that wasn't really holding up any longer after my major compiler refactoring (and it was then I failed to check the compile dir function).

There was a few other minor issues as well, that I've taken care of. There is one oddity I'd like to get rid of still, but it shouldn't be of critical nature (the list of dependencies can have relative path names.. I don't like that, as it's hard to know what dir they are relative to).

Non-existing dirs should work too, only you'll get a "empty" template with no render functions in them (there's a test in there now, verifying that it's at least compilable with a non-existing dir).

Let me know if there's anything else (bug or feature, I like 'em both ;)

kaos added a commit that referenced this issue Mar 10, 2014

Contributor

choptastic commented Mar 10, 2014

Thanks a ton for the quick fix and detailed explanation!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment