Products.cmfplone: PLIP: Get rid of portal_quickinstaller

[mauritsvanrees]

Proposer : Maurits van Rees

Seconder : @hvelarde

Abstract

No longer support installing via Extensions/install.py.

Motivation

There are two ways of installing products: GenericSetup and CMFQuickInstaller. Checking if a package is currently installed, means having to check portal_setup and portal_quickinstaller, or having code that makes sure those two are in sync. The syncing is mostly in place, with some recent changes, but supporting both remains tricky. Cleanup would make this code much easier to understand.

Assumptions

Some core packages need to actually get a GenericSetup profile instead of their current install.py (for example Products.Marshall). Some need to get an uninstall profile.

Proposal & Implementation

• Update prefs_install_products_form browser view to only handle GenericSetup profiles.
• Register this view also as @@installer for easier remembering.
• Possibly create a utility instead that handles the basic operations.
• When installing a product, use its default profile, instead of using the first profile found.
• Warn before install when a product has no uninstall profile.
• Change any core code that uses the portal_quickinstaller (mostly plone.app.upgrade and tests).
• CMFQuickInstaller code should remain in Plone 5.x, but possibly stripped down and with deprecation warnings. Remove it in Plone 6. (plone.app.upgrade can get bare bones module patches to avoid possible breakage.)

• Document what third party add-ons should do.

  Deliverables

• CMFPlone branch

• plone.app.upgrade branch
• CMFQuickInstaller branch
• Branches for core packages that have no default or no uninstall profile yet.

• Documentation

  Risks

• Third party add-ons that have no profile, are no longer installable.

• We lose the automatic partial cleanup that CMFQuickInstaller does on uninstall (remove added skins, css, etc). The 'partial' part of this may be a benefit: a proper uninstall profile is now required instead of add-ons relying on the default cleanup being good enough.
  

  Participants

Maurits van Rees

Note that CMFQuickInstaller is Plone specific, in spite of the CMF in the name. So compatibility with CMF is not an issue.

See http://sourceforge.net/p/plone/mailman/message/34390699/ for small initial discussion in August 2015.

[mauritsvanrees]

Working on this at the Alpine City Strategic Sprint 2016 in Innsbruck, Austria.
WIP on CMFPlone branch get-rid-of-qi:
https://github.com/plone/Products.CMFPlone/tree/get-rid-of-qi
The prefs_install_products_form on that branch no longer uses portal_quickinstaller.

[hvelarde]

+1 I think is clear, implemented with enough time in advance, documentation, deprecation warnings and so on; makes a lot of sense.

@keul thoughts? in order to run Python code, are import steps in setuphandlers.py easy and/or powerful enough in your opinion?

[keul]

No, I'm -1 about this.

Setuphandlers are powerful, I always use them, but aren't they related to an installation already running? On the other hand install.py is something can be run at the very beginning, maybe checking Python version (OK, not so useful anymore nowadays) or trying to perform some import assumption.

However I like the fact that magic uninstall operation done by quickinstaller will gone.

[mauritsvanrees]

@keul: You mean you sometimes need to run some code before applying the first import step? I have not needed that so far, but I can imagine it. Fair point. That could easily be handled by this GenericSetup pull request I created earlier:
https://github.com/zopefoundation/Products.GenericSetup/pull/24
I created that for the post_handler, but the pre_handler would help you.

[keul]

@mauritsvanrees yep! This can help (hoping to see this merged)

[hvelarde]

@mauritsvanrees what about reinstalls?

[mauritsvanrees]

Reinstalls are not supported. Basically that is already true in the current add-ons control panel.

But with the new setup a reinstall would really just be an uninstall and install, which is easy to do in an add-on if you need it. The old quick installer had a specific reinstall because this did not remove any portal objects that the install had added.

[mauritsvanrees]

Added plip config in coredev:
https://github.com/plone/buildout.coredev/blob/5.0/plips/plip1340-get-rid-of-qi.cfg

[gforcada]

And a jenkins job for it: http://jenkins.plone.org/view/PLIPs/job/plip-1340-QuickInstaller/

[mauritsvanrees]

Ah, sweet, thanks.

[ebrehault]

@mauritsvanrees, your PLIP has been approved by the Framework team, and @frapell is the one in charge of review your PR once it will be ready.

[mauritsvanrees]

Note to self. Todo: hidden products are no longer hidden:

