172331 - importxml.pl fails tests under perl 5.8

Status: RESOLVED FIXED

(Bugzilla :: Bug Import/Export & Moving, defect)

Description

[Bradley Baetz (:bbaetz)]

Using an array as a reference is deprecated at importxml.pl line 229.
Using an array as a reference is deprecated at importxml.pl line 263.
Using an array as a reference is deprecated at importxml.pl line 279.
Using an array as a reference is deprecated at importxml.pl line 292.
Using an array as a reference is deprecated at importxml.pl line 420.
Using an array as a reference is deprecated at importxml.pl line 422.

The tinderboxes don't see this, probably because they don't ahve the xml module
installed

my $bugqty = ($#{@{$tree}->[1]} +1 -3) / 4;

is line 229, others are similar.

I have no clue what this code is trying to do, but dropping the @{..} bit may do
something. Possibly.

Any volunteers?

Comment 1

[Bradley Baetz (:bbaetz)]

Fails tests, therefore its a blocker

Comment 2

[Tobias Burnus]

Attachment: v1: shot into the blind

> I have no clue what this code is trying to do.
Neither have I ;-)

> Any volunteers?
Try this patch. It removes the warning. Not sure whether it still does what it
supposed to do ;-)

Comment 3

[Jacob Steenhagen]

I don't think that's the right solution.  AIUI, the change attachment 108689 [details] [diff] [review]
makes is to use $tree as a hashref instead of an arrayref.  What it looks like
is happening to me is that the @{} is defreferencing $tree and the ->[1] is
telling it to use $tree as an arrayref.  I think Bradley has the right idea to
change this to:

