Skip Menu |
 

This queue is for tickets about the Module-CPANTS-Analyse CPAN distribution.

Report information
The Basics
Id: 97858
Status: resolved
Priority: 0/
Queue: Module-CPANTS-Analyse

People
Owner: Nobody in particular
Requestors: RURBAN [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)



Subject: wrong no_symlinks test in files not in MANIFEST
Download (untitled) / with headers
text/plain 821b
no_symlinks checks for symlinks in generated files (configure && make in a src subdir) which are not part of the distribution. Perfect::Hash $ pb -S prove -b t/z_kwalitee.t t/z_kwalitee.t .. These tests should not be running unless AUTHOR_TESTING=1 and/or RELEASE_TESTING=1! t/z_kwalitee.t .. 1/? # Failed test 'no_symlinks' # at t/z_kwalitee.t line 12. # Error: This distribution includes symbolic links (symlinks). This is bad, because there are operating systems that do not handle symlinks. # Details: # The following symlinks were found: cmph-2.0/src/.libs/libcmph.so.0;cmph-2.0/src/.libs/libcmph.so;cmph-2.0/src/.libs/libcmph.la;cmph-2.0/src/.libs/libcmph.dylib;cmph-2.0/lib/libcmph.so.0;cmph-2.0/lib/libcmph.so;cmph-2.0/lib/libcmph.dylib # Remedy: Remove the symlinks from the distribution. -- Reini Urban
Download (untitled) / with headers
text/plain 1.3k
On 2014-08-07 19:15:10, RURBAN wrote: Show quoted text
> no_symlinks checks for symlinks in generated files (configure && make > in a src subdir) > which are not part of the distribution. > > Perfect::Hash > $ pb -S prove -b t/z_kwalitee.t > t/z_kwalitee.t .. These tests should not be running unless > AUTHOR_TESTING=1 and/or RELEASE_TESTING=1! > t/z_kwalitee.t .. 1/? > # Failed test 'no_symlinks' > # at t/z_kwalitee.t line 12. > # Error: This distribution includes symbolic links (symlinks). This is > bad, because there are operating systems that do not handle symlinks. > # Details: > # The following symlinks were found: cmph- > 2.0/src/.libs/libcmph.so.0;cmph-2.0/src/.libs/libcmph.so;cmph- > 2.0/src/.libs/libcmph.la;cmph-2.0/src/.libs/libcmph.dylib;cmph- > 2.0/lib/libcmph.so.0;cmph-2.0/lib/libcmph.so;cmph- > 2.0/lib/libcmph.dylib > # Remedy: Remove the symlinks from the distribution.
I suppose the analyser would have to check against the MANIFEST file to be sure that the file actually was part of the distribution. Otherwise, you could add a skip_all if -d '.git', which would prevent the file from running in a bare repo dir (but this would only be useful if you were using Dist::Zilla, because otherwise the bare repo dir is the only place tests ever run). Sending over to the Module-CPANTS-Analyse queue for consideration; checking MANIFEST might be simple?
Download (untitled) / with headers
text/plain 2.1k
On Fri Aug 08 14:54:07 2014, ETHER wrote: Show quoted text
> On 2014-08-07 19:15:10, RURBAN wrote:
> > no_symlinks checks for symlinks in generated files (configure && make > > in a src subdir) > > which are not part of the distribution. > > > > Perfect::Hash > > $ pb -S prove -b t/z_kwalitee.t > > t/z_kwalitee.t .. These tests should not be running unless > > AUTHOR_TESTING=1 and/or RELEASE_TESTING=1! > > t/z_kwalitee.t .. 1/? > > # Failed test 'no_symlinks' > > # at t/z_kwalitee.t line 12. > > # Error: This distribution includes symbolic links (symlinks). This > > is > > bad, because there are operating systems that do not handle symlinks. > > # Details: > > # The following symlinks were found: cmph- > > 2.0/src/.libs/libcmph.so.0;cmph-2.0/src/.libs/libcmph.so;cmph- > > 2.0/src/.libs/libcmph.la;cmph-2.0/src/.libs/libcmph.dylib;cmph- > > 2.0/lib/libcmph.so.0;cmph-2.0/lib/libcmph.so;cmph- > > 2.0/lib/libcmph.dylib > > # Remedy: Remove the symlinks from the distribution.
> > I suppose the analyser would have to check against the MANIFEST file > to be sure that the file actually was part of the distribution. > > Otherwise, you could add a skip_all if -d '.git', which would prevent > the file from running in a bare repo dir (but this would only be > useful if you were using Dist::Zilla, because otherwise the bare repo > dir is the only place tests ever run). > > Sending over to the Module-CPANTS-Analyse queue for consideration; > checking MANIFEST might be simple?
It depends on the context. As for the CPANTS website, it's not sufficient to check only manifested stuff because hand-made distributions may have symlinks that are not manifested or hidden from the CPAN indices (there're even dists without MANIFEST). OTOH, it may be reasonable to skip stuff not in the MANIFEST for Test::Kwalitee and its friends, or it may not be. (After all, should Test::Kwalitee work correctly when unnecessary stuff is still left in the dist directory even though RELEASE_TESTING is set to true?) Test::Kwalitee has already a feature to disable specific metrics (such as -no_symlinks). So I feel this is just a matter of choice by a user, not by us the maintainers. However, I can change it to behave differently depending on the context. Any ideas?
Download (untitled) / with headers
text/plain 2.6k
On Fri Aug 08 02:50:35 2014, ISHIGAKI wrote: Show quoted text
> On Fri Aug 08 14:54:07 2014, ETHER wrote:
> > On 2014-08-07 19:15:10, RURBAN wrote:
> > > no_symlinks checks for symlinks in generated files (configure && > > > make > > > in a src subdir) > > > which are not part of the distribution. > > > > > > Perfect::Hash > > > $ pb -S prove -b t/z_kwalitee.t > > > t/z_kwalitee.t .. These tests should not be running unless > > > AUTHOR_TESTING=1 and/or RELEASE_TESTING=1! > > > t/z_kwalitee.t .. 1/? > > > # Failed test 'no_symlinks' > > > # at t/z_kwalitee.t line 12. > > > # Error: This distribution includes symbolic links (symlinks). This > > > is > > > bad, because there are operating systems that do not handle > > > symlinks. > > > # Details: > > > # The following symlinks were found: cmph- > > > 2.0/src/.libs/libcmph.so.0;cmph-2.0/src/.libs/libcmph.so;cmph- > > > 2.0/src/.libs/libcmph.la;cmph-2.0/src/.libs/libcmph.dylib;cmph- > > > 2.0/lib/libcmph.so.0;cmph-2.0/lib/libcmph.so;cmph- > > > 2.0/lib/libcmph.dylib > > > # Remedy: Remove the symlinks from the distribution.
> > > > I suppose the analyser would have to check against the MANIFEST file > > to be sure that the file actually was part of the distribution. > > > > Otherwise, you could add a skip_all if -d '.git', which would prevent > > the file from running in a bare repo dir (but this would only be > > useful if you were using Dist::Zilla, because otherwise the bare repo > > dir is the only place tests ever run). > > > > Sending over to the Module-CPANTS-Analyse queue for consideration; > > checking MANIFEST might be simple?
> > It depends on the context. As for the CPANTS website, it's not > sufficient to check only manifested stuff because hand-made > distributions may have symlinks that are not manifested or hidden from > the CPAN indices (there're even dists without MANIFEST). OTOH, it may > be reasonable to skip stuff not in the MANIFEST for Test::Kwalitee and > its friends, or it may not be. (After all, should Test::Kwalitee work > correctly when unnecessary stuff is still left in the dist directory > even though RELEASE_TESTING is set to true?) > > Test::Kwalitee has already a feature to disable specific metrics (such > as -no_symlinks). So I feel this is just a matter of choice by a user, > not by us the maintainers. However, I can change it to behave > differently depending on the context. Any ideas?
Sure. I had to use Test::Kwalitee->import(tests => ['-no_symlinks']) I found a previous report with a similar related problem: https://rt.cpan.org/Ticket/Display.html?id=26343 I believe if there is a MANIFEST it should be used. If there's none the logic can stay as it is. -- Reini Urban
Download (untitled) / with headers
text/plain 2.9k
On Fri Aug 08 23:34:16 2014, RURBAN wrote: Show quoted text
> On Fri Aug 08 02:50:35 2014, ISHIGAKI wrote:
> > On Fri Aug 08 14:54:07 2014, ETHER wrote:
> > > On 2014-08-07 19:15:10, RURBAN wrote:
> > > > no_symlinks checks for symlinks in generated files (configure && > > > > make > > > > in a src subdir) > > > > which are not part of the distribution. > > > > > > > > Perfect::Hash > > > > $ pb -S prove -b t/z_kwalitee.t > > > > t/z_kwalitee.t .. These tests should not be running unless > > > > AUTHOR_TESTING=1 and/or RELEASE_TESTING=1! > > > > t/z_kwalitee.t .. 1/? > > > > # Failed test 'no_symlinks' > > > > # at t/z_kwalitee.t line 12. > > > > # Error: This distribution includes symbolic links (symlinks). > > > > This > > > > is > > > > bad, because there are operating systems that do not handle > > > > symlinks. > > > > # Details: > > > > # The following symlinks were found: cmph- > > > > 2.0/src/.libs/libcmph.so.0;cmph-2.0/src/.libs/libcmph.so;cmph- > > > > 2.0/src/.libs/libcmph.la;cmph-2.0/src/.libs/libcmph.dylib;cmph- > > > > 2.0/lib/libcmph.so.0;cmph-2.0/lib/libcmph.so;cmph- > > > > 2.0/lib/libcmph.dylib > > > > # Remedy: Remove the symlinks from the distribution.
> > > > > > I suppose the analyser would have to check against the MANIFEST > > > file > > > to be sure that the file actually was part of the distribution. > > > > > > Otherwise, you could add a skip_all if -d '.git', which would > > > prevent > > > the file from running in a bare repo dir (but this would only be > > > useful if you were using Dist::Zilla, because otherwise the bare > > > repo > > > dir is the only place tests ever run). > > > > > > Sending over to the Module-CPANTS-Analyse queue for consideration; > > > checking MANIFEST might be simple?
> > > > It depends on the context. As for the CPANTS website, it's not > > sufficient to check only manifested stuff because hand-made > > distributions may have symlinks that are not manifested or hidden > > from > > the CPAN indices (there're even dists without MANIFEST). OTOH, it may > > be reasonable to skip stuff not in the MANIFEST for Test::Kwalitee > > and > > its friends, or it may not be. (After all, should Test::Kwalitee work > > correctly when unnecessary stuff is still left in the dist directory > > even though RELEASE_TESTING is set to true?) > > > > Test::Kwalitee has already a feature to disable specific metrics > > (such > > as -no_symlinks). So I feel this is just a matter of choice by a > > user, > > not by us the maintainers. However, I can change it to behave > > differently depending on the context. Any ideas?
> > Sure. I had to use > Test::Kwalitee->import(tests => ['-no_symlinks']) > > I found a previous report with a similar related problem: > https://rt.cpan.org/Ticket/Display.html?id=26343 > > I believe if there is a MANIFEST it should be used. > If there's none the logic can stay as it is.
Implemented in the master. https://github.com/cpants/Module-CPANTS-Analyse/commit/05407eec87d1544a7322a2fd3ff84be58706495b
Download (untitled) / with headers
text/plain 3.1k
On Sat Aug 09 17:37:09 2014, ISHIGAKI wrote: Show quoted text
> On Fri Aug 08 23:34:16 2014, RURBAN wrote:
> > On Fri Aug 08 02:50:35 2014, ISHIGAKI wrote:
> > > On Fri Aug 08 14:54:07 2014, ETHER wrote:
> > > > On 2014-08-07 19:15:10, RURBAN wrote:
> > > > > no_symlinks checks for symlinks in generated files (configure > > > > > && > > > > > make > > > > > in a src subdir) > > > > > which are not part of the distribution. > > > > > > > > > > Perfect::Hash > > > > > $ pb -S prove -b t/z_kwalitee.t > > > > > t/z_kwalitee.t .. These tests should not be running unless > > > > > AUTHOR_TESTING=1 and/or RELEASE_TESTING=1! > > > > > t/z_kwalitee.t .. 1/? > > > > > # Failed test 'no_symlinks' > > > > > # at t/z_kwalitee.t line 12. > > > > > # Error: This distribution includes symbolic links (symlinks). > > > > > This > > > > > is > > > > > bad, because there are operating systems that do not handle > > > > > symlinks. > > > > > # Details: > > > > > # The following symlinks were found: cmph- > > > > > 2.0/src/.libs/libcmph.so.0;cmph-2.0/src/.libs/libcmph.so;cmph- > > > > > 2.0/src/.libs/libcmph.la;cmph-2.0/src/.libs/libcmph.dylib;cmph- > > > > > 2.0/lib/libcmph.so.0;cmph-2.0/lib/libcmph.so;cmph- > > > > > 2.0/lib/libcmph.dylib > > > > > # Remedy: Remove the symlinks from the distribution.
> > > > > > > > I suppose the analyser would have to check against the MANIFEST > > > > file > > > > to be sure that the file actually was part of the distribution. > > > > > > > > Otherwise, you could add a skip_all if -d '.git', which would > > > > prevent > > > > the file from running in a bare repo dir (but this would only be > > > > useful if you were using Dist::Zilla, because otherwise the bare > > > > repo > > > > dir is the only place tests ever run). > > > > > > > > Sending over to the Module-CPANTS-Analyse queue for > > > > consideration; > > > > checking MANIFEST might be simple?
> > > > > > It depends on the context. As for the CPANTS website, it's not > > > sufficient to check only manifested stuff because hand-made > > > distributions may have symlinks that are not manifested or hidden > > > from > > > the CPAN indices (there're even dists without MANIFEST). OTOH, it > > > may > > > be reasonable to skip stuff not in the MANIFEST for Test::Kwalitee > > > and > > > its friends, or it may not be. (After all, should Test::Kwalitee > > > work > > > correctly when unnecessary stuff is still left in the dist > > > directory > > > even though RELEASE_TESTING is set to true?) > > > > > > Test::Kwalitee has already a feature to disable specific metrics > > > (such > > > as -no_symlinks). So I feel this is just a matter of choice by a > > > user, > > > not by us the maintainers. However, I can change it to behave > > > differently depending on the context. Any ideas?
> > > > Sure. I had to use > > Test::Kwalitee->import(tests => ['-no_symlinks']) > > > > I found a previous report with a similar related problem: > > https://rt.cpan.org/Ticket/Display.html?id=26343 > > > > I believe if there is a MANIFEST it should be used. > > If there's none the logic can stay as it is.
> > Implemented in the master. > > https://github.com/cpants/Module-CPANTS- > Analyse/commit/05407eec87d1544a7322a2fd3ff84be58706495b
Closed as 0.94 is released. Thanks.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.