Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.85k stars 527 forks source link

Carp is missing a dot #11814

Closed p5pRT closed 12 years ago

p5pRT commented 12 years ago

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

Searchable as RT106538$

p5pRT commented 12 years ago

From @cpansprout

This was brought up in #96672 but needs its own ticket​:

$ perl -e 'die' Died at -e line 1. $ perl -MCarp -e 'croak Died' Died at -e line 1

Where did my dot go?

The hard part is deciding what to do about cluck().


Flags​:   category=library   severity=low


Site configuration information for perl 5.15.5​:

Configured by sprout at Sun Dec 18 11​:26​:14 PST 2011.

Summary of my perl5 (revision 5 version 15 subversion 5) configuration​:   Snapshot of​: 5dca8ed9d28127c9f7a2e7ce5f8ba970da3608cd   Platform​:   osname=darwin\, osvers=10.5.0\, archname=darwin-2level   uname='darwin pint.local 10.5.0 darwin kernel version 10.5.0​: fri nov 5 23​:20​:39 pdt 2010; root​:xnu-1504.9.17~1release_i386 i386 '   config_args='-de -Dusedevel -DDEBUGGING=-g'   hint=recommended\, useposix=true\, d_sigaction=define   useithreads=undef\, usemultiplicity=undef   useperlio=define\, d_sfio=undef\, uselargefiles=define\, usesocks=undef   use64bitint=undef\, use64bitall=undef\, uselongdouble=undef   usemymalloc=n\, bincompat5005=undef   Compiler​:   cc='cc'\, ccflags ='-fno-common -DPERL_DARWIN -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'\,   optimize='-O3 -g'\,   cppflags='-fno-common -DPERL_DARWIN -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'   ccversion=''\, gccversion='4.2.1 (Apple Inc. build 5664)'\, gccosandvers=''   intsize=4\, longsize=4\, ptrsize=4\, doublesize=8\, byteorder=1234   d_longlong=define\, longlongsize=8\, d_longdbl=define\, longdblsize=16   ivtype='long'\, ivsize=4\, nvtype='double'\, nvsize=8\, Off_t='off_t'\, lseeksize=8   alignbytes=8\, prototype=define   Linker and Libraries​:   ld='env MACOSX_DEPLOYMENT_TARGET=10.3 cc'\, ldflags =' -fstack-protector -L/usr/local/lib'   libpth=/usr/local/lib /usr/lib   libs=-ldbm -ldl -lm -lutil -lc   perllibs=-ldl -lm -lutil -lc   libc=\, so=dylib\, useshrplib=false\, libperl=libperl.a   gnulibc_version=''   Dynamic Linking​:   dlsrc=dl_dlopen.xs\, dlext=bundle\, d_dlsymun=undef\, ccdlflags=' '   cccdlflags=' '\, lddlflags=' -bundle -undefined dynamic_lookup -L/usr/local/lib -fstack-protector'

Locally applied patches​:  


@​INC for perl 5.15.5​:   /usr/local/lib/perl5/site_perl/5.15.5/darwin-2level   /usr/local/lib/perl5/site_perl/5.15.5   /usr/local/lib/perl5/5.15.5/darwin-2level   /usr/local/lib/perl5/5.15.5   /usr/local/lib/perl5/site_perl   .


Environment for perl 5.15.5​:   DYLD_LIBRARY_PATH (unset)   HOME=/Users/sprout   LANG=en_US.UTF-8   LANGUAGE (unset)   LD_LIBRARY_PATH (unset)   LOGDIR (unset)   PATH=/usr/bin​:/bin​:/usr/sbin​:/sbin​:/usr/local/bin​:/usr/X11/bin​:/usr/local/bin   PERL_BADLANG (unset)   SHELL=/bin/bash

p5pRT commented 12 years ago

From @cpansprout

On Sun Dec 18 19​:56​:10 2011\, sprout wrote​:

This was brought up in #96672 but needs its own ticket​:

$ perl -e 'die' Died at -e line 1. $ perl -MCarp -e 'croak Died' Died at -e line 1

Where did my dot go?

The hard part is deciding what to do about cluck().

In commit 879b0cab8\, Zefram has solved this by adding the dot to messages but leaving it off for subsequent lines of cluck() output.

--

Father Chrysostomos

p5pRT commented 12 years ago

From [Unknown Contact. See original ticket]

On Sun Dec 18 19​:56​:10 2011\, sprout wrote​:

This was brought up in #96672 but needs its own ticket​:

$ perl -e 'die' Died at -e line 1. $ perl -MCarp -e 'croak Died' Died at -e line 1

Where did my dot go?

The hard part is deciding what to do about cluck().

In commit 879b0cab8\, Zefram has solved this by adding the dot to messages but leaving it off for subsequent lines of cluck() output.

--

Father Chrysostomos

p5pRT commented 12 years ago

@cpansprout - Status changed from 'new' to 'resolved'

p5pRT commented 12 years ago

From zefram@fysh.org

Father Chrysostomos wrote​:

Where did my dot go?

Added to Carp as 879b0cab8575cdc155c45c42eb82075648761936. This required changing an autodie test\, in partial violation of UPSTREAM=>"cpan". autodie will need a new CPAN release with that change. (Also had to change a handful of core-only tests.)

The hard part is deciding what to do about cluck().

I added the dot only to the base message\, and left the stack trace unaltered. I don't see what punctuation would be appropriate at the end of every stack trace line\, and I don't want to make the last line of it different from the others.

-zefram

p5pRT commented 12 years ago

From @Leont

On Thu\, Feb 2\, 2012 at 5​:39 PM\, Father Chrysostomos via RT \perlbug\-comment@​perl\.org wrote​:

In commit 879b0cab8\, Zefram has solved this by adding the dot to messages but leaving it off for subsequent lines of cluck() output.

I'm worried this will break tests of CPAN modules that rely a little too much on the exact error (probably including at least one of my modules).

Leon

p5pRT commented 12 years ago

From zefram@fysh.org

Leon Timmermans wrote​:

I'm worried this will break tests of CPAN modules

There presumably will be a few. It's an easy fix for each. I think it's an acceptable level of hassle. If you object\, you've got two weeks to render it contentious so that it has to be reverted. I'm holding off on CPAN release for this reason.

-zefram

p5pRT commented 12 years ago

From @Leont

On Thu\, Feb 2\, 2012 at 6​:00 PM\, Zefram \zefram@​fysh\.org wrote​:

There presumably will be a few.  It's an easy fix for each.  I think it's an acceptable level of hassle.  If you object\, you've got two weeks to render it contentious so that it has to be reverted.  I'm holding off on CPAN release for this reason.

I'm fine with updating my module\, it's trivial indeed. I'm worried though that the list of modules with this issue is larger than expected. We'll see soon enough how big the issue is.

Leon

p5pRT commented 12 years ago

From @toddr

On Thu Feb 02 09​:17​:06 2012\, LeonT wrote​:

On Thu\, Feb 2\, 2012 at 6​:00 PM\, Zefram \zefram@​fysh\.org wrote​:

There presumably will be a few. �It's an easy fix for each. �I think it's an acceptable level of hassle. �If you object\, you've got two weeks to render it contentious so that it has to be reverted. �I'm holding off on CPAN release for this reason.

I'm fine with updating my module\, it's trivial indeed. I'm worried though that the list of modules with this issue is larger than expected. We'll see soon enough how big the issue is.

So far\, I've found needed test fixes in​: autodie\, Error\, Log4perl and counting.

p5pRT commented 12 years ago

From zefram@fysh.org

Todd Rinaldo via RT wrote​:

So far\, I've found needed test fixes in​: autodie\, Error\, Log4perl and counting.

Not very many\, then. This is looking viable.

-zefram

p5pRT commented 12 years ago

From @hvds

"Todd Rinaldo via RT" \perlbug\-followup@​perl\.org wrote​: :On Thu Feb 02 09​:17​:06 2012\, LeonT wrote​: :> On Thu\, Feb 2\, 2012 at 6​:00 PM\, Zefram \zefram@​fysh\.org wrote​: :> > There presumably will be a few. �It's an easy fix for each. �I think it's :> > an acceptable level of hassle. �If you object\, you've got two weeks to :> > render it contentious so that it has to be reverted. �I'm holding off :> > on CPAN release for this reason. :> :> I'm fine with updating my module\, it's trivial indeed. I'm worried :> though that the list of modules with this issue is larger than :> expected. We'll see soon enough how big the issue is. : :So far\, I've found needed test fixes in​: autodie\, Error\, Log4perl and counting.

One thing worth mentioning​: along the lines of Nick's recent patch to separate two otherwise indistinguishable panics in perl core\, there is actually an advantage to being able to discover from where an error message came.

Maybe the presence or absence of a trailing full-stop is not the ideal way to signal provenance; but I think the general principle stands that you actually do lose something by making the two identical.

Hugo

p5pRT commented 12 years ago

From @tsee

On 02/10/2012 05​:01 PM\, Zefram wrote​:

Todd Rinaldo via RT wrote​:

So far\, I've found needed test fixes in​: autodie\, Error\, Log4perl and counting.

Not very many\, then. This is looking viable.

If somebody supplies me with two commits to run a CPAN smoke for\, then I can do that within a few days to get a more complete list.

--Steffen

p5pRT commented 12 years ago

From zefram@fysh.org

Steffen Mueller wrote​:

If somebody supplies me with two commits to run a CPAN smoke for\, then I can do that within a few days to get a more complete list.

Obvious choice is 62e90759156bee3f2bcbf1ac6fd055aeab81b9e4 (before) and 879b0cab8575cdc155c45c42eb82075648761936 (after).

-zefram

p5pRT commented 12 years ago

From @demerphq

On 11 February 2012 11​:15\, \hv@​crypt\.org wrote​:

"Todd Rinaldo via RT" \perlbug\-followup@​perl\.org wrote​: :On Thu Feb 02 09​:17​:06 2012\, LeonT wrote​: :> On Thu\, Feb 2\, 2012 at 6​:00 PM\, Zefram \zefram@​fysh\.org wrote​: :> > There presumably will be a few. �It's an easy fix for each. �I think it's :> > an acceptable level of hassle. �If you object\, you've got two weeks to :> > render it contentious so that it has to be reverted. �I'm holding off :> > on CPAN release for this reason. :> :> I'm fine with updating my module\, it's trivial indeed. I'm worried :> though that the list of modules with this issue is larger than :> expected. We'll see soon enough how big the issue is. : :So far\, I've found needed test fixes in​: autodie\, Error\, Log4perl and counting.

One thing worth mentioning​: along the lines of Nick's recent patch to separate two otherwise indistinguishable panics in perl core\, there is actually an advantage to being able to discover from where an error message came.

Maybe the presence or absence of a trailing full-stop is not the ideal way to signal provenance; but I think the general principle stands that you actually do lose something by making the two identical.

I agree. I'm pretty sure at least one port of perl adds error codes to all the error messages we provide. I have always thought that that is a very useful thing. Especially when messages are localized\, or whatever. Id like to see a unique error code added to every error and warning generating line of code in the core\, and if I thought it would be approved I'd do the work to make it happen.

cheers\, Yves

-- perl -Mre=debug -e "/just|another|perl|hacker/"

p5pRT commented 12 years ago

From @tsee

On 02/11/2012 11​:42 AM\, Zefram wrote​:

Steffen Mueller wrote​:

If somebody supplies me with two commits to run a CPAN smoke for\, then I can do that within a few days to get a more complete list.

Obvious choice is 62e90759156bee3f2bcbf1ac6fd055aeab81b9e4 (before) and 879b0cab8575cdc155c45c42eb82075648761936 (after).

Running at

http​://users.perl5.git.perl.org/~tsee/progress.html

Result to be at

http​://users.perl5.git.perl.org/~tsee/carp_errmsg/

Best regards\, Steffen

p5pRT commented 12 years ago

From @rurban

On Sat\, Feb 11\, 2012 at 7​:30 AM\, Steffen Mueller \smueller@​cpan\.org wrote​:

On 02/11/2012 11​:42 AM\, Zefram wrote​:

Steffen Mueller wrote​:

If somebody supplies me with two commits to run a CPAN smoke for\, then I can do that within a few days to get a more complete list.

Obvious choice is 62e90759156bee3f2bcbf1ac6fd055aeab81b9e4 (before) and 879b0cab8575cdc155c45c42eb82075648761936 (after).

Running at

http​://users.perl5.git.perl.org/~tsee/progress.html

Result to be at

http​://users.perl5.git.perl.org/~tsee/carp_errmsg/

Excellent. Muchas gracias! Problem was that maintainers often did not have blead and there is no Carp-1.25 in CPAN yet\, so it was not easy to detect or reproduce.

We provided patches for all but Sub-Uplevel-0.22 I think DAGOLDEN can handle it by himself. -- Reini Urban http​://cpanel.net/   http​://www.perl-compiler.org/

p5pRT commented 12 years ago

From @rjbs

* demerphq \demerphq@​gmail\.com [2012-02-11T06​:28​:21]

I agree. I'm pretty sure at least one port of perl adds error codes to all the error messages we provide. I have always thought that that is a very useful thing. Especially when messages are localized\, or whatever. Id like to see a unique error code added to every error and warning generating line of code in the core\, and if I thought it would be approved I'd do the work to make it happen.

This is something I'd hoped to see quite a while ago\, as part of a "exceptions are objects" update. I'd like to pursue this\, for sure\, but am probably not going to give it a front burner in my brain until 5.17. I will save and review all proposals on it\, though!

We had talked about giving exceptions tags as well\, at the time.

-- rjbs

p5pRT commented 12 years ago

From zefram@fysh.org

Steffen Mueller wrote​:

http​://users.perl5.git.perl.org/~tsee/carp_errmsg/

Not yet complete\, but now showing 28 distros that passed before and fail after and 20 that changed behaviour in some other way. Anyone want to argue that it's too many?

-zefram

p5pRT commented 12 years ago

From @rurban

On Wed\, Feb 15\, 2012 at 5​:42 AM\, Zefram \zefram@​fysh\.org wrote​:

Steffen Mueller wrote​:

http​://users.perl5.git.perl.org/~tsee/carp_errmsg/

Not yet complete\, but now showing 28 distros that passed before and fail after and 20 that changed behaviour in some other way.  Anyone want to argue that it's too many?

This made a lot of people angry\, not only people who had their CPAN modules changed for purley aesthetic consistency reasons.

Carp's only API is the string return value. Additionally carp is with die our primary exception mechanism.

Changing the API would have required an API bump\, like Carp2 or use Carp (-dot). But that is already over\, so I'm content with that change. It was only a night of work to get CPAN fixed and all authors took the fixes. -- Reini Urban http​://cpanel.net/   http​://www.perl-compiler.org/

p5pRT commented 12 years ago

From zefram@fysh.org

Reini Urban wrote​:

This made a lot of people angry\,

Anything you can point at? Blogs?

But that is already over\, so I'm content with that change.

It's not a done deal. The dot change only exists in blead. I've deliberately held off from making a CPAN release until we've made a decision on whether we actually want it. The smoke tests that we get from having it in blead are an important input to that decision.

In five days we'll have a 5.15.8 release\, and whatever version of Carp it has will also go to CPAN. If the dot remains in\, that's when it becomes widely visible. If we want the dot out\, best to take it out before then. That's why I'm asking now. If we leave it in at 5.15.8 and decide later that we need it out for 5.16\, we can do that\, but it's messier.

It was only a night of work to get CPAN fixed and all authors took the fixes.

By "CPAN" do you mean "all of the affected modules on CPAN"? I believe we don't have a final tally of them yet.

-zefram

p5pRT commented 12 years ago

From @rurban

On Wed\, Feb 15\, 2012 at 11​:07 AM\, Zefram \zefram@​fysh\.org wrote​:

Reini Urban wrote​:

This made a lot of people angry\,

Anything you can point at?  Blogs?

Not publicly. Normal people do not blog or complain upstream at the interior ministry. Ask around.

But that is already over\, so I'm content with that change.

It's not a done deal.  The dot change only exists in blead.  I've deliberately held off from making a CPAN release until we've made a decision on whether we actually want it.  The smoke tests that we get from having it in blead are an important input to that decision.

In five days we'll have a 5.15.8 release\, and whatever version of Carp it has will also go to CPAN.  If the dot remains in\, that's when it becomes widely visible.  If we want the dot out\, best to take it out before then. That's why I'm asking now.  If we leave it in at 5.15.8 and decide later that we need it out for 5.16\, we can do that\, but it's messier.

It was only a night of work to get CPAN fixed and all authors took the fixes.

By "CPAN" do you mean "all of the affected modules on CPAN"?  I believe we don't have a final tally of them yet.

"All of CPAN" we use\, that is 652 of 29916 packages. The most common fraction. -- Reini Urban http​://cpanel.net/   http​://www.perl-compiler.org/

p5pRT commented 12 years ago

From @doy

On Wed\, Feb 15\, 2012 at 11​:27​:01AM -0600\, Reini Urban wrote​:

On Wed\, Feb 15\, 2012 at 11​:07 AM\, Zefram \zefram@​fysh\.org wrote​:

Reini Urban wrote​:

This made a lot of people angry\,

Anything you can point at?  Blogs?

Not publicly. Normal people do not blog or complain upstream at the interior ministry. Ask around.

So how are we to know that they exist? We can't really go around asking every Perl user about every change that we make to core\, and we can't just stop making changes to core. What is the solution here?

-doy

p5pRT commented 12 years ago

From @rurban

On Wed\, Feb 15\, 2012 at 11​:36 AM\, Jesse Luehrs \doy@​tozt\.net wrote​:

On Wed\, Feb 15\, 2012 at 11​:27​:01AM -0600\, Reini Urban wrote​:

On Wed\, Feb 15\, 2012 at 11​:07 AM\, Zefram \zefram@​fysh\.org wrote​:

Reini Urban wrote​:

This made a lot of people angry\,

Anything you can point at?  Blogs?

Not publicly. Normal people do not blog or complain upstream at the interior ministry. Ask around.

So how are we to know that they exist? We can't really go around asking every Perl user about every change that we make to core\, and we can't just stop making changes to core. What is the solution here?

Make the right decisions. Don't upset people too much. use common sense. yes\, marc lehman is right most of the time -- Reini Urban http​://cpanel.net/   http​://www.perl-compiler.org/

p5pRT commented 12 years ago

From @doy

On Wed\, Feb 15\, 2012 at 12​:38​:39PM -0600\, Reini Urban wrote​:

On Wed\, Feb 15\, 2012 at 11​:36 AM\, Jesse Luehrs \doy@​tozt\.net wrote​:

On Wed\, Feb 15\, 2012 at 11​:27​:01AM -0600\, Reini Urban wrote​:

On Wed\, Feb 15\, 2012 at 11​:07 AM\, Zefram \zefram@​fysh\.org wrote​:

Reini Urban wrote​:

This made a lot of people angry\,

Anything you can point at?  Blogs?

Not publicly. Normal people do not blog or complain upstream at the interior ministry. Ask around.

So how are we to know that they exist? We can't really go around asking every Perl user about every change that we make to core\, and we can't just stop making changes to core. What is the solution here?

Make the right decisions.

Oh\, is it really that easy?

-doy

p5pRT commented 12 years ago

From @rurban

On Wed\, Feb 15\, 2012 at 12​:40 PM\, Jesse Luehrs \doy@​tozt\.net wrote​:

On Wed\, Feb 15\, 2012 at 12​:38​:39PM -0600\, Reini Urban wrote​:

On Wed\, Feb 15\, 2012 at 11​:36 AM\, Jesse Luehrs \doy@​tozt\.net wrote​:

On Wed\, Feb 15\, 2012 at 11​:27​:01AM -0600\, Reini Urban wrote​:

On Wed\, Feb 15\, 2012 at 11​:07 AM\, Zefram \zefram@​fysh\.org wrote​:

Reini Urban wrote​:

This made a lot of people angry\,

Anything you can point at?  Blogs?

Not publicly. Normal people do not blog or complain upstream at the interior ministry. Ask around.

So how are we to know that they exist? We can't really go around asking every Perl user about every change that we make to core\, and we can't just stop making changes to core. What is the solution here?

Make the right decisions.

Oh\, is it really that easy?

Sorry\, I forgot​: marketing.

Persuade people why they must change their code. Before people like me jump on them on other more-general aspects. -- Reini Urban http​://cpanel.net/   http​://www.perl-compiler.org/

p5pRT commented 12 years ago

From @doy

On Wed\, Feb 15\, 2012 at 12​:42​:02PM -0600\, Reini Urban wrote​:

On Wed\, Feb 15\, 2012 at 12​:40 PM\, Jesse Luehrs \doy@​tozt\.net wrote​:

On Wed\, Feb 15\, 2012 at 12​:38​:39PM -0600\, Reini Urban wrote​:

On Wed\, Feb 15\, 2012 at 11​:36 AM\, Jesse Luehrs \doy@​tozt\.net wrote​:

On Wed\, Feb 15\, 2012 at 11​:27​:01AM -0600\, Reini Urban wrote​:

On Wed\, Feb 15\, 2012 at 11​:07 AM\, Zefram \zefram@​fysh\.org wrote​:

Reini Urban wrote​:

This made a lot of people angry\,

Anything you can point at?  Blogs?

Not publicly. Normal people do not blog or complain upstream at the interior ministry. Ask around.

So how are we to know that they exist? We can't really go around asking every Perl user about every change that we make to core\, and we can't just stop making changes to core. What is the solution here?

Make the right decisions.

Oh\, is it really that easy?

Sorry\, I forgot​: marketing.

Persuade people why they must change their code. Before people like me jump on them on other more-general aspects.

Which people? The only method of communication we have with the majority of Perl users is through deprecation warnings.

-doy

p5pRT commented 12 years ago

From @rjbs

* Zefram \zefram@​fysh\.org [2012-02-15T06​:42​:56]

Steffen Mueller wrote​:

http​://users.perl5.git.perl.org/~tsee/carp_errmsg/

Not yet complete\, but now showing 28 distros that passed before and fail after and 20 that changed behaviour in some other way. Anyone want to argue that it's too many?

It's a bummer that\, if released\, this will be the second change to this behavior. I hope nobody will be too put out having to fix tests a second time.

I think this should be corrected. Hopefully the next changes we have to make on this front will be "replace the stack and exception mechanism with something was awesomer" and not "add a missing dot!"

I had a quick glance over the smoke changes looking for core modules and the only one that leaped out at me was autodie; presumably we can fix that in core if we can't get Paul to issue a new version immediately.

Reini suggested on IRC\, today\, that he had already been in touch with all the affected authors. Has this really been done? That would be great!

-- rjbs

p5pRT commented 12 years ago

From @tsee

On 02/16/2012 02​:03 AM\, Ricardo Signes wrote​:

* Zefram\zefram@​fysh\.org [2012-02-15T06​:42​:56]

Steffen Mueller wrote​:

http​://users.perl5.git.perl.org/~tsee/carp_errmsg/

Not yet complete\, but now showing 28 distros that passed before and fail after and 20 that changed behaviour in some other way. Anyone want to argue that it's too many?

It's a bummer that\, if released\, this will be the second change to this behavior. I hope nobody will be too put out having to fix tests a second time.

I think this should be corrected. Hopefully the next changes we have to make on this front will be "replace the stack and exception mechanism with something was awesomer" and not "add a missing dot!"

I had a quick glance over the smoke changes looking for core modules and the only one that leaped out at me was autodie; presumably we can fix that in core if we can't get Paul to issue a new version immediately.

Reini suggested on IRC\, today\, that he had already been in touch with all the affected authors. Has this really been done? That would be great!

Now up to ~30-35 degradations. Reini qualified his statement as "the authors of all modules that we [presumably cPanel] use" which was on the order of 600-700 out of tens of thousands modules on the CPAN. Since authors of infrequently used modules are as likely to hard-core error messages\, I'd say that this is not yet a done deal.

I expect that at least a handful of the modules would require taking over to make a new release or else they're dead wood.

--Steffen

p5pRT commented 12 years ago

From zefram@fysh.org

Ricardo Signes wrote​:

I had a quick glance over the smoke changes looking for core modules and the only one that leaped out at me was autodie; presumably we can fix that in core

My commit to modify Carp did edit the autodie test in core. But not in the correct way for the CPAN version​: I changed it to *require* the dot\, which is OK in core\, but the CPAN version needs to make the dot optional.

-zefram

p5pRT commented 12 years ago

From @rurban

Am 16.02.2012 um 00​:53 schrieb Steffen Mueller​:

On 02/16/2012 02​:03 AM\, Ricardo Signes wrote​:

* Zefram\zefram@​fysh\.org [2012-02-15T06​:42​:56]

Steffen Mueller wrote​:

http​://users.perl5.git.perl.org/~tsee/carp_errmsg/

Not yet complete\, but now showing 28 distros that passed before and fail after and 20 that changed behaviour in some other way. Anyone want to argue that it's too many?

It's a bummer that\, if released\, this will be the second change to this behavior. I hope nobody will be too put out having to fix tests a second time.

I think this should be corrected. Hopefully the next changes we have to make on this front will be "replace the stack and exception mechanism with something was awesomer" and not "add a missing dot!"

I had a quick glance over the smoke changes looking for core modules and the only one that leaped out at me was autodie; presumably we can fix that in core if we can't get Paul to issue a new version immediately.

Reini suggested on IRC\, today\, that he had already been in touch with all the affected authors. Has this really been done? That would be great!

Me and toddr checked the 600-700 modules we package as rpm and filed bugreports with patches. I guess for Carp it was about 4-5\, for defined(@​%) it was more. Todd has the numbers and ticket id's. Sub​::UpLevel\, log4perl and Test​::Warn being the most important.

All them were accepted resp. fixed in a better way\, but the authors wait until this new Carp 1.25 is actually released. I did the check with a $Carp​::VERSION compare.

Now up to ~30-35 degradations. Reini qualified his statement as "the authors of all modules that we [presumably cPanel] use" which was on the order of 600-700 out of tens of thousands modules on the CPAN. Since authors of infrequently used modules are as likely to hard-core error messages\, I'd say that this is not yet a done deal.

I expect that at least a handful of the modules would require taking over to make a new release or else they're dead wood.

Yes\, thanks for your full test.

p5pRT commented 12 years ago

From zefram@fysh.org

Reini Urban wrote​:

I did the check with a $Carp​::VERSION compare.

That's not a great idea. Better to just accept messages with or without the dot. You don't need to know which version of Carp you're using.

-zefram

p5pRT commented 12 years ago

From @rurban

On Thu\, Feb 16\, 2012 at 9​:28 AM\, Zefram \zefram@​fysh\.org wrote​:

Reini Urban wrote​:

I did the check with a $Carp​::VERSION compare.

That's not a great idea.  Better to just accept messages with or without the dot.  You don't need to know which version of Carp you're using.

Sure. I didn't know that time that it will be changed again. Currently Log4perl and Test​::Warn will have to be fixed again\, no big deal. Test​::Warn is bummer. -- Reini

p5pRT commented 12 years ago

From @tsee

On 02/16/2012 04​:23 PM\, Reini Urban wrote​:

Yes\, thanks for your full test.

That's done now. Do note the high number of distributions that could not be tested on the patched perl​: This is because they depend on one of the distributions that failed tests. So in theory\, once those are fixed\, we'd have to run the smoke again to get a full coverage.

--Steffen

p5pRT commented 12 years ago

From @andk

On Sat\, 18 Feb 2012 14​:31​:56 +0100\, Steffen Mueller \smueller@​cpan\.org said​:

  > On 02/16/2012 04​:23 PM\, Reini Urban wrote​:

Yes\, thanks for your full test.

  > That's done now. Do note the high number of distributions that could   > not be tested on the patched perl​: This is because they depend on one   > of the distributions that failed tests. So in theory\, once those are   > fixed\, we'd have to run the smoke again to get a full coverage.

Maybe now's the time. If you can\, please install CHORNY/Test-Warn-0.23_01.tar.gz and rerun.

-- andreas

p5pRT commented 12 years ago

From @tsee

On 02/26/2012 05​:41 PM\, Andreas J. Koenig wrote​:

On Sat\, 18 Feb 2012 14​:31​:56 +0100\, Steffen Mueller\smueller@​cpan\.org said​:

On 02/16/2012 04​:23 PM\, Reini Urban wrote​:

Yes\, thanks for your full test.

That's done now. Do note the high number of distributions that could not be tested on the patched perl​: This is because they depend on one of the distributions that failed tests. So in theory\, once those are fixed\, we'd have to run the smoke again to get a full coverage.

Maybe now's the time. If you can\, please install CHORNY/Test-Warn-0.23_01.tar.gz and rerun.

Should be running with an updated CPAN mirror (so expect some other differences).

Best regards\, Steffen

p5pRT commented 12 years ago

From alex.hartmaier@gmail.com

I was one of the upset people Reini talked about. If a new Perl version brings such a change noted in its changelog thats fine\, releasing a new version of a dual-lifed module to CPAN without a fat warning in the changelog isn't.

The reason(s) why such an unimportant looking but fail introducting change was made would also be welcome.

-Alex (abraxxa)

On Sun\, Feb 26\, 2012 at 8​:03 PM\, Steffen Mueller \smueller@​cpan\.org wrote​:

On 02/26/2012 05​:41 PM\, Andreas J. Koenig wrote​:

On Sat\, 18 Feb 2012 14​:31​:56 +0100\, Steffen Mueller\smueller@​cpan\.org

said​:

On 02/16/2012 04​:23 PM\, Reini Urban wrote​: Yes\, thanks for your full test.

That's done now. Do note the high number of distributions that could not be tested on the patched perl​: This is because they depend on one of the distributions that failed tests. So in theory\, once those are fixed\, we'd have to run the smoke again to get a full coverage.

Maybe now's the time. If you can\, please install CHORNY/Test-Warn-0.23_01.tar.**gz and rerun.

Should be running with an updated CPAN mirror (so expect some other differences).

Best regards\, Steffen

p5pRT commented 12 years ago

From chris@prather.org

On Tue\, Feb 28\, 2012 at 3​:21 PM\, Alexander Hartmaier \alex\.hartmaier@​gmail\.com wrote​:

I was one of the upset people Reini talked about. If a new Perl version brings such a change noted in its changelog thats fine\, releasing a new version of a dual-lifed module to CPAN without a fat warning in the changelog isn't.

The reason(s) why such an unimportant looking but fail introducting change was made would also be welcome.

Having consistent things be consistent is a nice feature.

The change log for Carp does include mention of this change. At the time of the release no more than 35 modules out of >14K tested were showing any possible change due to this patch\, it seems reasonable that a simple mention was all that was expected to be required.

No-one (including Reini) pointed out more than a fraction of a percentage of affected modules in the two weeks between when the patch was made and the development release of Perl that this dual-lifed module was bundled with was released. (Or if they did it is not recorded in this thread). Reini only brought up that a group of unnamed people such as yourself had issues with this change the day before the release.

The nice part about the development process for Perl5 is that any interested parties\, such as yourself\, can participate to make sure that your concerns are noted and dealt with *before* a release happens.

-Chris

p5pRT commented 12 years ago

From @tsee

On 02/29/2012 02​:19 AM\, Chris Prather wrote​:

On Tue\, Feb 28\, 2012 at 3​:21 PM\, Alexander Hartmaier \alex\.hartmaier@​gmail\.com wrote​:

I was one of the upset people Reini talked about. If a new Perl version brings such a change noted in its changelog thats fine\, releasing a new version of a dual-lifed module to CPAN without a fat warning in the changelog isn't.

The reason(s) why such an unimportant looking but fail introducting change was made would also be welcome.

Having consistent things be consistent is a nice feature.

The change log for Carp does include mention of this change. At the time of the release no more than 35 modules out of>14K tested were showing any possible change due to this patch\, it seems reasonable that a simple mention was all that was expected to be required.

No-one (including Reini) pointed out more than a fraction of a percentage of affected modules in the two weeks between when the patch was made and the development release of Perl that this dual-lifed module was bundled with was released. (Or if they did it is not recorded in this thread). Reini only brought up that a group of unnamed people such as yourself had issues with this change the day before the release.

The nice part about the development process for Perl5 is that any interested parties\, such as yourself\, can participate to make sure that your concerns are noted and dealt with *before* a release happens.

The CPAN smoke did show\, though\, that there are a few distributions that break and ripple to a good fraction of the CPAN. Fixing Test​::Warn has done away with a good chunk of that\, but let's wait for the resmoke at​:

http​://users.perl5.git.perl.org/~tsee/progress.html http​://users.perl5.git.perl.org/~tsee/carp_errmsg/

--Steffen

p5pRT commented 12 years ago

From @andk

On Tue\, 28 Feb 2012 20​:19​:57 -0500\, Chris Prather \chris@​prather\.org said​:

  > The change log for Carp does include mention of this change. At the   > time of the release no more than 35 modules out of >14K tested were   > showing any possible change due to this patch\, it seems reasonable   > that a simple mention was all that was expected to be required.

Nope. I'd say we blew it major way.

Changing such a heavily used module in a rush a few days before freeze is not as excusable as you put it. Such a relatively irrelevant change should have been postponed to the early phases of the next release cycle. It was put into blead trunk. Instead of a smoke-me branch. And Steffen's parallel smoke setup should have been involved *before* a merge into blead trunk was even considered.

  > No-one (including Reini) pointed out more than a fraction of a   > percentage of affected modules in the two weeks between when the patch   > was made and the development release of Perl that this dual-lifed   > module was bundled with was released.

Sub​::Uplevel was effected and reported. Since it is part of the toolchain it broke the smoke process for *everything* for a few days.

  > (Or if they did it is not recorded in this thread). Reini only   > brought up that a group of unnamed people such as yourself had   > issues with this change the day before the release.

  > The nice part about the development process for Perl5 is that any   > interested parties\, such as yourself\, can participate to make sure   > that your concerns are noted and dealt with *before* a release   > happens.

A cheap excuse. We should stand to our mistakes. Development should slow down in February and such changes should not be tried out in the trunk ever. Let's put this into our book of mistakes that should not have happened.

-- andreas

p5pRT commented 12 years ago

From @demerphq

On 29 February 2012 08​:18\, Andreas J. Koenig \andreas\.koenig\.7os6VVqR@​franz\.ak\.mind\.de wrote​:

On Tue\, 28 Feb 2012 20​:19​:57 -0500\, Chris Prather \chris@​prather\.org said​:

 > The change log for Carp does include mention of this change. At the  > time of the release no more than 35 modules out of >14K tested were  > showing any possible change due to this patch\, it seems reasonable  > that a simple mention was all that was expected to be required.

Nope. I'd say we blew it major way.

Changing such a heavily used module in a rush a few days before freeze is not as excusable as you put it. Such a relatively irrelevant change should have been postponed to the early phases of the next release cycle. It was put into blead trunk. Instead of a smoke-me branch. And Steffen's parallel smoke setup should have been involved *before* a merge into blead trunk was even considered.

 > No-one (including Reini) pointed out more than a fraction of a  > percentage of affected modules in the two weeks between when the patch  > was made and the development release of Perl that this dual-lifed  > module was bundled with was released.

Sub​::Uplevel was effected and reported. Since it is part of the toolchain it broke the smoke process for *everything* for a few days.

 > (Or if they did it is not recorded in this thread). Reini only  > brought up that a group of unnamed people such as yourself had  > issues with this change the day before the release.

 > The nice part about the development process for Perl5 is that any  > interested parties\, such as yourself\, can participate to make sure  > that your concerns are noted and dealt with *before* a release  > happens.

A cheap excuse. We should stand to our mistakes. Development should slow down in February and such changes should not be tried out in the trunk ever. Let's put this into our book of mistakes that should not have happened.

I think you have somewhat of a point\, especially as regards timing. However I think the counterpoint is that people should not be testing raw output from core modules in the way that caused problems. We make no commitments as to a particular output format in Carp\, so hard testing a snapshot of its output is unwise. In git terms Carp is not plumbing\, it is porcelain\, so establishing hard dependencies on how it works is an unforced error by the module author\, not by the people changing Carp.

Let me put this another way\, almost for sure these errors come down to someone snapshotting the output of Carp. The authors who did this were too lazy to write their test to be robust to possible changes in Carp\, For instance\, they could have checked to see if a fragment of the carp output that was relevant to them. Instead they established a hard dependency on something that has no API guarantees\, and it cost them. That is not Perls fault. It is much the same as using a private internals routine and then getting upset when it is removed.

On the other hand\, we knew that this was going to cause turbulence and should have been more proactive in addressing it before the fact.

cheers\, Yves

-- perl -Mre=debug -e "/just|another|perl|hacker/"

p5pRT commented 12 years ago

From chris@prather.org

On Wed\, Feb 29\, 2012 at 2​:18 AM\, Andreas J. Koenig \andreas\.koenig\.7os6VVqR@​franz\.ak\.mind\.de wrote​:

On Tue\, 28 Feb 2012 20​:19​:57 -0500\, Chris Prather \chris@​prather\.org said​:

 > The change log for Carp does include mention of this change. At the  > time of the release no more than 35 modules out of >14K tested were  > showing any possible change due to this patch\, it seems reasonable  > that a simple mention was all that was expected to be required.

Nope. I'd say we blew it major way.

I'm not arguing that we didn't. Hindsight is a wonderfully clarifying lens.

Changing such a heavily used module in a rush a few days before freeze is not as excusable as you put it. Such a relatively irrelevant change should have been postponed to the early phases of the next release cycle. It was put into blead trunk. Instead of a smoke-me branch. And Steffen's parallel smoke setup should have been involved *before* a merge into blead trunk was even considered.

Your definition of "in a rush" and "a few days" differs from mine. Even then I'm not sure it is fair\, or accurate\, to blame the outcome on a last-minute-rush-job. You first reported a bug (in what is apparently an important piece of the toolchain) within two days of the patch landing on blead. Two weeks should have been more than enough time to triage this enough to at least back it out. Obviously (to be very American for a moment) mistakes were made.

 > No-one (including Reini) pointed out more than a fraction of a  > percentage of affected modules in the two weeks between when the patch  > was made and the development release of Perl that this dual-lifed  > module was bundled with was released.

Sub​::Uplevel was effected and reported. Since it is part of the toolchain it broke the smoke process for *everything* for a few days.

See that's useful to know.

I know you put a great deal of effort into smoking and reporting errors. I've been the recipient of this service myself\, and I'm grateful for it. I see that you filed a perlbug on on 2012-02-04\, and are cited in the Changes for Sub​::Uplevel on 2012-02-07. However from the context in this thread it seems that Zefram didn't know about this as of the 10th when the only modules mentioned explicitly as broken so far were autodie\, error\, and Log4perl; or if he did know he didn't recognize the significance.

 > (Or if they did it is not recorded in this thread). Reini only  > brought up that a group of unnamed people such as yourself had  > issues with this change the day before the release.

 > The nice part about the development process for Perl5 is that any  > interested parties\, such as yourself\, can participate to make sure  > that your concerns are noted and dealt with *before* a release  > happens.

A cheap excuse. We should stand to our mistakes. Development should slow down in February and such changes should not be tried out in the trunk ever. Let's put this into our book of mistakes that should not have happened.

It wasn't intended to be an excuse\, cheap or otherwise. One of the best parts about OpenSource is that people are free to participate at any stage in the process. I did let my annoyance show through in the phrasing though\, and I apologize to abraxxa for my tone\, it wasn't helpful or necessary.

I agree with you there were mistakes. I'm not sure they were obviously avoidable. What can we do to make sure they don't happen again?

You have suggested that any contentious change be put it into a smoke-me branch. I agree. The problem was that this change wasn't seen as being contentious. You're free to argue if it should have been or not\, but I am certain that sometime we will face a situation again where a seemingly trivial change causes unforeseen trauma. The fact that the change is seemingly trivial means you won't know ahead of time that it should be in a smoke-me branch.

Should then *all* changes go through a smoke-me change? I treat all of my patches to important multi-person projects this way and do all of my work in topic branch and then get someone else to review it. The downside to this is that things don't always get merged before they've bit-rotted. Also smoking a branch for downstream changes is a multi-day process (as I'm sure you're aware). I'm not sure this is the optimal solution. Maybe this should be part of the 'Contentious changes freeze' process? It is certainly part of the 'User-visible changes freeze'\, so maybe the dates for the various freezes need to be adjusted? We can also clarify better what commits should be automatically put into a smoke-me branch regardless of how trivial they may seem regardless of the current slushiness of blead. If someone can point me in the direction of the proper documentation to patch I'm willing to make sure it is properly recorded.

Obviously the issue for this change *was* known within the two week window Zefram had for rolling back the change. You filed a perlbug for it two day later. Why wasn't this perlbug noticed sometime before the 2012-02-16 when Zefram made the dual-life release? I'm going to guess that the parties interested in failures for the Carp change (obviously) didn't see the perlbug because they were busy. I'm also going to guess that you didn't see this thread asking for failures for the same reason. This is a volunteer process so people have to be allowed to be busy. Is there some way we can make these things more noticeable?

I didn't realize that Sub​::Uplevel was such a core part of the toolchain. It's not part of core\, so I can understand why Zefram didn't test it when he made his initial patch. Obviously you noticed the Sub​::Uplevel change within 2 days of the patch landing in blead. Was this serendipity or were you working in this specific area? I would love to have a list of non-core toolchain modules to Task​::Kensho​::Toolchain. This could then provide perhaps a quicker turn-around smoke for the important bits of the toolchain first.

Is there anything else that can be done to help prevent this class of mistakes from cropping up?

-Chris

p5pRT commented 12 years ago

From @nwc10

[ignoring all other details\, one point below]

On Wed\, Feb 29\, 2012 at 05​:32​:57AM -0500\, Chris Prather wrote​:

On Wed\, Feb 29\, 2012 at 2​:18 AM\, Andreas J. Koenig \andreas\.koenig\.7os6VVqR@​franz\.ak\.mind\.de wrote​:

On Tue\, 28 Feb 2012 20​:19​:57 -0500\, Chris Prather \chris@​prather\.org said​:

 > The change log for Carp does include mention of this change. At the  > time of the release no more than 35 modules out of >14K tested were  > showing any possible change due to this patch\, it seems reasonable  > that a simple mention was all that was expected to be required.

Nope. I'd say we blew it major way.

I'm not arguing that we didn't. Hindsight is a wonderfully clarifying lens.

Changing such a heavily used module in a rush a few days before freeze is not as excusable as you put it. Such a relatively irrelevant change should have been postponed to the early phases of the next release cycle. It was put into blead trunk. Instead of a smoke-me branch. And Steffen's parallel smoke setup should have been involved *before* a merge into blead trunk was even considered.

You have suggested that any contentious change be put it into a smoke-me branch. I agree. The problem was that this change wasn't seen as being contentious. You're free to argue if it should have been or not\, but I am certain that sometime we will face a situation again where a seemingly trivial change causes unforeseen trauma. The fact that the change is seemingly trivial means you won't know ahead of time that it should be in a smoke-me branch.

Putting it in a smoke-me branch would not have helped. That tests building the core with different configurations on different platforms.

All tests passed for all core modules. That's all it can tell.

The thing that *would* have got it is a "build all of CPAN"\, which we don't have the resources to do for every change. So it gets down to - how do we spot which changes need that sort of thing?

Should then *all* changes go through a smoke-me change? I treat all of

I'd actually be happy if (nearly) all non-doc changes *did*. But it wouldn't have caught this one.

(We'd need to improve the smoke-me reporting infrastructure to make this useful though)

I didn't realize that Sub​::Uplevel was such a core part of the toolchain. It's not part of core\, so I can understand why Zefram didn't test it when he made his initial patch. Obviously you noticed the Sub​::Uplevel change within 2 days of the patch landing in blead. Was this serendipity or were you working in this specific area? I would love to have a list of non-core toolchain modules to Task​::Kensho​::Toolchain. This could then provide perhaps a quicker turn-around smoke for the important bits of the toolchain first.

Right. What *is* the toolchain? I was under the impression that the core shipped pretty much all of the toolchain. (Although\, thanks to configure_requires support now being widespread\, you shouldn't actually notice if it doesn't.)

Is there anything else that can be done to help prevent this class of mistakes from cropping up?

I think the key thing that was missing here was realising that this change broke the CPAN smokers *reporting* setup. Which would have cut off reports\, and hence hidden the problems.

Regular automated smoke testing to be sure that the reporting setup isn't broken seems like the first thing to fix. Unlike "all of CPAN"\, doing that on a fast cycle seems tractable.

Nicholas Clark

p5pRT commented 12 years ago

From chris@prather.org

On Wed\, Feb 29\, 2012 at 5​:57 AM\, Nicholas Clark \nick@​ccl4\.org wrote​:

[ignoring all other details\, one point below]

[ditto]

Putting it in a smoke-me branch would not have helped. That tests building the core with different configurations on different platforms.

All tests passed for all core modules. That's all it can tell.

The thing that *would* have got it is a "build all of CPAN"\, which we don't have the resources to do for every change. So it gets down to - how do we spot which changes need that sort of thing?

If it's not clear from context\, I meant "smoke-me" as a substitute for 'Enhanced Smoking Techniques' we can apply to a branch. It's my fault for mis-using a term in an imprecise fashion.

Should then *all* changes go through a smoke-me change? I treat all of

I'd actually be happy if (nearly) all non-doc changes *did*. But it wouldn't have caught this one.

(We'd need to improve the smoke-me reporting infrastructure to make this useful though)

You yourself have pointed out in other threads that even *doc* changes can affect things like diagnostics.pm. Going down this road leads to a place where we'll get lost in the intractable problems which is why I wanted to cut it off early. I would love to have an infrastructure where we could smoke every commit against all of CPAN. The things we could do with such a tool would be amazing! Sadly we're not there yet.

I think the key thing that was missing here was realising that this change broke the CPAN smokers *reporting* setup. Which would have cut off reports\, and hence hidden the problems.

Having a failure mode that hides the complexity of the problem is a double whammy. Having a process to define where we need more canaries would be useful too.

Regular automated smoke testing to be sure that the reporting setup isn't broken seems like the first thing to fix. Unlike "all of CPAN"\, doing that on a fast cycle seems tractable.

I'm certainly willing to help figure out how to make it happen.

-Chris

p5pRT commented 12 years ago

From @nwc10

On Wed\, Feb 29\, 2012 at 06​:11​:49AM -0500\, Chris Prather wrote​:

On Wed\, Feb 29\, 2012 at 5​:57 AM\, Nicholas Clark \nick@​ccl4\.org wrote​:

[ignoring all other details\, one point below]

[ditto]

Putting it in a smoke-me branch would not have helped. That tests building the core with different configurations on different platforms.

All tests passed for all core modules. That's all it can tell.

The thing that *would* have got it is a "build all of CPAN"\, which we don't have the resources to do for every change. So it gets down to - how do we spot which changes need that sort of thing?

If it's not clear from context\, I meant "smoke-me" as a substitute for 'Enhanced Smoking Techniques' we can apply to a branch. It's my fault for mis-using a term in an imprecise fashion.

It wasn't clear. I think that's obvious in hindsight :-)

smoke-me specifically means platform testing from branches of   git​://perl5.git.perl.org/perl.git BBC specifically means "Bleadperl Breaks CPAN" - Andreas' testing setup for   trying to build CPAN against a current(ish) build\, and then (*very*   usefully) trying to bisect down to the particular commit that broke things   (which I think is only on x86_64 Linux\, but I might be wrong)

keep those terms for those things.

everything else doesn't exist yet.

Should then *all* changes go through a smoke-me change? I treat all of

I'd actually be happy if (nearly) all non-doc changes *did*. But it wouldn't have caught this one.

(We'd need to improve the smoke-me reporting infrastructure to make this useful though)

You yourself have pointed out in other threads that even *doc* changes can affect things like diagnostics.pm. Going down this road leads to a

Sure\, but they are not platform or configuration dependent. If people run the tests before they commit\, such problems are spotted early. (Otherwise Jenkins will waggle a finger at them in public. The shame...)

place where we'll get lost in the intractable problems which is why I wanted to cut it off early. I would love to have an infrastructure where we could smoke every commit against all of CPAN. The things we could do with such a tool would be amazing! Sadly we're not there yet.

Which I think means that it's not as bad as that.

I think the key thing that was missing here was realising that this change broke the CPAN smokers *reporting* setup. Which would have cut off reports\, and hence hidden the problems.

Having a failure mode that hides the complexity of the problem is a double whammy. Having a process to define where we need more canaries would be useful too.

Regular automated smoke testing to be sure that the reporting setup isn't broken seems like the first thing to fix. Unlike "all of CPAN"\, doing that on a fast cycle seems tractable.

I'm certainly willing to help figure out how to make it happen.

That would be cool. I don't really think that I have the time to do more than this brain dump​:

It (first) struck me as something that could be automated with Jenkins. But I'm not *sure*. Does

a) Jenkins maintain state? b) does it have a concept of a skip?

In that\, we need to avoid spamming with false positives. What *we* here are interested in is whether a change to blead broke the reporter modules. We don't want to confuse that with the modules themselves breaking. And it's not *that* easy to control the local CPAN mirror updates. So\, probably one wants to do this​:

0) snapshot CPAN. (eg\, private local copy of CPAN\, updated by this build   script) 1) build blead with *parent of current commit* (5 minutes on fast hardware   in parallel) 2) build reporter toolchain   a) if it fails at this point\, bail out as a "skip". (Warn about this?)   b) otherwise continue 3) clean everything 4) build blead with current commit 5) build reporter toolchain (identical CPAN)

and that's then "pass" or "fail"\, and conclusively did *this* blead commit break things

(for the tested platform\, which likely is the easy 90% to get right for starters)

I also wonder whether anyone at the QA hackathon currently projectless would find this one interesting.

Nicholas Clark

p5pRT commented 12 years ago

From zefram@fysh.org

Chris Prather wrote​:

You have suggested that any contentious change be put it into a smoke-me branch. I agree. The problem was that this change wasn't seen as being contentious.

Not quite the right criterion. We *did* know that this change would break some proportion of CPAN\, and that a full-CPAN smoke test would be useful. This is not the case for most core changes\, contentious or otherwise. We could (and I won't dispute that we should) have run the full-CPAN smoke test with the change parked on a branch\, rather than putting it into blead first.

I think I was emboldened to do that by the recent "\, \<$fh> line 123" addition in the same place\, which raised exactly the same potential range of breakage (while actually hitting a considerably smaller proportion of CPAN). I was definitely made comfortable about applying straight to blead by the clear opportunity to revert it from blead before 5.15.8​: I ensured we'd have time to make a decision on that. I didn't pay much attention to which modules turned out to be affected\, just to how many (because that's how much effort it takes to fix)\, and ultimately I delegated the decision to our glorious pumpking.

What would have made a difference here\, and in similar situations\, is an easy way to invoke the full-CPAN smoke test. We have smoke-me branches for testing a change across architectures and build options; I'd like to see a cpan-smoke-me mechanism for testing a change across CPAN distros. This would have given us the smoke results without so much (perceived) pressure for the change to go into a particular imminent release. It sounds like having some time to preemptively fix the affected modules would have resulted in fewer ruffled feathers.

Overall\, I don't think we totally bungled this\, as Andreas surmised. We anticipated pain\, and we took steps to measure and manage that pain\, albeit imperfectly. It's a could-do-better.

-zefram

p5pRT commented 12 years ago

From @rjbs

* "Andreas J. Koenig" [2012-02-29T02​:18​:50]

On Tue\, 28 Feb 2012 20​:19​:57 -0500\, Chris Prather \chris@&#8203;prather\.org said​:

[ ... ] Nope. I'd say we blew it major way.

Changing such a heavily used module in a rush a few days before freeze is not as excusable as you put it. Such a relatively irrelevant change should have been postponed to the early phases of the next release cycle. It was put into blead trunk. Instead of a smoke-me branch. And Steffen's parallel smoke setup should have been involved *before* a merge into blead trunk was even considered.

Steffen's smoke setup *was* involved\, and the output was compared\, and we found some 25-30 breakages. I said we should merge the change on the assumption that getting it into blead would be more likely to get more problems to the surface\, and that should these problems seriously change the scope of the expected breakage\, we could revert the change.

There were some problems with this assumption\, some of which have been pointed out\, and some of which have not.

* "Andreas J. Koenig" [2012-02-29T02​:18​:50]

Sub​::Uplevel was effected and reported. Since it is part of the toolchain it broke the smoke process for *everything* for a few days.

For example\, I didn't realize that the smoke reporting toolchain was affected like this. I'd seen Sub-Uplevel on the list\, and considered the number of things hiding behind it\, but not how they'd affect the reporters themselves. This was a tactical blunder.

* "Andreas J. Koenig" [2012-02-29T02​:18​:50]

A cheap excuse. We should stand to our mistakes. Development should slow down in February and such changes should not be tried out in the trunk ever. Let's put this into our book of mistakes that should not have happened.

With hindsight\, I would have done things differently. I don't agree that "changes should not be tried in the trunk\, ever" though. As it stands now\, smoke-me branches tend to get only smoking of the core tests. We don't have smoke-me CPAN smokers\, other than Steffen\, who must be asked to do it\, and must set it up by hand. That means that getting an experimental change into blead\, if the change doesn't break core\, is a totally viable way to *then* get CPAN smoke reports.

...as long as it doesn't break smoke reports.

As for it being February​: yes\, it was pretty late in the game. As Zefram suggested\, I was emboldened by the *other* recent changes to Carp\, which had already been release as stable to CPAN without causing massive chaos.

What I overlooked as that they changed the error message in a *much* smaller set of cases. That is​: I was thinking that the same set of people\, give or take a small fraction\, would be affected by this change. I think that's the most significant mistake I made. Had that been true\, I think I'd still be pretty sanguine with how things went.

* demerphq [2012-02-29T03​:17​:05]

I think you have somewhat of a point\, especially as regards timing. However I think the counterpoint is that people should not be testing raw output from core modules in the way that caused problems. We make no commitments as to a particular output format in Carp\, so hard testing a snapshot of its output is unwise.

I'm not really happy with this counter-argument. The problem is that yes\, people shouldn't be relying on the exact string output. They should be relying on an unchanging simple error identification API... which we don't provide.

I thought about this\, too\, and hoped that by the next time we wanted to talk about changing this stuff\, it would be to create the core exception types\, and that we'd be doing the one last bodge-without-massive-improvement change now\, preparing things for the future when these errors would be the same type of thing\, and would necessarily stringify the same way.

This was only a very minor consideration\, though.

* demerphq [2012-02-29T03​:17​:05]

For instance\, they could have checked to see if a fragment of the carp output that was relevant to them.

In a nutshell​: many did. For example\, they only looked at the first line\, which changed. If they tested for m{^You died at Foo.pm\, line 2$} and we complain that this was too specific\, what would we have them change? It's reasonable to check that it came from the same place. So they should've amended that to m{...line 2(?​:[^0-9]|$)} or something? Bleh. I think that the affected test cases I saw were not unreasonable\, in most cases.

* Chris Prather [2012-02-29T05​:32​:57]

Should then *all* changes go through a smoke-me change?

Certainly\, many more of them should than now do. More of them should have a CPAN regression test. There are quite a few problems with making this a hard requirement\, though\, which boil down to shortages in resources\, both human and computer.

* Zefram [2012-02-29T06​:40​:12]

Not quite the right criterion. We *did* know that this change would break some proportion of CPAN\, and that a full-CPAN smoke test would be useful. This is not the case for most core changes\, contentious or otherwise. We could (and I won't dispute that we should) have run the full-CPAN smoke test with the change parked on a branch\, rather than putting it into blead first.

Well\, we *did* run it through a CPAN smoke. The problem was not looking at the list of affected dists\, noting which were dependencies for other libraries (especially significant trees)\, fixing those\, and then re-smoking.

As I said much earlier\, I thought we'd be able to revert the change if we determined it was a bigger problem than we'd expected. Now I'm concerned that this is not true\, because somehow this got propagated as a way to fix the problem​:

  $pattern .= $Carp​::VERSION gt "1.24" ? "." :"";

...which means that a reversion in 1.30 would require all the libraries that were fixed in this way to be fixed *again*!

* Zefram [2012-02-29T06​:40​:12]

What would have made a difference here\, and in similar situations\, is an easy way to invoke the full-CPAN smoke test. We have smoke-me branches for testing a change across architectures and build options; I'd like to see a cpan-smoke-me mechanism for testing a change across CPAN distros.

I agree wholeheartedly. It's a lot of work to make this happen\, but it would be phenomenally useful\, even if it was limited to one architecture and build. Especially if the mechanism was easy to re-use as more computing resources became available.

* Zefram [2012-02-29T06​:40​:12]

Overall\, I don't think we totally bungled this\, as Andreas surmised. We anticipated pain\, and we took steps to measure and manage that pain\, albeit imperfectly. It's a could-do-better.

This is my feeling\, too. I made a judgment call\, and in hindsight\, I would have made it differently. Frustratingly\, I believe I had all the information I needed to make the right one\, and simply didn't put it all together at the time.

But I don't plan to fall on my sword over this. I just plan to keep it in mind at similar junctures in the future.

-- rjbs

p5pRT commented 12 years ago

From @demerphq

On 29 February 2012 15​:26\, Ricardo Signes \perl\.p5p@&#8203;rjbs\.manxome\.org wrote​:

* demerphq [2012-02-29T03​:17​:05]

I think you have somewhat of a point\, especially as regards timing. However I think the counterpoint is that people should not be testing raw output from core modules in the way that caused problems. We make no commitments as to a particular output format in Carp\, so hard testing a snapshot of its output is unwise.

I'm not really happy with this counter-argument.  The problem is that yes\, people shouldn't be relying on the exact string output.  They should be relying on an unchanging simple error identification API... which we don't provide.

That focuses too much on the details of the instant case. You need to zoom out and see how this is just an instance of a class of issues that we have in Perl dev\, and that we simply cannot provide the kind of guarantee that people want for this kind of stuff.

On the other hand\, I agree with your point. When I dealt with the regex modifiers I actually created routines so people could access their patterns in such way that they could test that they had created the right ones without being sensitive to things like what modifiers we include in the pattern. So I am sympathetic with your proposed solution to the instant problem. But your reservations don't address the bigger issue. We simply cannot be hand-cuffed in what we change in Perl by people establishing hard dependencies on things we have not made promises about. If we do then we basically will never be able to release a change\, fix bugs\, correct typos\, whatnot.

I thought about this\, too\, and hoped that by the next time we wanted to talk about changing this stuff\, it would be to create the core exception types\, and that we'd be doing the one last bodge-without-massive-improvement change now\, preparing things for the future when these errors would be the same type of thing\, and would necessarily stringify the same way.

This was only a very minor consideration\, though.

I like the general idea. But again\, it does not address my core point of commenting on this thread.

* demerphq [2012-02-29T03​:17​:05]

For instance\, they could have checked to see if a fragment of the carp output that was relevant to them.

In a nutshell​: many did.  For example\, they only looked at the first line\, which changed.  If they tested for m{^You died at Foo.pm\, line 2$} and we complain that this was too specific\, what would we have them change?  It's reasonable to check that it came from the same place.  So they should've amended that to m{...line 2(?​:[^0-9]|$)} or something?  Bleh.  I think that the affected test cases I saw were not unreasonable\, in most cases.

How about

  ok($@​=~/You died/ && $@​=~/Foo\.pm/ && $@​=~/line 2\b/);

Anyway\, the core point here is that if we have not made a promise about some feature then no-one can complain when change it. Its that simple. If a programmer decides to use unpublished API's in the Perl core and we remove them then its that programmers problem\, not ours.

I mean consider the massive list of things that we might have to worry about. Do we have to worry about backwards compatibility if we change the format of Devel​::Peek output? What about the re=debug output? What about correcting typos in warning messages? The list is virtually endless.

Anyway\, all this aside\, my view is that our default choice of action should always be to do things in such a way that it causes the least friction in the community but that we reserve the right to change things that we have not committed to keeping unchanged.

Yves

-- perl -Mre=debug -e "/just|another|perl|hacker/"

p5pRT commented 12 years ago

From @rjbs

* demerphq \demerphq@&#8203;gmail\.com [2012-02-29T09​:52​:01]

So I am sympathetic with your proposed solution to the instant problem. But your reservations don't address the bigger issue. We simply cannot be hand-cuffed in what we change in Perl by people establishing hard dependencies on things we have not made promises about. If we do then we basically will never be able to release a change\, fix bugs\, correct typos\, whatnot.

I totally agree with you on this general point.

-- rjbs

p5pRT commented 12 years ago

From @greerga

On Wed\, 29 Feb 2012\, Nicholas Clark wrote​:

In that\, we need to avoid spamming with false positives. What *we* here are interested in is whether a change to blead broke the reporter modules. We don't want to confuse that with the modules themselves breaking. And it's not *that* easy to control the local CPAN mirror updates. So\, probably one wants to do this​:

0) snapshot CPAN. (eg\, private local copy of CPAN\, updated by this build script) 1) build blead with *parent of current commit* (5 minutes on fast hardware in parallel) 2) build reporter toolchain a) if it fails at this point\, bail out as a "skip". (Warn about this?) b) otherwise continue 3) clean everything 4) build blead with current commit 5) build reporter toolchain (identical CPAN)

and that's then "pass" or "fail"\, and conclusively did *this* blead commit break things

I had a script that did CPAN testers smoking against blead/maint-X that I stopped running a while ago because of the switch to metabase. Essentially it​:

1) updated a minicpan 2) pulled and built blead or maint-X 3) did a CPAN testers module install from CPAN #1 with perl #2 4) ran CPAN testers against all the modules from #1 with perl from #2 5) go to #1

I had wanted to make it smarter so it kept state of the modules but dealing with all the modules that hung during tests was a pain and took up a lot of time\, even with Andreas' blacklist.

When I get home I'll see if I still have that around\, at least. It might've been deleted during one of the Test​::Smoke "delete everything" events.

-- George Greer

p5pRT commented 12 years ago

From @greerga

On Wed\, 29 Feb 2012\, Nicholas Clark wrote​:

The thing that *would* have got it is a "build all of CPAN"\, which we don't have the resources to do for every change. So it gets down to - how do we spot which changes need that sort of thing?

If I can get a copy of the "build all of CPAN" script that the comparisons from a while ago were done with\, I can try to set it up on a rolling basis. That is\, it would compare blead from the last pass with whatever was blead at the start of the next pass. So it wouldn't be every change but it would at least be more than never.

Should then *all* changes go through a smoke-me change? I treat all of

I'd actually be happy if (nearly) all non-doc changes *did*. But it wouldn't have caught this one.

(We'd need to improve the smoke-me reporting infrastructure to make this useful though)

What's on your wishlist?

-- George Greer