You have no idea what environments users may run in. I run my own code in
git repositories all the time, and I do not want "use strictures" behaving
differently depending on this. Also:
!!((caller)[1] =~ /^(?:t|xt|lib|blib)/
Really? Where's even the trailing slash? And "lib" is a common directory
name; in fact it's a top level directory of all my Perl repos

> Fri Sep 07 18:09:23 2012: Request 79504 was acted upon.
> Transaction: Ticket created by CHIPS
> Queue: strictures
> Subject: "smells like VCS"? really? this is not right; it is not even wrong
> Broken in: 1.004001
> Severity: Critical
> Owner: Nobody
> Requestors: chip@pobox.com
> Status: new
> Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=79504 >
>
>
> You have no idea what environments users may run in. I run my own code in
> git repositories all the time, and I do not want "use strictures" behaving
> differently depending on this. Also:
>
> !!((caller)[1] =~ /^(?:t|xt|lib|blib)/
>
> Really? Where's even the trailing slash? And "lib" is a common directory
> name; in fact it's a top level directory of all my Perl repos

This is the best possible heuristic (in the Klortho sense, indeed) that we've been able to discern for the purpose. We'd welcome a better one that doesn't sacrifice too much simplicity. This is very much a least worst sort of thing, and this is the least worst solution we have so far. Everything else fails in more common cases.

I'm trying to remain calm but this is just stupid. You can't argue the
quality of the heuristic when the point of it is so deeply misguided.
Read slowly:
A module. Should not behave differently. Depending on whether $PWD has
.git in it.
This is very simple, it is obviously correct, and I can't imagine why you
would imagine otherwise.

> Queue: strictures
> Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=79504 >
>
> I'm trying to remain calm but this is just stupid. You can't argue the
> quality of the heuristic when the point of it is so deeply misguided.
>
> Read slowly:
>
> A module. Should not behave differently. Depending on whether $PWD has
> .git in it.
>
> This is very simple, it is obviously correct, and I can't imagine why you
> would imagine otherwise.

>
> This is the best possible heuristic (in the Klortho sense, indeed) that we've been able to discern for the purpose.

Then perhaps the whole idea is flaed? I am with Chip on this one - it
is incredibly icky. Combine this with the amount of fallout we have
seen over the past year as more and more things start including
Moo - it's a slippery slope.
So how about making an explicit switch? A $HOME/.perl_strictures
or somesuch. So you don't have to dick around with any envvars,
and your dev-environment is your dev-environment.
Thoughts?

> Queue: strictures
> Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=79504 >
>
> I'm trying to remain calm but this is just stupid. You can't argue the
> quality of the heuristic when the point of it is so deeply misguided.
>
> Read slowly:
>
> A module. Should not behave differently. Depending on whether $PWD has
> .git in it.
>
> This is very simple, it is obviously correct, and I can't imagine why you
> would imagine otherwise.

I can read, Chip. When I'm at a real computer I'll show you the bug report asking for what you are now calling a bug.

> A module. Should not behave differently. Depending on whether $PWD has
> .git in it.
>
> This is very simple, it is obviously correct

It obviously isn't so obvious, or this discussion would not have arisen.
The documentation clearly states that the intention is that behaviour is
different when the author is running his own code (via a unit test, or in
an "uninstalled" location. You may disagree as to the wisdom of this, but
that's one of the express missions of this module.

I should have written: Imagine me saying this very slowly. Because
that's how I want to say it.
Show quoted text

>> A module. Should not behave differently. Depending on whether $PWD has
>> .git in it.
>>
>> This is very simple, it is obviously correct

> It obviously isn't so obvious, or this discussion would not have arisen.

On its own terms, that statement *is* obviously correct. I think anyone
who writes any code other than libraries would find it equally obvious.
But the rot has crept in. First Catalyst and now strictures has fallen
into the trap that all Perl people write is libraries, so "I am under
source control" must mean "This is only testing, not yet released."
Which is WRONG. And it also assumes "not yet released" must mean "being
extra strict is fine, i.e. not even behaving as it will when deployed."
Which is DOUBLY wrong, because it means that what you test is not what
you deploy.
Show quoted text

>
> The documentation clearly states that the intention is that behaviour is
> different when the author is running his own code (via a unit test, or in
> an "uninstalled" location. You may disagree as to the wisdom of this, but
> that's one of the express missions of this module.
>
>
>

> Then perhaps the whole idea is flaed? I am with Chip on this one - it
> is incredibly icky. Combine this with the amount of fallout we have
> seen over the past year as more and more things start including
> Moo - it's a slippery slope.

Thank you.
Show quoted text

> So how about making an explicit switch? A $HOME/.perl_strictures
> or somesuch. So you don't have to dick around with any envvars,
> and your dev-environment is your dev-environment.

No. "dev" vs. "prod" is *not* a binary for a given user on a given
machine. What if I run a program in /usr/local/bin that uses
/usr/local/lib/strictures.pm ... do I want that program to be extra
strict just because I happen to be in the git repo for, say, my C++
project?
Why not an environment variable if you're dead set on the cockamamie
idea that dev and prod should have different strictures? If you start
hitting files in $HOME when I "use" your module, I'm going to be still
quite annoyed.

> > Then perhaps the whole idea is flaed? I am with Chip on this one - it
> > is incredibly icky. Combine this with the amount of fallout we have
> > seen over the past year as more and more things start including
> > Moo - it's a slippery slope.

>
> Thank you.
>

> > So how about making an explicit switch? A $HOME/.perl_strictures
> > or somesuch. So you don't have to dick around with any envvars,
> > and your dev-environment is your dev-environment.

>
> No. "dev" vs. "prod" is *not* a binary for a given user on a given
> machine. What if I run a program in /usr/local/bin that uses
> /usr/local/lib/strictures.pm ... do I want that program to be extra
> strict just because I happen to be in the git repo for, say, my C++
> project?

Yes, it sounds mildly counterintuitive, but this afaiu is *precisely* what
the author wants as behavior. Hence leaving the option to explicitly turn
this on for good is... good.
Show quoted text

> If you start hitting files in $HOME when I "use" your module, I'm going
> to be still quite annoyed.

> >
> > For what it's worth, there actually is $ENV{PERL_STRICTURES_EXTRA} for
> > that purpose.

>
> Hey cool! That means the entire VCS detection logic can be removed
> immediately, with no loss of functionality.

The ENV is primarily there to explicitly disable things when something isn't
right. Hence not exactly what you are looking for. A system-wide "yes I want
strictures to behave differently for me" setting is a reasonable half-way
compromise imho.

> > No. "dev" vs. "prod" is *not* a binary for a given user on a given
> > machine. What if I run a program in /usr/local/bin that uses
> > /usr/local/lib/strictures.pm ... do I want that program to be extra
> > strict just because I happen to be in the git repo for, say, my C++
> > project?

>
> Yes, it sounds mildly counterintuitive, but this afaiu is *precisely*

what
Show quoted text

> the author wants as behavior.

Just because he wants it doesn't mean he's right. Surely this has been
obvious ever since the first badly written program.
To make clear just how stupid this is: I will fork Moo before I let it
stand. I'm a busy guy, but I *will* make the time. This is that
stupid. Deeply and profoundly.
Show quoted text

> > If you start hitting files in $HOME when I "use" your module, I'm

going
Show quoted text

> > to be still quite annoyed.

>
> Why... ?

Because what I am developing and what I am using are two independent
things. Sometimes more than two. So using one global flag in my $HOME
to indicate which non-global mode I am in is stupid.

> Then perhaps the whole idea is flaed? I am with Chip on this one - it
> is incredibly icky. Combine this with the amount of fallout we have
> seen over the past year as more and more things start including
> Moo - it's a slippery slope.
>
> So how about making an explicit switch? A $HOME/.perl_strictures
> or somesuch. So you don't have to dick around with any envvars,
> and your dev-environment is your dev-environment.

I don't think $HOME would be the right place for that, but a generic
.is_development file in $PWD may be an appropriate measure, that would
also be useful for other purposes than just strictures.
Leon

Here's my current train of thought -
The extra testing is all stuff that only ever blows up at compile time;
this is intentional. So your assertion that it's different code being
tested is only sort of the case - none of the modules involved affect
the final optree to my knowledge, so the author gets some additional
compile time crashes which he/she then fixes, and the rest of the
testing is completely valid for all environments.
The point of the extra testing - especially 'no indirect' - is to catch
mistakes that newbie users won't even realise are mistakes without help.
For example,
foo { ... };
where foo is an & prototyped sub that you forgot to import - this is
pernicious to track down since all -seems- fine until it gets called and
you get a crash. Worse still, you can fail to have imported it due to a
circular require, at which point you have a load order dependent bug
which I've seen before now -only- show up in production due to tiny
differences between the production and the development environment.
As such, in my experience so far the strictures extra testing has
-avoided- production versus development differences, not caused them.
Additionally, strictures' policy is very much "try and provide as much
protection as possible for newbies - who won't think about whether
there's an option to turn on or not" - so having only the environment
variable is not sufficient to achieve that (I get to explain that you
need to add 'use strict' at least once a week on freenode #perl -
newbies sometimes completely skip steps because they don't understand
that that step is important).
I'd be open to having a '.no_strictures_extra' file that you create to
disable the behaviour for repositories you want to deploy from. I'd be
open to having 'use Moo -no_extra_tests;' or something of that ilk.
I'm fairly open to pretty much any idea here, but the thing is, you were
quite able to find the environment variable and to figure out what was
going on. I want to make it easy for experts to figure out what's
happening and make it do what they want.
I really, *really* don't want to accept a change that means that people
who haven't thought about what they want will stop getting the extra
protections - and that's what the heuristic is for. I'm not claiming
it's a good approach, but I -am- claiming it's definitely saved users
from bugs that they would've found much harder to track down otherwise,
and I'm unwilling to simply abandon that affordance.
All thoughts gratefully received.

I've put a tweaked and slightly expanded version of this rationale into
master.
So far as I can see, the "obvious" solutions would be:
1) detect .no_strictures_extra next to the .git/.svn and disable the
checks in that case
2) provide 'use strictures -extra_testing => 0;' and an equivalent
option in Moo (where setting the environment variable to 1 overrides this)
I think I'd like to see both implemented, but it's unlikely to float to
the top of my tuit list immediately. Feel free to bikeshed the names I
proposed above; the mechanism (and the retention of the newbie
protection while removing the deleterious effect on Chip's blood
pressure) is the interesting part to me.

> So far as I can see, the "obvious" solutions would be:
>
> 1) detect .no_strictures_extra next to the .git/.svn and disable the
> checks in that case
> 2) provide 'use strictures -extra_testing => 0;' and an equivalent
> option in Moo (where setting the environment variable to 1 overrides

this)
I'm sorry, Matt, I like you a lot, but you're pushing people to use Moo,
Moo uses strictures, and strictures is being evil.
Unless extra_testing is the default, there will be a fork. Unless the
filesystem is no longer consulted, there will be a fork.
This is non-negotiable.

