Bug #99935 for Dist-Zilla-Plugin-MinimumPerl: Code fails on non-perl file

Fri Oct 31 10:22:51 2014 nigelgregoire [...] gmail.com - Ticket created

Subject:	Code fails on non-perl file
Date:	Fri, 31 Oct 2014 14:22:40 +0000
To:	bug-Dist-Zilla-Plugin-MinimumPerl [...] rt.cpan.org
From:	Nigel Gregoire <nigelgregoire [...] gmail.com>

Hello! The latest release (1.004) removed the check on filename added for https://rt.cpan.org/Public/Bug/Display.html?id=67355 next unless $file->name =~ /\.(?:t|p[ml])$/i; One of internal repos at work started failing because we have a testing certificate that passes the "return if $file->is_bytes" test. So it dies with the "Unable to parse" message. -- Nigel

Fri Oct 31 12:10:10 2014 ether [...] cpan.org - Correspondence added

Show quoted text

> we have a testing certificate that passes the "return if $file->is_bytes" test

Can you elaborate on this? Why is the encoding not set to bytes if it is an unparsable file?

Fri Oct 31 12:10:11 2014 The RT System itself - Status changed from 'new' to 'open'

Fri Oct 31 12:11:21 2014 ether [...] cpan.org - Correspondence added

On 2014-10-31 09:10:10, ETHER wrote: Show quoted text

> > we have a testing certificate that passes the "return if $file-

> > >is_bytes" test

> > Can you elaborate on this? Why is the encoding not set to bytes if it > is an unparsable file?

BTW, you can also pass a filefinder to the plugin to indicate what exact files you do and don't want to parse -- see the docs for Dist::Zilla::Role::FileFinderUser. I suspect this is why the filename check was removed from the code.

Fri Oct 31 13:08:42 2014 nigelgregoire [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #99935] Code fails on non-perl file
Date:	Fri, 31 Oct 2014 17:08:32 +0000
To:	bug-Dist-Zilla-Plugin-MinimumPerl [...] rt.cpan.org
From:	Nigel Gregoire <nigelgregoire [...] gmail.com>

Ah, I figured it out. Someone used Dist::Zilla::Plugin::Encoding to explicitly set the encoding of the offending file to "latin1". Changing it to "bytes" resolves the problem. As for Dist::Zilla::Role::FileFinderUser, ":testFiles" returns all the files under test, not just unit tests. So the regex would still be needed. On 31 October 2014 16:11, Karen Etheridge via RT <bug-Dist-Zilla-Plugin-MinimumPerl@rt.cpan.org> wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=99935 > > > On 2014-10-31 09:10:10, ETHER wrote:

>> > we have a testing certificate that passes the "return if $file-

>> > >is_bytes" test

>> >> Can you elaborate on this? Why is the encoding not set to bytes if it >> is an unparsable file?

> > BTW, you can also pass a filefinder to the plugin to indicate what exact files you do and don't want to parse -- see the docs for Dist::Zilla::Role::FileFinderUser. I suspect this is why the filename check was removed from the code.

-- Nigel http://www.forgreatjustice.ca/

Fri Oct 31 13:29:27 2014 APOCAL [...] cpan.org - Correspondence added

Hello, I've reviewed what you and Karen said and I agree that I was a bit hasty in removing the filename check. I assumed that the filefinders would DTRT but again, as history has shown it's best to double-check everything :) Look forward to v1.005 on CPAN soon! On Fri Oct 31 13:08:42 2014, nigelgregoire@gmail.com wrote: Show quoted text

> Ah, I figured it out. Someone used Dist::Zilla::Plugin::Encoding to > explicitly set the encoding of the offending file to "latin1". > Changing it to "bytes" resolves the problem. > > As for Dist::Zilla::Role::FileFinderUser, ":testFiles" returns all the > files under test, not just unit tests. So the regex would still be > needed. > > On 31 October 2014 16:11, Karen Etheridge via RT > <bug-Dist-Zilla-Plugin-MinimumPerl@rt.cpan.org> wrote:

> > <URL: https://rt.cpan.org/Ticket/Display.html?id=99935 > > > > > On 2014-10-31 09:10:10, ETHER wrote:

> >> > we have a testing certificate that passes the "return if $file-

> >> > > is_bytes" test

> >> > >> Can you elaborate on this? Why is the encoding not set to bytes if > >> it > >> is an unparsable file?

> > > > BTW, you can also pass a filefinder to the plugin to indicate what > > exact files you do and don't want to parse -- see the docs for > > Dist::Zilla::Role::FileFinderUser. I suspect this is why the > > filename check was removed from the code.

-- ~Apocalypse

Fri Oct 31 13:29:27 2014 APOCAL [...] cpan.org - Status changed from 'open' to 'resolved'

Fri Oct 31 13:42:35 2014 ether [...] cpan.org - Correspondence added

On 2014-10-31 10:29:27, APOCAL wrote: Show quoted text

> Hello, > > I assumed that the filefinders would DTRT

You DO need to still configure the filefinders to specify what you do and don't want :) The entire point of allowing filefinders is to give the user *total* control over the file inputs. The plugin does not need to do anything more than that. @Nigel: Show quoted text

> As for Dist::Zilla::Role::FileFinderUser, ":testFiles" returns all the files under test, not just unit tests. So the regex would still be needed.

If :TestFiles doesn't do the right thing for you, then configure your own! You are not restricted to just the builtins: See [FileFinder::Filter], [FileFinder::ByName].

Fri Oct 31 14:07:27 2014 ether [...] cpan.org - Correspondence added

I've sent some fixes in https://github.com/apocalypse/perl-dist-zilla-plugin-minimumperl/pull/5