GenericSetup step loop: portlets -> content -> plone-final -> portlets

[rossp]

When registering an import step with a dependency on "content" (possibly the others), the step will fail to run after the "content" step due to a loop in the step dependencies. Specifically, in plone/app/portlets/exportimport/configure.zcml:23 the "portlets" step depends on "content". The "content" step depends on "plone-final" in CMFPlone/profiles/default/import_steps.xml:38. Finally, the "plone-final" step depends on "portlets" in CMFPlone/profiles/default/import_steps.xml:115.

I've seen this in Plone 3.1.4 and several older versions. I'm not sure where is the right place to break the cycle, sorry. I've worked around it by removing the dependency on "content" in "portlets" in my ZCML, but I don't know if this would break anything in Plone:

    <genericsetup:importStep
        name="portlets"
        title="Portlets"
        description="Import portlet managers, types, assignments and blacklistings"
        handler="plone.app.portlets.exportimport.portlets.importPortlets">
        <depends name="componentregistry"/>
    </genericsetup:importStep>

Maybe the condition in the GS dependency resolution in GenericSetup/utils.py:852 that excepts circular loops should log an error?

[charleso]

I just experienced the same issue. I have a generic setup import step with a dependency of plone-final. This actually worked for one site due to the random nature of how the nodes are thrown into the graph list. On another site using the same code I was affected by the following line of code:

if len(unresolved) == len(graph):

# Nothing was resolved in this loop. There must be circular or # missing dependencies. Just add them to the end. We can't # raise an error, because checkComplete relies on this method. for node, edges in unresolved:

result.append(node)

By default the plone-final depends on portlets, portlets depends on content, and content depends on plone-final. Therefore you cannot reliably make a step depend on one of these steps

These steps should not depend on each other or at least some kind of warning should be shown if you try to use one as a dependency

[charleso]

here's my work around, i hope this helps someone else. i removed the import_steps.xml file completely and added this to my configure.zcml :

<genericsetup:importStep

name="do_it" title="DO IT Install Some Product Content" description="This import step installs all the necessary default content objects into a pre-determined location." handler="mypackage.policy.setuphandlers.do_it">

<depends name="plone-final"/>

</genericsetup:importStep>

[b_mathieu]

I made a monkey patch for the sorting function. Before inserting unresolved nodes it will detect nodes involved in a cycle and insert them, giving a better chance for remaining nodes to be inserted with better dependencies order. My patch also skip dependencies if there's no import step with that name. This behaviour may be questionable, I don't know.

import logging
from Products.GenericSetup import tool, utils, registry

LOG = logging.getLogger(__name__)

def is_cyclic(step, steps_map, visited=None):

    if visited is None:
        visited = []

    if step in visited:
        # we want to say True only if the node is involved in the cycle,
        # not if the cycle is in one of its dependencies
        return visited.index(step) == 0

    visited.append(step)
    deps = steps_map.get(step, ())

    for d in deps:
        tmp_visited = list(visited)
        if is_cyclic(d, steps_map, tmp_visited):
            return True
    return False

def _patched_computeTopologicalSort( steps ):
    result = []
    graph = [ ( x[ 'id' ], x[ 'dependencies' ] ) for x in steps]

    unresolved = []
    cyclics_inserted = False
    existing_edges = frozenset((x['id'] for x in steps))

    while True:
        for node, edges in graph:

            after = -1
            resolved = 0

            for edge in edges:

                if edge not in existing_edges:
                    # missing edged, don't bother
                    resolved += 1

                elif edge in result:
                    resolved += 1
                    after = max( after, result.index( edge ) )

            if len(edges) > resolved:
                unresolved.append((node, edges))
            else:
                result.insert( after + 1, node )

        if not unresolved:
            break
        if len(unresolved) == len(graph):
            # Nothing was resolved in this loop. There must be circular or
            # missing dependencies. Just add them to the end. We can't
            # raise an error, because checkComplete relies on this method.

            # 1st pass: insert cyclics in result before others, and let others
            # have a chance to be inserted with dependencies better resolved
            if not cyclics_inserted:
                cyclics_inserted = True
                node_map = dict(graph)
                graph = []
                unresolved = []
                for step in node_map:
                    if is_cyclic(step, node_map):
                        result.append(step)
                    else:
                        graph.append((step, node_map[step]))
                continue

            # 2nd pass: cyclics have been inserted, add remaining nodes
            for node, edges in unresolved:
                print repr((node, edges))
                result.append(node)
            break
        graph = unresolved
        unresolved = []

    return result

