The strictures.pm interface

[Ævar Arnfjörð Bjarmason]

On Mon, Feb 18, 2013 at 9:54 AM, Matt S Trout <m...@shadowcat.co.uk> wrote:
>> Add that it uses strictures which makes your code act different
>> depending on whether a ".git" directory is in $PWD
>
> Strictly, it checks for .git and for being a file within a normal
> module structure - i.e. lib, t/ etc. - the exact algorithm is as
> documented in the POD.
>
> The extra testing modules so enabled make no change at all to the final
> optree though - the only thing they do is, in some circumstances, cause a
> compilation failure with an error message explaining why (and if they're
> not available a warning is emitted explaining what's going on).
>
> I'd be perfectly happy to see a stanza added to the reference to Moo
> mentioning that it turns on additional compile time safety features when
> it thinks it's running out of a checkout; from my point of view that's
> very definitely a feature.
>
>> I've raised this issue with mst and he's told me to pound sand.
>
> You told me "this isn't just not right, it's not even wrong".
>
> I asked for a concrete example of a problem it was causing.
>
> You reminded me that you have more experience developing infrastructure
> software than I do, and therefore I should agree with you.
>
> You told me that I was thinking like an anti-vaxxer if I didn't agree with you.
>
> You told me that I was thinking like a flat earther if I didn't agree with you.
>
> When I explained that I didn't find those arguments convincing on their own,
> you forked the module.
>
> I am aware that if you want to deploy via 'git pull' and not install the
> extra test dependencies, you need to set PERL_STRICTURES_EXTRA to 0 in your
> environment or in a BEGIN block before strictures is loaded to indicate
> that this particular checkout is not a development environment, and that
> that's a trifle on the irritating side the first time you encounter it.
>
> However, the combination of (1) Moo being fatpackable and deployable via
> rsync/etc. due to a lack of non-core required-XS dependencies (2) new users
> automatically getting the additional safety of 'no indirect',
> 'no bareword::filehandles' and 'no multidimensional' by default when
> running from a checkout is something I would really prefer not to lose if
> at all possible. After all, defaults where possible need to help protect
> newbies from themselves, since they're the people least likely to find and
> enable such features, whereas experts can easily find the off switch by
> reading the docs.
>
> So for the moment I'm pretty comfortable with the trade offs I've chosen
> here.

FWIW the reasons I wouldn't deploy this in production are.

* It turns on warnings FATAL in both dev/production. This is bad
because:

- "Use FATAL" in general is pretty useless for the reasons noted
by Tom C. in another thread while back. A lot of the time it
causes you to die way too early and actually hides whatever the
problem is with your code (which would be a later compilation
or stricture error).

A better way to do this is to have something at the top-level
that catches warnings via $SIG{__WARN__} and dies in dev (or
warns, in prod) if you emit any warnings.

- Almost no code actually wants to die if it encounters a
warning, especially in production. I've written some code where
e.g. interpolating an undef string meant something really bad
*really* was happening (mostly in cronjobs) and we wanted to
die *right now*. But most code isn't like that and will carry
on just fine with the stray warning.

* Loading no indirect/multidimensional/bareword::filehandles only in
the dev mode means your dev envs aren't running like
production. I've seen way too many cases where that's failed in
some unexpected case. I'd be especially weary of doing it with a
pragma that hooks into scary bits of perl's guts.

A better approach IMO would be to always enable all of these, but
have some of them support warning/dying in dev/prod (but only die
if it's the kind of pragma where you die at compile time, not
runtime).

The implementation of this is also really fishy. You depend on the
$CWD instead of using git/svn to find if there's a checkout in the
directory *you're in*. That means you end up with all sorts of
hairy stuff like:

ssh dev-box
perl -MSome::Prod::Module::Because::We::Set::Up::PERL5LIB -wle1
# works like prod
cd /prod/dev/env
perl -MSome::Prod::Module::Because::We::Set::Up::PERL5LIB -wle1
# fails because this is where the git checkout is

Which'll end up being really hard to debug. Yeah you can set your
dev boxes up so they set PERL_STRICTURES_EXTRA in bashrc, but
people will run the code on their own laptops etc.

Just my $0.02. I think the idea of having a wrapper module for
sensible pragmas is nice.

But since different people obviously have vastly different
requirements for such a thing writing it as something that supports
different profiles you can enable would be a much nicer use for a
general name like "strictures" rather than something.

Lastly this whole this is probably wildly off-topic, but there is that
bit in perldoc somewhere asking people to discuss pragmas on p5p :)

Anyway, thanks a lot for your continuing contributions to CPAN and
perl.

I mostly just wrote this E-Mail to give you something that didn't
reference your opinions about whether the Earth was flat or not (which
Chip may or may not have done, dunno) in a critique of strictures.pm

[Tom Christiansen]

On Mon, 18 Feb 2013 13:26:05 +0100, Ævar Arnfjörð Bjarmason <ava...@gmail.com>
wrote many good things about problems with warnings in production.

Let me add one more.

When you process millions and even billions of records per day, just a single
per-record warning in production can easily fill up your filesystem faster
than you can do anthing about it.

There really needs to an easier way to make any given warning activate just
once for any given line of code than currently exists. The only standard way
to do this that I am aware of is my old diagnostics pragma.

There should be a better and easier way.

A fancier interface might extend that to no more N times for any given line of
code, or per unique stack trace, or per source line + datafile and/or datafile
line. But all that’s so much chrome. Basic idempotence should be achievable
other than through manual $SIG{__WARN__} hacks. Those are kinda nasty, and
seldom stack well.

It's gotten so I have much too much of this kind of thing in my code:

use warnings; # compile-time ones in this scope
use warnings FATAL => qw(closure closed unopened severe precedence utf8);
use if ($] >= 5.010) "warnings", qw(FATAL illegalproto);

use Carp qw(carp croak confess cluck);

# now run-time fatals in any scope; wish it were just this one!
$SIG{__WARN__} = sub {
confess "[INTERNAL ERROR] fatalized runtime warning: @_";
};

or

{
my %_Seen_Warning;
$SIG{__WARN__} = sub {
my($msg) = @_;
cluck "[INTERNAL WARNING] $msg" unless $_Seen_Warning{$msg}++;
};
}

or some such.

And that doesn't even have some kind of a CHECK block thingie that detects
whether you have emitted any non-fatal compile-time warnings and aborts the
run if so.

There's a great deal of untapped potential here for making the world
a better place.

--tom

[Nicholas Clark]

On Mon, Feb 18, 2013 at 06:48:36AM -0700, Tom Christiansen wrote:

> There really needs to an easier way to make any given warning activate just
> once for any given line of code than currently exists. The only standard way
> to do this that I am aware of is my old diagnostics pragma.

I think that on the implementation side, this basic suggestion wouldn't be
*that* hard. Warnings are stored as bitmasks. (Actually, pairs of bits, IIRC,
enabled/not and fatal/not)

So a bitmask of "have we warned about this yet?" could be stored for each
location in the code. "Each location in the code" corresponds to a COP. I
suspect that giving each COP another pointer (or something indirected for
ithreads) would be fairly space inefficient, as most lines don't warn. So
a per-interpreter lookup hash of locations, and record COP locations as and
when they warn. Yes, this makes the "warning" case slower for "warn once" -
no big deal - the programmer asked for it.


