intersystems / ipm

InterSystems ObjectScript Package Manager
MIT License
29 stars 19 forks source link

Some resource types require custom unconfigure code #616

Open janihur opened 1 week ago

janihur commented 1 week ago

I'm not sure if this is a bug or just a normal IPM pattern (if it is a valid pattern then it should be described in the wiki I guess).

I noticed a global imported in manifest:

<Resource Name="OSEX.ipm.demo.GBL"/>

Is not automatically removed during uninstall unlike the package. However IMO the problem here is asymmetry (different resource types behave differently) is not optimal developer experience.

The obvious (?) workaround is the module has to provide it's own cleanup/unconfigure code:

<Invoke Phase="Unconfigure" Class="OSEX.ipm.demo.Main" Method="Unconfigure"/>

Further pondering:

<Resource> do have ProcessorClass attribute that seems a useful concept (i.e. resource processors). Unfortunately thanks to my subpar ObjectScript skills I don't have a clear picture how to write your own. The relevant classes in (0.7.x) seems to be:

janihur commented 1 week ago

Managed to write my first resource processor! AFAICS a custom resource processor can't be used to redefine a single phase of an existing resource processor (because how OnBeforePhase(), OnAfterPhase() and OnPhase() have been defined in %ZPM.PackageManager.Developer.Processor.Abstract). It's not absolutely impossible but the custom processor needs to consider and potentially reimplement the original processor's functionality.

In the long term it probably would be a good idea to make redefinition of a single step easier.

isc-tleavitt commented 1 week ago

@janihur you're right that .GBL is a bit of an outlier. Not killing globals on uninstall by default is a feature, IMO. But it would be reasonable to have a flag in the Global resource processor to make it easy to do that.

Two reasons we shouldn't kill globals on uninstall by default:

In resource processors it's common to override On.*Phase and call ##super() in cases other than the one phase of interest.

janihur commented 1 week ago

override On.*Phase and call ##super() in cases other than the one phase of interest.

Yes, I can see that now 🤦‍♂️


if (pPhase = "Configure") {
  // my stuff here ...
  return $$$OK
} else {
  return ##super(pPhase,.pParams)
}
isc-tleavitt commented 1 week ago

Looking at %IPM.ResourceProcessor.Default.Global there's code that lets you provide the global name, and if you do it'll be killed on uninstallation. The resource name just ends up being used as a path to the file.

This needs to be documented.