[Zope-CMF] GenericSetup: Apply profile dependencies only once

[yuppie]

Hi Maurits,

Maurits van Rees wrote:
> Op 25-09-15 om 10:31 schreef yuppie:
>> But I hope these assertions are true:
>>
>> - a profile that depends on more than one base profile is broken anyway
> 
> Agreed.
> 
>> - if there is a base profile in the chain, it is always the first in the
>> chain
> 
> Not necessarily, though I do hope so.
> 
> I am expecting that the base profile is the first and only profile in
> the chain.
> 
> But in the tests I explicitly test what would happen if the base profile
> itself has a dependency, although this makes no sense to me.  In this
> case the base profile would be the second in the chain.

why are you testing features that don't make sense to you? Are the names
BASE and EXTENSION not clear enough? Why would someone expect that you
can use an EXTENSION profile as the base and extend it with a BASE profile?

> Then the purge
> of all versions should happen right before applying the base profile, so
> after its dependency profile.

Why would someone want to import the first profile in the chain (version
is set automatically) and remove the version data again in the next step?

>> So it might be ok to purge versions inside the loop. But I don't think
>> it makes sense to purge versions if we don't reapply the base profile in
>> purge mode. I would make the change after the BeforeProfileImportEvent.
> 
> Problem may then be that this part of the code is never reached. Between
> those two spots are the checks to see whether the profile that is about
> to be imported has already been applied previously.  And we use the
> profile upgrade versions for this.  When a base profile is in this way
> applied a second time, the checks would conclude it has already been
> applied and will continue with the next profile in the for loop, without
> purging the versions.

Why would an upgrade step (re-)apply a complete base profile in purge
mode? Why should we purge the versions if the code that purges the old
configuration is never reached?

>> In that place it should be possible to use the shouldPurge method
>> instead of checking the profile type. Or is anyone running extension
>> profiles in purge mode? In that case we have to check for both.
> 
> Let me think.
[...]
> So I would say: we purge the profile upgrade versions only if a base
> profile is imported, and should_purge is None or True.  I have updated
> the pull request with that.
> 
> Does that sound reasonable?

Well, I would say:

There are several ways to use the import machinery. Only a few ways
should normally be used. All other options are some kind of expert mode
that allows to (re-)apply selected parts of all kinds of profiles. You
should use those options only if you know exactly what you do.

I started the discussion about purging versions automatically. And I
think we should do that only in the normal use case where someone
obviously wants to start from scratch. And that's the case if we are in
purge mode and (re-)apply a "complete" profile. Base profiles, snapshots
and tarballs are usually complete profiles. But snapshots and tarballs
don't care about versions and upgrades, so it might be ok to ignore them.

Your updated pull request still purges versions if we depend on a base
profile, but don't (re-)apply it. That's not what I would expect.


Cheers,

	Yuppie