The interface side, I'm not sure about. And I don't think that it's my forté.
Given that the current interface is this:

> use warnings FATAL => qw(closure closed unopened severe precedence utf8);

would this work?

use warnings ONCE => qw(closure closed unopened severe precedence utf8);

Nicholas Clark

[demerphq]

On 18 February 2013 15:14, Nicholas Clark <ni...@ccl4.org> wrote:
> On Mon, Feb 18, 2013 at 06:48:36AM -0700, Tom Christiansen wrote:
>
>> There really needs to an easier way to make any given warning activate just
>> once for any given line of code than currently exists. The only standard way
>> to do this that I am aware of is my old diagnostics pragma.
>
> I think that on the implementation side, this basic suggestion wouldn't be
> *that* hard. Warnings are stored as bitmasks. (Actually, pairs of bits, IIRC,
> enabled/not and fatal/not)
>
> So a bitmask of "have we warned about this yet?" could be stored for each
> location in the code. "Each location in the code" corresponds to a COP. I
> suspect that giving each COP another pointer (or something indirected for
> ithreads) would be fairly space inefficient, as most lines don't warn. So
> a per-interpreter lookup hash of locations, and record COP locations as and
> when they warn. Yes, this makes the "warning" case slower for "warn once" -
> no big deal - the programmer asked for it.

Coincidentally today ive been working on code that would make a hash
of ptrs easier to manage.

See a4c171affab792d1ea2c4b05cc4a305c42a43f17
es
Yv


--
perl -Mre=debug -e "/just|another|perl|hacker/"

[Ed Avis]

FWIW, my production system runs from an svn working copy. Pushing a new release
to production is 'svn update'. (We could instead tag specific versions to
release instead of svn HEAD, but haven't needed to so far.) In fact, I am wary
of putting any file anywhere on the machine unless it's (a) in a version control
working copy or (b) managed by the RPM package manager. So in short, the
presence of git or svn droppings in the current directory is not any reliable
indication of 'production-ness'.

We moved away from 2-arg open because it does random unexpected things if you
have filenames that begin with a > character. Let's not introduce new random
unexpected things depending on whether a .git directory happens to exist. It's
great to have CPAN modules for this stuff but tutorial and introductory material
should not recommend them without at least mentioning the side effects.

--
Ed Avis <e...@waniasset.com>

[Christian Walde]

> FWIW the reasons I wouldn't deploy this in production are.

I'm a person who has used this plenty and who has deployed it in plenty
production situations. I've been faring quite well, since most of your
issues are valid in a purely theoretical sense, they turn in practice out
to be non-issues.

> A lot of the time it
> causes you to die way too early and actually hides whatever the
> problem is with your code (which would be a later compilation
> or stricture error).

Yes, i even did encounter this actually. Exactly once in many months
though. I had a weird syntax error in my code that caused a warning at
compile time, which hid the actual syntax error. That was a bit confusing,
but not much of a problem at the time.

More importantly: There's no possible chance this could affect production,
since it can only happen when your code in the lexical scope is completely
broken and to get to production it would need to go through exactly zero
testing or execution before being deployed, at which point the presence or
lack of strictures.pm is irrelevant.

> A better way to do this is to have something at the top-level
> that catches warnings via $SIG{__WARN__} and dies in dev (or
> warns, in prod) if you emit any warnings.

That's not lexical though.

>
> - Almost no code actually wants to die if it encounters a
> warning, especially in production.

As a counter point: I've written code that calculated prices of items
according to fairly complex flows. Due to changes in inputs that code
began using undef in a place where the default was the numerical 1. The
result was that many prices were horribly wrong. In the log i did find a
bunch of warnings, but at that time i REALLY wished those warnings had
been fatal errors.

Fatal warnings also help rationalize to management the fixing of what
would otherwise seem entirely trivial messages (to be squashed by removal
of use warnings), which can be very useful in less-than-enlightened
environments.

> * Loading no indirect/multidimensional/bareword::filehandles only in
> the dev mode means your dev envs aren't running like
> production.

Actually they do. These modules are nothing more than linters at compile
time, the actual runtime is not changed by their presence. As such, any
message they directly generate can only actually come to production if
you're in the habit of deploying untested code (or editing code on your
production servers).

The only tangible difference is that in production code would use slightly
less ram/cpu-time.

> I've seen way too many cases where that's failed in
> some unexpected case.

You've seen cases where strictures.pm's behavior in re
indirect/multidim/bareword failed in unexpected cases? Please tell us
about them. We've been asking for those for months, but nobody has
actually given us *any*.

> Which'll end up being really hard to debug.

Please show us a case where this actually *will* be appreciably hard to
debug. In many months of using and testing them, we have not found a
single one. Keep in mind that the purpose of these modules is exclusively
to die at runtime if certain dangerous patterns are found in the lexical
scope.

--
With regards,
Christian Walde

[Christian Walde]

> Add that it uses strictures which makes your code act different
> depending on whether a ".git" directory is in $PWD

Many people seem to be taking this as actual gospel without actually
investigating further into what actually happens, so i'll take this
separate email to point out an important fact:

strictures.pm does not simply run differently when .git is found.

The actual algorithm is found in the code:

https://metacpan.org/source/ETHER/strictures-1.004004/lib/strictures.pm#L41

In summary it checks whether the code file being run is inside
t/xt/lib/blib and additionally whether the current directory smells like a
VCS.

I ask everyone who intends to opine on this to actually look at the code
and form an opinion on the factual reality, instead of playing Chinese
Whispers.

[Ed Avis]

Christian Walde <walde.christian <at> gmail.com> writes:

>strictures.pm does not simply run differently when .git is found.

>In summary it checks whether the code file being run is inside
>t/xt/lib/blib and additionally whether the current directory smells like a
>VCS.

In my case, the production code runs from an svn working copy, and libraries are
kept in lib/, so yes it would trigger this logic. But that's not really the
point; I don't mind so much whether warnings are fatal or not as that the
behaviour be consistent and not depend in any way on odd details of the
filesystem or the caller's filename. At least, not unless the programmer
in full knowledge agreed to it. A message at startup
"looks like I'm running in development, making warnings fatal" would take
care of it.

--
Ed Avis <e...@waniasset.com>

[Ævar Arnfjörð Bjarmason]

On Mon, Feb 18, 2013 at 5:09 PM, Christian Walde
<walde.c...@gmail.com> wrote:
>> A lot of the time it
>> causes you to die way too early and actually hides whatever the
>> problem is with your code (which would be a later compilation
>> or stricture error).
>
>
> Yes, i even did encounter this actually. Exactly once in many months though.
> I had a weird syntax error in my code that caused a warning at compile time,
> which hid the actual syntax error. That was a bit confusing, but not much of
> a problem at the time.

Really? I encounter this every other time I run into "use warnings
FATAL", which isn't often these days, but just a couple of weeks ago I
kept monkeypatching a module I was hacking on to comment out the
"FATAL" line to get sensible errors.

