Bug 16104 - Warnings "used only once: possible typo" should be removed
Summary: Warnings "used only once: possible typo" should be removed
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Galen Charlton
QA Contact: Testopia
URL:
Keywords:
: 16101 17238 (view as bug list)
Depends on: 11998
Blocks: 16105
  Show dependency treegraph
 
Reported: 2016-03-18 13:29 UTC by Jonathan Druart
Modified: 2018-02-26 16:00 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 16104: Remove warnings "used only once: possible typo" (1.39 KB, patch)
2016-03-18 13:34 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16104: Remove warnings "used only once: possible typo" (1.61 KB, patch)
2016-03-18 18:59 UTC, M. Tompsett
Details | Diff | Splinter Review
[PASSED QA] Bug 16104: Remove warnings "used only once: possible typo" (1.68 KB, patch)
2016-04-03 15:07 UTC, Katrin Fischer
Details | Diff | Splinter Review
Revert "Bug 16104: Remove warnings "used only once: possible typo"" (934 bytes, patch)
2016-05-02 11:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Revert "Bug 16104: Remove warnings "used only once: possible typo"" (1.11 KB, patch)
2016-05-02 11:22 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2016-03-18 13:29:40 UTC
Since bug 11998, the following warnings are raised:
Name "Tie::Hash::FIELDS" used only once: possible typo at /usr/share/perl/5.22/fields.pm line 135.
Name "Cache::RemovalStrategy::LRU::FIELDS" used only once: possible typo at /usr/share/perl/5.22/fields.pm line 135.
Name "Cache::RemovalStrategy::FIELDS" used only once: possible typo at /usr/share/perl/5.22/fields.pm line 135.

It comes from the Koha::Cache

103     if ( can_load( modules => { 'Cache::Memory' => undef } ) ) {
Comment 1 Jonathan Druart 2016-03-18 13:34:50 UTC Comment hidden (obsolete)
Comment 2 M. Tompsett 2016-03-18 18:55:11 UTC
*** Bug 16101 has been marked as a duplicate of this bug. ***
Comment 3 M. Tompsett 2016-03-18 18:59:15 UTC Comment hidden (obsolete)
Comment 4 M. Tompsett 2016-03-18 19:01:46 UTC
Also did an:
1) apply patch
2) perl -cw C4/AuthoritiesMarc.pm
   -- no issues
3) checked out master
4) repeat step 2
   -- expected failures.
