Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bleadperl v5.21.1-11-g4077a6b breaks GFUJI/Text-Xslate-3.2.4.tar.gz #13952

Closed
p5pRT opened this issue Jun 23, 2014 · 20 comments
Closed

Bleadperl v5.21.1-11-g4077a6b breaks GFUJI/Text-Xslate-3.2.4.tar.gz #13952

p5pRT opened this issue Jun 23, 2014 · 20 comments

Comments

@p5pRT
Copy link

p5pRT commented Jun 23, 2014

Migrated from rt.perl.org#122159 (status was 'resolved')

Searchable as RT122159$

@p5pRT
Copy link
Author

p5pRT commented Jun 23, 2014

From @andk

git bisect


commit 4077a6b
Author​: Ævar Arnfjörð Bjarmason <avar@​cpan.org>
Date​: Sun Jan 19 16​:27​:58 2014 +0000

  Add a new warning about redundant printf arguments

also affected


TOBYINK/Kavorka-0.030.tar.gz
SATOH/Teng-0.23.tar.gz
JMATES/App-MusicTools-1.16.tar.gz
CFAERBER/Unicode-Stringprep-1.104.tar.gz
HKOBA/YATT-v0.0.8.tar.gz
TEAM/EntityModel-Log-0.005.tar.gz

sample fail report


http​://www.cpantesters.org/cpan/report/c8e16e60-f97a-11e3-b4ce-9dae0a370852

perl -V


Summary of my perl5 (revision 5 version 21 subversion 2) configuration​:
  Commit id​: 4077a6b
  Platform​:
  osname=linux, osvers=3.14-1-amd64, archname=x86_64-linux-thread-multi
  uname='linux k83 3.14-1-amd64 #1 smp debian 3.14.5-1 (2014-06-05) x86_64 gnulinux '
  config_args='-Dprefix=/home/sand/src/perl/repoperls/installed-perls/perl/v5.21.1-11-g4077a6b/9980 -Dmyhostname=k83 -Dinstallusrbinperl=n -Uversiononly -Dusedevel -des -Ui_db -Duseithreads -Uuselongdouble -DDEBUGGING=-g'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=define, usemultiplicity=define
  use64bitint=define, use64bitall=define, uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
  optimize='-O2 -g',
  cppflags='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
  ccversion='', gccversion='4.8.3', gccosandvers=''
  intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
  ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
  alignbytes=8, prototype=define
  Linker and Libraries​:
  ld='cc', ldflags =' -fstack-protector -L/usr/local/lib'
  libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/4.8/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib
  libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread -lc -lgdbm_compat
  perllibs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
  libc=libc-2.19.so, so=so, useshrplib=false, libperl=libperl.a
  gnulibc_version='2.19'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
  cccdlflags='-fPIC', lddlflags='-shared -O2 -g -L/usr/local/lib -fstack-protector'

Characteristics of this binary (from libperl)​:
  Compile-time options​: HAS_TIMES MULTIPLICITY PERLIO_LAYERS
  PERL_DONT_CREATE_GVSV
  PERL_HASH_FUNC_ONE_AT_A_TIME_HARD
  PERL_IMPLICIT_CONTEXT PERL_MALLOC_WRAP
  PERL_NEW_COPY_ON_WRITE PERL_PRESERVE_IVUV
  PERL_USE_DEVEL USE_64_BIT_ALL USE_64_BIT_INT
  USE_ITHREADS USE_LARGE_FILES USE_LOCALE
  USE_LOCALE_COLLATE USE_LOCALE_CTYPE
  USE_LOCALE_NUMERIC USE_PERLIO USE_PERL_ATOF
  USE_REENTRANT_API
  Built under linux
  Compiled at Jun 21 2014 16​:37​:26
  %ENV​:
  PERL5LIB=""
  PERL5OPT=""
  PERL5_CPANPLUS_IS_RUNNING="19951"
  PERL5_CPAN_IS_RUNNING="19951"
  PERL_MM_USE_DEFAULT="1"
  @​INC​:
  /home/sand/src/perl/repoperls/installed-perls/perl/v5.21.1-11-g4077a6b/9980/lib/site_perl/5.21.2/x86_64-linux-thread-multi
  /home/sand/src/perl/repoperls/installed-perls/perl/v5.21.1-11-g4077a6b/9980/lib/site_perl/5.21.2
  /home/sand/src/perl/repoperls/installed-perls/perl/v5.21.1-11-g4077a6b/9980/lib/5.21.2/x86_64-linux-thread-multi
  /home/sand/src/perl/repoperls/installed-perls/perl/v5.21.1-11-g4077a6b/9980/lib/5.21.2
  .
