collective / collective.roster

Personnel Roster
Other
1 stars 4 forks source link

Cannot install on Plone 4.3 (also 16 to 17 upgrade broken) #6

Closed adaugherity closed 7 years ago

adaugherity commented 7 years ago
Traceback (innermost last):
  Module ZPublisher.Publish, line 138, in publish
  Module ZPublisher.mapply, line 77, in mapply
  Module ZPublisher.Publish, line 48, in call_object
  Module Products.CMFCore.FSPythonScript, line 127, in __call__
  Module Shared.DC.Scripts.Bindings, line 322, in __call__
  Module Shared.DC.Scripts.Bindings, line 359, in _bindAndExec
  Module Products.PythonScripts.PythonScript, line 344, in _exec
  Module script, line 11, in prefs_reinstallProducts
   - <FSPythonScript at /my/site/portal_quickinstaller/prefs_reinstallProducts>
   - Line 11
  Module Products.CMFPlone.QuickInstallerTool, line 100, in upgradeProduct
  Module Products.GenericSetup.upgrade, line 194, in doStep
  Module Products.GenericSetup.tool, line 349, in runImportStepFromProfile
  Module Products.GenericSetup.tool, line 1226, in _doRunImportStep
   - __traceback_info__: plone.app.registry
  Module plone.app.registry.exportimport.handler, line 49, in importRegistry
  Module plone.app.registry.exportimport.handler, line 94, in importDocument
  Module plone.app.registry.exportimport.handler, line 281, in importRecords
  Module zope.dottedname.resolve, line 38, in resolve
ImportError: No module named IBundleRegistry

Obviously the step itself (added in 07ac1008) is Plone 5 specific, but this product still supports 4.3, right? Probably this step should be flagged as "only for Plone 5" so the control panel doesn't keep prompting me to do an upgrade which will fail.

I guess a corner case is if this is fixed like this (so the 16/17 step is ignored in Plone 4), but I later upgrade the site to Plone 5, would I need to manually run the step afterwards?

datakurre commented 7 years ago

Thanks for the report.

Alternative would be to use https://pypi.python.org/pypi/plone.app.registry/1.5, which supports the condition in the registry.xml. Would that work for you?

adaugherity commented 7 years ago

The standard Plone buildout pins plone.app.registry to 1.2.4, which I guess is too old. Is version 1.5 compatible with Plone 4.3? (I'm guessing no, since plone/plone.app.registry/README.rst says "1.3.x versions are for Plone 5. 1.2.x versions are for Plone 4.")

datakurre commented 7 years ago

That's surprising, because feature in question was supposed to help keeping products 4.3 compatible. Thanks for reporting. I'll try to backport the changes to earlier registry version.

On 17. helmikuuta 2017 klo 22.10 +0200, Andrew Daugherity notifications@github.com, wrote:

The standard Plone buildout (http://dist.plone.org/release/4.3-latest/versions.cfg) pins plone.app.registry to 1.2.4, which I guess is too old. Is version 1.5 compatible with Plone 4.3? (I'm guessing no, since plone/plone.app.registry/README.rst says "1.3.x versions are for Plone 5. 1.2.x versions are for Plone 4.")

— You are receiving this because you commented. Reply to this email directly, view it on GitHub (https://github.com/collective/collective.roster/issues/6#issuecomment-280753836), or mute the thread (https://github.com/notifications/unsubscribe-auth/AAJyv1bNJPg-PAOrlK9Sj61ZdHi9OQI8ks5rdf6lgaJpZM4MDsL_).

adaugherity commented 7 years ago

It's actually worse than I thought -- you can't even install collective.roster into a Plone 4.3 site:

Traceback (innermost last):

    Module ZPublisher.Publish, line 138, in publish
    Module ZPublisher.mapply, line 77, in mapply
    Module ZPublisher.Publish, line 48, in call_object
    Module Products.PloneHotfix20160830.redirect_qi, line 14, in QuickInstallerTool_installProducts
    Module Products.CMFQuickInstallerTool.QuickInstallerTool, line 686, in installProducts
    Module Products.CMFQuickInstallerTool.QuickInstallerTool, line 603, in installProduct
    __traceback_info__: ('collective.roster',)
    Module Products.GenericSetup.tool, line 379, in runAllImportStepsFromProfile
    __traceback_info__: profile-collective.roster:default
    Module Products.GenericSetup.tool, line 1414, in _runImportStepsFromContext
    Module Products.GenericSetup.tool, line 1226, in _doRunImportStep
    __traceback_info__: plone.app.registry
    Module plone.app.registry.exportimport.handler, line 49, in importRegistry
    Module plone.app.registry.exportimport.handler, line 94, in importDocument
    Module plone.app.registry.exportimport.handler, line 281, in importRecords
    Module zope.dottedname.resolve, line 38, in resolve

ImportError: No module named IBundleRegistry 

As you can see, the last part of the traceback (and, presumably, the underlying cause) is the same. AIUI, the version of plone.app.registry used in Plone 4.3 doesn't support the condition="have plone-5" syntax so it tries to import the stuff in profiles/default/registry.xml anyway, right?

If I pin to 4bce0e3 (the last checkout before registry.xml was added), then I am able to install collective.roster once more.

I came across https://github.com/plone/Products.CMFPlone/issues/1406 which seems to indicate that this condition stuff was only recently added (mostly by you), and not yet backported to the 1.2.x branch of plone.app.registry?

datakurre commented 7 years ago

I backported registry conditions for plone.app.registry 1.2.x. Once those are merged and released, I update setup.py to require at least that version of plone.app.registry. As you said, an alternative fix would be to have different profiles for Plone 4 and Plone 5, but I'd choose the registry fix, because that was supposed to fix this exact compatibility issue.

https://github.com/plone/plone.app.registry/pull/25

adaugherity commented 7 years ago

I see that PR was accepted, so I added to my [sources] section:

plone.app.registry = git https://github.com/plone/plone.app.registry.git branch=1.2.x

and now collective.roster (2.1.2, latest from PyPI) installation works fine, as do the upgrade steps. Nice work! 👍

adaugherity commented 7 years ago

FYI, plone.app.registry v1.2.5 has been released with that fix, and I have tested that it works.