Uninstall Profile would encourage good practise

[djowett]

Hi, I'm willing to do some work on this, but before I go to far, can I test opinion here?

I think it would be good to have uninstall profiles by default (at least when gs profiles are asked for). This would help people create products that really cleanly uninstall. And is especially useful as there are3 or 4 "hoops" to jump thru to get uninstall profiles working which folk won't bother with figuring out. I also would plan to add a test that checks that installing & uninstalling a product leaves the same (number of):

• Skin layers
• Browser layers
• CSS files in registry
• JS files in registry

I guess this won't work in all cases, but it would be a good baseline to further enourage clean uninstalls, right?

Your thoughts?

[djowett]

I should add ... as far as I know, in order for an uninstall profile to work the changes in ab2eb80 will need to be reverted. Correct me if I'm wrong....

[tisto]

An uninstall profile is nice to have. It is important that removing a package does not break anything.

Though, if we need to add lots of code that confuses people, I tend to be against it. A clean package layout that is easy to understand for people is much more important than an uninstall profile that removes every trace of the package.

Do we have an example package that does a full clean uninstall like the one you described above?

[djowett]

I have the same aversion to lots of code that confuses people - unfortunately, an uninstall profile requires a few more changes than it should, but that's what we have in Plone at the moment. By my recokoning that's changes in:

• /__init__.py
• /configure.zcml
• /Extensions/Install.py
• /profiles/uninstall/...

This is meant to be a good example: https://github.com/keul/example.gs

[davisagli]

The reason an uninstall method in Extensions/Install.py (and therefore being a zope2 product) is necessary is that the quick installer doesn't look for a profile named uninstall. That would not be too hard to fix.

[gforcada]

For the time being, we could add a comment on configure.zcml and/or init.py that points to this example.gs for instructions on how to create an uninstall profile.

[thet]

+1 for an default uninstall profile. IMO, it's necessary for a package template, which wants to show off good coding practice. many packages, including some of my own, are lacking uninstall profiles. removing a package and not having the browserlayer unregistered leaves your site broken, for example.

+1 for fixing the quickinstaller along the way! if not, i wouldn't bother that the quickinstaller doesn't actually use an uninstall profile, as long as the docs say that you have to manually import it from portal_setup.

[pbauer]

I agree with @thet but would also really like to see Products.CMFQuickinstaller automatically run an uninstall-profile to not have to add Extensions/install.py. The addon-story in Plone will improve if more have sane default uninstall-profiles.

[djowett]

+1 (or +2 if I can) for fixing uninstaller, though unfortunately, unless that fix is backported a lot, we will still need the Extensions/install.py for any product that could be installed in older Plone 4 versions.

@gforcada I think more than a comment is required. Same as we add codeanalysis rather than a comment about PEP8 :-)

[gforcada]

@djowett of course, my idea was mostly around @tisto's approach to not bloat the structure with files that you don't understand/probably you will not need anyway :)

[tisto]

We can either have full backwards compatibility or make this package reflect our current best practice. It is just impossible to have both. I would want bobtemplates.plone to be a clean package, that reflects our current best practice. It should be a good basis for Plone 5 development, not cover every possible use case. mr.bob gives us the chance not to repeat mistakes from the past. The mr.bob approach is to be minimalistic and not to try to solve all use cases. The same should be true for our templates in my opinion.

[djowett]

But even your Plone 5 packages may want to be backwards compatible. I think you'll find others in the community not being able to move so fast.

In any case, I think you mean "I would want the output from bobtemplates.plone to be a clean package. Right?

We could add a "support Plone 4?" question which if answered negatively could strip out the backwards compatible cruft. Obviously that qu only has to be answered if is_plone5 = True.

[tisto]

Hey @djowett: what about starting with fixing the quickinstaller as @davisagli suggested? Maybe we can even backport this fix to older Plone versions. If we have that in place, we can discuss the best way to fix the uninstall profile in bobtemplates.plone.

@davisagli Do you have any pointers for us to make it easier to get started?

[pbauer]

Products.CMFQuickInstallerTool.InstalledProduct.InstalledProduct.uninstall is the place where this has to happen if reinstall is False

Questions:

• Should we only run the uninstall-profile if getUninstallMethod did not find a method in Extensions/Install.py?
• Should we skip the _cascadeRemove if there is a uninstall-profile? I think what _cascareremove does is pretty crazy...

[djowett]

Hey, I've added a branch so you can see what I had in mind. Use it, scrap it, as you like, but at least I got it off my chest....

@tisto - sorry I won't have time at the moment to get into that, especially if it brings up deep questions as raised by @pbauer just now

[djowett]

Btw, the test_browserlayers_removed test is broken - best examples I can find of something similar that works are:

• https://github.com/collective/collective.nitf/blob/master/src/collective/nitf/tests/test_setup.py
• https://github.com/plone/plone.browserlayer/blob/master/plone/browserlayer/README.txt

but I can't replicate their success.

The only thing I spotted was that a Browser Layer interface can have different superclasses - I wonder which one is really correct? Again see:

• https://github.com/collective/collective.nitf/commit/752266edd8ef736cd963e76b9aaf3abc1972616d
• https://github.com/plone/plone.browserlayer/blob/master/plone/browserlayer/tests/interfaces.py

[davisagli]

Re @pbauer's questions:

• Yes, the the uninstall profile should be used only if there's no uninstall method in Extensions/Install.py (this matches how we already handle the default profile for installation).
• I actually like _cascadeRemove and the fact that it's smart enough to automatically remove some things without having to write a special uninstall profile. In the cases where it doesn't do the right thing or doesn't handle enough things, we should try to fix that. Better to write and test code once to automatically remove browser layers, for example, than require every add on to do so in its uninstall profile AND need to maintain sample code for that in this template. My 2 cents, anyway.

[djowett]

"Better to write and test code once to automatically remove browser layers, for example, than require every add on to do so in its uninstall profile AND need to maintain sample code for that in this template."

Absolutely, but if Plone doesn't do the job properly now, then IMHHO it would be good to add that to uninstall profiles of addons intended to work in current & older Plone versions.

[do3cc]

I side with @tisto here, if it adds a lot of logic to the template, I am against it. This all needs to be maintained for years. Backward compatible fixes have the tendency to never get removed and to add a LOT of maintenance burden over time.

Please keep in mind that it is easy with mr.bob to use templates from git directly. We can easily have a branch with Plone 4.3 compat and a branch with Plone 5. Then the burden is shifted to those who actually need it.

[gforcada]

As @mauritsvanrees made uninstall profiles more than a nice to have (and actually is already in bobtemplates.plone) this can be closed.