> typo before: unless extra_testing is *not* the default is what I meant to
> write

package Moo::NXT;
sub import {
$ENV{STRICTURES_EXTRA_TESTS} = 0;
require Moo;
goto &Moo::import;
}
should work fine to force the default the way you want for your code.
I'll happily take a patch to avoid checking the filesystem if the env
var's already been set explicitly.
As for the rest - currently it seems to me that you're using "evil" to
mean "annoys me due to behaviour designed to help newbies". You're an
ex-pumpking. I'm allowed to expect you to read the docs. I -know- they
won't.
I, currently, believe that the positive value that newbies derive from
this behaviour exceeds the negative value of the annoyance that it's
going to cause people - and I've done my best to express how I came to
that conclusion.
You clearly believe that conclusion to be in error - could we please
discuss *why* rather than playing chicken? I expressed my arguments at
length with the intent that counterarguments to them could be provided.
"this is not even wrong" and "this is evil" are somewhat useful to me to
gauge the vehemence of your disagreement by, but fall a little short of
actually being counterarguments that we can discuss :)

> As for the rest - currently it seems to me that you're using "evil" to
> mean "annoys me due to behaviour designed to help newbies". You're an
> ex-pumpking. I'm allowed to expect you to read the docs. I -know- they
> won't.

I understand your getting that message, but that isn't what I'm
sending. Consider:
My message is that it is a *severe* violation of the principle of least
astonishment for most Perl modules -- especially those like "strictures"
whose effects should be limited to guiding or limiting a developer -- to
act different, in *any* way, depending on (-e '.git') or any other
vagary of the current directory. Or, for that matter, any other vagary
of the filesystem outside of $ENV{PATH} and @INC. I'm going to *run*
these programs that these n00bs contribute to. When I run them, I don't
want them to act different because I'm in my Linux kernel git
directory. Do you? Do you want lsdiff or splain to act differently
depending on whether you're in the libxml git tree? I doubt it.
To violate that principle is not about depriving newbies. It is about
enhancing newbies' experience of CPAN code as not booby-trapped.
Surely there is SOME other way to serve the n00bs that does not violate
POLA.
I can see caller pathname testing again "blib" ... just barely.
Otherwise, I don't see a way to get what you want without violating POLA.