• Products.CMFUid (which is marked as not installed, probably because it has no metadata.xml)
• plonetheme.barceloneta
• plone.app.caching
• plone.app.intid

[frapell]

@mauritsvanrees Do we have something analogue to the "isProductInstalled" we have in qi ? this is very helpful, and I remember having issues to know if a specific profile for a package was ran or not (for packages that include several profiles)

[mauritsvanrees]

With the new setup, we still have an is_product_installed method (or isProductInstalled as deprecated alias). It gets the default profile of the product. It then checks the only canonical place left: portal_setup. If the default profile has a last version other than unknown then we say it is installed.

This last check is done with a single line that does a call to portal_setup, but for ease of use I have added this as method is_profile_installed on the installer view. You can pass this a full profile id and get back True or False.

[mauritsvanrees]

Random idea out of scope for this plip.

Situation:

• Install product A. This depends on profile B.
• Uninstall product A. Profile B remains installed: uninstall routines for package A do not usually run an uninstall for profile B, because profile B may be required by other packages.

If the author of product A is really sure, he may add a metadata.xml in his uninstall profile that depends on the uninstall profile of package B.

Practically speaking: ATContentTypes depends on a profile from Archetypes, but when uninstalling ATContentTypes we are probably not going to run an uninstall profile for Archetypes. Well, in this case we might actually do it: if someone uninstalls ATCT there is no good reason to keep Archetypes installed.

An idea may be to add a cleanup control panel. It may be a part of the add-ons control panel. Or it may be more general, where it can also be a third party add-on (maybe part of ftw.upgrade). For our use case here: add a form that lists all products that are:

• hidden from the add-ons control panel (INonInstallable)
• currently installed
• no other currently installed profiles depend on them

Just a few half baked ideas, but if someone wants to take this on, it may be good.

[hvelarde]

I have another idea/request: fire a couple of new events on add-on install and uninstall; if someone give us a hand, we can dedicate some resources to this.

[mauritsvanrees]

This is largely finished. I will need to change plone.app.testing to use the new get_installer instead of QI. And probably I will discover a few more things that need to be changed.

Meanwhile, I would like some feedback on how much deprecation is needed.

For example, the old QI has isProductInstalled. In the new one you should use is_product_installed. But I have added isProductInstalled as allowed name for backwards compatibility. It does roughly the same as before, but there are differences. And all keyword arguments are ignored.
Is this useful? Or confusing?
If someone does getToolByName(context, 'portal_quickinstaller') then the old name is still there and does the exist same as before. That may be the best backwards compatibility.
See this plone.app.upgrade commit for some examples of what needs to be changed in code currently using QI:
https://github.com/plone/plone.app.upgrade/commit/93b13114fb6cedf081fccc4e46ab92cd050f3f55
Without the extra backwards compatibility in get_installer we would need a bit of extra code there, using either the old or the new name, but it is not hard. We already need to do slightly different things.

My recommendation: let's remove isProductInstalled and friends from the new installer. It's cleaner.

See https://github.com/plone/Products.CMFPlone/blob/get-rid-of-qi/Products/CMFPlone/GET_RID_OF_QI.rst which shows this and other changes. This file is a rough sketch for a documentation update.

It also has an idea for Plone 5.0 that I would like feedback on: add the new get_installer function and let it return the old portal_quickinstaller with getToolByName.

• Good: this way you can use get_installer in all Plone 5 versions (well, starting from 5.0.something). Use get_installer and the deprecated methods and it will work in 5.0 and higher.
• Bad: then the availability of get_installer does not tell you whether you get the old tool or the new view. If you try to use the new methods, they will fail because they do not exist.

I came up with this idea, but my recommendation now is: let's not add this function in Plone 5.0.

@hvelarde I thought you listed yourself as seconder, but now I don't see this anymore. Maybe I remember wrongly. But do you have feedback on my questions?
Feedback by others is welcome too of course. And a seconder. :-)
BTW, I am offline for the next week.

[hvelarde]

@mauritsvanrees sorry about that; let me know how can I help you.

[jensens]

This PLIP is officially under review by the FWT now.

[frapell]

I added my review in https://github.com/plone/buildout.coredev/blob/5.1/plips/reviews/plip1340-review-frapell.rst

I left two suggestions about consistency in the code and a very minor addition to documentation.

Other than that, +1 for merging, it looks fantastic

[mauritsvanrees]

@frapell Thanks for the review. BTW, the review has the title of a different plip. :-)

I have updated the CMFPlone code to use the get_installer function where needed. And I have added a sub section on deprecating upgradeProduct.
I have rebased all branches to pull in the latest changes from master and have started a new run of the plip job: http://jenkins.plone.org/view/PLIPs/job/plip-1340-QuickInstaller/5/

[mauritsvanrees]

BTW, on the CMFQuickInstaller branch I have a commit where I comment out two event subscribers. The events notified the old QI that a profile has been installed, so that the QI can register some information about, and register the product as installed.

My intention is to actually _not_ merge that commit. The commit is only there to show that these events are no longer needed in Plone. But to me it seems nicer to keep the events: add-on authors can then still rely on them to work the same way as in Plone 5.0.

[frapell]

@mauritsvanrees Thanks! it is fixed now, I used the other one as template :P

[jensens]

After FWT meeting we decided this one is ready for merge. Thanks @frapell for the review and many more thanks @mauritsvanrees for the idea and implementations.

I'am going to merge now, just need to figure out what :D

[hvelarde]

may I ask where is the documentation before you merge and close this?

[jensens]

Glad you are asking. I found a few occurrences at https://github.com/plone/documentation/blob/5.0/develop/addons/components/genericsetup.rst which need some love. I'll open an issue at the documentation tracker after merge.

[mauritsvanrees]

Good news! Thanks for the reviews.

The documentation is currently a text file in the CMFPLone repo. I intend to use that as basis for an update to the docs. You can open an issue and assign it to me. Note that during the creation of this PLIP I already updated the genericsetup.rst doc to describe the current situation, and mostly the various xml files.

Jens, on what to merge:

• see the plip config file
• but don't merge the change in the QI repo as that was just for showing that the event is not needed anymore in core. It is best if the QI for the moment does remain working as is if people explicitly want to use the ZMI for their addon.

[jensens]

Note to self:

plip config at https://github.com/plone/buildout.coredev/blob/5.0/plips/plip1340-get-rid-of-qi.cfg

So we have those merges:

• [x] Add uninstall profile plone/Products.CMFFormController#4
• [x] Add uninstall profile plone/Products.Archetypes#54
• [x] Added uninstall profile plone/plone.session#7
• [x] Added uninstall profile plone/Products.ATContentTypes#32
• [x] Use get_installer and make profile upgrade versions persistent plone/plone.app.testing#29
• [x] Get rid of qi plone/Products.CMFQuickInstallerTool#15
• [x] Get rid of qi plone/Products.CMFPlone#1772

Also all GenericSetup changes were merged already.

Do not forget to create an issue in the docs repository.

[jensens]

@mauritsvanrees if you like to merge this yourself, I'am fine. You know better than I....

[mauritsvanrees]

Yeah, I might as well merge it myself. Will do.

[mauritsvanrees]

As basis for the docs, I have copied the GET_RID_OF_QI.rst from the CMFPlone plip branch to this new docs branch:
https://github.com/plone/documentation/tree/plip-1340-deprecate-qi

[mauritsvanrees]

The PLIP job is happy, but I want to do a final check before merging.

Two pull requests should be fine for Plone 5.0:

• https://github.com/plone/plone.app.testing/pull/29
• https://github.com/plone/plone.app.upgrade/pull/86

I test them in combination here: http://jenkins.plone.org/job/pull-request-5.0/1434/

For Plone 5.1 we have the same, plus two extra, for which the packages in 5.0 already use other branches:

• https://github.com/plone/plone.app.testing/pull/29
• https://github.com/plone/plone.app.upgrade/pull/86
• https://github.com/plone/Products.ATContentTypes/pull/32
• https://github.com/plone/Products.CMFPlone/pull/1772

I test them in combination here: http://jenkins.plone.org/job/pull-request-5.1/630/

[mauritsvanrees]

The 5.1 job succeeded, the 5.0 one had a few failures that should be easy to fix. Doing that now.

[mauritsvanrees]

The 5.0 job passes too.

But I should test https://github.com/plone/plone.app.upgrade/pull/86 on Plone 4.3 as well: http://jenkins.plone.org/job/pull-request-4.3/406/

[mauritsvanrees]

4.3 passes too. Merging all now.

[mauritsvanrees]

And it's a wrap!

Handling the documentation here: https://github.com/plone/documentation/pull/696