If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Dependency graphs should be able to be limited by relationship

RESOLVED FIXED in Bugzilla 3.2

Status

()

Bugzilla
Bugzilla-General
P3
major
RESOLVED FIXED
16 years ago
6 years ago

People

(Reporter: myk, Assigned: Frédéric Buclin)

Tracking

2.15
Bugzilla 3.2
Bug Flags:
approval +

Details

(Whiteboard: [applied to b.m.o])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

16 years ago
showdependencygraph.cgi creates graphs containing any bug with any relationship
to the bug whose graph is being requested.  In some cases this means that
Bugzilla generates a webdot file with thousands of dependencies, which is too
many for webdot to parse, so showdependencygraph.cgi returns a page that hangs
for five minutes and then times out waiting for the graph image to be displayed.

showdependencygraph.cgi should provide options for limiting the level of
recursion or other mechanisms to make it usable with bugs that have a large
number of dependencies.  It could use the same logic used by showdependencytree.cgi
 to limit the number of bugs it displays.
Priority: -- → P3
Target Milestone: --- → Bugzilla 2.18
I think a very useful way of limiting this which is not available in the tree
view and which makes little sense in the tree view but a lot of sense for graphs
is a way to stop recursing when you hit a bug with more than <n> dependencies
(with <n> being some large user-customisable number default to, e.g., 20).

I think this would massively decrease the size of the graphs being generated,
because I expect most of the time the graph snowballs out of control when the
chain hits one of the uber meta bugs. Nobody really cares about going up then
down through those uber meta bugs. They either want to see down from that bug,
or up to that bug.

When bugs are trimmed, the graph should show this (6 is the uber meta bug):

                             +---------+
                             |   123   |
                             +---------+
                              /   |   \
         +------+            /    |    \        +-----+
         |  45  |-----------'     |     '-------|  6  |
         +------+              +-----+          +-----+
                               | 789 |             |
                               +-----+             o
                                                   o
                                                   o
We no longer rely on webdot, but we still could use some depth-limiting, even
for the local case.
Summary: showdependencygraph.cgi creates over-complex graphs → Dependency graphs should be able to be limited by depth

Updated

14 years ago
Blocks: 164085

Comment 3

14 years ago
It would be necessary to have the depth limit as a default setting, so that
people can access it in the first place where the infinite depth has caused it
to fail.
All 2.18 bugs that haven't been touched in over 60 days and aren't flagged as
blockers are getting pushed out to 2.20
Target Milestone: Bugzilla 2.18 → Bugzilla 2.20
Myk: is this a dupe of bug 193947 (or vice versa?)

Updated

13 years ago
Severity: normal → enhancement

Comment 6

13 years ago
This bug has not been touched by its owner in over six months, even though it is
targeted to 2.20, for which the freeze is 10 days away. Unsetting the target
milestone, on the assumption that nobody is actually working on it or has any
plans to soon.

If you are the owner, and you plan to work on the bug, please give it a real
target milestone. If you are the owner, and you do *not* plan to work on it,
please reassign it to nobody@bugzilla.org or a .bugs component owner. If you are
*anybody*, and you get this comment, and *you* plan to work on the bug, please
reassign it to yourself if you have the ability.
Target Milestone: Bugzilla 2.20 → ---
Reassigning bugs that I'm not actively working on to the default component owner
in order to try to make some sanity out of my personal buglist.  This doesn't
mean the bug isn't being dealt with, just that I'm not the one doing it.  If you
are dealing with this bug, please assign it to yourself.
Assignee: justdave → general
QA Contact: mattyt-bugzilla → default-qa
No longer blocks: 164085
*** Bug 164085 has been marked as a duplicate of this bug. ***

Comment 9

12 years ago
*** Bug 313563 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 10

12 years ago
*** Bug 193947 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 11

12 years ago
*** Bug 331587 has been marked as a duplicate of this bug. ***
*** Bug 339642 has been marked as a duplicate of this bug. ***
The lack of ability to do this causes denial of service attacks on bmo...
Severity: enhancement → major
Target Milestone: --- → Bugzilla 3.2

Comment 14

11 years ago
Slight understatement - the ability _not_ to do this can lead to DoS attacks.  I now think there should be a maximum depth set in the configuration of the Bugzilla installation.

Comment 15

11 years ago
Created attachment 249426 [details] [diff] [review]
hint patch

I just finished a patch to my own 2.20 and 2.22, one which seems to do most of what you're thinking -- mine omits bugs which aren't in the dep or block path of the IDs given.  You'll see that you will need to pass 'constrain=1' at the URL bar if you want to see it.  I haven't even thought about template modification to support it, although bug 248166 has a very similar hack and DOES have some template mods we can use.