> Do you? Do you want lsdiff or splain to act differently
> depending on whether you're in the libxml git tree? I doubt it.

They don't. They won't. strictures doesn't do that.
The only thing that strictures does is:
If .git is present in pwd, *and* the file in question is in t/, xt/,
lib/ or blib/ - and note that the regexp is anchored at the front, so it
has to have been loaded as a path relative to pwd - extra testing is
enabled.
/usr/bin/splain will not be affected.
/usr/blah/blah/blah/lib/Some/Module.pm will not be affected.
I'm not sure where you developed the idea that they would be, unless
you're actually filing a bug against a pre-1.004 version without testing
the current logic.
Now, I *can* see an argument that for people deploying straight off a
checkout, scripts shouldn't be subject to the extra testing, so I'd
suggest therefore augmenting the logic to check if $0 starts with 'lib/'
or 't/' - this should ensure we activate both on running of tests and on
casual compilation checks of a single module in the application's tree -
i.e.
our $Smells_Like_VCS = ($0 =~ /^(t|lib)\// and ...current checks...);
and I think that would be a good thing; it'd also avoid useless stat()s
in cases where we probably weren't going to turn anything on.
However, you appear to be utterly confused about how strictures
currently works, so I'd ask that you stop, step back, take a deep
breath, and give me -actual- examples of the behaviour that you can
-demonstrate- against the latest release. We can't usefully argue about
whether strictures is right if you're accusing it of being wrong for
things that it doesn't actually do. Also, I can't turn things that don't
actually happen into test cases if you manage to produce a genuine problem.

> Now, I *can* see an argument that for people deploying straight off a
> checkout, scripts shouldn't be subject to the extra testing, so I'd
> suggest therefore augmenting the logic to check if $0 starts with 'lib/'
> or 't/' - this should ensure we activate both on running of tests and on
> casual compilation checks of a single module in the application's tree -
> i.e.
>
> our $Smells_Like_VCS = ($0 =~ /^(t|lib)\// and ...current checks...);
>
> and I think that would be a good thing; it'd also avoid useless stat()s
> in cases where we probably weren't going to turn anything on.

Just spotted lib::if::dev on CPAN, which checks for the existence for
Makefile.PL, Build.PL, or dist.ini. That sounds like a good probe to
distinguish between deploy-from-git and develop-in-git.
Leon

> Just spotted lib::if::dev on CPAN, which checks for the existence for
> Makefile.PL, Build.PL, or dist.ini. That sounds like a good probe to
> distinguish between deploy-from-git and develop-in-git.

Except that people often retain one of the first two for 'make
installdeps' or './Build --installdeps' or 'cpanm --installdeps .' - and
equally people often delete them because they don't understand and then
deploy via FTP :)
Basically, the key problem here is that some people are confusing their
version control system and a deployment tool, and we need to accommodate
that and cause them a minimum of pain until they learn better.

> > > Do you? Do you want lsdiff or splain to act differently
> > > depending on whether you're in the libxml git tree? I doubt it.

> >
> > They don't. They won't. strictures doesn't do that.

>
> Yes, they do:
>

> > The only thing that strictures does is:
> >
> > If .git is present in pwd, *and* the file in question is in t/, xt/,
> > lib/ or blib/ - and note that the regexp is anchored at the front, so

> it

> > has to have been loaded as a path relative to pwd - extra testing is
> > enabled.

>
> That describes many Perl programs I run in my day job and my primary
> hobby project.

That's nice.
It does NOT describe things like lsdiff and splain that live in /usr/bin
Show quoted text

> Are you calling me a liar? I think not.

I'm calling you hyperbolic to the point where any genuine point is obscured.
Show quoted text

> So please stop beating around the bush and fix this damn bug.

Once again, please provide a CONCRETE EXAMPLE of a problem case such
that if I agree that it needs to be fixed I can turn it into a test case.
This is now the THIRD time I have asked you for something approximating
an actual bug report rather than merely a string of insults to me/the code.
As such, I'm marking the ticket 'stalled'. If your next reply doesn't
turn this back into a proper bug discussion rather than a rant, the next
status will be 'rejected'.

> > Basically, the key problem here is that some people are confusing their
> > version control system and a deployment tool,

>
> No, some people are confusing *my* version control directory for *their*
> version control directory.

Ok. This ticket is -so- not going to go anywhere useful.
However, I'm going to correspond with Chip via email for a bit, and once
we've got something we can both have a civilised debate about, we'll
bring it back into RT.
Otherwise one or both of us is going to turn on the flamethrower within
the next two replies and it's not going to be good.
I'm marking this ticket as rejected *but* I give you my word that there
*will* be another ticket, and this one will get linked to it once it exists.
As such, please don't re-open this one. If you think it's been too long
and can't see the new ticket, you all know how to find me and shout at
me in a way that won't re-open this one.
Thanks.