--
andreas

@p5pRT
Copy link
Author

p5pRT commented Jun 24, 2014

From @jkeenan

On Sun Jun 22 23​:04​:08 2014, andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

git bisect
----------
commit 4077a6b
Author​: Ævar Arnfjörð Bjarmason <avar@​cpan.org>
Date​: Sun Jan 19 16​:27​:58 2014 +0000

Add a new warning about redundant printf arguments

also affected
-------------
TOBYINK/Kavorka-0.030.tar.gz
SATOH/Teng-0.23.tar.gz
JMATES/App-MusicTools-1.16.tar.gz
CFAERBER/Unicode-Stringprep-1.104.tar.gz
HKOBA/YATT-v0.0.8.tar.gz
TEAM/EntityModel-Log-0.005.tar.gz

sample fail report
------------------
http​://www.cpantesters.org/cpan/report/c8e16e60-f97a-11e3-b4ce-
9dae0a370852

I confirmed each of these failures with a program evolved from khw's debug program.

Before we go submitting patches to the various CPAN authors, I would like to ask​: Have we decided to include this new new feature in 5.22?

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

p5pRT commented Jun 24, 2014

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Oct 17, 2014

From @jkeenan

On Tue Jun 24 13​:39​:43 2014, jkeenan wrote​:

On Sun Jun 22 23​:04​:08 2014, andreas.koenig.7os6VVqR@​franz.ak.mind.de
wrote​:

git bisect
----------
commit 4077a6b
Author​: Ævar Arnfjörð Bjarmason <avar@​cpan.org>
Date​: Sun Jan 19 16​:27​:58 2014 +0000

Add a new warning about redundant printf arguments

also affected
-------------
TOBYINK/Kavorka-0.030.tar.gz
SATOH/Teng-0.23.tar.gz
JMATES/App-MusicTools-1.16.tar.gz
CFAERBER/Unicode-Stringprep-1.104.tar.gz
HKOBA/YATT-v0.0.8.tar.gz
TEAM/EntityModel-Log-0.005.tar.gz

sample fail report
------------------
http​://www.cpantesters.org/cpan/report/c8e16e60-f97a-11e3-b4ce-
9dae0a370852

I confirmed each of these failures with a program evolved from khw's
debug program.

Before we go submitting patches to the various CPAN authors, I would
like to ask​: Have we decided to include this new new feature in 5.22?

Four months ago, I asked for opinions on the question of this proposed new warning and -- surprisingly, for this list -- got none!

So how shall we proceed?

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Oct 17, 2014

From @khwilliamson

On Thu Oct 16 18​:13​:59 2014, jkeenan wrote​:

On Tue Jun 24 13​:39​:43 2014, jkeenan wrote​:

On Sun Jun 22 23​:04​:08 2014, andreas.koenig.7os6VVqR@​franz.ak.mind.de
wrote​:

git bisect
----------
commit 4077a6b
Author​: Ævar Arnfjörð Bjarmason <avar@​cpan.org>
Date​: Sun Jan 19 16​:27​:58 2014 +0000

Add a new warning about redundant printf arguments

also affected
-------------
TOBYINK/Kavorka-0.030.tar.gz
SATOH/Teng-0.23.tar.gz
JMATES/App-MusicTools-1.16.tar.gz
CFAERBER/Unicode-Stringprep-1.104.tar.gz
HKOBA/YATT-v0.0.8.tar.gz
TEAM/EntityModel-Log-0.005.tar.gz

sample fail report
------------------
http​://www.cpantesters.org/cpan/report/c8e16e60-f97a-11e3-b4ce-
9dae0a370852

I confirmed each of these failures with a program evolved from khw's
debug program.

Before we go submitting patches to the various CPAN authors, I would
like to ask​: Have we decided to include this new new feature in
5.22?

Four months ago, I asked for opinions on the question of this proposed
new warning and -- surprisingly, for this list -- got none!

So how shall we proceed?

Thank you very much.

FWIW, I think this is useful to include in 5.22
--
Karl Williamson

@p5pRT
Copy link
Author

p5pRT commented Nov 7, 2014