Comment 5 Katrin Fischer 2016-04-03 15:07:28 UTC Comment hidden (obsolete)
Comment 6 Brendan Gallagher 2016-04-20 20:26:46 UTC
Pushed to Master - Should be in the May 2016 Release.  Thanks!
Comment 7 Julian Maurice 2016-04-22 10:03:51 UTC
Patch pushed to 3.22.x, will be in 3.22.6
Comment 8 Jacek Ablewicz 2016-04-25 10:04:07 UTC
(In reply to Jonathan Druart from comment #1)

> QA note: this may have an impact on performance but I have not found any
> other workarounds.

This

   if ( can_load( modules => { 'Cache::Memory' => undef, nocache => 1 } ) ) {

is not a right syntax for 'noncache' parameter.. If called that way, can_load() tries to load Cache::Memory and (nonexisting) 'nocache' module (version >= 1.0), and fails - so it simply prevents Cache::Memory loading (hence no more warnings). This has and enormous impact on performance in all setups which do not use memcached as a caching system; after that, memchached is the only caching system still working in Koha.

It also causes 'prove t/Cache.t' to fail in the default setup (w/o memcached).
Comment 9 Jacek Ablewicz 2016-04-25 10:29:33 UTC
I found another workaround:

+can_load( modules => { 'Cache::Memory' => undef } ) && Cache::Memory->new();
our %L1_cache;

-    if ( can_load( modules => { 'Cache::Memory' => undef, nocache => 1 } ) ) {
+    if ( can_load( modules => { 'Cache::Memory' => undef } ) ) {

which seems to work (?, "perl -wc C4/AuthoritiesMarc.pm" does not emit any warnings) without introducing noticeable performance issues, but I have no idea how exactly/why it works.. I have a feeling that it only hides the problem (whatever it really is) instead of fixing it properly.
Comment 10 Jonathan Druart 2016-04-25 15:23:41 UTC
(In reply to Jacek Ablewicz from comment #8)
> This
> 
>    if ( can_load( modules => { 'Cache::Memory' => undef, nocache => 1 } ) ) {
> 
> is not a right syntax for 'noncache' parameter.. If called that way,
> can_load() tries to load Cache::Memory and (nonexisting) 'nocache' module
> (version >= 1.0), and fails - so it simply prevents Cache::Memory loading
> (hence no more warnings). This has and enormous impact on performance in all
> setups which do not use memcached as a caching system; after that,
> memchached is the only caching system still working in Koha.

Erk! You are right, I misread the doc :-/

@RM please revert this patch
Comment 11 Jonathan Druart 2016-05-02 09:56:07 UTC
(In reply to Jonathan Druart from comment #10)
> @RM please revert this patch

ping!

Above all, it fails 2 tests in t/db_dependent/Context.t
Comment 12 Jonathan Druart 2016-05-02 10:08:47 UTC
(In reply to Jonathan Druart from comment #11)
> (In reply to Jonathan Druart from comment #10)
> > @RM please revert this patch
> 
> ping!
> 
> Above all, it fails 2 tests in t/db_dependent/Context.t

And t/db_dependent/sysprefs.t
Comment 13 Julian Maurice 2016-05-02 10:10:45 UTC
Patch reverted in 3.22.x
Comment 14 Jonathan Druart 2016-05-02 11:14:30 UTC Comment hidden (obsolete)
Comment 15 Marcel de Rooy 2016-05-02 11:22:51 UTC
Created attachment 51088 [details] [review]
Revert "Bug 16104: Remove warnings "used only once: possible typo""

This reverts commit 6b319a2d487257e8a08bc26af9c6b2e7fa2ece2c.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested with memcached and without MEMCACHED_SERVERS.
Cache.t, Context.t and sysprefs.t pass now.
Comment 16 Kyle M Hall 2016-05-02 15:29:48 UTC
Revert pushed to master
Comment 17 Julian Maurice 2016-05-16 07:36:59 UTC
Revert will be in 3.22.7
Comment 18 Marcel de Rooy 2016-05-27 09:18:27 UTC
If you compile (perl -cw) a script or module now that contains a use C4::Context, you have the following warnings:

Name "Tie::Hash::FIELDS" used only once: possible typo at /usr/share/perl/5.22/fields.pm line 135.
Name "Cache::RemovalStrategy::LRU::FIELDS" used only once: possible typo at /usr/share/perl/5.22/fields.pm line 135.
Name "Cache::RemovalStrategy::FIELDS" used only once: possible typo 
at /usr/share/perl/5.22/fields.pm line 135.

[ PS koha-qa does the same. ]

Are these warnings somehow related to perl's Cache module containing things like below?

use base qw(Tie::Hash);

my $strategy = $args->{removal_strategy} || 'Cache::RemovalStrategy::LRU';
Comment 19 Jonathan Druart 2016-06-05 16:46:55 UTC
(In reply to Marcel de Rooy from comment #18)
> If you compile (perl -cw) a script or module now that contains a use
> C4::Context, you have the following warnings:
> 
> Name "Tie::Hash::FIELDS" used only once: possible typo at
> /usr/share/perl/5.22/fields.pm line 135.
> Name "Cache::RemovalStrategy::LRU::FIELDS" used only once: possible typo at
> /usr/share/perl/5.22/fields.pm line 135.
> Name "Cache::RemovalStrategy::FIELDS" used only once: possible typo 
> at /usr/share/perl/5.22/fields.pm line 135.
> 
> [ PS koha-qa does the same. ]
> 
> Are these warnings somehow related to perl's Cache module containing things
> like below?
> 
> use base qw(Tie::Hash);
> 
> my $strategy = $args->{removal_strategy} || 'Cache::RemovalStrategy::LRU';

Yes, more or less, but I have never managed to track down completely the problem.
There is something weird happening...
To recreate easily, see bug 11998 comment 62
Comment 20 Katrin Fischer 2016-07-15 08:24:52 UTC
Hm, this has been reverted - changing status.
Comment 21 Jonathan Druart 2016-07-22 10:42:23 UTC
FYI I have added a temporary fix to the qa tools
  commit d73431289d4c72d7751b006137f518851d2ee605
    Do not consider "used only once: possible typo" as a warning (and make the tests pass!)
Comment 22 Jonathan Druart 2016-09-06 08:12:16 UTC
*** Bug 17238 has been marked as a duplicate of this bug. ***