diff options
Diffstat (limited to 'doc/bugs/trail_excess_dependencies.mdwn')
-rw-r--r-- | doc/bugs/trail_excess_dependencies.mdwn | 95 |
1 files changed, 95 insertions, 0 deletions
diff --git a/doc/bugs/trail_excess_dependencies.mdwn b/doc/bugs/trail_excess_dependencies.mdwn new file mode 100644 index 000000000..f806a62eb --- /dev/null +++ b/doc/bugs/trail_excess_dependencies.mdwn @@ -0,0 +1,95 @@ +I've just modified the trail plugin to use only presence, and not +content dependencies. Using content dependencies, particularly to the page +that defines the trail, meant that every time that page changed, *every* +page in the trail gets rebuilt. This leads to users setting up sites that +have horrible performance, if the trail is defined in, for example, the top +page of a blog. + +Unfortunatly, this change to presence dependencies has +introduced a bug. Now when an existing trail is removed, the pages in the +trail don't get rebuilt to remove the trail (both html display and state). + +> Actually, this particular case is usually OK. Suppose a trail `untrail` +> contains `untrail/a` (as is the case in the regression +> test I'm writing), and you build the wiki, then edit `untrail` to no +> longer be a trail, and refresh. `untrail` has changed, so it is +> rendered. Assuming that the template of either `untrail` or another +> changed page happens to contain the `TRAILS` variable (which is not +> guaranteed, but is highly likely), `I::P::t::prerender` +> is invoked. It notices that `untrail/a` was previously a trail +> member and is no longer, and rebuilds it with the diagnostic +> "building untrail/a, its previous or next page has changed". +> +> Strictly speaking, I should change `I::P::t::build_affected` +> so it calls `prerender`, so we're guaranteed to have done the +> recalculation. Fixed in my branch. --[[smcv]] + +I think that to fix this bug, the plugin should use a hook to +force rebuilding of all the pages that were in the trail, when +the trail is removed (or changed). + +> The case of "the trail is changed" is still broken: +> if the order of items changes, or the trail is removed, +> then the logic above means it's OK, but if you +> change the `\[[!meta title]]` of the trail, or anything else +> used in the prev/up/next bar, the items won't show that +> change. Fixed in my branch. --[[smcv]] + +There's a difficulty in doing that: The needsbuild hook runs before the scan +hook, so before it has a chance to see if the trail directive is still there. +It'd need some changes to ikiwiki's hooks. + +> That's what `build_affected` is for, and trail already used it. --s + +(An improvement in this area would probably simplify other plugins, which +currently abuse the needsbuild hook to unset state, to handle the case +where the directive that resulted in that state is removed.) + +I apologise for introducing a known bug, but the dependency mess was too +bad to leave as-is. And I have very little time (and regrettably, even less +power) to deal with it right now. :( --[[Joey]] + +[[!template id=gitbranch branch=smcv/ready/trail author="[[Simon_McVittie|smcv]]"]] +[[!tag patch]] + +> I believe my `ready/trail` branch fixes this. There are regression tests. +> +> Here is an analysis of how the trail pages interdepend. +> +> * If *trail* contains a page *member* which does exist, *member* depends +> on *trail*. This is so that if the trail directive is deleted from +> *trail*, or if *trail*'s "friendly" title or trail settings are changed, +> the trail navigation bar in *member* will pick up that change. This is +> now only a presence dependency, which isn't enough to make those happen +> correctly. [Edited to add: actually, the title is the only thing that +> can affect *member* without affecting the order of members.] +> +> * If *trail* contains consecutive pages *m1* and *m2* in that order, +> *m1* and *m2* depend on each other. This is so that if one's +> "friendly" title changes, the other is rebuilt. This is now only +> a presence dependency, which isn't enough to make those happen +> correctly. In my branch, I explicitly track the "friendly" title +> for every page that's edited and is involved in a trail somehow. +> +> * If *trail* has *member* in its `pagenames` but there is no page called +> *member*, then *trail* must be rebuilt if *member* is created. This +> was always a presence dependency, and is fine. +> +> In addition, the `trail` plugin remembers the maps +> { trail => next item in that trail } and { trail => previous item in +> that trail } for each page. If either changes, the page gets rebuilt +> by `build_affected`, with almost the same logic as is used to update +> pages that link to a changed page. My branch extends this to track the +> "friendly title" of each page involved in a trail, either by being +> the trail itself or a member (or both). +> +> I think it's true to say that the trail always depends on every member, +> even if it doesn't display them. This might mean that we can use +> "render the trail page" as an opportunity to work out whether any of +> its members are also going to need re-rendering? +> [Edited to add: actually, I didn't need this to be true, but I made the +> regression test check it anyway.] +> +> --[[smcv]] + +>>> Thanks **very** much! [[done]] --[[Joey]] |