From @andk

also affected​:
SKINGTON/warnings-everywhere-0.007.tar.gz
http​://www.cpantesters.org/cpan/report/f631aad6-5e1c-11e4-ab7d-50d5e0bfc7aa
--
andreas

@p5pRT
Copy link
Author

p5pRT commented Nov 17, 2014

From @cpansprout

On Thu Oct 16 18​:49​:50 2014, khw wrote​:

FWIW, I think this is useful to include in 5.22

I have just looked at Badger, and the offending code is this subroutine in lib/Badger/Utils.pm​:

sub xprintf {
  my $format = shift;
  my @​args = @​_;
  $format =~
  s{ < (\d+)
  (?​: :( [#\-\+ ]? [\w\.]+ ) )?
  (?​: \| (.*?) )?
  >
  }
  { defined $3
  ? _xprintf_ifdef(\@​args, $1, $2, $3)
  : '%' . $1 . '$' . ($2 || 's')
  }egx;
  sprintf($format, @​_);
}

Getting code like that to work around the warning is going to be a pain. And, to me, this code seems perfectly idiomatic. I’m not sure this warning is so helpful after all. (Especially consider that redundant arguments are generally harmless, whereas missing arguments are generally indications of a bug.)

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Nov 18, 2014

From @cpansprout

On Sun Nov 16 16​:56​:51 2014, sprout wrote​:

On Thu Oct 16 18​:49​:50 2014, khw wrote​:

FWIW, I think this is useful to include in 5.22

I have just looked at Badger, and the offending code is this
subroutine in lib/Badger/Utils.pm​:

sub xprintf {
my $format = shift;
my @​args = @​_;
$format =~
s{ < (\d+)
(?​: :( [#\-\+ ]? [\w\.]+ ) )?
(?​: \| (.*?) )?

}
{ defined $3
? _xprintf_ifdef(\@​args, $1, $2, $3)
: '%' . $1 . '$' . ($2 || 's')
}egx;
sprintf($format, @​_);
}

Getting code like that to work around the warning is going to be a
pain. And, to me, this code seems perfectly idiomatic. I’m not sure
this warning is so helpful after all. (Especially consider that
redundant arguments are generally harmless, whereas missing arguments
are generally indications of a bug.)

However, ‘printf "%s", $foo, $bar’ is almost undoubtedly a mistake. So maybe what we really want to do is make this a compile-time warning that only applies when we know ahead of time that the list will overflow the format.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Nov 23, 2014

From @rjbs

* Father Chrysostomos via RT <perlbug-followup@​perl.org> [2014-11-16T19​:56​:51]

Getting code like that to work around the warning is going to be a pain.
And, to me, this code seems perfectly idiomatic. I’m not sure this warning
is so helpful after all.

I fixed a real bug in PAUSE because of this warning, this week.

--
rjbs

@p5pRT
Copy link
Author

p5pRT commented Nov 24, 2014

From @cpansprout

On Sun Nov 23 14​:18​:01 2014, perl.p5p@​rjbs.manxome.org wrote​:

* Father Chrysostomos via RT <perlbug-followup@​perl.org> [2014-11-
16T19​:56​:51]

Getting code like that to work around the warning is going to be a
pain.
And, to me, this code seems perfectly idiomatic. I’m not sure this
warning
is so helpful after all.

I fixed a real bug in PAUSE because of this warning, this week.

And since I wrote that I have seen about 3 or 4 cases of false positives, and one case of clearly badly-written code. The latter would have been caught by my suggestion of doing a compile-time check. What do you think of that suggestion, BTW? Would it have caught the PAUSE bug? It’s basically to warn only about sprintf "constant string", $known, $number, $of, $arguments.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Feb 8, 2015

From @rjbs

* Father Chrysostomos via RT <perlbug-followup@​perl.org> [2014-11-23T19​:06​:38]

And since I wrote that I have seen about 3 or 4 cases of false positives, and
one case of clearly badly-written code. The latter would have been caught by
my suggestion of doing a compile-time check. What do you think of that
suggestion, BTW? Would it have caught the PAUSE bug? It’s basically to warn
only about sprintf "constant string", $known, $number, $of, $arguments.

It would not, fwiw, have caught the PAUSE bug, which passed the arguments in an
array whose size was constant, wrong, and not inferrable from the Perl code.

In https://rt.perl.org/Public/Bug/Display.html?id=122159#txn-1318809 you vited
changes needed to Badget, saying it would be a pain to work around the warning.
But the workaround is just "no warnings 'redundant'".

I don't have really strong feelings about this warning, but I remain leaning
pretty firmly toward "it's still worth it."

--
rjbs

@p5pRT
Copy link
Author

p5pRT commented Feb 8, 2015

From @cpansprout

On Sat Feb 07 18​:48​:00 2015, perl.p5p@​rjbs.manxome.org wrote​:

* Father Chrysostomos via RT <perlbug-followup@​perl.org> [2014-11-
23T19​:06​:38]

And since I wrote that I have seen about 3 or 4 cases of false
positives, and
one case of clearly badly-written code. The latter would have been
caught by
my suggestion of doing a compile-time check. What do you think of
that
suggestion, BTW? Would it have caught the PAUSE bug? It’s basically
to warn
only about sprintf "constant string", $known, $number, $of,
$arguments.

It would not, fwiw, have caught the PAUSE bug, which passed the
arguments in an
array whose size was constant, wrong, and not inferrable from the Perl
code.

In https://rt.perl.org/Public/Bug/Display.html?id=122159#txn-1318809
you vited
changes needed to Badget, saying it would be a pain to work around the
warning.
But the workaround is just "no warnings 'redundant'".

I don't have really strong feelings about this warning, but I remain
leaning
pretty firmly toward "it's still worth it."

In that case I relent, especially since I don’t want to go fiddling with that type of op tree examination this close to 5.22.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Mar 25, 2015

From @cpansprout

One thing we really ought to do, in addition to patching the failing modules, is fix this message, which I get when starting the CPAN shell, since it is embarrassing​:

Redundant argument in sprintf at /usr/local/lib/perl5/5.21.11/CPAN.pm line 314.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Mar 25, 2015

From @andk

On Wed, 25 Mar 2015 09​:06​:25 -0700, "Father Chrysostomos via RT" <perlbug-followup@​perl.org> said​:

  > One thing we really ought to do, in addition to patching the failing
  > modules, is fix this message, which I get when starting the CPAN
  > shell, since it is embarrassing​: Redundant argument in sprintf at
  > /usr/local/lib/perl5/5.21.11/CPAN.pm line 314.

Oh yes, how embarrassing. I never saw it until now, sorry for that.
Proposed patch follows. I'll jump to 2.12 then in my copy and will not
release a 2.11. Let me know if there is a better solution.

From 0aae0334bc38975f7ebea5fd9bf244d3f3684e5d Mon Sep 17 00​:00​:00 2001
From​: Andreas Koenig <andk@​cpan.org>
Date​: Wed, 25 Mar 2015 23​:47​:01 +0100
Subject​: [PATCH] Fix "Redundant argument in sprintf", reported by Father
Chrysostomos


lib/CPAN.pm | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

Inline Patch
diff --git a/lib/CPAN.pm b/lib/CPAN.pm
index 60a5e6c..731765c 100644
--- a/lib/CPAN.pm
+++ b/lib/CPAN.pm
@@ -2,7 +2,7 @@
 # vim: ts=4 sts=4 sw=4:
 use strict;
 package CPAN;
-$CPAN::VERSION = '2.10';
+$CPAN::VERSION = '2.11';
 $CPAN::VERSION =~ s/_//;
 
 # we need to run chdir all over and we would get at wrong libraries
@@ -318,7 +318,6 @@ Enter 'h' for help.
 
 },
                                  $CPAN::VERSION,
-                                 $rl_avail
                                 )
     }
     my($continuation) = "";
-- 
2.1.4

--
andreas

@p5pRT
Copy link
Author

p5pRT commented Mar 25, 2015

From @andk

0001-Fix-Redundant-argument-in-sprintf-reported-by-Father.patch
From 0aae0334bc38975f7ebea5fd9bf244d3f3684e5d Mon Sep 17 00:00:00 2001
From: Andreas Koenig <andk@cpan.org>
Date: Wed, 25 Mar 2015 23:47:01 +0100
Subject: [PATCH] Fix "Redundant argument in sprintf", reported by Father
 Chrysostomos

---
 lib/CPAN.pm | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/CPAN.pm b/lib/CPAN.pm
index 60a5e6c..731765c 100644
--- a/lib/CPAN.pm
+++ b/lib/CPAN.pm
@@ -2,7 +2,7 @@
 # vim: ts=4 sts=4 sw=4:
 use strict;
 package CPAN;
-$CPAN::VERSION = '2.10';
+$CPAN::VERSION = '2.11';
 $CPAN::VERSION =~ s/_//;
 
 # we need to run chdir all over and we would get at wrong libraries
@@ -318,7 +318,6 @@ Enter 'h' for help.
 
 },
                                  $CPAN::VERSION,
-                                 $rl_avail
                                 )
     }
     my($continuation) = "";
