Hi,

Delurking, because the latest version of Dancer is causing our unit tests at 
$WORK to start creating empty log files inside the current working directory. 
This is easily-reproducible: e.g.

> sam@chimera64:tmp$ cat spamlogfile.pl 
> #!/usr/bin/env perl
> 
> use Dancer qw(halt);
> sam@chimera64:tmp$ rm -fr logs; perl spamlogfile.pl
> sam@chimera64:tmp$ ls -lah logs/
> total 40K
> drwxr-xr-x  2 sam  sam  4.0K Jun 22 17:01 .
> drwxrwxrwt 10 root root  36K Jun 22 17:01 ..
> -rw-r--r--  1 sam  sam     0 Jun 22 17:01 development.log

A workaround is to say use Dancer qw(:syntax), but there are two problems with 
that:

(1) this is a change in behaviour that wasn’t documented as such, and it 
requires all auxiliary non-server Dancer code to be changed; and

(2) passing :syntax imports all sorts of Dancer symbols that can potentially 
clash with a parent class.

I’ve spent the afternoon debugging point 2. Our end-to-end tests run under 
Test::Dancer, and we have a separate class that tests individual method calls 
as if they were Dancer requests (including setting the status code in case of 
errors). It previously said use Dancer () and explicitly called Dancer::status 
- and under Dancer 1.3138, running unit tests using this class would generate 
spurious empty log files.

I patched that to say use Dancer qw(:syntax) - and now the code fails because 
the parent class implements methods true, false and redirect, which the method 
call class never gets to call because it’s had Dancer’s symbols stuck in its 
own package instead. So this now needs to say use Dancer qw(:syntax !true 
!false !redirect), and in practice I needed to write an automated test script 
that checked that all of a parent class’s methods were explicitly not imported 
from Dancer, because if I change anything upstream I need to change the import 
statement in the child classes.

The problem appears to have been introduced in 
https://github.com/PerlDancer/Dancer/commit/e9e4e087528c2734dd6e9c922105eac3cd2e53e7

The actual problem is further down in Dancer::Config - line 227 in 1.3138 - 
where the settings are read. $SETTINGS at this point contains default values, 
and part of setting confdir involves creating a logs subdirectory. Previously 
the code wouldn’t get this far.

Presumably a workaround would be to not create a log directory immediately, but 
then if you were running a real Dancer server you’d want to know as soon as 
possible that your log directory wasn’t writable, so that’s not going to be 
acceptable to many people.

Alternatively, any code that says e.g. use Dancer qw(halt status) should be 
seen as clearly not wanting to actually start a Dancer server or anything like 
that. That sounds like a scary change to make, though.

Or you could defer parsing of the Dancer config until something actually 
happens. That’s also vague and terrifying.

Or just revert that particular commit / PR. I don’t fully understand why it 
does what it does, though. If there’s a way of cleanly telling apart (1) a hash 
of settings that was explicitly passed to Dancer from (2) the default settings 
that, chances are, you didn’t intend to use, that would  be the best solution 
to my mind.

Sam
-- 
Website: http://www.illuminated.co.uk/

_______________________________________________
dancer-users mailing list
[email protected]
http://lists.preshweb.co.uk/mailman/listinfo/dancer-users

Reply via email to