my $bugqty = ($#{$tree->[1]} +1 -3) / 4;

but I'd have to study the code that creates $tree to know for sure, and I don't
have time to do that tonight...

Comment 4

[Bradley Baetz (:bbaetz)]

Comment on attachment 108689 [details] [diff] [review]
v1: shot into the blind

Yeah, thats definately wrong, and won't work.

Removing the defined keyword should be enough, I think.

Comment 5

[Pedro Melo]

Attachment: v2: another approach

another way: cleanup the result of XML::Parser first, and then use that.

Comment 6

[Pedro Melo]

attachment 111943 [details] [diff] [review] is another way of looking at the problem.

the ouput of the Tree Style of XML::Parser can be a bit dauting. Also, the
current  code was assuming the existence of whitespace between each bug and each
bug field.

In this new approach, first we cleanup the output of the XML::Parser to a more
clean and consistent structure and then use that in the the rest of the code.

We could probably clean up a lot more code with this new approach too...

I could not test this (it passes perl -Tc though) because I broke my Mysql setup
:(. I'll test it properly tomorow, though. Sorry about that.

Comment 7

[Bradley Baetz (:bbaetz)]

Comment on attachment 111943 [details] [diff] [review]
v2: another approach

I'm not really set up to test the import stuff. It looks ok, though

myk? You use import stuff...

Comment 8

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v3: updates syntax for perl 5.8

I think we should deal with the blocker issue first and then worry about larger
changes to the way this script works.  Here's a patch that updates the syntax
of the five deprecated expressions.  I left my debugging code in so you can see
how it works when you test it.

To test the patch, apply it, comment out line 66 (require "CGI.pl") of the
script, use http://bugzilla.mozilla.org/xml.cgi to get a bug as XML, save the
XML to your local drive, make sure the moved-default-product and
moved-default-component parameters are set, and run the script against the XML:


./importxml.pl < saved-bug.xml

My results are as follows:

----------------------------------------------------------------------
[myk@myk bztip]$ ./importxml.pl < export-sample.xml
Using an array as a reference is deprecated at ./importxml.pl line 228.
Using an array as a reference is deprecated at ./importxml.pl line 228.
Using an array as a reference is deprecated at ./importxml.pl line 262.
Using an array as a reference is deprecated at ./importxml.pl line 262.
Using an array as a reference is deprecated at ./importxml.pl line 282.
Using an array as a reference is deprecated at ./importxml.pl line 282.
Using an array as a reference is deprecated at ./importxml.pl line 296.
Using an array as a reference is deprecated at ./importxml.pl line 296.
Using an array as a reference is deprecated at ./importxml.pl line 427.
old ARRAY(0x868f27c) (6) is like new ARRAY(0x868f27c) (6)
old ARRAY(0x868f210) (166) is like new ARRAY(0x868f210) (166)
old ARRAY(0x86c4f4c) (14) is like new ARRAY(0x86c4f4c) (14)
old ARRAY(0x868f210) (166) is like new ARRAY(0x868f210) (166)
old 1.0 (1.0) is like new ARRAY(0x86b1648) (1.0)
<b>Email sent to:</b> [e-mail]<br>
<br><center>If you wish to tweak the kinds of mail Bugzilla sends you, you can
<a href="userprefs.cgi?tab=email">change your preferences</a></center>
----------------------------------------------------------------------

I'm not sure why that last debug statement doesn't print equivalent before and
after results, but I think the change there is still correct.

Comment 9

[Bradley Baetz (:bbaetz)]

This isn't strictly a blocker - its just a warning, not an error.

It may or may not be an error in perl5.10, but I don't think we really have to
worry about it.

Lets just do this right.

Comment 10

[Dave Miller [:justdave]]

I accidently discovered this afternoon that the compile test on the 2.16 branch
was slightly broken.  I accidently fixed it this afternoon.  The 2.16/5.8.0 tree
on tinderbox lit up orange.  Same error as the trunk on 5.8.0, in the same spot
in the same file.  So we need to fix this on the 2.16 branch, too.  That file is
touched infrequently enough I'd bet the same patch would even apply cleanly.

Comment 11

[Dave Miller [:justdave]]

Any action here lately?

Comment 12

[Dave Miller [:justdave]]

what's the status here?  we're rolling for 2.16.3 real shortly here and we need
this for it.  I see a review pending bbaetz...  if you're not sure, I'm not
afraid of breaking bug moving, Netscape<->bmo seems to be the only place that
uses it anyway, and I'm around to fix it if it does. :)

Comment 13

[Bradley Baetz (:bbaetz)]

Myk's doesn't work because its wrong - he's moved teh @{} stuff arround.

Let me try to look into this later today.

Comment 14

[Bradley Baetz (:bbaetz)]

Attachment: patch

Given an arrayref, @{$foo} produces an array, not a ref, so you need to lose
the ->. However, the @{} wasn't actully needed for any sort of grouping here,
so Ijust removed it. The versions bit is the only place I'm not 100% sure
about.

This is untested except for |perl -wc| justdave? Please test the case where the
version in teh bug to be imported doesn't exist in the new system.

Comment 15

[Dave Miller [:justdave]]

patch has been applied to b.m.o (only convenient place to test it).  I have to
run for a bit right now, but I'll send a bug over from Bugscape when I get back
and see what it does.

Comment 16

[Bradley Baetz (:bbaetz)]

dave: ping?

Comment 17

[Dave Miller [:justdave]]

ping what?  I was waiting for you.  You said the count bug had to be ready to go
first, so both of these could be checked in at the same time.

Comment 18

[Dave Miller [:justdave]]

nevermind that last comment, wrong bug :)  I'm testing...

Comment 19

[Dave Miller [:justdave]]

Comment on attachment 121150 [details] [diff] [review]
patch

ok, it worked on bugscape->mothra without breaking :)

Comment 20

[Dave Miller [:justdave]]

-> patch author

a= justdave for both branches (it does successfully apply to both without errors :)

Comment 21

[Bradley Baetz (:bbaetz)]

Fixed:

Checking in importxml.pl;
/cvsroot/mozilla/webtools/bugzilla/importxml.pl,v  <--  importxml.pl
new revision: 1.32; previous revision: 1.31
done

Checking in importxml.pl;
/cvsroot/mozilla/webtools/bugzilla/importxml.pl,v  <--  importxml.pl
new revision: 1.23.2.2; previous revision: 1.23.2.1
done

Comment 22

[Bradley Baetz (:bbaetz)]

Comment on attachment 114065 [details] [diff] [review]
patch v3: updates syntax for perl 5.8

r-; the other patch was checked in.