Last Comment Bug 143743 - Warning generated in dependency tree
: Warning generated in dependency tree
Product: Bugzilla
Classification: Server Software
Component: Bugzilla-General (show other bugs)
: 2.15
: All All
-- normal (vote)
: Bugzilla 2.16
Assigned To: Jouni Heikniemi
: default-qa
Depends on:
  Show dependency treegraph
Reported: 2002-05-11 11:40 PDT by Jouni Heikniemi
Modified: 2012-12-18 20:46 PST (History)
2 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---

Patch (699 bytes, patch)
2002-05-11 11:41 PDT, Jouni Heikniemi
myk: review+
myk: review+
Details | Diff | Splinter Review
patch v2: fixes problem in showdependencytree.cgi (570 bytes, patch)
2002-05-14 15:17 PDT, Myk Melez [:myk] [@mykmelez]
bbaetz: review+
justdave: review+
Details | Diff | Splinter Review

Description User image Jouni Heikniemi 2002-05-11 11:40:13 PDT
The dependency tree template generates a warning because not all dependencies
have dependencies themselves, and there is one condition in display_tree block
which doesn't check for this. The warning message:

Argument "" isn't numeric in numeric gt (>) at 
data/template/en/default/bug/dependency-tree.html.tmpl line 284 (#1) (W 
numeric) The indicated string was fed as an argument to an operator that 
expected a numeric value instead. If you're fortunate the message will identify 
which operator was so unfortunate. [Sat May 11 19:00:28 2002] 

Patch follows. Ccing Burnus and Myk (template authors) for comments/review.
Comment 1 User image Jouni Heikniemi 2002-05-11 11:41:03 PDT
Created attachment 83210 [details] [diff] [review]

Adds one condition, killing the warning.
Comment 2 User image Jouni Heikniemi 2002-05-11 11:42:45 PDT
The fix should be trivial enough, so proposing milestone 2.16 to get this into
the branch as well.
Comment 3 User image Myk Melez [:myk] [@mykmelez] 2002-05-13 13:22:08 PDT
I wonder if it would be better for every bug record to have a dependencies field.
Comment 4 User image Dave Miller [:justdave] ( 2002-05-13 19:08:35 PDT
uh, is that anything like the "keyword cache"?  I'm not particularly fond of
that idea...
Comment 5 User image Myk Melez [:myk] [@mykmelez] 2002-05-13 20:07:54 PDT
No, I mean that when generating the Perl data structure representing the
dependency tree, we should initialize each Perl bug record with an empty list of
dependencies.  Then we wouldn't have to check whether or not the list of
dependencies exists, we could just check its size (like we currently do).
Comment 6 User image Dave Miller [:justdave] ( 2002-05-13 20:14:08 PDT
Oh, for the values passed to the template you mean, not the DB schema.  Yeah,
that makes perfect sense. would fit well for feeding bugs to templates
ya know...  are we using it?  (just an off-topic aside :)
Comment 7 User image Jouni Heikniemi 2002-05-13 23:24:56 PDT
Before implementing any solution, the true meaning of these undef'd fields in
template data structures should be considered.

This is not the first bug (I think it's the fourth for 2.16) I've filed with
exactly the same characteristics. We should consider whether "nulls" should be
passed as empty lists or undef'd variables and then make it a standard coding
practice. The check "foo && foo.size > 0" is actually redundant in most of the
cases, since either one of the conditions would suffice if the code was designed

So, there should be a design document either stating that lists with empty
meaning should be passed as undef'd or a document stating that there should
always be a list, empty if appropriate. This way all template authors could
trust on certain convention being used.

I still think this bug should be fixed using my patch, since it's quick and
easy. If something is decided on the more generic issue presented above, a new
bug should be filed to convert all the existing empty list code to use whichever
method is chosen. 
Comment 8 User image Myk Melez [:myk] [@mykmelez] 2002-05-14 11:57:07 PDT
Comment on attachment 83210 [details] [diff] [review]

As it turns out, the code in showdependencytree.cgi already initializes 
the Perl bug records with empty arrays (line 170), but Perl returns 
the undefined value when evaluating an empty array in a scalar context.

I think the Template Toolkit should be handling this situation 
by returning numeric zero instead of the undefined value in this situation.
I'll report this bug to the Template Toolkit folks.  In the meantime,
Jouni's patch is the right workaround, so 2xr=myk.
Comment 9 User image Myk Melez [:myk] [@mykmelez] 2002-05-14 15:17:34 PDT
Created attachment 83603 [details] [diff] [review]
patch v2: fixes problem in showdependencytree.cgi

Ok, the problem turns out to be that I'm initializing the array reference with
an empty list instead of an array reference.  This patch fixes the problem.
Comment 10 User image Bradley Baetz (:bbaetz) 2002-05-16 17:46:25 PDT
Comment on attachment 83603 [details] [diff] [review]
patch v2: fixes problem in showdependencytree.cgi

r=bbaetz, per discussion on TT list.
Comment 11 User image Dave Miller [:justdave] ( 2002-05-20 20:32:58 PDT
Comment on attachment 83603 [details] [diff] [review]
patch v2: fixes problem in showdependencytree.cgi

r= justdave
Comment 12 User image Myk Melez [:myk] [@mykmelez] 2002-05-21 07:25:30 PDT
Checking in bztip/showdependencytree.cgi;
/cvsroot/mozilla/webtools/bugzilla/showdependencytree.cgi,v  <-- 
new revision: 1.18; previous revision: 1.17
Checking in bz216/showdependencytree.cgi;
/cvsroot/mozilla/webtools/bugzilla/showdependencytree.cgi,v  <-- 
new revision:; previous revision: 1.17

Note You need to log in before you can comment on or make changes to this bug.