utils._computeTopologicalSort = _patched_computeTopologicalSort
tool._computeTopologicalSort = _patched_computeTopologicalSort
registry._computeTopologicalSort = _patched_computeTopologicalSort
LOG.info('PATCHED utils._computeTopologicalSort to have a fix for dependencies'
         ' properly sorted (https://dev.plone.org/plone/ticket/8350)')

[b_mathieu]

More useful, here are the doctests for is_cyclic:

    >>> steps = dict([('base', ()),
    ...               (u'my-final', (u'plone-final', u'portlets')),
    ...               (u'plone-final', (u'portlets',)),
    ...               (u'portlets', (u'content',)),
    ...               (u'content', (u'plone-final',)),
    ...              ])
    >>> is_cyclic(u'base', steps)
    False
    >>> is_cyclic(u'plone-final', steps)
    True
    >>> is_cyclic(u'portlets', steps)
    True
    >>> is_cyclic(u'content', steps)
    True
    >>> is_cyclic(u'my-final', steps)
    False

and _computeTopologicalSort. It should fail on current implementation and succeed with mine (missing edges case is not tested, though).

    >>> steps = [{'id': u'base', 'dependencies': ()},
    ...          {'id': u'final', 'dependencies': (u'intermediate',)},
    ...          {'id': u'intermediate', 'dependencies': (u'base',)}
    ...          ]
    >>> _computeTopologicalSort(steps)
    [u'base', u'intermediate', u'final']

    now 'my-final' will depend on 'plone-final' and 'portlets', which are both
    involved in a cycle. We want 'my-final' to be placed after both.

    >>> steps = [{'id': u'base', 'dependencies': ()},
    ...          {'id': u'my-final', 'dependencies': (u'plone-final', u'portlets')},
    ...          {'id': u'plone-final', 'dependencies': (u'portlets',)},
    ...          {'id': u'portlets', 'dependencies': (u'content',)},
    ...          {'id': u'content', 'dependencies': (u'plone-final',)},
    ...          ]
    >>> _computeTopologicalSort(steps)
    [u'base', u'content', u'plone-final', u'portlets', u'my-final']

[wichert]

GenericSetup is not maintained as part of Plone, but as part of CMF. Please submit a bugreport for the CMF project at launchpad.

[b_mathieu]

Ok, Reported GenericSetup issue:  https://bugs.launchpad.net/zope-cmf/+bug/314010

But you have closed the ticket too fast: it's summary says that CMFPlone has circular dependency in its profile, this is not of GS concern. AFAIK the import step loop is still defined in the plone profile: I reopen the ticket. Please correct me if the profile code has been fixed. BTW I don't think that it should be listed in "portlets" component.

[claytron]

b_mathieu is correct, the circular loops just needs to be fixed in Plone. I am removing optilude as the owner as I know he is to busy to take care of this. We'll see if we can take care of this during the next tune-up.

There are two issues with the current import steps.

1. as noted. the 'content' step depends on 'plone-final', which ends up depending on 'content' by way of the 'portlets' step
2. the 'plonepas' step in Products.PlonePAS:default profile depends on a non-existent step named 'plonepas-contents'. this renders that step 'unresolved' as well.

[lucie]

I am thinking about no more having "plone-final" depending on "portlets" (I commented out the dependency into Products/CMFPlone/profiles/default/import_steps.xml). I succeeded to create a plone site with no problems so far, but I want to run all the plone steps to make sure nothing is broken. It will happen during the next tune up.

If this works, then we can also have "portlets" no more depending on "componentregistry", because "portlets" is already depending on "content", which is indirectly depending on "componentregistry"

[do3cc]

Dependency graph of import steps for 3.3rc3

[do3cc]

I got struck by this while finding out why my import step cannot depend on content. I do not understand the code enough to be able to untangle that dependency mess, but maybe the dependency graphs helps anybody who can.

[alecm]

I'm not sure which of these dependencies if any is needed and why they were added. In particular the dependence of the content step on plone-final seems odd based on nomenclature alone. In any case, the circular dependency has consequences for developers. Effectively, it means that the 'plone-final', 'portlets', and 'content' steps as well as any other steps that depend on those steps directly or indirectly will be run in a random order with dependencies ignored. IMO, this is a significant issue.

[aclark]

I can't reproduce this. I tried creating 'my.package' with import_steps containing 'mandatory-step' and 'circular-step' as described in  https://bugs.launchpad.net/zope-cmf/+bug/314010

Then added 'my-step' via zcml and ran the installer. No errors, am I missing something? My code:

From my.package/my/package/configure.zcml:

  <gs:importStep
      name="my-step"
      handler="my.package.setuphandlers.importVarious"
      title="test"
      description="test">
  </gs:importStep>

From my.package/my/package/profiles/default/import_steps.xml:

<import-steps>
    <import-step id='mandatory-step' version='1' handler='my.package.setuphandlers.importVarious'>
        <dependency step='my-step' />
    </import-step>
    <import-step id='circular-step' version='1' handler='my.package.setuphandlers.importVarious'>
        <dependency step='mandatory-step' />
    </import-step>
</import-steps>

[do3cc]

Yes, there is no exception if the dependency is broken, but they are just added in random order. There should be an error message and there should not be a circular dependency in plone, see my dependency graph earlier.

The problem is in Products.Genericsetup.utils._computeTopologicalSort

[do3cc]

Same graph as svg

[do3cc]

This bug is still an issue in Plone 4. I added a logging message to GenericSetup trunk that emits a warning if there is a problem. The Warning also contains data that can be fed into the dot tool from the graphviz package. It visualizes the dependencies and highlights the problematic parts.

When I added the log, GenericSetup 1.6.2 was current. I assume in 1.6.3 it will be included.

[kiorky]

Also ran into this bug, plone 4.0.5

[kiorky]

I fixed the plonepas step (now registered in zcml and drop the unexistant plonepas-content step). Its commited in r237801 (collective).

Looking if i can do something for the other issue.

[kiorky]

I partially fixed the bug in collective -r13592. In Products.MimeTypesRegistry, i also converted the import step to be registered in ZCML. Without that, if the products isn't installed, we will throw into this cyclic dependencies bug.

Playing around with Plone egg code to fix the last problem with portlet -> content -> plone-final

[kiorky]

Ooops, for MimeTypesRegistry, it's on archetypes repo, of course !

[kiorky]

Also commited a related patch to collective.flowplayer.

[kiorky]

Commited another change in -r48735 on plone.app.upgrade (plone)

[kiorky]

oups, my bad -r48950

[kiorky]

Related commit in -r48959

[kiorky]

Port of -r48950 in -r48960 for plone41.

This bug is now closed.

But, BUT, but.

Be aware of products using legacy import/exports steps.xml files to declare their steps.

If:

• OTHERS products rely on those declared steps
• This product is not specifically installed into your plonesite

that will trigger dependencies problems in the generic setup dependency machinery triggering circular dependencies errors.

You have then two possible fixes:

• Install the product which has the step onto your plonesite to have the step. BAD.
• Fix the product to register the step with a ZCML slug as:

for an import step

  <genericsetup:importStep
      name="plone-difftool"
      handler="Products.CMFDiffTool.exportimport.difftool.importDiffTool"
      title="Diff Tool"
      description="Import CMFDiffTool settings">
  </genericsetup:importStep>

Or for an export step

  <genericsetup:exportStep
      name="content"
      title="Content"
      description="Export the site's structure and content."
      handler="Products.CMFCore.exportimport.content.exportSiteStructure"
      />

And delete the .xml files ;)

Finally, you may also have to purge your portal_setup registries from thoses step with such a migration code as example:

from Products.GenericSetup.registry import _import_step_registry
from zope.app.component.hooks import setSite

logger = logging.getLogger('update Portal Setup Registries')
logger.warn('running')
setSite(plone)
setup_tool = plone['portal_setup']
irs = setup_tool.getImportStepRegistry(test=True)
rewrite = ["plonepas"]
for registry in irs, _import_step_registry:
    registered = registry._registered
    for key in registered.keys():
        step = registered.get(key)
        if 'plonepas' in key:
            step["dependencies"] = (u'componentregistry', u'memberdata-properties', u'rolemap')
        if not key in rewrite:continue
        logger.warn("Rewriting GenericSetup import step : %s" % key)
        if not isinstance(registry, dict):
            registry.unregisterStep(key)
            registry.registerStep(step["id"], step["version"], step["handler"],
                                  step["dependencies"] , step["title"], step["description"])

[kiorky]

new proposed dependencies

[kiorky]

(In [48968]) another will to fix #8350

[kiorky]

(In [48969]) another will to fix #8350

[kiorky]

(In [48970]) another will to fix #8350

[kiorky]

A better fix in -r48968 -r48969 48970.

[maurits]

(In [48977]) Let the portlets import step depend on the content import step again. Refs #8350.

[maurits]

(In [48978]) Let the content import step (importing the site structure) not depend on plone-final but on typeinfo. Solves circular dependency. Refs #8350.

[maurits]

(In [48980]) Merged r48977 from trunk: Let the portlets import step depend on the content import step again. Refs #8350.

[maurits]

(In [48981]) Merged r48978 from Products.CMFPlone/trunk: Let the content import step (importing the site structure) not depend on plone-final but on typeinfo. Solves circular dependency. Refs #8350.