Anyway, it depends on the code, but my (actually Tom's) point stands
that it's much more useful to die later even if you want to die on
warnings.

> Fatal warnings also help rationalize to management the fixing of what would
> otherwise seem entirely trivial messages (to be squashed by removal of use
> warnings), which can be very useful in less-than-enlightened environments.

Well it depends a lot on your company and what kind of code it
writes.

Where I work at least 99% of warnings are bothersome but aren't
actually cases where you'd rather throw an error at the user than
render a page with a couple of warnings.

Using fatal warnings to "help rationalize to management" only works
until they ask the obvious question: Why is it better to throw
*errors* in a production system instead of just carrying on and making
a note to fix relatively small issues later.

>> * Loading no indirect/multidimensional/bareword::filehandles only in
>> the dev mode means your dev envs aren't running like
>> production.
>
>
> Actually they do. These modules are nothing more than linters at compile
> time, the actual runtime is not changed by their presence. As such, any
> message they directly generate can only actually come to production if
> you're in the habit of deploying untested code (or editing code on your
> production servers).

From my perspective that's not the issue. Sure these modules have no
side-effect in theory, but they're thousands of lines of C code
hooking into the perl guts.

If I was going to run it I'd run them in *both* production and
development. Running different code in prod/dev, especiallyC-level
code is just a recipe for disaster, particularly when you end up
having to debug something on the C level.

But to each his own. I just think this code is overly clever,
especially since it seemingly has behavior like "are you running in a
git checkout and does the letter 't' exist anywhere in the path".

[Christian Walde]

On Mon, 18 Feb 2013 17:25:43 +0100, Ed Avis <e...@waniasset.com> wrote:

> Christian Walde <walde.christian <at> gmail.com> writes:
>> strictures.pm does not simply run differently when .git is found.
>>
>> In summary it checks whether the code file being run is inside
>> t/xt/lib/blib and additionally whether the current directory smells
>> like a
>> VCS.
>
> In my case, the production code runs from an svn working copy, and
> libraries are kept in lib/, so yes it would trigger this logic.

You run it like this to trigger it?

/home/ed/project # perl lib/Project.pm

This would not trigger it:

/home/ed/project # perl -e 'use lib "lib"; use Project;'

> I don't mind so much whether warnings are fatal or not as that the
> behaviour be consistent and not depend in any way on odd details of the
> filesystem or the caller's filename.

That is actually the case here. strictures.pm *always* makes warnings
fatal. Again, please read the code. The only conditional is whether it
*tries* to also load `no indirect/multidim/barewords`. (It's not even a
hard dependency, it simply tries to.)

[Ed Avis]

Christian Walde <walde.christian <at> gmail.com> writes:

>This would not trigger it:
>
>/home/ed/project # perl -e 'use lib "lib"; use Project;'

Ah, I misunderstood, sorry.

>strictures.pm *always* makes warnings
>fatal. Again, please read the code. The only conditional is whether it
>*tries* to also load `no indirect/multidim/barewords`. (It's not even a
>hard dependency, it simply tries to.)

Hmm - which means that if 'no indirect' was not installed but becomes installed
(perhaps being pulled in automatically as a dependency of some unrelated module)
then the semantics of some code changes. Doesn't this break the usual assumption
in Perl (and other languages) that merely installing a file is a safe operation,
since it won't be loaded unless the programmer asks for it?

--
Ed Avis <e...@waniasset.com>

[Christian Walde]

On Mon, 18 Feb 2013 17:59:56 +0100, Ed Avis <e...@waniasset.com> wrote:

> Hmm - which means that if 'no indirect' was not installed but becomes
> installed (perhaps being pulled in automatically as a dependency of some
> unrelated module) then the semantics of some code changes. Doesn't this
> break the usual assumption in Perl (and other languages) that merely
> installing a file is a safe operation, since it won't be loaded unless
> the programmer asks for it?

That would mean the original author of the code did not have indirect.pm
installed and actively ignored a warning when running tests in their
development environment. In that case you've worse problems to worry about
and a failure of this kind would be valuable, as it would alert you to the
bad practices of your coders or upstream dependencies.

Plus: It's easy to switch this behavior off with a single ENV entry.

[Christian Walde]

On Mon, 18 Feb 2013 17:50:24 +0100, Ævar Arnfjörð Bjarmason
<ava...@gmail.com> wrote:

>> Yes, i even did encounter this actually. Exactly once in many months
>> though. I had a weird syntax error in my code that caused a warning at
>> compile time, which hid the actual syntax error. That was a bit
>> confusing, but not much of a problem at the time.
>
> Really? I encounter this every other time I run into "use warnings
> FATAL", which isn't often these days, but just a couple of weeks ago I
> kept monkeypatching a module I was hacking on to comment out the
> "FATAL" line to get sensible errors.

I can't even imagine how that code would have been able to run at all,
given that i've only encountered this issue when warnings masked fatal

errors.

> Anyway, it depends on the code, but my (actually Tom's) point stands
> that it's much more useful to die later even if you want to die on
> warnings.

That i entirely agree with. When i first encountered this my conclusion
was: Bug in Perl, the warnings/syntax errors are thrown in the wrong
order. As i only encountered it once in my entire time of using Perl i
didn't think of reporting it though.

> Using fatal warnings to "help rationalize to management" only works
> until they ask the obvious question: Why is it better to throw
> *errors* in a production system instead of just carrying on and making
> a note to fix relatively small issues later.

If you agree that a certain issue is trivial, then you can easily add `no
warnings 'whatever';` to disable the warning lexically AND leave a note
that it needs to be looked at. :)

> From my perspective that's not the issue. Sure these modules have no
> side-effect in theory, but they're thousands of lines of C code
> hooking into the perl guts.

Is this worry about the three modules or are you actually aware of any
specific problems with them? Personally i trust their authors and am not
aware of any reasons to distrust them.

Also keep in mind that when you're feeling particularly paranoid, setting
that environment variable is trivial and one should inspect their
dependency chain either way.

In conclusion: If this leads to a reordering of how warnings and errors at
compile time are thrown, i'd be very happy.

[Jesse Luehrs]

Also, fatal warnings have some weird bugs:
https://rt.perl.org/rt3/Ticket/Display.html?id=96332 for instance. They
were added to the warnings system after the fact, and there are places
in the core that can throw warnings, but aren't expected to be able to
throw exceptions. This makes me a bit wary to trust the concept at all.

-doy

[Reverend Chip]

On 2/18/2013 8:54 AM, Christian Walde wrote:
> This would not trigger it:
>
> /home/ed/project # perl -e 'use lib "lib"; use Project;'

You are mistaken. See this pattern from the distributed, buggy strictures:

!!((caller)[1] =~ /^(?:t|xt|lib|blib)/

it isn't even anchored! It will trigger on any directory that *starts*
with with "t", "xt", "lib", *or* "blib".

[Reverend Chip]

On 2/18/2013 9:13 AM, Ed Avis wrote:
> any magic behaviour that depends on filenames,
> no matter how helpful, needs to be explicitly chosen by the
> programmer. It can't be silently included as a side effect of loading
> a seemingly unrelated library.

^^^^^^ Anyone who thinks I don't have a point, please consider that Ed
does. And it's the same point.

[Jan Dubois]

On Mon, Feb 18, 2013 at 11:44 AM, Jan Dubois <ja...@activestate.com> wrote:
>>
>> !!((caller)[1] =~ /^(?:t|xt|lib|blib)/
>>
>> it isn't even anchored! It will trigger on any directory that *starts*
>> with with "t", "xt", "lib", *or* "blib".
>

> Seriously? What does the '^' character do at the beginning of the regexp?

Ok, semantics quibbles about "anchors" aside, I do agree that
heuristics like this are bad in production code, which is why they
should be off by default. Remember MJD's take on heuristics "which is
a fancy way of saying that it doesn't work".

Cheers,
-Jan

[Reverend Chip]

On 2/18/2013 11:44 AM, Jan Dubois wrote:

> Seriously? What does the '^' character do at the beginning of the regexp?
>

> Cheers,
> -Jan

It's the missing '$' that I meant to draw attention to. Sorry for not
making clear which anchor I meant.

[Jan Dubois]

On Mon, Feb 18, 2013 at 11:40 AM, Reverend Chip <rev....@gmail.com> wrote:

[Jesse Luehrs]

+1.

-doy

[Christian Walde]

On Mon, 18 Feb 2013 20:50:06 +0100, Reverend Chip <rev....@gmail.com>

wrote:

> On 2/18/2013 11:44 AM, Jan Dubois wrote:
>> On Mon, Feb 18, 2013 at 11:40 AM, Reverend Chip <rev....@gmail.com>
>> wrote:
>>> On 2/18/2013 8:54 AM, Christian Walde wrote:
>>>> This would not trigger it:
>>>>
>>>> /home/ed/project # perl -e 'use lib "lib"; use Project;'
>>> You are mistaken. See this pattern from the distributed, buggy
>>> strictures:
>>>
>>> !!((caller)[1] =~ /^(?:t|xt|lib|blib)/
>>>
>>> it isn't even anchored! It will trigger on any directory that *starts*
>>> with with "t", "xt", "lib", *or* "blib".
>> Seriously? What does the '^' character do at the beginning of the
>> regexp?
>>
>> Cheers,
>> -Jan
>
> It's the missing '$' that I meant to draw attention to. Sorry for not
> making clear which anchor I meant.
>

I agree this smells like a bug and should probably be made more specific.
Thank you for pointing that out. :)

[Reverend Chip]

Adding the $ just makes it a slightly less broken broken thing. The
only way to win is not to play.

[Reverend Chip]

My apologies to readers; I was a bit distracted when I wrote the above.
Please ignore the specific suggestion of '$'. What's obviously intended
in the code is a pattern that reliably matches the first element of a
pathname component, but doing so portably requires Path::Spec or the
like, not an elaboration of the regex. Of course it shouldn't be
intending any such thing, but still; let's be precise.

[Ed Avis]

It sounds (correct me if I'm wrong) as if the magic filename / .svn / .git
checking in strictures.pm is just a heuristic to check 'is this running in the
test suite'? If so, perhaps Perl should define a special builtin variable or
other official mechanism for seeing whether you are running as part of tests.
Then strictures.pm could use it, everyone would understand how to turn it on and
off, and nobody would have to worry about code that works most of the time but
strangely does something different in a different part of the filesystem.

At the moment tests may be run with a command like

% PERL_DL_NONLAZY=1 /usr/bin/perl -MExtUtils::Command::MM \
-e "test_harness(0, 'blib/lib', 'blib/arch')" t/*.t

Perhaps a new core module:

package Test::IsTest;
our $IsTest = 1; # you can set this if you want
1;

Then the test suite command line would add -MTest::IsTest.

--
Ed Avis <e...@waniasset.com>

[Rafael Garcia-Suarez]

On 19 February 2013 10:24, Ed Avis <e...@waniasset.com> wrote:
> It sounds (correct me if I'm wrong) as if the magic filename / .svn / .git
> checking in strictures.pm is just a heuristic to check 'is this running in the
> test suite'? If so, perhaps Perl should define a special builtin variable or
> other official mechanism for seeing whether you are running as part of tests.
> Then strictures.pm could use it, everyone would understand how to turn it on and
> off, and nobody would have to worry about code that works most of the time but
> strangely does something different in a different part of the filesystem.

Some tests test the existence of TestInit.pm. The standard test
harness sets various environment variables, for example TEST_HARNESS.

> At the moment tests may be run with a command like
>
> % PERL_DL_NONLAZY=1 /usr/bin/perl -MExtUtils::Command::MM \
> -e "test_harness(0, 'blib/lib', 'blib/arch')" t/*.t
>
> Perhaps a new core module:
>
> package Test::IsTest;
> our $IsTest = 1; # you can set this if you want
> 1;
>
> Then the test suite command line would add -MTest::IsTest.

Why not use TestInit.pm ? since t/harness already sets
$ENV{HARNESS_ACTIVE}, t/TEST could do it too.

[Ed Avis]

Rafael Garcia-Suarez <rgs <at> consttype.org> writes:

>>Perl should define a special builtin variable or
>>other official mechanism for seeing whether you are running as part of tests.

>Some tests test the existence of TestInit.pm. The standard test
>harness sets various environment variables, for example TEST_HARNESS.

Hmm, what is TestInit.pm? It doesn't show up on my system and a random
(MakeMaker-based) package from CPAN does not include it. But yes, you
could check for C<defined $TestInit::VERSION> if this module is to be loaded
whenever tests are running.

An environment variable is probably okay. It is also a kind of magic hidden
behaviour, but we already accept that programs implemented in Perl will do
different things depending on the value of PERL5LIB (or even LD_LIBRARY_PATH).
However, since we are only discussing Perl here and not programs implemented
in any other language, we might as well use the facilities Perl provides and
set the flag as a Perl variable in some namespace - not the OS environment.

>since t/harness already sets $ENV{HARNESS_ACTIVE}, t/TEST could do it too.

It would be better not to change any of the files in t/, but have MakeMaker or
Module::Build add the necessary incantation to the command line that runs tests.

--
Ed Avis <e...@waniasset.com>

[Rafael Garcia-Suarez]

On 19 February 2013 11:32, Ed Avis <e...@waniasset.com> wrote:
> Rafael Garcia-Suarez <rgs <at> consttype.org> writes:
>
>>>Perl should define a special builtin variable or
>>>other official mechanism for seeing whether you are running as part of tests.
>
>>Some tests test the existence of TestInit.pm. The standard test
>>harness sets various environment variables, for example TEST_HARNESS.
>
> Hmm, what is TestInit.pm? It doesn't show up on my system and a random
> (MakeMaker-based) package from CPAN does not include it. But yes, you
> could check for C<defined $TestInit::VERSION> if this module is to be loaded
> whenever tests are running.

I thought you were referring to core tests, or cpan tests run from
within core tests.

In that case the solution to your problem might be simpler :
Test::Harness sets HARNESS_ACTIVE. TAP::Harness should probably have
something similar.

[Ed Avis]

Rafael Garcia-Suarez <rgarciasuarez <at> gmail.com> writes:

>Test::Harness sets HARNESS_ACTIVE. TAP::Harness should probably have
>something similar.

Ah, and Test::Harness is what MakeMaker uses by default. So that would cover
most cases if TAP::Harness does the same.

So how about this patch to strictures.pm:

--- strictures.pm~ 2012-11-12 19:04:07.000000000 +0000
+++ strictures.pm 2013-02-19 11:06:58.750665369 +0000
@@ -38,7 +38,8 @@
}
$ENV{PERL_STRICTURES_EXTRA};
} elsif (! _PERL_LT_5_8_4) {
- !!((caller)[1] =~ /^(?:t|xt|lib|blib)/
+ $ENV{TEST_HARNESS}
+ and !!((caller)[1] =~ /^(?:t|xt|lib|blib)/
and $Smells_Like_VCS)
}
};

The magic filesystem-checking behaviour is now enabled only inside a test suite.
(The POD documentation would also need a small patch.)

--
Ed Avis <e...@waniasset.com>

[Matt S Trout]

On Mon, Feb 18, 2013 at 01:26:05PM +0100, Ęvar Arnfjörš Bjarmason wrote:
> FWIW the reasons I wouldn't deploy this in production are.
>
> * It turns on warnings FATAL in both dev/production. This is bad
> because:
>
> - "Use FATAL" in general is pretty useless for the reasons noted
> by Tom C. in another thread while back. A lot of the time it
> causes you to die way too early and actually hides whatever the
> problem is with your code (which would be a later compilation
> or stricture error).

When writing code under strictures, I write it such that if a warning
is emitted at all, it indicates that a situation has occured that my
code did not handle.

I don't want my code to carry on running in an undefined state. I want
it to die.

> * Loading no indirect/multidimensional/bareword::filehandles only in
> the dev mode means your dev envs aren't running like

> production. I've seen way too many cases where that's failed in
> some unexpected case. I'd be especially weary of doing it with a
> pragma that hooks into scary bits of perl's guts.

As previously stated, these modules do not modify anything in the optree.

All they can do is cause a die() during development, which the author would
then be expected to fix.

I'm still not considering this a problem unless and until somebody points
out a concrete problem.

> A better approach IMO would be to always enable all of these, but
> have some of them support warning/dying in dev/prod (but only die
> if it's the kind of pragma where you die at compile time, not
> runtime).
>
> ssh dev-box
> perl -MSome::Prod::Module::Because::We::Set::Up::PERL5LIB -wle1
> # works like prod
> cd /prod/dev/env
> perl -MSome::Prod::Module::Because::We::Set::Up::PERL5LIB -wle1
> # fails because this is where the git checkout is

Only if that module is under lib/ - which means it would have been caught
during development.

If I'm missing something here, as always I'm open to a failing test.

--
Matt S Trout - Shadowcat Systems - Perl consulting with a commit bit and a clue

http://shadowcat.co.uk/blog/matt-s-trout/ http://twitter.com/shadowcat_mst/

Email me now on mst (at) shadowcat.co.uk and let's chat about how our Catalyst
commercial support, training and consultancy packages could help your team.

[Matt S Trout]

On Mon, Feb 18, 2013 at 06:48:36AM -0700, Tom Christiansen wrote:
> On Mon, 18 Feb 2013 13:26:05 +0100, Ęvar Arnfjörš Bjarmason <ava...@gmail.com>
> wrote many good things about problems with warnings in production.
>
> Let me add one more.
>
> When you process millions and even billions of records per day, just a single
> per-record warning in production can easily fill up your filesystem faster
> than you can do anthing about it.

This is another reason I prefer to make the code die().

If there was a means to make any given warning only fire the first time it
was encountered, I would be happy to see strictures use that feature for
runtime warnings.

If there was a means to make a compile time warning reliably provoke a die()
at the -end- of compilation so that other compile errors were still visible,
I would be happy to see strictures use that feature for compile time warnings.

If I'm missing ways to do either of these on an appropriately
scoped basis, I'd love to hear them.

[Matt S Trout]

On Mon, Feb 18, 2013 at 05:50:24PM +0100, Ęvar Arnfjörš Bjarmason wrote:
> But to each his own. I just think this code is overly clever,
> especially since it seemingly has behavior like "are you running in a
> git checkout and does the letter 't' exist anywhere in the path".

The ^ in the regexp says 'anywhere in the path' is wrong.

Please, please criticse my code as actually written.

[Matt S Trout]

On Mon, Feb 18, 2013 at 12:21:22PM -0600, Jesse Luehrs wrote:
> Also, fatal warnings have some weird bugs:
> https://rt.perl.org/rt3/Ticket/Display.html?id=96332 for instance. They
> were added to the warnings system after the fact, and there are places
> in the core that can throw warnings, but aren't expected to be able to
> throw exceptions. This makes me a bit wary to trust the concept at all.

Overall, I find the number of bugs this catches to be hugely greater than
the number it causes (I can think of only one, in fact, where an exception
during an IPC::Open3 call caused me some confusion - but it was confusion as
to the cause, the fix was trivial and obvious).

If I refuse to use a core feature until it's perfect, I don't see how it'll
ever get tested well enough to be perfect.

In spite of my tendency to try and support 'pure perl back to 5.8', on the
whole I do still like to have nice things :)

[Matt S Trout]

> Ok, semantics quibbles about "anchors" aside, I do agree that
> heuristics like this are bad in production code, which is why they
> should be off by default. Remember MJD's take on heuristics "which is
> a fancy way of saying that it doesn't work".

As I keep saying, so far this has worked well enough that I get a lot more
"omg this found a bug I'd never have realised was there" comments than I do
complaints about the way it currently works.

It's a trade off. But it's a trade off that newbies love, and experts only
come and complain to me about after they've already found the way to turn it
off and decided to complain anyway.

[Ævar Arnfjörð Bjarmason]

On Tue, Feb 19, 2013 at 1:54 PM, Matt S Trout <m...@shadowcat.co.uk> wrote:
> On Mon, Feb 18, 2013 at 01:26:05PM +0100, Ęvar Arnfjörš Bjarmason wrote:
>> FWIW the reasons I wouldn't deploy this in production are.
>>
>> * It turns on warnings FATAL in both dev/production. This is bad
>> because:
>>
>> - "Use FATAL" in general is pretty useless for the reasons noted
>> by Tom C. in another thread while back. A lot of the time it
>> causes you to die way too early and actually hides whatever the
>> problem is with your code (which would be a later compilation
>> or stricture error).
>
> When writing code under strictures, I write it such that if a warning
> is emitted at all, it indicates that a situation has occured that my
> code did not handle.
>
> I don't want my code to carry on running in an undefined state. I want
> it to die.

Sure, and that's fair enough. All I'm saying is that *I* wouldn't use
it since it's very much geared towards your particular use case. The
code I work on should definitely *not* die on warnings.

That by itself wouldn't be very notable (oh noes there's a CPAN module
that doesn't work for me) but you a) did ask for feedback b) it's a
pragma using a very general name doing a thing very specific to your
usecase.

>> * Loading no indirect/multidimensional/bareword::filehandles only in
>> the dev mode means your dev envs aren't running like
>> production. I've seen way too many cases where that's failed in
>> some unexpected case. I'd be especially weary of doing it with a
>> pragma that hooks into scary bits of perl's guts.
>
> As previously stated, these modules do not modify anything in the optree.
>
> All they can do is cause a die() during development, which the author would
> then be expected to fix.
>
> I'm still not considering this a problem unless and until somebody points
> out a concrete problem.

I'd consider it an inherent problem to be running non-trivial code in
dev that isn't run in production.

Any large combination of code that shouldn't impact anything and is
surely bug free will eventually get to the size and complexity that
it's going to cause some issue where production and dev are behaving
differently.

Better just to avoid that caveat altogether by making prod and dev the same.

>> A better approach IMO would be to always enable all of these, but
>> have some of them support warning/dying in dev/prod (but only die
>> if it's the kind of pragma where you die at compile time, not
>> runtime).
>>
>> ssh dev-box
>> perl -MSome::Prod::Module::Because::We::Set::Up::PERL5LIB -wle1
>> # works like prod
>> cd /prod/dev/env
>> perl -MSome::Prod::Module::Because::We::Set::Up::PERL5LIB -wle1
>> # fails because this is where the git checkout is
>
> Only if that module is under lib/ - which means it would have been caught
> during development.
>
> If I'm missing something here, as always I'm open to a failing test.

This assumes that you use the CPAN toolchain to deploy your code. E.g.
we use git-deploy[1] with a hook that just syncs out a git tree, so
production code is run from a lib/ directory.

Lot of people do that in production, and they also have git checkouts.
So "git && looks like a non-CPAN install" is broken as a "is dev"
heuristic.

1. https://github.com/git-deploy/git-deploy

[Ed Avis]

What do you think of the suggestion to check $ENV{TEST_HARNESS} as part of the
heuristic that looks for a test suite? That ought to keep both the 'newbie'
and 'expert' factions happy.

--
Ed Avis <e...@waniasset.com>

[Matt S Trout]

On Tue, Feb 19, 2013 at 02:45:47PM +0100, Ęvar Arnfjörš Bjarmason wrote:
> On Tue, Feb 19, 2013 at 1:54 PM, Matt S Trout <m...@shadowcat.co.uk> wrote:
> > When writing code under strictures, I write it such that if a warning
> > is emitted at all, it indicates that a situation has occured that my
> > code did not handle.
> >
> > I don't want my code to carry on running in an undefined state. I want
> > it to die.
>
> Sure, and that's fair enough. All I'm saying is that *I* wouldn't use
> it since it's very much geared towards your particular use case. The
> code I work on should definitely *not* die on warnings.

It's an opinion about what default should be being used for new code.

Moose and others also have a built in opinion.

It's only a default, and you're welcome to override it.

For code written with this default in mind, I don't find it to be an issue.

> I'd consider it an inherent problem to be running non-trivial code in
> dev that isn't run in production.
>
> Any large combination of code that shouldn't impact anything and is
> surely bug free will eventually get to the size and complexity that
> it's going to cause some issue where production and dev are behaving
> differently.
>
> Better just to avoid that caveat altogether by making prod and dev the same.

If you want to do that, add

BEGIN { $ENV{PERL_STRICTURES_EXTRA} = 1 }

to your scripts.

I have quite a few downstream users who don't consider required XS
dependencies by default to be acceptable.

> This assumes that you use the CPAN toolchain to deploy your code. E.g.
> we use git-deploy[1] with a hook that just syncs out a git tree, so
> production code is run from a lib/ directory.
>
> Lot of people do that in production, and they also have git checkouts.
> So "git && looks like a non-CPAN install" is broken as a "is dev"
> heuristic.

It's imperfect. If you need to make a version control checkout not be
treated as a development environment, you need to add

BEGIN { $ENV{PERL_STRICTURES_EXTRA} = 0 }

to your scripts.

The point here is to maximise safety for new users who haven't configured
anything. In doing so, yes, I'm causing a minor annoyance when you get as
far as the deployment part, in that you have to actually read the docs.

It's a trade off. Any proposals that reduce the number of false positives
without increasing the number of false negatives for naive users would be
very well received.

[Darin McBride]

On Tuesday February 19 2013 1:04:37 PM Matt S Trout wrote:
> On Mon, Feb 18, 2013 at 12:21:22PM -0600, Jesse Luehrs wrote:
> > Also, fatal warnings have some weird bugs:
> > https://rt.perl.org/rt3/Ticket/Display.html?id=96332 for instance. They
> > were added to the warnings system after the fact, and there are places
> > in the core that can throw warnings, but aren't expected to be able to
> > throw exceptions. This makes me a bit wary to trust the concept at all.
>
> Overall, I find the number of bugs this catches to be hugely greater than
> the number it causes (I can think of only one, in fact, where an exception
> during an IPC::Open3 call caused me some confusion - but it was confusion as
> to the cause, the fix was trivial and obvious).

+1

I run with full strict and FATAL => 'all' warnings, test, production. And
yes, we do get the odd fatal error in production, generally in error codepaths
that aren't handled well. But it becomes so obviously a problem (and so
obvious what the problem is) that I think it has quickly resulted in a much
stronger and robust code base.

_Sometimes_ we have a "no strict 'something'" or a "no warnings 'something'"
in there, lexically scoped to as small of an area as possible. But that
serves as documentation that we're doing something odd on purpose.

Oh, and near the top level of the code, we run almost everything inside one
big eval block where we can catch all these errors and log them. But we still
exit with an error as immediately thereafter as possible.

These features save my bacon. Multiple times over. I grant that they may not
be seen as such by all, but I have to admit that I fail to grasp the rationale
behind it.

[Karen Etheridge]

> So how about this patch to strictures.pm:

> ...
> + $ENV{TEST_HARNESS}

>
> The magic filesystem-checking behaviour is now enabled only inside a test
> suite.

TEST_HARNESS is only enabled by 'prove' and friends, not when running a
single test as a one-off.

This would catch more situations:

$^C # perl -Ilib -c lib/Foo.pm
|| $0 =~ /\.pm$/ # same, but without -c
|| $INC{'lib/Test/Builder.pm'} # most test libraries
|| $ENV{AUTHOR_TESTING}
|| $ENV{AUTOMATED_TESTING}
|| $ENV{RELEASE_TESTING}

This thread really has spiralled off into bikeshedding territory now!
The original thread was about how to mention Moo in perlootut. Has that
point been sufficiently hashed out yet?

- Karen Etheridge
et...@cpan.org

[Dave Rolsky]

On Tue, 19 Feb 2013, Karen Etheridge wrote:

> This thread really has spiralled off into bikeshedding territory now!
> The original thread was about how to mention Moo in perlootut. Has that
> point been sufficiently hashed out yet?

I just pushed a few updates to perlootut to add a bit more about Moo.

You can all go back to arguing about the thing that had basically nothing
to do with my original question now ;)


-dave

/*============================================================
http://VegGuide.org http://blog.urth.org
Your guide to all that's veg House Absolute(ly Pointless)
============================================================*/

[Christian Walde]

On Tue, 19 Feb 2013 14:45:47 +0100, Ævar Arnfjörð Bjarmason
<ava...@gmail.com> wrote:

> Sure, and that's fair enough. All I'm saying is that *I* wouldn't use
> it since it's very much geared towards your particular use case. The
> code I work on should definitely *not* die on warnings.
>
> That by itself wouldn't be very notable (oh noes there's a CPAN module
> that doesn't work for me) but you a) did ask for feedback b) it's a
> pragma using a very general name doing a thing very specific to your
> usecase.

I would like to point out here that all evidence available to me points
towards your use case being in the crass minority. It is of course a valid
use case. However in light of Perl having a reputation for being proven by
reality to have chosen the wrong defaults in being too lenient, i would
say strictures.pm is not only a right step for all the newbies starting to
learn and use it, but also for the overall health of Perl the language.

[Christian Walde]

On Tue, 19 Feb 2013 02:40:49 +0100, Reverend Chip <rev....@gmail.com>

wrote:

> On 2/18/2013 11:50 AM, Reverend Chip wrote:
>> On 2/18/2013 11:44 AM, Jan Dubois wrote:
>>> On Mon, Feb 18, 2013 at 11:40 AM, Reverend Chip <rev....@gmail.com>
>>> wrote:
>>>> On 2/18/2013 8:54 AM, Christian Walde wrote:
>>>>> This would not trigger it:
>>>>>
>>>>> /home/ed/project # perl -e 'use lib "lib"; use Project;'
>>>>>
>>>> You are mistaken. See this pattern from the distributed, buggy
>>>> strictures:
>>>>
>>>> !!((caller)[1] =~ /^(?:t|xt|lib|blib)/
>>>>
>>>> it isn't even anchored! It will trigger on any directory that
>>>> *starts*
>>>> with with "t", "xt", "lib", *or* "blib".
>>>
>>> Seriously? What does the '^' character do at the beginning of the
>>> regexp?
>>

>> It's the missing '$' that I meant to draw attention to. Sorry for not
>> making clear which anchor I meant.
>
> My apologies to readers; I was a bit distracted when I wrote the above.
> Please ignore the specific suggestion of '$'. What's obviously intended
> in the code is a pattern that reliably matches the first element of a
> pathname component, but doing so portably requires Path::Spec or the
> like, not an elaboration of the regex.

Don't worry, that is how it was understood. :)

[Christian Walde]

On Tue, 19 Feb 2013 14:49:40 +0100, Ed Avis <e...@waniasset.com> wrote:

> What do you think of the suggestion to check $ENV{TEST_HARNESS} as part
> of the heuristic that looks for a test suite? That ought to keep both
> the 'newbie' and 'expert' factions happy.

We're unhappy with that because that would not apply the extra tests
during syntax checks of pm files. In a perfect world people would not
write syntax errors, but in a perfect world they would also write tests
and run them regularly.

Sadly, in reality neither happens to be the common case, however syntax
checking is much more common than tests, so it is important for new
developers to experience the benefits of these tests in that situation.

[Christian Walde]

On Mon, 18 Feb 2013 18:13:24 +0100, Ed Avis <e...@waniasset.com> wrote:

>> That would mean the original author of the code did not have
>> indirect.pm installed and actively ignored a warning when running
>> tests in their development environment.
>
> That does assume their 'development environment' looks the same
> as the code is expecting.

If someone manages to run their code in a production setting that looks
like a development setting and ALSO manages to develop in a setting that
does NOT look like a development setting then they're either so far down
the rabbit hole that there's no helping them, or they know enough to not
need help with something this trivial.

> For in-house code where I work (in fact,
> everywhere I have worked) there isn't the blib/t/lib setup.

So, construct a legit case here. How would you end up with a production
deployment that would trigger the extra tests of strictures if your
development environment doesn't even look like one?

> I am just concerned

Let me tell you a little story: One time i looked at some code and talked
to mst because i thought it was broken. We argued over it and he insisted
it was fine. He told me to give him a failing test. I toiled for two
evenings to produce a failing test, but every assumption i made about how
the code could fail proved to be wrong. I ended up being unable to provide
a failing test, because it turned out that every issue i imagined was
imagined because of an incomplete understanding of the code at hand.

So, i implore you: Walk with me on this path. Do not let concern rule your
thinking and acting. Take strictures.pm and Make It Break. Do your best.
You will end up with one of two things:

* an actually proven problem which will lead to a fix or change in
strictures.pm
or
* alleviated concerns, replaced by certainty

[Christian Walde]

On Tue, 19 Feb 2013 23:23:58 +0100, Christian Walde
<walde.c...@gmail.com> wrote:

> crass minority

Apologies. I misused the english language. In german the word "krass"
refers to extreme, and as such i meant to say "extreme minority".

[Reverend Chip]

On 2/19/2013 2:27 PM, Christian Walde wrote:
>> My apologies to readers; I was a bit distracted when I wrote the above.
>> Please ignore the specific suggestion of '$'. What's obviously intended
>> in the code is a pattern that reliably matches the first element of a
>> pathname component, but doing so portably requires Path::Spec or the
>> like, not an elaboration of the regex.
>
> Don't worry, that is how it was understood. :)

I'm glad of that. I am less glad that Matt has not seen fit to act on
the bug.

[Johan Vromans]

"Christian Walde" <walde.c...@gmail.com> writes:

> Let me tell you a little story: One time i looked at some code and
> talked to mst because i thought it was broken. We argued over it and
> he insisted it was fine. He told me to give him a failing test. I
> toiled for two evenings to produce a failing test, but every
> assumption i made about how the code could fail proved to be wrong. I
> ended up being unable to provide a failing test, because it turned
> out that every issue i imagined was imagined because of an incomplete
> understanding of the code at hand.

Does that make you happy? Or sad?

Code that is so hard to understand is very likely to become a
maintenance nightmare sooner or later.

-- Johan

[demerphq]

I dont really agree your premise that "warnings should not die" use
cases are in the extreme minority.

It really depends what kind of business you are doing.

The use case that Avar refers to is that of a web server showing a
customer a 500 versus showing the customer a page with one field
missing[1].

On the other hand there are use cases that also come up at work, such
as internal report generators, where dieing on bad data is the right
thing. Better to die than make business decisions on bad data.

I think it is unwise for you to assume that the "all warnings are
critical" use case is the most common. I suspect in terms of code it
is about half-and-half. In terms of business transactions, were it
possible to accurately determine it, I would bet very heavily that
orders of magnitude more occur in the "warnings dont matter much to
the customer" environments than occur in the opposite.

In the interest of full disclosure I will note I work at the same
place Avar does.

cheers,
Yves
[1] and if you disagree with this point I'd love to know what web site
you work for so I can short your stock. :-)

--
perl -Mre=debug -e "/just|another|perl|hacker/"

[Ed Avis]

There is a difference of assumptions here that can lead to people talking past
each other. Let me try to explain.

One way of seeing things is to weigh up the advantages and disadvantages of a
behaviour in typical situations. If there are more cases (likely to be
encountered in practice) where it helps than where it hurts, it is worth doing.

The other viewpoint is to define a notion of correctness and ask if there are
any situations where the code is not correct by that definition. Either the
code is correct for 100% of inputs or it has a bug. By that rule, even
if the incorrect behaviour only triggers when the program is run seventeen
subdirectories deep on Tuesday by a user whose name contains no vowels, it is
still, in principle, buggy.

As programmers we mix these two philosophies in our work. Usually, the more
general-purpose and low-level the component, the more it must follow the '100%'
rule rather than the heuristic rule. I have written plenty of code which only
works on filenames that do not contain spaces. Out of caution I'll make it
die if it finds a bad filename, but it still doesn't work 100%. That is fine
for my use case, and such code runs reliably in production, but it would not
be suitable for distribution as a general-purpose library. I think it was
Stroustrup who wrote that library code needs to be held to a higher standard
(of performance, robustness etc) because you just don't know what requirements
the user of the library will have. So where reuse is likely, one takes care
to write code which handles even pathological cases and gives No Surprises(tm).
It gives a nice warm glow to finish writing a function which is (believed to be)
correct for all possible inputs, and does not require any caveats or gotchas
to be mentioned in the documentation.

An example in Perl is 2-arg versus 3-arg open(). It is true that 2-arg open()
will trip up if a filename begins >, but how likely is that? Surely in practice
the extra convenience of being able to give pipes on the command line is worth
it? And if you really need to handle those obscure cases, you are surely
expert enough to know how to turn off the magic. Yet the recommendation for
beginners and for new code in general is to use the 3-arg form, which opens the
filename given with 'no ifs and no buts'. This is because as a fundamental
part of the language, to be used in all sorts of places we don't even know of,
it must work 100% rather than having helpful heuristic behaviours. The magic
is still there for those who want it, but it cannot be the recommended default.

The strictures.pm library itself embodies the '100% or buggy' philosophy in its
disabling of indirect object syntax. The indirect syntax 'new Foo()' works fine
most of the time and can lead to more readable and expressive code. It goes
wrong only if you define a subroutine which has the same name as a method, and
if that does happen, the programmer can surely figure it out. Yet many think
that it is more important to have a method invocation that always works, without
any caveats or guessing.

That is why suggestions to improve the regular expression that checks directory
names will not satisfy those who don't think the code should ever behave
differently based on directory names (unless the programmer has explicitly
asked for it). As far as these people ('we') are concerned, if there is any
magic filename at all, there can still be a latent bug in that case.

There is another point which throws off the discussion, and that is the idea
that an expert user knows how to disable feature X, so it's okay to have X on
by default. If X is some magic behaviour which usually does nothing but
sometimes turns on, then the issue is not that an individual user can fix it
if that happens, but that your innocently written program (whether written by
beginner or expert) has a latent bug. Yes, perhaps anyone foolish enough to
put their executables in 'lib' or create filenames with '>' can also be expected
to deal with the consequences; but the program itself is faulty in some sense.
The default should be that code written without any special precautions is
safe and predictable in all cases.

You asked me to take strictures.pm and Make It Break. From my point of view,
I have done so. I have found an example directory structure where code written
loading strictures.pm will start behaving differently - which implies that
in general, programs loading the library will have this magic behaviour. From
my point of view, that means a bug, even if the only case that triggered it
were invoking the program as /$#!$!$@!#$/54385904353/myprog. If I found a
similar case in any other program I'd report it as a bug, and indeed lots of
bugs do depend on 'extremely unlikely' filenames and so on.

(There is nothing wrong with magic behaviour *if the programmer is fully aware*.
But not if the programmer just loaded some other library which happens to use
Moo which happens to load strictures.pm which now globally changes the semantics
of the program depending on the filesystem.)

Now, is there a way to keep both sides happy? Although it's generally accepted
that running from a different place in the filesystem should not change the
semantics of a program, that is not true for environment variables. We all
agree that if you set PERL5LIB differently then your Perl code may change, and
this is not considered a bug in the program or in perl itself. So I suggest
making any magic depend on environment variables which are set using testing
but not otherwise. Karen E. suggested also checking $^C to see if running
under 'perl -c'. With the tests she proposed, I think the dependency on the
filesystem can be removed altogether, and everyone will be happy.

--
Ed Avis <e...@waniasset.com>

[Christian Walde]

On Wed, 20 Feb 2013 08:34:28 +0100, Johan Vromans <jvro...@squirrel.nl>
wrote:

You jump to an early conclusion here. The code wasn't hard to understand.
I was worrying so much about it that i ended up imagining things, only to
be proven wrong by reality.

I'm not sure how to better express this, but if i were to try and
summarize the point:

If the code is simple and I'm worried about it, but don't have an actual
breaking case, it's very likely that the problem is not with the code, but
with myself.

Or in yet other words: Don't waste your and other's time by pondering
endlessly what might break, but go and actually break it, or let people be
productive.

[Matt S Trout]

On Wed, Feb 20, 2013 at 10:46:43AM +0100, demerphq wrote:
> I dont really agree your premise that "warnings should not die" use
> cases are in the extreme minority.

I don't either, because I think the entire concept of 'warnings should die'
versus 'warnings should not die' being a 'use case' is ridiculous and I don't
know why two people smart enough that I enjoy drinking beer with them are
even discussing it.

Code written with the expectation that warnings will not be fatal but the
intention that they should be rare ... will be written such that making them
die would be stupid.

After about the first year of using fatal warnings in everything I wrote, I
found that I'd naturally adjusted to their existence and that the code I
wrote as a result didn't die any more often than the code I wrote beforehand,
but was significantly easier to debug.

By this point, writing code so it doesn't warn doesn't feel onerous any
more than declaring variables with 'my' does - and it's a set of habits
rather than anything else, so that's no declaration of technical ability
on my part; if anything, it's a declaration that I think I'm too stupid to
not have them turned on.

After a year or so of writing non-custommer code using fatal warnings and
customer code without, I concluded that I was substantially more
productive and ended up with production broken less often when I had
fatal warnings on - so when I wrote strictures, that's what it did.

While I can't expect other people spend a year experimenting before having
an opinion like I did, I would suggest that the people who've been using
fatal warnings so long it's muscle memory and the people who've never used
them for a significant sized project are rather talking past each other
here.

[Jesse Luehrs]

Thank you. I had been trying to think of how to say exactly this, but
couldn't really figure out how to phrase it.

-doy

[Reverend Chip]

On 2/20/2013 7:15 AM, Jesse Luehrs wrote:
> On Wed, Feb 20, 2013 at 10:41:48AM +0000, Ed Avis wrote:
>> You asked me to take strictures.pm and Make It Break. From my point of view,
>> I have done so. I have found an example directory structure where code written
>> loading strictures.pm will start behaving differently - which implies that
>> in general, programs loading the library will have this magic behaviour. From

>> my point of view, that means a bug [...]

>>
>> So I suggest making any magic depend on environment variables which are
>> set using testing but not otherwise.

> Thank you. I had been trying to think of how to say exactly this, but
> couldn't really figure out how to phrase it.

Likewise. Thank you.

[Tom Hukins]

On Mon, Feb 18, 2013 at 02:14:32PM +0000, Nicholas Clark wrote:
> On Mon, Feb 18, 2013 at 06:48:36AM -0700, Tom Christiansen wrote:
>

> > There really needs to an easier way to make any given warning activate just
> > once for any given line of code than currently exists.
>
> The interface side, I'm not sure about. And I don't think that it's my fort�.
> Given that the current interface is this:
>
> > use warnings FATAL => qw(closure closed unopened severe precedence utf8);
>
> would this work?
>
> use warnings ONCE => qw(closure closed unopened severe precedence utf8);

I'm a different Tom to the one who suggested this feature, but that
interface seems sensible and intuitive to me.

Tom