PLIP: Remove portal_quickinstaller in Plone 6.0

mauritsvanrees commented 8 years ago

Proposer : Maurits van Rees

Seconder : @hvelarde

Abstract

No longer ship with Products.CMFQuickInstallerTool in Plone 6.0

Motivation

Since Plone 5.1 the add-ons control panel no longer uses portal_quickinstaller. In Plone 6.0, for clarity, we should completely get rid of this old way of installing and managing add-ons.

Assumptions

Plone 5.1 is released with PLIP #1340 merged [done].

Proposal & Implementation

Let's not mention everything here, as we can search for use of portal_quickinstaller in code, but let's only list a few important places.

In Products.CMFPlone:

Remove portal_quickinstaller from required tools in profiles/default/toolset.xml
Remove tests/testQuickInstallerTool.py
Remove QuickInstallerTool.py. Maybe leave a dummy class in place for backwards compatibility if needed.
Remove Products.CMFQuickInstallerTool from setup.py. Same for plone.app.collection and Products.Archetypes, which can probably happen today.

In plone.app.upgrade:

Remove the portal_quickinstaller object from the site when migrating.
Remove Products.CMFQuickInstallerTool from setup.py.
Add dummy implementation of the tool and InstalledProduct so portal_quickinstaller keeps working well enough to support upgrades.
Remove use of portal_quickinstaller from all upgrades, in favor of the new get_installer from Plone 5.1. This may mean removing old upgrades completely. We may want to refuse any upgrades from Plone 5.0 or lower.

In plone.app.testing:

Remove use of portal_quickinstaller from all code, in favor of the new get_installer from Plone 5.1. But that may need no change: we already prefer get_installer if we can import it.

In Products.CMFQuickInstallerTool:

Maybe create a completely stripped down version to support upgrades from older Plone versions, instead of having a dummy version in plone.app.upgrade. But it is probably good to keep a working version around in case an add-on really really needs the QI.
Deliverables
New versions of at least CMFPlone and plone.app.upgrade.
documentation
Risks

Add-ons that rely on CMFQuickInstallerTool should be rare after Plone 5.1. They can add Products.CMFQuickinstallerTool to their setup.py and it should still work. They would need to update their install instructions, because the user would first need to add an instance of portal_quickinstaller.

Participants

Maurits van Rees

Let's first ship Plone 5.1 and then worry about this. But I had parts of this text in a file for PLIP 1340, so I wanted to create this issue based on it.

idgserpro commented 8 years ago

We're not exactly following the full roadmap of Plone 5.1 or 5.2, but shouldn't a deprecation warning be added in 5.x telling about this removal? Do these deprecation warnings already exist?

hvelarde commented 8 years ago

@idgserpro is all in here #1340

ebrehault commented 7 years ago

@mauritsvanrees the FWT discussed this PLIP today. We think it is interesting, and we would like to know if you are stil volunteering to work on that (as you submitted one year ago).

mauritsvanrees commented 7 years ago

Yes, I am still volunteering.

mauritsvanrees commented 6 years ago

On the way home in the train from the Alpine City Sprint I wanted to work on something that did not interfere with the sprint topic. So I made preparations for this PLIP, mostly updating code that used portal_quickinstaller to use get_installer instead, or have the QI only as backwards compatibility code. I made several pull requests that should be fine to merge already in Plone 5.2 or earlier:

Some more that could be done already before Plone 6:

[x] Make a GenericSetup profile for Products.Marshall. I thought I did that already, but apparently not. https://github.com/plone/Products.Marshall/pull/6
[x] Products.PlonePAS has an Extensions/Install.py with only some backwards compatibility imports used by borg.localrole and plone.app.upgrade. It does not block anything, but would be good to clean up. https://github.com/plone/Products.PlonePAS/pull/31
[x] plone.app.multilingual has an Extensions/install.py for uninstall, but it calls an uninstall profile there. That code can be safely removed, because uninstall profiles are already picked up automatically. https://github.com/plone/plone.app.multilingual/pull/301
[x] plone.app.testing loads the zcml of CMFQuickInstallerTool. This can be done conditionally. https://github.com/plone/plone.app.testing/pull/47
[x] plone.app.upgrade: remove from setup.py and use INonInstallable from CMFPlone. https://github.com/plone/plone.app.upgrade/pull/150
[x] plone.app.upgrade: the previous line needs a slightly better fix. https://github.com/plone/plone.app.upgrade/pull/151
[x] Take over some tests from CMFPlone/tests/testQuickInstallerTool.py in a new file and let them test the get_installer. https://github.com/plone/Products.CMFPlone/pull/2298 and https://github.com/plone/Products.CMFPlone/pull/2299
[x] Copy methods and tests from CMFPlone/QuickInstallerTool.py to Products.CMFQuickInstaller. https://github.com/plone/Products.CMFQuickInstallerTool/pull/18
[x] Remove the methods and tests of the previous point from CMFPlone/QuickInstallerTool.py. https://github.com/plone/Products.CMFPlone/pull/2304

TODO when we really want to get rid of the CMFQuickInstallerTool:

