On Mon Aug 08 01:36:01 2016, PERLANCAR wrote:
Show quoted text> On Fri, 5 Aug 2016 04:36:13 GMT, ISHIGAKI wrote:
> > On Thu Aug 04 17:00:08 2016, PERLANCAR wrote:
> > > Hi,
> > >
> > > I haven't looked at the code, but scanning against the lib/ of my
> > > repo
> > >
https://github.com/perlancar/perl-App-LintPrereqs results in these
> > > requirements as being 'suggests':
> > >
> > > Perl::PrereqScanner
> > > Perl::PrereqScanner::Lite
> > > Perl::PrereqScanner::NotQuiteLite
> > >
> > > While it is true that these modules are 'require'd inside an if
> > > block,
> > > Perl::PrereqScanner is actually a requires because it is the
> > > default
> > > choice in the application.
> > >
> > > So, what is the criteria for a module being in the 'suggests'
> > > requirements vs in 'requires'? It looks like anything inside an if
> > > block is categorized as 'suggests'.
> >
> > Hi. I'm supposing you're using one of the recent versions of
> > Perl::PrereqScanner::NotQuiteLite. If not, ignore most of the
> > following.
> >
> > As CPAN::Meta::Spec says, "requires" is for modules that *must* be
> > installed for proper completion of the phase. In your case, none of
> > the Perl::PrereqScanner(::*) modules are necessary. So, they are not
> > for "requires", but "recommends". In other words, "requires" is for
> > modules that are "use"d (or "no"ed) outside "eval".
> >
> > "suggests" is mainly for modules that are "use"d (or "require"d or
> > "no"ed) inside "eval". They are optional, and nothing should break if
> > they are not installed.
> >
> > "recommends" is mainly for modules that are "require"d somewhere in a
> > block. It is not always necessary (because the block may not be
> > executed, depending on conditions), but if the block is executed,
> > those modules are necessary, so we recommend to install them
> > beforehand (if they are installable).
> >
> > Modules that are "require"d outside a block should *probably* be
> > considered "requires", because they will always be required, if
> > there's no "exit" or equivalents before them. If "exit" is found,
> > those modules should be considered "recommends". (However, this last
> > bit is not implemented yet, and 0.49_01/02 didn't have "recommends"
> > and merged all the optional requirements into "suggests".)
> >
> > So, back to your case. Because all the Perl::PrereqScanner modules
> > are
> > "require"d in a block, they are considered "recommends", at least
> > from
> > the scanner's point of view. You can upgrade some of them to
> > "requires" manually, even if your app actually runs without them (by
> > passing an option).
> >
> > Is there anything that I have overlooked?
>
> Yes, I tried 0.49_02.
>
> When I read CPAN::Meta::Spec, I don't see any of the above statements
> about "suggests == inside eval", "recommends == inside block". It only
> says:
>
> requires
> These dependencies must be installed for proper completion
> of the phase.
>
> recommends
> Recommended dependencies are strongly encouraged and should be
> satisfied except in resource constrained environments.
>
> suggests
> These dependencies are optional, but are suggested for enhanced
> operation of the described distribution.
>
> Anyway, you are of course free to interpret this anyway you want. But
> consider my case:
>
> $option{scanner} //= 'regular'; # the default scanner, if user doesn't
> specify --scanner option
> if ($option{scanner} eq 'lite') {
> require Perl::PrereqScanner::Lite;
> $scanner = Perl::PrereqScanner::Lite->new(...);
> } elsif ($option{scanner} eq 'nqlite') {
> require Perl::PrereqScanner::NotQuiteLite;
> $scanner = Perl::PrereqScanner::NotQuiteLite->new(...);
> } else {
> require Perl::PrereqScanner;
> $scanner = Perl::PrereqScanner->new(...);
> }
>
> In this case, *at least one* of the scanner modules is required. The
> application needs a scanner to function. And in this case, I would
> argue that Perl::PrereqScanner is a requires.
>
> Perhaps, you can make modules required inside an else block as
> requires? But I also think this assumes a bit much and might fail in
> weird cases.
No, it doesn't make sense. Consider the following case:
if ($^O eq 'MSWin32') { require 'Module::For::Win32' }
else { require 'Module::For::Others::Which::Doesnt::Install::Under::Win32' }
If we make the one in the else block "requires", most probably Win32 users wouldn't be able to install your module. Don't call this a weird case :)
Your suggestion works only in your case. So, it's you (or your tool) that should change the scanned result manually to reflect your opinion; not a neutral scanner which only cares if a module is loaded in a BEGIN phase or not (if it's loaded in a BEGIN phase, it won't be affected by a normal 'if's, thus it'll be unconditionally loaded = "requires"; otherwise, it should be "recommends" or "suggests").
Show quoted text>
> All in all, I think it's nice that you try to guess requires vs
> recommends vs suggests, but I suspect this will a non-negligible rate
> of false positives/negatives. In my App::LintPrereqs BTW, I currently
> lump the results from requires/suggests (and recommends) as simply
> requires/prereqs (the categorization comes manually from dist.ini).
Your tool is written from a different point of view, so it's fine if your tool changes the phase/severity to your taste. More is better for you, but less is better for this module, whose main purpose is to test prereqs that may or may not be generated by other tools like yours.