Either way, I made this mod and thought maybe it'd scratch someone else's itch.  It'll need to be cleaned up probably, in addition to the template fixes, but I'm hoping I've done the heavier lifting already.

ignore the arrow reversal at the head of the patch -- the reverse dependency graph layout always annoyed me, like we can either choose a) backwards, or b) upside-down for our graph layout  ;-)

HTH
(Assignee)

Comment 16

11 years ago
(In reply to comment #15)
> ignore the arrow reversal at the head of the patch -- the reverse dependency
> graph layout always annoyed me

This is bug 370885. :)
(Assignee)

Updated

11 years ago
Duplicate of this bug: 348166
(Assignee)

Comment 18

11 years ago
Created attachment 257847 [details] [diff] [review]
patch, v1

In this patch, I consider "depth" as restricting bugs to those directly blocking or depending on the current bug(s). This will severely limit the number of bugs displayed, even if a meta bug is selected as root of the tree.
Assignee: general → LpSolit
Attachment #249426 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #257847 - Flags: review?(justdave)
(Assignee)

Comment 19

11 years ago
Comment on attachment 257847 [details] [diff] [review]
patch, v1

/me hopes myk will be faster than dave.
Attachment #257847 - Flags: review?(myk)
(Assignee)

Comment 20

11 years ago
Comment on attachment 257847 [details] [diff] [review]
patch, v1

Dave told me he was too busy to review it.
Attachment #257847 - Flags: review?(justdave) → review?(wicked+bz)
I've implemented patch,v1 (attachment 257847 [details] [diff] [review]) instead of my solution in Bug 348166. Patch,v1 works fine for me.
Comment on attachment 257847 [details] [diff] [review]
patch, v1

This is a nice patch and seems to do what it's supposed to do. Following problems can be fixed on checkin or you can carry on my r+ to a patch with only those changes.

However, I don't think this really fixes this bug as described. This patch restricts by relationship and not by depth. It does cut down on nodes rendered so maybe it's a correct fix for the "too much to handle" problem.

BTW, this CGI has a nasty habit of spitting out an ugly Internal Error when going to it directly. Instead it should show no graph but ask for parameters. :)

>Index: template/en/default/bug/dependency-graph.html.tmpl
>===================================================================
>+      <td><input id="dep_id" name="id" value="[% bug_id %]"></td>

Use same value for both name and id to avoid confusion and problems.

>+            Restrict to [% terms.bugs %] having a direct relationship with this bug</option>
>+            Show all [% terms.bugs %] having any relationship with this bug</option>

Term "this bug" should be plural as you can input more than one bug ID. Maybe something like "these bugs" or "selected bugs" or "entered bugs". Also, shouldn't those words use terms.bugs too?

>+      <th align="left"><label for="rankdir">Orientation:</label></th>

Drop "Orient" words from the options now that this has a label.

I wonder why we don't show all valid values for this one. All four orientations are supported but only two of them are selectable here.
Attachment #257847 - Flags: review?(wicked+bz)
Attachment #257847 - Flags: review?(myk)
Attachment #257847 - Flags: review+
Flags: approval?
(Assignee)

Comment 23

11 years ago
(In reply to comment #22)
> However, I don't think this really fixes this bug as described. This patch
> restricts by relationship and not by depth. It does cut down on nodes rendered
> so maybe it's a correct fix for the "too much to handle" problem.

Yes, I think that's the right approach for now. Speaking of depth when the relationship is random doesn't make sense to me. justdave suggested this alternative anyway, but he didn't convince me. ;)
Flags: approval? → approval+
(Assignee)

Comment 24

11 years ago
Created attachment 260878 [details] [diff] [review]
patch, v1.1

I fixed all comments made by wicked. Carrying forward his r+.
Attachment #257847 - Attachment is obsolete: true
Attachment #260878 - Flags: review+
(Assignee)

Comment 25

11 years ago
Checking in showdependencygraph.cgi;
/cvsroot/mozilla/webtools/bugzilla/showdependencygraph.cgi,v  <--  showdependencygraph.cgi
new revision: 1.57; previous revision: 1.56
done
Checking in template/en/default/bug/dependency-graph.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/dependency-graph.html.tmpl,v  <--  dependency-graph.html.tmpl
new revision: 1.13; previous revision: 1.12
done
Summary: Dependency graphs should be able to be limited by depth → Dependency graphs should be able to be limited by relationship
(Assignee)

Updated

11 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Updated

10 years ago
Keywords: relnote
Blocks: 401005
Whiteboard: [wanted-bmo]
Whiteboard: [wanted-bmo] → [applied to b.m.o]

Comment 26

9 years ago
Added to the release notes for Bugzilla 3.2 in a patch on bug 432331.
Keywords: relnote
You need to log in before you can comment on or make changes to this bug.