-- 
2.1.4

@p5pRT
Copy link
Author

p5pRT commented Apr 2, 2015

From @tonycoz

Most of the modules mentioned in this ticket have been updated, the exceptions are​:

HKOBA/YATT-v0.0.8.tar.gz - this has been broken since inlinable constants changed
in 5.19.x, the author appeared confused about what was happening, I've provided a
hopefully coherent note in https://rt.cpan.org/Ticket/Display.html?id=95311

ANDK/CPAN - as discussed in this ticket.

One of the dists doesn't seem to have had a format issue​:

SATOH/Teng-0.23.tar.gz - now at 0.28, the only change (in 0.25) I saw involving
parameters to a *printf() didn't change the format string or the number of
arguments. In any case, it passes on blead now.

The following have been fixed​:

GFUJI/Text-Xslate-3.2.4.tar.gz - fixed in 3.3.1 (Unicode-Stringprep is a dep too
and caused problems)

TOBYINK/Kavorka-0.030.tar.gz - fixed in 0.032.

JMATES/App-MusicTools-1.16.tar.gz - fixed in 1.18.

CFAERBER/Unicode-Stringprep-1.104.tar.gz - fixed in 1.105

TEAM/EntityModel-Log-0.005.tar.gz - fixed in 0.006

SKINGTON/warnings-everywhere-0.007.tar.gz - fixed in 0.006