[ ] CMFPlone: remove from setup.py
[ ] CMFPlone: remove from factory.py getNonInstallableProfiles
[ ] CMFPlone: remove two permission checks from testSiteAdminRole.py (although nothing goes wrong if we leave it)
[ ] CMFPlone: remove QuickInstallerTool.py (or maybe leave a BBB), also from __init__.py. It has three extra methods, which are moved to get_installer or no longer needed. It could inherit from CMFQuickInstaller if the package is there, and else probably inherit from SimpleItem. It needs no methods. I should test this theory.
[ ] CMFPlone: from profiles/default/toolset.xml remove <required tool_id="portal_quickinstaller" class="Products.CMFPlone.QuickInstallerTool.QuickInstallerTool"/>
[ ] CMFPlone: remove from tests, especially tests/testQuickInstallerTool.py, and test_zmi.py: test_portal_quickinstaller
[ ] CMFPlone: remove from setuphandlers.py assignTitles.
[ ] plone.app.upgrade: add code to remove portal_quickinstaller. Maybe rerun some old upgrade steps for syncing qi and portal_setup? No, not needed.

Initial cleanup in CMFPlone is in branch remove-qi-completely-master which builds on branch remove-qi-additions-and-tests-master.

So when the preparation is done correctly, the removal is really just that: remove several lines in Products.CMFPlone and plone.app.upgrade.

[Note: I am editing this comment as I go along.]

mauritsvanrees commented 6 years ago

Analog to https://github.com/plone/Products.CMFPlone/issues/2390 I would say that Products.CMFQuickInstallerTool should not be ported to Python 3.

tisto commented 5 years ago

@plone/framework-team is this PLIP accepted?

tisto commented 5 years ago

@mauritsvanrees your PLIP has been approved by the framework team and planned for Plone 6. Please go ahead with the implementation.

mauritsvanrees commented 4 years ago

I did the work for this today. See the plip config.

We would need a PLIP job for that. @gforcada Could you arrange that? Or is there someone else who can do this?

Alternatively I could do this on a branch of coredev. That is easy as well. But it may take a while before this is reviewed, making it harder to keep the branch up to date.

gforcada commented 4 years ago

@mauritsvanrees jobs are there: https://jenkins.plone.org/view/PLIPs/ give them a try and let me know if they are broken :sweat_smile:

mauritsvanrees commented 4 years ago

Thanks @gforcada. Seems to work. Well, I only tried the 3.6 job, and that has a few errors, but that is up to me to fix. The Python 2.7 job is not needed, so could be removed.

mauritsvanrees commented 4 years ago

The three PLIP jobs are green.

mauritsvanrees commented 4 years ago

For documentation this should be enough:

The portal_quickinstaller tool is longer added to new sites. In existing sites, it will be removed during upgrade. For installation code in an add-on you should no longer create an Extensions/install.py file, but create a GenericSetup profile, as has been the norm for years.

In Plone 5.1 deprecation warnings were added for no longer supported methods. These methods have now been removed.

The best source of information for dealing with this change, is the upgrade guide for add-ons from Plone 5.0 to 5.1: https://docs.plone.org/develop/addons/upgrade_to_51.html#installation-code

mauritsvanrees commented 4 years ago

This has been ready for review since three months. I probably need to rebase the various branches by now, and can do so when asked. But what is the next step? Does someone from the @plone/framework-team need to step up as champion?

ale-rt commented 4 years ago

I am in charge of reviewing it, sorry for the delay.

I actually started to review this in the weekend but I was distracted by https://github.com/plone/plone.app.theming/pull/189 in the process.

I am writing a document on that that I will present (when ready) to the next framework team meeting. The code changes look good (IIRC there where some commented code in plone.app.upgrade that could be removed but that is just a minor thing). Creating a new Plone 6 site with this PLIP cfg already does not show the portal quickinstaller. Installing/uninstalling add ons was working.

I plan to do some other manual tests (like taking a Plone 5.2 site, upgrading it and verifying portal_quickinstaller is gone and the site works). When this done I will present the document in the FWT meeting and ask for a green light.

So for this reason I would not rebase all the branches right now.

jensens commented 4 years ago

This one is ready for merge. @mauritsvanrees I think best is you merge it yourself. Or are any takers here?

mauritsvanrees commented 4 years ago

I should first rebase or merge master into the various branches, like the PLIP review says, and do final Jenkins runs before merging.

mauritsvanrees commented 4 years ago

I am cherry-picking from the older branch from this PLIP into new branches so we have the latest code everywhere. I think all four packages can be tested and merged separately. Note that the master branch for all except Products.CMFPlone is used on Plone 5.2, so the tests should remain working there as well.

PRs :

mauritsvanrees commented 4 years ago

https://github.com/plone/Products.CMFPlone/pull/3189 gave problems, but with the extra changes in https://github.com/plone/buildout.coredev/pull/683 it was okay. I have merged all. I have opened an issue for documentation.

When Jenkins stays green, this PLIP is done.

mauritsvanrees commented 4 years ago

All green. Done.

plone / Products.CMFPlone