1070117 - Chart throws deprecation warnings with Perl 5.16 - shows up in testserver.pl output

Status: RESOLVED FIXED

(Bugzilla :: Installation & Upgrading, defect)

Description

[Gervase Markham [:gerv]]

gerv@hare:/usr/src/bugzilla$ ./testserver.pl http://bugzilla.localhost/
TEST-OK Webserver is running under group id in $webservergroup.
...
TEST-OK GD library generated a good PNG image.
defined(@array) is deprecated at /usr/share/perl5/Chart/Base.pm line 181.
	(Maybe you should just omit the defined()?)
defined(@array) is deprecated at /usr/share/perl5/Chart/Base.pm line 233.
	(Maybe you should just omit the defined()?)
TEST-OK Chart library generated a good PNG image.
...


This is https://rt.cpan.org/Public/Bug/Display.html?id=79658 , a one-line code change unfixed for two years. I guess the Chart module is unmaintained.

These warnings confuse the output. I've tried sticking "no warnings 'all'" everywhere I can think of but I can't suppress them. So we should either:

* find some way to suppress them; or
* remove this test from testserver.pl

Gerv

Comment 1

[Frédéric Buclin]

Removing a test just because it generates a warning doesn't make sense. It's a warning, not an error. And the reason I reported the bug to rt.cpan.org instead of here 2 years ago is because it's a bug in Chart::Base, not in the Bugzilla code. Personally, I would close this bug as INVALID and wait for the Chart module to be updated. And meanwhile, if someone complains about these warnings, he is free to manually apply the fix I suggested on RT.

Comment 2

[Gervase Markham [:gerv]]

(In reply to Frédéric Buclin from comment #1)
> Removing a test just because it generates a warning doesn't make sense. It's
> a warning, not an error. 

I want to write in the docs: "Run testserver.pl; if everything is TEST-OK, you are fine." Warnings confuse the issue - people will worry that they've done something wrong. It's obvious to _you_ that it's not a problem, but that doesn't mean it's obvious to everyone.

Let's look at this another way: is the thing this test is testing _ever_ a problem any more? It used to be that we got a lot of support questions about the Charting modules. But I've not seen one in years. Whatever the library incompatibilities and version mismatches were, they seem to have resolved themselves.

> And the reason I reported the bug to rt.cpan.org
> instead of here 2 years ago is because it's a bug in Chart::Base, not in the
> Bugzilla code. Personally, I would close this bug as INVALID and wait for
> the Chart module to be updated.

...which will be never.

> And meanwhile, if someone complains about
> these warnings, he is free to manually apply the fix I suggested on RT.

I can't manually apply the fix to the copy of Chart::Base used by lots of people installing Bugzilla for the first time!

Gerv

Comment 3

[Frédéric Buclin]

(In reply to Gervase Markham [:gerv] from comment #2)
> > Bugzilla code. Personally, I would close this bug as INVALID and wait for
> > the Chart module to be updated.
> 
> ...which will be never.

If all bugs older than 2 years mean "will never be fixed", then Bugzilla has a lot of bugs which will never been fixed either.

Comment 4

[Gervase Markham [:gerv]]

Are there many one-liner, patch-provided bugs in Bugzilla which are over two years old and not fixed?

Also, _some_ bugs in Bugzilla are getting fixed. By contrast, the last time a bug filed in RT was fixed in Chart was 11 years ago:
https://rt.cpan.org/Public/Dist/Display.html?Status=Resolved;Name=Chart

Gerv

Comment 5

[Frédéric Buclin]

Then maybe it's time to drop GD and Chart if favor of something more shiny and alive and able to support UTF-8 natively (remember bug 287682?). For instance, I would love to be able to hover a graph and get some additional data thanks to JS.

Comment 6

[Gervase Markham [:gerv]]

That would indeed be nice. But in the mean time, I'd like to make the output of testserver.pl not have confusing warnings in it, please.

If there's any way for the caller of a module to suppress warnings coming from that module, that would be great. But otherwise, I suggest we remove the test.

Gerv

Comment 7

[Dylan Hardison [:dylan] (he/him)]

I have directly contacted the upstream author for Chart, asking if they would like help doing a new release. If they are gone from the internet, the next step would be emailing module-authors to see if anyone else ha CO-MAINT on that namespace.

Comment 8

[Gervase Markham [:gerv]]

:dylan: any news?

Gerv

Comment 9

[Dylan Hardison [:dylan] (he/him)]

I have yet to actually post to module-authors. I never got a response from directly emailing the maintainer however.

Comment 10

[Gervase Markham [:gerv]]

dylan: can you take the next step in this process ("posting to module-authors"?), please?

Gerv

Comment 11

[Dylan Hardison [:dylan] (he/him)]

It would appear that https://rt.cpan.org/Public/Bug/Display.html?id=79658 has been resolved,
but I don't see that version as released yet. Perhaps they forgot to upload to PAUSE. :)

Comment 12

[Gervase Markham [:gerv]]

https://metacpan.org/pod/release/CHARTGRP/Chart-2.4.8/Chart.pod ?
http://search.cpan.org/~chartgrp/Chart-2.4.8/ ?

So we should set Chart 2.4.8 as the minimum version on trunk?

Gerv

Comment 13

[Frédéric Buclin]

(In reply to Gervase Markham [:gerv] from comment #12)
> So we should set Chart 2.4.8 as the minimum version on trunk?

yes!

Comment 14

[Gervase Markham [:gerv]]

Hmm. It will take a long time for Chart 2.4.8 to be packaged for all the distributions. In the mean time, we will be requiring most or all admins to install a special local copy of Chart, just to avoid one warning.

Are we sure this is worth it?

Gerv

Comment 15

[Frédéric Buclin]

(In reply to Gervase Markham [:gerv] from comment #14)
> Hmm. It will take a long time for Chart 2.4.8 to be packaged for all the
> distributions. In the mean time, we will be requiring most or all admins to
> install a special local copy of Chart, just to avoid one warning.
> 
> Are we sure this is worth it?

Yes. As said upstream, Perl 5.21 makes your application crash instead of throwing a warning. This is not acceptable. And we are talking about Bugzilla 5.2/6.0, not 5.0. This gives a plenty of time to see Chart 2.4.8 being packaged before 5.2/6.0 is released.

Comment 16

[Gervase Markham [:gerv]]

Hmm:

gerv@hare:/usr/src/bugzilla$ /usr/bin/perl install-module.pl Chart::Lines
...
Installing Chart::Lines version 2.004006...
Chart::Lines is up to date (2.4.6).
gerv@hare:/usr/src/bugzilla$

dylan was right: it seems not to be fully released in some way.

Gerv

Comment 17

[Gervase Markham [:gerv]]

:dylan: I don't understand the complexities of the CPAN release process... are you able to shed light on what needs to happen for install-module.pl to find this new version, and how soon those steps are likely to be executed by someone?

Gerv

Comment 18

[Dylan Hardison [:dylan] (he/him)]

There is an on-going discussion on the upstream RT ticket. https://rt.cpan.org/Ticket/Display.html?id=79658#txn-1459567

Comment 19

[Dylan Hardison [:dylan] (he/him)]

tl;dr of the last update on the associated rt.cpan.org ticket: Another module author got ownership of the name "Chart", some people are working to get that corrected. Then the module author will be able to re-upload the fixed version of Chart.

Comment 20

[Frédéric Buclin]

Chart 2.4.10 is now available on CPAN:

Installing Chart::Lines version 2.004010...
  CHARTGRP/Chart-2.4.10.tar.gz
  /usr/bin/make install  -- OK

Comment 21

[Gervase Markham [:gerv]]

Attachment: Patch v.1

Update required Chart version to new 2.4.10.

Gerv

Comment 22

[Frédéric Buclin]

Comment on attachment 8589696 [details] [diff] [review]
Patch v.1

r=LpSolit

Comment 23

[Gervase Markham [:gerv]]

To ssh://[e-mail]/bugzilla/bugzilla.git
   3d40dae..8a1de00  master -> master

Gerv