Again, tracking this at

https://docs.google.com/spreadsheets/d/1aBWdK3Y-dDM5rxg70P0af8FfwmdEubTUtPV0li5eb3M/edit?usp=sharing

Tony

@p5pRT
Copy link
Author

p5pRT commented Apr 7, 2015

From @jkeenan

On Wed Apr 01 21​:13​:20 2015, tonyc wrote​:

Most of the modules mentioned in this ticket have been updated, the
exceptions are​:

HKOBA/YATT-v0.0.8.tar.gz - this has been broken since inlinable
constants changed
in 5.19.x, the author appeared confused about what was happening, I've
provided a
hopefully coherent note in
https://rt.cpan.org/Ticket/Display.html?id=95311

And HKOBA has responded with gratitude​: https://rt.cpan.org/Ticket/Display.html?id=95311#txn-1480881

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Apr 20, 2015

From @iabyn

On Wed, Mar 25, 2015 at 11​:51​:15PM +0100, Andreas Koenig wrote​:

On Wed, 25 Mar 2015 09​:06​:25 -0700, "Father Chrysostomos via RT" <perlbug-followup@​perl.org> said​:
One thing we really ought to do, in addition to patching the failing
modules, is fix this message, which I get when starting the CPAN
shell, since it is embarrassing​: Redundant argument in sprintf at
/usr/local/lib/perl5/5.21.11/CPAN.pm line 314.

Oh yes, how embarrassing. I never saw it until now, sorry for that.
Proposed patch follows. I'll jump to 2.12 then in my copy and will not
release a 2.11. Let me know if there is a better solution.
[snip]
-$CPAN​::VERSION = '2.10';
+$CPAN​::VERSION = '2.11';
$CPAN​::VERSION =~ s/_//;
[snip]
- $rl_avail

I've applied this as 3198fda.
All tests seemed to pass. I don't know whether it messes up any of the
consistency cheres that are run only by release pumpkins and not by the
standard test suite; and I'm not sure whether the same change needs to
released as 2.11 on CPAN.

Other than that, I don't think this ticket needs to be a blocker for 5.22.

--
This is a great day for France!
  -- Nixon at Charles De Gaulle's funeral

@p5pRT
Copy link
Author

p5pRT commented Apr 23, 2015

From @rjbs

This ticket is no longer a blocker, and I am closing it.

--
rjbs

@p5pRT
Copy link
Author

p5pRT commented Apr 23, 2015

@rjbs - Status changed from 'open' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant