Closed Bug 706753 Opened 13 years ago Closed 12 years ago

Bugzilla will not work with newest version of JSON::RPC 1.01 due to non-backward compatibility

Categories

(Bugzilla :: Creating/Changing Bugs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 3.6

People

(Reporter: erkiha, Assigned: LpSolit)

References

Details

Attachments

(2 files, 2 obsolete files)

User Agent: Mozilla/5.0 (Ubuntu; X11; Linux x86_64; rv:9.0) Gecko/20100101 Firefox/9.0
Build ID: 20111124182712

Steps to reproduce:

Enter new bug and start entering summary. Automatic duplicate detection runs but gives message "Data error.".

I think that it happened due recent perl update with cpan. Maybe JSON::RPC or JSON::XS modules?

I'm running debian sid, which recently updated its perl to 5.14. This did not break dup detection. But cpan perl update after that did. I remember one JSON module stated before upgrade of some possible incompatible changes. 


Actual results:

Instead of duplicate list i get Data error. message


Expected results:

Should have gotten duplicate bug list
are there any error messages in your web server's error log?
(In reply to Byron Jones ‹:glob› from comment #1)
> are there any error messages in your web server's error log?

Apache error log shows this:

[Thu Dec 01 16:42:10 2011] [error] [client 10.10.20.167] [Thu Dec  1 16:42:10 2011] jsonrpc.cgi: Undefined subroutine &JSON::RPC::Procedure::check called at /usr/local/share/perl/5.14.2/JSON/RPC/Server.pm line 316., referer: https://bugzilla.erki.net/bugzilla/enter_bug.cgi?classification=Test
JSON::RPC::Procedure info on that machine:

CPAN Terminal> l JSON::RPC::Procedure

Details for 'JSON::RPC::Procedure'
Author                   Makamaka Hannyaharamitu (makamaka@cpan.org)  
Description              None given                                   
Development Stage        Unknown                                      
Installed File           /usr/share/perl5/JSON/RPC/Procedure.pm       
Interface Style          Unknown                                      
Language Used            Unknown                                      
Package                  JSON-RPC-0.96.tar.gz                         
Public License           Unknown                                      
Support Level            Unknown                                      
Version Installed        0.90                                         
Version on CPAN          0.90                                         
Contains:                JSON::RPC::Client                             0.93      
                         JSON::RPC::Procedure                          0.90      
                         JSON::RPC::ReturnObject                       0.93      
                         JSON::RPC::Server                             0.92      
                         JSON::RPC::Server::Apache2                    0.05      
                         JSON::RPC::Server::CGI                        0.92      
                         JSON::RPC::Server::Daemon                     0.03      
                         JSON::RPC::Server::system                     0.92      
                         JSON::RPC::ServiceObject                      0.93      
                         JSONRPC                                       1.01
Got it working again by removing cpan upgraded JSON::RPS and JSON::XS.

repo has json version 2.32.

when doing cpan upgrade there is following message, and after this duplicate detection gets broken:

CPAN.pm: Building M/MA/MAKAMAKA/JSON-2.53.tar.gz

Welcome to JSON (v.2.53)
=============================
You have JSON::XS (v.2.32), so JSON can work very fast!!

 ************************** CAUTION **************************
 * This is 'JSON version 2' and there are many differences   *
 * to version 1.xx                                           *
 * Please check your applications useing old version.        *
 *   See to 'INCOMPATIBLE CHANGES TO OLD VERSION' and 'TIPS' *
 *************************************************************
Thanks for the bug report! This is probably actually more a support issue than a bug in Bugzilla itself, so the best place to go for any further assistance would be the support resources described at:

  http://www.bugzilla.org/support/
Status: UNCONFIRMED → RESOLVED
Closed: 13 years ago
Resolution: --- → INVALID
(In reply to Max Kanat-Alexander from comment #5)
> Thanks for the bug report! This is probably actually more a support issue
> than a bug in Bugzilla itself, so the best place to go for any further
> assistance would be the support resources described at:
> 
>   http://www.bugzilla.org/support/

I disagree on that, it's a bug when you have perl JSON::RPC version 2.53
(In reply to erkiha@gmail.com from comment #6)
> I disagree on that, it's a bug when you have perl JSON::RPC version 2.53

JSON::RPC 2.53 doesn't exist. Maybe you meant JSON 2.53. That's the version I have with Perl 5.14.2 for weeks now, and the automatic duplicate detection feature works fine. So this is not a bug with this package. Maybe it's a bug in the Debian package, in which case you must report this problem to them.
The error is because you have JSON::RPC 1.01 installed which is not backwards compatible with the version that Bugzilla is written against (0.96). You will need to install JSON::RPC 0.96 for now until this problem is resolved and Bugzilla will work with the new version.

From the JSON::RPC 1.01 man page:

If you are using old JSON::RPC code (up to 0.96), DO NOT EXPECT
YOUR CODE TO WORK WITH THIS VERSION. THIS VERSION IS
****BACKWARDS INCOMPATIBLE****

dkl
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: INVALID → ---
Summary: Automatic duplicate detection gives message "Data error." → Bugzilla will not work with newest version of JSON::RPC 1.01 due to non-backward compatibility
WTF. WTG CPAN.
(In reply to David Lawrence [:dkl] from comment #9)
> The error is because you have JSON::RPC 1.01 installed which is not
> backwards compatible with the version that Bugzilla is written against
> (0.96). You will need to install JSON::RPC 0.96 for now until this problem
> is resolved and Bugzilla will work with the new version.

Doesn't this mean that Bugzilla should check specifically for 0.96, instead of requiring "any" version?
FWIW, it is a one line change to make it all work again with JSON-RPC-1.01 by doing the following in Bugzilla/WebService/Server/JSONRPC.pm:

-use base qw(JSON::RPC::Server::CGI Bugzilla::WebService::Server);
+use base qw(JSON::RPC::Legacy::Server::CGI Bugzilla::WebService::Server);

But we would have to add more code to check for the version and load the correct module which I think would not be worth it. We should just blacklist the version for 4.0/4.2
and make 5.0 work with the newer version natively.

dkl
(In reply to Bernd Kuemmerlen from comment #11)
> (In reply to David Lawrence [:dkl] from comment #9)
> > The error is because you have JSON::RPC 1.01 installed which is not
> > backwards compatible with the version that Bugzilla is written against
> > (0.96). You will need to install JSON::RPC 0.96 for now until this problem
> > is resolved and Bugzilla will work with the new version.
> 
> Doesn't this mean that Bugzilla should check specifically for 0.96, instead
> of requiring "any" version?

Yeah we could fix the version in Bugzilla/Install/Requirements.pm to throw an error if not 0.96 and add 1.0 or higher to the blacklist array. This would work for people who use CPAN to install their Bugzilla modules. Not too helpful for those who install theirs via RPM or other distro package managers. Fedora is still on 0.96 but they could update at any time. RHEL/Centos will probably not update to 1.01 until their next major release.

dkl
Patch that adds the version bits to Bugzilla/Install/Requirements.pm

dkl
Attachment #583494 - Flags: review?(mkanat)
Comment on attachment 583494 [details] [diff] [review]
Patch to blacklist JSON-RPC-1.x and require 0.96 for 4.0 and 4.2 (v1)

This is not enough. checksetup.pl will ban any 1.x version, and running install-module.pl will always try to install the most recent version, i.e. 1.x, which will immediately banned by checksetup.pl again. See how I did it for LWP to fix the problem.
Attachment #583494 - Flags: review?(mkanat) → review-
Thanks LpSolit. I was able to use your example for LWP to update Bugzilla/Install/CPAN.pm to make install-module.pl install the proper version.

New patch
dkl
Attachment #583494 - Attachment is obsolete: true
Attachment #583506 - Flags: review?(LpSolit)
(In reply to David Lawrence [:dkl] from comment #12)
> But we would have to add more code to check for the version and load the
> correct module which I think would not be worth it.

Hum, I'm not that sure. Maybe it worths the effort. Some administrators refuse to install Perl modules themselves using CPAN and rely entirely on their Linux distro to provide them the necessary packages. For instance, Mageia 2 has JSON::RPC 1.01, but not 0.96.

Rather than banning the 1.x series of JSON::RPC, maybe we should fix our code to detect which version of JSON::RPC is installed. Sort of: use base qw() if ...; (if this syntax exists).
Marking as blocker for 4.2 final (not rc1). Meanwhile, I added it to the "Outstanding Issues" in the release notes for rc1.
Flags: blocking4.2+
(In reply to Frédéric Buclin from comment #17)
> Hum, I'm not that sure. Maybe it worths the effort. Some administrators
> refuse to install Perl modules themselves using CPAN and rely entirely on
> their Linux distro to provide them the necessary packages. For instance,
> Mageia 2 has JSON::RPC 1.01, but not 0.96.

I was merely stating that we could require 0.96 for the remaining 4.x series and then 
make 5.0 adhere to the new 1.01 structure requiring people to upgrade at that time if
any of the distros were still behind.
 
> Rather than banning the 1.x series of JSON::RPC, maybe we should fix our
> code to detect which version of JSON::RPC is installed. Sort of: use base
> qw() if ...; (if this syntax exists).

We could do this using the facilities in Bugzilla/Install/Util.pm and I have a patch somewhere where I started trying to do this detection. I will try to get it uploaded.

dkl
Target Milestone: --- → Bugzilla 4.0
Assignee: create-and-change → dkl
(In reply to David Lawrence [:dkl] from comment #12)
> FWIW, it is a one line change to make it all work again with JSON-RPC-1.01
> by doing the following in Bugzilla/WebService/Server/JSONRPC.pm:
> 
> -use base qw(JSON::RPC::Server::CGI Bugzilla::WebService::Server);
> +use base qw(JSON::RPC::Legacy::Server::CGI Bugzilla::WebService::Server);
> 
> But we would have to add more code to check for the version and load the
> correct module which I think would not be worth it. We should just blacklist
> the version for 4.0/4.2
> and make 5.0 work with the newer version natively.
> 
> dkl

Is this patch working?
I'm asking because FreeBSD has JSON-RPC-1.01 in the tree and I will prepare a diff for the update to version 3.6.7 and 4.0.3.

Maybe also update the page http://www.bugzilla.org/releases/4.0.3/release-notes.html and change the lines
JSON::RPC | (Any) | JSON-RPC Interface
to 
JSON::RPC | 0.96 | JSON-RPC Interface
It will work if you apply the change manually once you have updated

(In reply to olli hauer from comment #20)
> Is this patch working?
> I'm asking because FreeBSD has JSON-RPC-1.01 in the tree and I will prepare
> a diff for the update to version 3.6.7 and 4.0.3.

It will work if you apply the change manually once you have updated to 1.01. We are still working on an intelligent coding solution so that it makes the change automatically based
on which version of JSON::RPC is installed.

> Maybe also update the page
> http://www.bugzilla.org/releases/4.0.3/release-notes.html and change the
> lines
> JSON::RPC | (Any) | JSON-RPC Interface
> to 
> JSON::RPC | 0.96 | JSON-RPC Interface

Yeah we should definitely change that.

dkl
Hi David,

thanks for the update.

Maybe this can be done by checksetup
 
If JSON-RPC > 0.96 is detected simply do a 
sed -i '' -e 's/JSON::RPC::Server/JSON::RPC::Legacy::Server/' \
  Bugzilla/WebService/Server/JSONRPC.pm

I will implement a simmilar solution as workaround in the FreeBSD ports Makefile.
(In reply to olli hauer from comment #22)
> Hi David,
> 
> thanks for the update.
> 
> Maybe this can be done by checksetup
>  
> If JSON-RPC > 0.96 is detected simply do a 
> sed -i '' -e 's/JSON::RPC::Server/JSON::RPC::Legacy::Server/' \
>   Bugzilla/WebService/Server/JSONRPC.pm
> 
> I will implement a simmilar solution as workaround in the FreeBSD ports
> Makefile.

That is one way. Realistically at some point Bugzilla code will need to either 1) move to another JSON RPC library or 2) port over to the new structure of JSON::RPC 1.01 since the Legacy code will probably be obsolete and maintained from this point on.

The solution for now I envision will be Bugzilla::WebService::Server::JSONRPC using the utility function Bugzilla::Install::Requirements::have_vers to determine the version of JSON-RPC installed and load the appropriate module in a BEGIN block.

dkl
(In reply to olli hauer from comment #20)
> Maybe also update the page
> http://www.bugzilla.org/releases/4.0.3/release-notes.html and change the
> lines
> JSON::RPC | (Any) | JSON-RPC Interface
> to 
> JSON::RPC | 0.96 | JSON-RPC Interface

No, this is incorrect. We don't require 0.96 as a minimum.
(In reply to Frédéric Buclin from comment #24)
> (In reply to olli hauer from comment #20)
> > Maybe also update the page
> > http://www.bugzilla.org/releases/4.0.3/release-notes.html and change the
> > lines
> > JSON::RPC | (Any) | JSON-RPC Interface
> > to 
> > JSON::RPC | 0.96 | JSON-RPC Interface
> 
> No, this is incorrect. We don't require 0.96 as a minimum.

But we should at least note in the docs that 0.96 is currently the 'maximum'. It now seems
some distros are starting to ship the 1.01 code.

dkl
(In reply to David Lawrence [:dkl] from comment #25)
> But we should at least note in the docs that 0.96 is currently the
> 'maximum'.

The release notes already mention this problem:

 http://www.bugzilla.org/releases/4.2/release-notes.html#v42_issues

 
> It now seems some distros are starting to ship the 1.01 code.

Sure. Mageia 2 is already shipping 1.01. They patched their Bugzilla RPM a few days ago to be compatible with 1.01.
(In reply to Frédéric Buclin from comment #26)
> (In reply to David Lawrence [:dkl] from comment #25)
> > But we should at least note in the docs that 0.96 is currently the
> > 'maximum'.
> 
> The release notes already mention this problem:
> 
>  http://www.bugzilla.org/releases/4.2/release-notes.html#v42_issues

Cool. I forgot that that was in the known issues list already. My original thought was maybe a small note in the requirements table as well but maybe it is not needed since it
is clearly stated in the issues list.

dkl
Oh, I was following the release notes for 4.03 and 3.6.7 and there is no comment about this outstanding issue. A small footnote like "* see outstanding issues" will be helpful.
Status: REOPENED → ASSIGNED
Flags: blocking4.0.4+
Comment on attachment 583506 [details] [diff] [review]
Patch to blacklist JSON-RPC-1.x and require 0.96 for 4.0 and 4.2 (v2)

I don't want to block JSON::RPC 1.x. We should rather check which module to use.
Attachment #583506 - Flags: review?(LpSolit) → review-
Attached patch patch, v3Splinter Review
I think this is a better alternative.
Assignee: dkl → LpSolit
Attachment #583506 - Attachment is obsolete: true
Attachment #585592 - Flags: review?(mkanat)
Attachment #585592 - Flags: review?(dkl)
Comment on attachment 585592 [details] [diff] [review]
patch, v3

Review of attachment 585592 [details] [diff] [review]:
-----------------------------------------------------------------

This doesn't work as both 0.96 and 1.01 have a JSON::RPC::Server::CGI so the loading always passes. You should test instead for
JSON::RPC::Client which is not present in 1.01. Like this:

+use Bugzilla::WebService::Server;
+
+BEGIN {
+    our @ISA = qw(Bugzilla::WebService::Server);
+    eval { require JSON::RPC::Client; };
+    if ($@) {
+        require JSON::RPC::Legacy::Server::CGI;
+        push(@ISA, 'JSON::RPC::Legacy::Server::CGI');
+    }
+    else {
+        require JSON::RPC::Server::CGI;
+        push(@ISA, 'JSON::RPC::Server::CGI');
+    }
+}
Attachment #585592 - Flags: review?(dkl) → review-
(In reply to David Lawrence [:dkl] from comment #31)
> This doesn't work as both 0.96 and 1.01 have a JSON::RPC::Server::CGI so the
> loading always passes. You should test instead for
> JSON::RPC::Client which is not present in 1.01. Like this:

Crap. When I built a RPM of perl-JSON-RPC-1.01 and installed it, it mistakenly left the 0.96 files in place :( So I was wrong about my earlier testing remarks as it looked like the new version had similar file structure. Just now doing a 'rpm -qpl perl-JSON-RPC-1.01' I see that in fact the JSON::RPC::Server::CGI module is missing as you stated so your code would work properly. I will figure out sometime why the rpm update does not remove the old files as it should. Will retest now and change my review status.

dkl
Comment on attachment 585592 [details] [diff] [review]
patch, v3

Re-requesting review based on comment 32.
Attachment #585592 - Flags: review- → review?(dkl)
Actually I see now that at some point in the past Fedora decided to break up perl-JSON-RPC-0.96 into perl-JSON-RPC (client) and perl-JSON-RPC-server (server) which got me messed up.
Comment on attachment 585592 [details] [diff] [review]
patch, v3

Review of attachment 585592 [details] [diff] [review]:
-----------------------------------------------------------------

Ok, now that I worked out my RPM issues, it works properly. I think the way I posted in comment 31 is slightly
cleaner but I am not opposed to your structure as it also works. r=dkl
Attachment #585592 - Flags: review?(dkl) → review+
Comment on attachment 585592 [details] [diff] [review]
patch, v3

Review of attachment 585592 [details] [diff] [review]:
-----------------------------------------------------------------

::: Bugzilla/WebService/Server/JSONRPC.pm
@@ +24,5 @@
>  use strict;
> +use Bugzilla::WebService::Server;
> +BEGIN {
> +    eval { require JSON::RPC::Server::CGI; };
> +    if ($@) {

That should just be "if (eval {" -- see Install::CPAN and makedocs.pl for examples.

@@ +29,5 @@
> +        require JSON::RPC::Legacy::Server::CGI;
> +        our @ISA = qw(JSON::RPC::Legacy::Server::CGI Bugzilla::WebService::Server);
> +    }
> +    else {
> +        our @ISA = qw(JSON::RPC::Server::CGI Bugzilla::WebService::Server);

Does this actually work? I would expect you'd have to define @ISA outside of the block and then unshift the right CGI module onto it.
Attachment #585592 - Flags: review?(mkanat) → review+
Approved back to 3.6 on discussion with LpSolit. (Approval can be granted when new patch attached with fixes.)
Flags: approval?
Flags: approval4.2?
Flags: approval4.0?
Flags: approval3.6?
Target Milestone: Bugzilla 4.0 → Bugzilla 3.6
(In reply to Max Kanat-Alexander from comment #36)
> @@ +29,5 @@
> > +        require JSON::RPC::Legacy::Server::CGI;
> > +        our @ISA = qw(JSON::RPC::Legacy::Server::CGI Bugzilla::WebService::Server);
> > +    }
> > +    else {
> > +        our @ISA = qw(JSON::RPC::Server::CGI Bugzilla::WebService::Server);
> 
> Does this actually work? I would expect you'd have to define @ISA outside of
> the block and then unshift the right CGI module onto it.

It did work for me.

dkl
I can test tomorrow the patch from LpSolit on a cloned live system.

Maybe it's an option to change the test (first test for JSON-RPC > 0.96) in case RPM/cpan or ??? cleanup was not correct.

-use base qw(JSON::RPC::Server::CGI Bugzilla::WebService::Server);
+use Bugzilla::WebService::Server;
+BEGIN {
+    eval { require JSON::RPC::Legacy::Server::CGI; };
+    if ($@) {
+        require JSON::RPC::Server::CGI;
+        our @ISA = qw(JSON::RPC::Server::CGI Bugzilla::WebService::Server);
+    }
+    else {
+        our @ISA = qw(JSON::RPC::Legacy::Server::CGI Bugzilla::WebService::Server);
+    }
+}
Attached patch patch, v3.1Splinter Review
Carrying forward r+.

|our @ISA = qw(...)| must be in the BEGIN block, else the BEGIN block will be executed before the definition of @ISA, and Bugzilla indeed crashes if you try to do so.
Attachment #585930 - Flags: review+
Flags: approval?
Flags: approval4.2?
Flags: approval4.2+
Flags: approval4.0?
Flags: approval4.0+
Flags: approval3.6?
Flags: approval3.6+
Flags: approval+
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/WebService/Server/JSONRPC.pm
Committed revision 8057.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.2/
modified Bugzilla/WebService/Server/JSONRPC.pm
Committed revision 7994.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.0/
modified Bugzilla/WebService/Server/JSONRPC.pm
Committed revision 7675.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/3.6/
modified Bugzilla/WebService/Server/JSONRPC.pm
Committed revision 7270.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago12 years ago
Resolution: --- → FIXED
I also removed this bug from the Outstanding Bugs section of the release notes, now that this bug is fixed.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified template/en/default/pages/release-notes.html.tmpl
Committed revision 8065.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.2/
modified template/en/default/pages/release-notes.html.tmpl
Committed revision 8000.
You need to log in before you can comment on or make changes to this bug.