Re: [PATCH] v2 upgrade process for obsolete options

On 20/10/10 07:09, Alex Rousskov wrote:
> On 10/16/2010 07:22 AM, Amos Jeffries wrote:
>>
>> After Alex suggestion/request the DOC_* comments can be used to
>> hard-code the upgrade process documentation in cf.data.pre instead of
>> cache_cf.cc.
>>
>> In total:
>>
>> One problem we currently have with upgrades is leaving the parser able
>> to avoid its bungled/unknown option message for directives which have
>> been fully removed or massively syntax altered.
>>
>> We are able to handle this for flags and option syntax easily but the
>> parser has been particularly dense and strict on the directives (first
>> word of each line).
>>
>> This patch updates the cf_* and cfgman code to allow a special directive
>> type "obsolete" which pumps old config lines off to a parse_obsolete()
>> function for handling without causing the directives to remain in the
>> publicly visible squid.conf documentation.
>>
>> cf.data.pre has entries added for all the 2.6-3.1 directives I could
>> find that were removed up to 3.HEAD
>>
>> allows DOC_START / DOC_END comments to be written in cf.data.pre
>> describing the upgrade actions that need to be taken. This text is
>> dumped to cache.log verbatim when the configuration option is sighted.
>> If "-k parse" is used it is displayed at debug level 0 otherwise it's
>> displayed at debug level 1. One line indicating a generic "directive X
>> is obsolete" is always displayed at level 0 for backwards compatibility
>> with admin expectations of a high level "bungled" message.
>>
>> After all this text display, parse_obsolete(char*) is called with the
>> option name. This function exists in cache_cf.cc and can be coded to
>> selectivey do more complex handling of the directive. ie for upgrade
>> actions deeper than removal.
>>
>> One useful side-effect was the removal of some LOC: assert(). This
>> allows directives to be configured with no LOC: entry being identical to
>> a previously explicit "LOC: none".
>
> Hi Amos,
>
> I like the overall approach and appreciate the time you took to document
> obsolete options. I am also happy this feature required few code changes.
>
> I am not sure we should add smart upgrading code to src/cache_cf.cc
> except under very unusual conditions. I would remove both TODO clauses
> from the parse_obsolete() body and rely on cf.data.pre text for upgrade
> instructions. I would leave parse_obsolete() API itself, in case we need
> it in an emergency. If somebody wants to automate upgrades, they can
> write a script. This is a weak preference.

Dropped the header_access set.

I feel that the *_concurrency case is one which warrants such exception.
If not done any old configs upgrading will get a surprise when their
concurrent helpers stop working.
I've dumped a loud exception warning and parsed the old config entry as
the master. Overriding any new setting until its manually fixed.

>
>> + Remove this line. Since squid-3.2 configure FTP page display details
>
> s/this line Since/this line. Since/ig
>

Fixed.

> s/Since/Starting with/g [optional]
>
> and add a comma after "Starting with squid-x.y" or "Since squid-x.y".
>

The original is one sentence. I've dropped the whole temporal prefix on
all of them now and left the bare explanations instead.

>
>> +
>> +// if (strcmp(ptr, "obsolete") == 0) {
>> +// /* add to list of entries */
>> +// curr->next = entries;
>> +// entries = curr;
>> +// }
>> +
>
> Remove?
>

Done.

>> - gen_parse_alias (name, alias,entry, fp);
>> + gen_parse_alias(name, alias, entry, fp);
>
> Avoid whitespace changes.
>
>
>> (!entry->loc || strcmp(entry->loc, "none") == 0)
>
> This is repeated a few times. Make it an Entry::none() method and/or
> remove "LOC: none" from .pre?

Unfortunately I was wrong about it being identical. "LOC: none" is
depended on by the earlier parser upgrade attempts. Altering that
involves touching a lot more code.

At present:
  * !entry->loc means there is no parsing function at all.
  * entry->loc == "none" means there is a global parsing function which
does not have a global Config object passed as a parameter.

The HotConf project when we get around to it will have to look at doing
that as it updates both old parsers.

Applied after tweaks to 3.HEAD.

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.8
   Beta testers wanted for 3.2.0.2