You are viewing a plain text version of this content. The canonical link for it is here.
Posted to modperl@perl.apache.org by Rici Lake <ri...@ricilake.net> on 2004/09/17 22:38:06 UTC
Fwd: [mp2] patch for addvar/setvar merging
Begin forwarded message:
> From: Rici Lake <ri...@ricilake.net>
> Date: September 17, 2004 3:26:00 PM GMT-05:00
> To: dev@perl.apache.org
> Subject: [mp2] patch for addvar/setvar merging
>
> Following is a patch which I believe fixes the mp2 outstanding issue
> with PerlAddVar/PerlSetVar ordering. As a little bonus, it adds the
> directive PerlUnsetVar; the implementation was trivial given the
> algorithm I used, so I threw it in in case it is useful to anyone.
>
> For the record, the basic algorithm is as follows:
>
> A "configuration" is a series of directives, either "Set key val" or
> "Add key val". We can rewrite the
> former as "Unset key; Add key val". Once this is done, we can
> canonicalise the sequence by moving all the Unsets to the beginning,
> deleting any Add's for the same key that we pass over. The result of
> the transformation is to turn:
> (<set-directive> | <add-directive>)*
> into
> <unset-directive>* <add-directive>*
>
> Now, we can merge two configurations by canonicalising the
> concatenation of the canonicalised configurations; that is:
>
> merge(a, b) == canonicalise(concat(canonicalise(a), canonicalise(b)))
>
> This operation is fully associative and obviously preserves
> canonicalisation.
>
> In practice, we represent a configuration as a list of unset's (this
> could be an unordered set, but we don't have such a beast to play with
> in APR) and an ordered list of add's. To avoid changing too many
> structure member names, the unsets are called setvars and the adds are
> called configvars, which were the names from the existing source code.
> config->addvars has vanished.
>
> With this representation, the merge step is effectively the following:
>
> merge({a->unset, a->add}, {b->unset, b->add}) ==>
> {union(a->unset, b->unset), a->add - b->unset + b->add}
>
>
> The patch follows:
>
Re: [mp2] patch for addvar/setvar merging
Posted by Rici Lake <ri...@ricilake.net>.
On 17-Sep-04, at 4:55 PM, Stas Bekman wrote:
> Geoffrey Young wrote:
>> Rici Lake wrote:
> [...]
>>> As a little bonus, it adds the
>>>
>>>> directive PerlUnsetVar
>> hmm... I guess this would be useful. but what does it do, unset the
>> last
>> value (of multiple adds) or clear the table entry entirely? if the
>> latter
>> maybe PerlResetVar or PerlClearVar instead? also, I'm not entirely
>> sure
>> about ITERATE - maybe TAKE1 is better? others can weigh in, too :)
>
> I'd rather not add new directives, but have:
>
> PerlSetVar Foo
>
> set the value to an empty value, which internally could be
> impelemented as: remove that table entry completely. We already use
> this technique in more than one place (e.g. set_handlers,
> add_handlers), so it'd be nice to be consistent.
That would be easy enough... The use case I had in mind for the ITERATE
setting was:
# clear the environment
PerlUnSetVar secret1 secret2 secret3 thisOneAsWell
Although now that I think of it, unset within some scope only unsets
directory variables; the server variables (if any) continue to exist.
So maybe it's too confusing to be useful.
It could just get eliminated altogether, I don't care enough to make a
case one way or the other. Just because something is easy to implement
doesn't mean it's a good idea.
>> oh, and a new test for the new dorective much appreciated :)
>
> seconded. any new features or bug fixes *must* come with a test
> reproducing the problem/exercising the new feature, *before* they will
> be committed. ah, and documented too if that's a new feature.
I'll see what I can do over the weekend, if anyone pops up on the list
to say, "Great, what a cool feature."
R.
--
Report problems: http://perl.apache.org/bugs/
Mail list info: http://perl.apache.org/maillist/modperl.html
List etiquette: http://perl.apache.org/maillist/email-etiquette.html
Re: [mp2] patch for addvar/setvar merging
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
Rici Lake wrote:
> On 17-Sep-04, at 4:55 PM, Stas Bekman wrote:
>
>> I'd rather not add new directives, but have:
>
>
> On reflection, I think I agree. It is possible to achieve virtually
> the same effect with
>
> PerlSetVar foo ""
>
> So I trimmed a few lines out of the patch, ran the tests again, and
> this time I will cut and paste it into the message:
while my head is still spinning from all of this, I've reviewed the patch
and committed it with only a few style changes.
boy, I'm glad I took the time to write all those merge tests while it was
all still fresh in my head :)
nice work rici. thanks a million.
--Geoff
--
Report problems: http://perl.apache.org/bugs/
Mail list info: http://perl.apache.org/maillist/modperl.html
List etiquette: http://perl.apache.org/maillist/email-etiquette.html
Re: [mp2] patch for addvar/setvar merging
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
Rici Lake wrote:
> On 17-Sep-04, at 4:55 PM, Stas Bekman wrote:
>
>> I'd rather not add new directives, but have:
>
>
> On reflection, I think I agree. It is possible to achieve virtually
> the same effect with
>
> PerlSetVar foo ""
>
> So I trimmed a few lines out of the patch, ran the tests again, and
> this time I will cut and paste it into the message:
while my head is still spinning from all of this, I've reviewed the patch
and committed it with only a few style changes.
boy, I'm glad I took the time to write all those merge tests while it was
all still fresh in my head :)
nice work rici. thanks a million.
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] patch for addvar/setvar merging
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> So I trimmed a few lines out of the patch, ran the tests again, and
> this time I will cut and paste it into the message:
hi.
I'm starting to review this now but I can't get the patch to apply.
can you please re-post the patch as an attachment and make sure that it's a
unified diff with context - it doesn't look like a context diff, which makes
it difficult for me to read on it's own. the patch should apply cleanly
against current cvs with
[mod_perl-2.0]$ patch -p0 < perlsetvar.patch
thanks
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] patch for addvar/setvar merging
Posted by Stas Bekman <st...@stason.org>.
Rici Lake wrote:
>
> On 18-Sep-04, at 8:37 AM, Stas Bekman wrote:
>
>> What about the unset via set "" functionality? I don't remember it
>> being in mp1.
>
>
> I didn't actually do anything to implement that; as in mp1 (I presume)
> it sets
> the variable to an empty string, discarding any previous values. Unless
> someone
> actually clamours for an unset function, I think that is probably
> sufficient,
> given the discussion up to this point.
I guess you are correct, but we better have an explicit test for that and
documentation, so it we make sure that it sticks around and won't
disappear one day.
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
--
Report problems: http://perl.apache.org/bugs/
Mail list info: http://perl.apache.org/maillist/modperl.html
List etiquette: http://perl.apache.org/maillist/email-etiquette.html
Re: [mp2] patch for addvar/setvar merging
Posted by Rici Lake <ri...@ricilake.net>.
On 18-Sep-04, at 8:37 AM, Stas Bekman wrote:
> What about the unset via set "" functionality? I don't remember it
> being in mp1.
I didn't actually do anything to implement that; as in mp1 (I presume)
it sets
the variable to an empty string, discarding any previous values. Unless
someone
actually clamours for an unset function, I think that is probably
sufficient,
given the discussion up to this point.
R.
--
Report problems: http://perl.apache.org/bugs/
Mail list info: http://perl.apache.org/maillist/modperl.html
List etiquette: http://perl.apache.org/maillist/email-etiquette.html
Re: [mp2] patch for addvar/setvar merging
Posted by Stas Bekman <st...@stason.org>.
Rici Lake wrote:
>
> On 17-Sep-04, at 7:36 PM, Stas Bekman wrote:
>
>> and if you can send the missing tests and docs that will ensure that
>> your patch will be committed promptly.
>>
>> As I can see src/docs/2.0/user/config/config.pod's PerlSetVar entry is
>> completely empty :( So it needs to be ported from the mp1 docs first
>> and then add the new functionality. If you could do that it will be
>> great! Let me know if you need any guiding to get the docs right...
>> Thanks Rici.
>
>
> Actually, there is now no new functionality,
What about the unset via set "" functionality? I don't remember it being
in mp1.
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
--
Report problems: http://perl.apache.org/bugs/
Mail list info: http://perl.apache.org/maillist/modperl.html
List etiquette: http://perl.apache.org/maillist/email-etiquette.html
Re: [mp2] patch for addvar/setvar merging
Posted by Rici Lake <ri...@ricilake.net>.
On 17-Sep-04, at 7:36 PM, Stas Bekman wrote:
> and if you can send the missing tests and docs that will ensure that
> your patch will be committed promptly.
>
> As I can see src/docs/2.0/user/config/config.pod's PerlSetVar entry is
> completely empty :( So it needs to be ported from the mp1 docs first
> and then add the new functionality. If you could do that it will be
> great! Let me know if you need any guiding to get the docs right...
> Thanks Rici.
Actually, there is now no new functionality, aside from the fact that
it works as (presumably) intended. This also pretty well voids the need
for testing new features. The behaviour should conform to the
as-yet-unported v1 documentation,
I think there ought to be a note about the difference between request
variables and server variables, but I don't feel like I'm the right
person to write it because I only have a fuzzy idea about what the
expected use case is. In fact, I'm not 100% comfortable with the
conflation of the two; if I understand correctly, in a server context:
PerlSetVar foo bar
sets *both* the request and the server variables (independently); there
is no way to just set the server context PerlVar, thus clouding the
fact that there are really two objects in independent
(case-insensitive) namespaces. But perhaps this is not really an issue
for anyone.
R.
--
Report problems: http://perl.apache.org/bugs/
Mail list info: http://perl.apache.org/maillist/modperl.html
List etiquette: http://perl.apache.org/maillist/email-etiquette.html
Re: [mp2] patch for addvar/setvar merging
Posted by Stas Bekman <st...@stason.org>.
[Geoff, please don't cross-post the thread, Rici is not subscribed and
it'll be just confusing as half of the posts won't make it to the dev list]
Rici Lake wrote:
> On 17-Sep-04, at 4:55 PM, Stas Bekman wrote:
>
>> I'd rather not add new directives, but have:
>
>
> On reflection, I think I agree. It is possible to achieve virtually
> the same effect with
>
> PerlSetVar foo ""
>
> So I trimmed a few lines out of the patch, ran the tests again, and
> this time I will cut and paste it into the message:
rici++
and if you can send the missing tests and docs that will ensure that your
patch will be committed promptly.
As I can see src/docs/2.0/user/config/config.pod's PerlSetVar entry is
completely empty :( So it needs to be ported from the mp1 docs first and
then add the new functionality. If you could do that it will be great! Let
me know if you need any guiding to get the docs right... Thanks Rici.
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
--
Report problems: http://perl.apache.org/bugs/
Mail list info: http://perl.apache.org/maillist/modperl.html
List etiquette: http://perl.apache.org/maillist/email-etiquette.html
Re: [mp2] patch for addvar/setvar merging
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> So I trimmed a few lines out of the patch, ran the tests again, and
> this time I will cut and paste it into the message:
hi.
I'm starting to review this now but I can't get the patch to apply.
can you please re-post the patch as an attachment and make sure that it's a
unified diff with context - it doesn't look like a context diff, which makes
it difficult for me to read on it's own. the patch should apply cleanly
against current cvs with
[mod_perl-2.0]$ patch -p0 < perlsetvar.patch
thanks
--Geoff
--
Report problems: http://perl.apache.org/bugs/
Mail list info: http://perl.apache.org/maillist/modperl.html
List etiquette: http://perl.apache.org/maillist/email-etiquette.html
Re: [mp2] patch for addvar/setvar merging
Posted by Rici Lake <ri...@ricilake.net>.
On 17-Sep-04, at 4:55 PM, Stas Bekman wrote:
> I'd rather not add new directives, but have:
On reflection, I think I agree. It is possible to achieve virtually
the same effect with
PerlSetVar foo ""
So I trimmed a few lines out of the patch, ran the tests again, and
this time I will cut and paste it into the message:
--------------------------------------------------------
Index: modperl-2.0/src/modules/perl/mod_perl.h
===================================================================
RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/mod_perl.h,v
retrieving revision 1.68
diff -u -r1.68 mod_perl.h
--- modperl-2.0/src/modules/perl/mod_perl.h 10 Jul 2004 00:36:19
-0000 1.68
+++ modperl-2.0/src/modules/perl/mod_perl.h 18 Sep 2004 00:16:21 -0000
@@ -129,9 +129,10 @@
#define MgTypeExt(mg) (mg->mg_type == '~')
-typedef void MP_FUNC_T(modperl_table_modify_t) (apr_table_t *,
- const char *,
- const char *);
+typedef void MP_FUNC_T(modperl_var_modify_t) (apr_table_t *,
+ apr_table_t *,
+ const char *,
+ const char *);
/* we need to hook a few internal things before APR_HOOK_REALLY_FIRST
*/
#define MODPERL_HOOK_REALLY_REALLY_FIRST (-20)
Index: modperl-2.0/src/modules/perl/modperl_cmd.c
===================================================================
RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/modperl_cmd.c,v
retrieving revision 1.65
diff -u -r1.65 modperl_cmd.c
--- modperl-2.0/src/modules/perl/modperl_cmd.c 9 Sep 2004 22:16:37
-0000 1.65
+++ modperl-2.0/src/modules/perl/modperl_cmd.c 18 Sep 2004 00:16:22
-0000
@@ -266,64 +266,50 @@
}
}
-static MP_CMD_SRV_DECLARE2(handle_vars)
+static void modperl_cmd_addvar_func(apr_table_t *configvars,
+ apr_table_t *setvars,
+ const char *key, const char *val)
{
- MP_dSCFG(parms->server);
- modperl_config_dir_t *dcfg = (modperl_config_dir_t *)mconfig;
- const char *name = parms->cmd->name;
-
- /* PerlSetVar and PerlAddVar logic. here's the deal...
- *
- * cfg->configvars holds the final PerlSetVar/PerlAddVar
configuration
- * for a given server or directory. however, getting to that point
- * is kind of tricky, due to the add-style nature of PerlAddVar.
- *
- * the solution is to use cfg->setvars to hold PerlSetVar entries
- * and cfg->addvars to hold PerlAddVar entries, each serving as a
- * placeholder for when we need to know what's what in the merge
routines.
- *
- * however, for the initial pass, apr_table_setn and apr_table_addn
- * will properly build the configvars table, which will be visible
to
- * startup scripts trying to access per-server configurations.
- *
- * the end result is that we need to populate all three tables in
order
- * to keep things straight later on see merge_table_config_vars in
- * modperl_config.c
- */
- modperl_table_modify_t func =
- strEQ(name, "PerlSetVar") ? apr_table_setn : apr_table_addn;
+ apr_table_addn(configvars, key, val);
+}
- apr_table_t *table =
- strEQ(name, "PerlSetVar") ? dcfg->setvars : dcfg->addvars;
+/* Conceptually, setvar is { unsetvar; addvar; } */
- func(table, arg1, arg2);
- func(dcfg->configvars, arg1, arg2);
+static void modperl_cmd_setvar_func(apr_table_t *configvars,
+ apr_table_t *setvars,
+ const char * key, const char *val)
+{
+ apr_table_setn(setvars, key, val);
+ apr_table_setn(configvars, key, val);
+}
+static const char *modperl_cmd_modvar(modperl_var_modify_t varfunc,
+ cmd_parms *parms,
+ modperl_config_dir_t *dcfg,
+ const char *arg1, const char
*arg2)
+{
+ varfunc(dcfg->configvars, dcfg->setvars, arg1, arg2);
MP_TRACE_d(MP_FUNC, "%s DIR: arg1 = %s, arg2 = %s\n",
- name, arg1, arg2);
-
- /* make available via Apache->server->dir_config */
+ parms->cmd->name, arg1, arg2);
if (!parms->path) {
- table = strEQ(name, "PerlSetVar") ? scfg->setvars :
scfg->addvars;
-
- func(table, arg1, arg2);
- func(scfg->configvars, arg1, arg2);
-
+ MP_dSCFG(parms->server);
+ varfunc(scfg->configvars, scfg->setvars, arg1, arg2);
MP_TRACE_d(MP_FUNC, "%s SRV: arg1 = %s, arg2 = %s\n",
- name, arg1, arg2);
+ parms->cmd->name, arg1, arg2);
}
-
return NULL;
}
-MP_CMD_SRV_DECLARE2(set_var)
+MP_CMD_SRV_DECLARE2(add_var)
{
- return modperl_cmd_handle_vars(parms, mconfig, arg1, arg2);
+ modperl_config_dir_t *dcfg = (modperl_config_dir_t *)mconfig;
+ return modperl_cmd_modvar(modperl_cmd_addvar_func, parms, dcfg,
arg1, arg2);
}
-MP_CMD_SRV_DECLARE2(add_var)
+MP_CMD_SRV_DECLARE2(set_var)
{
- return modperl_cmd_handle_vars(parms, mconfig, arg1, arg2);
+ modperl_config_dir_t *dcfg = (modperl_config_dir_t *)mconfig;
+ return modperl_cmd_modvar(modperl_cmd_setvar_func, parms, dcfg,
arg1, arg2);
}
MP_CMD_SRV_DECLARE2(set_env)
Index: modperl-2.0/src/modules/perl/modperl_config.c
===================================================================
RCS file:
/home/cvspublic/modperl-2.0/src/modules/perl/modperl_config.c,v
retrieving revision 1.81
diff -u -r1.81 modperl_config.c
--- modperl-2.0/src/modules/perl/modperl_config.c 9 Sep 2004 22:39:11
-0000 1.81
+++ modperl-2.0/src/modules/perl/modperl_config.c 18 Sep 2004 00:16:24
-0000
@@ -43,6 +43,11 @@
* note that this is equivalent to apr_table_overlap except a new
* table is generated, which is required (otherwise we would
clobber
* the existing parent or child configurations)
+ *
+ * note that this is *not* equivalent to apr_table_overlap,
although
+ * I think it should be, because apr_table_overlap seems to clear
+ * its first argument when the tables have different pools. I think
+ * this is wrong -- rici
*/
apr_table_t *merge = apr_table_overlay(p, base, add);
@@ -59,54 +64,27 @@
#define merge_table_overlap_item(item) \
mrg->item = modperl_table_overlap(p, base->item, add->item)
-static apr_table_t *merge_table_config_vars(apr_pool_t *p,
- apr_table_t *configvars,
- apr_table_t *set,
- apr_table_t *add)
+static apr_table_t *merge_config_add_vars(apr_pool_t *p,
+ const apr_table_t *base,
+ const apr_table_t *unset,
+ const apr_table_t *add)
{
- apr_table_t *base = apr_table_copy(p, configvars);
- apr_table_t *merged_config_vars;
+ apr_table_t *temp = apr_table_copy(p, base);
const apr_array_header_t *arr;
apr_table_entry_t *entries;
int i;
- /* configvars already contains a properly merged
PerlSetVar/PerlAddVar
- * configuration for the base (parent), so all we need to do is
merge
- * the add (child) configuration into it properly.
- *
- * any PerlSetVar settings in the add (child) config need to reset
- * existing entries in the base (parent) config, or generate a
- * new entry where none existed previously. PerlAddVar settings
- * are merged into that.
- *
- * unfortunately, there is no set of apr functions to do this for
us -
- * apr_compress_table would be ok, except it always merges
mulit-valued
- * keys into one, regardless of the APR_OVERLAP_TABLES flag. that
is,
- * regardless of whether newer entries are set or merged into
existing
- * entries, the entire table is _always_ compressed. this is no
good -
- * we need separate entries for existing keys, not a single
(compressed)
- * entry.
- *
- * fortunately, the logic here is simple. first, (re)set the base
(parent)
- * table where a PerlSetVar entry exists in the add (child)
configuration.
- * then, just overlay the PerlAddVar configuration into it.
- */
-
- arr = apr_table_elts(set);
+ /* for each key in unset do apr_table_unset(temp, key); */
+ arr = apr_table_elts(unset);
entries = (apr_table_entry_t *)arr->elts;
/* hopefully this is faster than using apr_table_do */
for (i = 0; i < arr->nelts; i++) {
- apr_table_setn(base, entries[i].key, entries[i].val);
+ if (entries[i].key) apr_table_unset(temp, entries[i].key);
}
-
- /* at this point, all the PerlSetVar merging has happened. add in
the
- * add (child) PerlAddVar entries and we're done
- */
- merged_config_vars = apr_table_overlay(p, base, add);
-
- return merged_config_vars;
+
+ return apr_table_overlay(p, temp, add);
}
#define merge_handlers(merge_flag, array) \
@@ -141,18 +119,10 @@
merge_table_overlap_item(SetEnv);
/* this is where we merge PerlSetVar and PerlAddVar together */
- mrg->configvars = merge_table_config_vars(p,
- base->configvars,
- add->setvars,
add->addvars);
-
- /* note we don't care about merging dcfg->setvars or dcfg->addvars
- * specifically - what is important to merge is dfcg->configvars.
- * but we need to keep track of the entries for this config, so
- * the merged values are simply the values for the add (current)
- * configuration.
- */
- mrg->setvars = add->setvars;
- mrg->addvars = add->addvars;
+ mrg->configvars = merge_config_add_vars(p,
+ base->configvars,
+ add->setvars,
add->configvars);
+ merge_table_overlap_item(setvars);
/* XXX: check if Perl*Handler is disabled */
for (i=0; i < MP_HANDLER_NUM_PER_DIR; i++) {
@@ -187,7 +157,6 @@
scfg->argv = apr_array_make(p, 2, sizeof(char *));
scfg->setvars = apr_table_make(p, 2);
- scfg->addvars = apr_table_make(p, 2);
scfg->configvars = apr_table_make(p, 2);
scfg->PassEnv = apr_table_make(p, 2);
@@ -223,7 +192,6 @@
dcfg->flags = modperl_options_new(p, MpDirType);
dcfg->setvars = apr_table_make(p, 2);
- dcfg->addvars = apr_table_make(p, 2);
dcfg->configvars = apr_table_make(p, 2);
dcfg->SetEnv = apr_table_make(p, 2);
@@ -322,18 +290,10 @@
merge_table_overlap_item(PassEnv);
/* this is where we merge PerlSetVar and PerlAddVar together */
- mrg->configvars = merge_table_config_vars(p,
- base->configvars,
- add->setvars,
add->addvars);
-
- /* note we don't care about merging dcfg->setvars or dcfg->addvars
- * specifically - what is important to merge is dfcg->configvars.
- * but we need to keep track of the entries for this config, so
- * the merged values are simply the values for the add (current)
- * configuration.
- */
- mrg->setvars = add->setvars;
- mrg->addvars = add->addvars;
+ mrg->configvars = merge_config_add_vars(p,
+ base->configvars,
+ add->setvars,
add->configvars);
+ merge_table_overlap_item(setvars);
merge_item(threaded_mpm);
merge_item(server);
Index: modperl-2.0/src/modules/perl/modperl_types.h
===================================================================
RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/modperl_types.h,v
retrieving revision 1.76
diff -u -r1.76 modperl_types.h
--- modperl-2.0/src/modules/perl/modperl_types.h 18 Aug 2004 22:05:16
-0000 1.76
+++ modperl-2.0/src/modules/perl/modperl_types.h 18 Sep 2004 00:16:25
-0000
@@ -127,7 +127,6 @@
typedef struct {
MpHV *setvars;
- MpHV *addvars;
MpHV *configvars;
MpHV *SetEnv;
MpHV *PassEnv;
@@ -160,7 +159,6 @@
MpAV *handlers_per_dir[MP_HANDLER_NUM_PER_DIR];
MpHV *SetEnv;
MpHV *setvars;
- MpHV *addvars;
MpHV *configvars;
modperl_options_t *flags;
#ifdef USE_ITHREADS
Index: modperl-2.0/t/response/TestModperl/merge.pm
===================================================================
RCS file: /home/cvspublic/modperl-2.0/t/response/TestModperl/merge.pm,v
retrieving revision 1.7
diff -u -r1.7 merge.pm
--- modperl-2.0/t/response/TestModperl/merge.pm 9 Jul 2004 18:53:01
-0000 1.7
+++ modperl-2.0/t/response/TestModperl/merge.pm 18 Sep 2004 00:16:27
-0000
@@ -90,7 +90,7 @@
$hash = $1;
# skip .htaccess merges for now - they are still broken
- plan tests => 10, under_construction;
+ # plan tests => 10, under_construction;
} elsif ($uri =~ m/(merge2)/) {
$hash = $1;
} else {
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] patch for addvar/setvar merging
Posted by Rici Lake <ri...@ricilake.net>.
On 17-Sep-04, at 4:55 PM, Stas Bekman wrote:
> I'd rather not add new directives, but have:
On reflection, I think I agree. It is possible to achieve virtually
the same effect with
PerlSetVar foo ""
So I trimmed a few lines out of the patch, ran the tests again, and
this time I will cut and paste it into the message:
--------------------------------------------------------
Index: modperl-2.0/src/modules/perl/mod_perl.h
===================================================================
RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/mod_perl.h,v
retrieving revision 1.68
diff -u -r1.68 mod_perl.h
--- modperl-2.0/src/modules/perl/mod_perl.h 10 Jul 2004 00:36:19
-0000 1.68
+++ modperl-2.0/src/modules/perl/mod_perl.h 18 Sep 2004 00:16:21 -0000
@@ -129,9 +129,10 @@
#define MgTypeExt(mg) (mg->mg_type == '~')
-typedef void MP_FUNC_T(modperl_table_modify_t) (apr_table_t *,
- const char *,
- const char *);
+typedef void MP_FUNC_T(modperl_var_modify_t) (apr_table_t *,
+ apr_table_t *,
+ const char *,
+ const char *);
/* we need to hook a few internal things before APR_HOOK_REALLY_FIRST
*/
#define MODPERL_HOOK_REALLY_REALLY_FIRST (-20)
Index: modperl-2.0/src/modules/perl/modperl_cmd.c
===================================================================
RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/modperl_cmd.c,v
retrieving revision 1.65
diff -u -r1.65 modperl_cmd.c
--- modperl-2.0/src/modules/perl/modperl_cmd.c 9 Sep 2004 22:16:37
-0000 1.65
+++ modperl-2.0/src/modules/perl/modperl_cmd.c 18 Sep 2004 00:16:22
-0000
@@ -266,64 +266,50 @@
}
}
-static MP_CMD_SRV_DECLARE2(handle_vars)
+static void modperl_cmd_addvar_func(apr_table_t *configvars,
+ apr_table_t *setvars,
+ const char *key, const char *val)
{
- MP_dSCFG(parms->server);
- modperl_config_dir_t *dcfg = (modperl_config_dir_t *)mconfig;
- const char *name = parms->cmd->name;
-
- /* PerlSetVar and PerlAddVar logic. here's the deal...
- *
- * cfg->configvars holds the final PerlSetVar/PerlAddVar
configuration
- * for a given server or directory. however, getting to that point
- * is kind of tricky, due to the add-style nature of PerlAddVar.
- *
- * the solution is to use cfg->setvars to hold PerlSetVar entries
- * and cfg->addvars to hold PerlAddVar entries, each serving as a
- * placeholder for when we need to know what's what in the merge
routines.
- *
- * however, for the initial pass, apr_table_setn and apr_table_addn
- * will properly build the configvars table, which will be visible
to
- * startup scripts trying to access per-server configurations.
- *
- * the end result is that we need to populate all three tables in
order
- * to keep things straight later on see merge_table_config_vars in
- * modperl_config.c
- */
- modperl_table_modify_t func =
- strEQ(name, "PerlSetVar") ? apr_table_setn : apr_table_addn;
+ apr_table_addn(configvars, key, val);
+}
- apr_table_t *table =
- strEQ(name, "PerlSetVar") ? dcfg->setvars : dcfg->addvars;
+/* Conceptually, setvar is { unsetvar; addvar; } */
- func(table, arg1, arg2);
- func(dcfg->configvars, arg1, arg2);
+static void modperl_cmd_setvar_func(apr_table_t *configvars,
+ apr_table_t *setvars,
+ const char * key, const char *val)
+{
+ apr_table_setn(setvars, key, val);
+ apr_table_setn(configvars, key, val);
+}
+static const char *modperl_cmd_modvar(modperl_var_modify_t varfunc,
+ cmd_parms *parms,
+ modperl_config_dir_t *dcfg,
+ const char *arg1, const char
*arg2)
+{
+ varfunc(dcfg->configvars, dcfg->setvars, arg1, arg2);
MP_TRACE_d(MP_FUNC, "%s DIR: arg1 = %s, arg2 = %s\n",
- name, arg1, arg2);
-
- /* make available via Apache->server->dir_config */
+ parms->cmd->name, arg1, arg2);
if (!parms->path) {
- table = strEQ(name, "PerlSetVar") ? scfg->setvars :
scfg->addvars;
-
- func(table, arg1, arg2);
- func(scfg->configvars, arg1, arg2);
-
+ MP_dSCFG(parms->server);
+ varfunc(scfg->configvars, scfg->setvars, arg1, arg2);
MP_TRACE_d(MP_FUNC, "%s SRV: arg1 = %s, arg2 = %s\n",
- name, arg1, arg2);
+ parms->cmd->name, arg1, arg2);
}
-
return NULL;
}
-MP_CMD_SRV_DECLARE2(set_var)
+MP_CMD_SRV_DECLARE2(add_var)
{
- return modperl_cmd_handle_vars(parms, mconfig, arg1, arg2);
+ modperl_config_dir_t *dcfg = (modperl_config_dir_t *)mconfig;
+ return modperl_cmd_modvar(modperl_cmd_addvar_func, parms, dcfg,
arg1, arg2);
}
-MP_CMD_SRV_DECLARE2(add_var)
+MP_CMD_SRV_DECLARE2(set_var)
{
- return modperl_cmd_handle_vars(parms, mconfig, arg1, arg2);
+ modperl_config_dir_t *dcfg = (modperl_config_dir_t *)mconfig;
+ return modperl_cmd_modvar(modperl_cmd_setvar_func, parms, dcfg,
arg1, arg2);
}
MP_CMD_SRV_DECLARE2(set_env)
Index: modperl-2.0/src/modules/perl/modperl_config.c
===================================================================
RCS file:
/home/cvspublic/modperl-2.0/src/modules/perl/modperl_config.c,v
retrieving revision 1.81
diff -u -r1.81 modperl_config.c
--- modperl-2.0/src/modules/perl/modperl_config.c 9 Sep 2004 22:39:11
-0000 1.81
+++ modperl-2.0/src/modules/perl/modperl_config.c 18 Sep 2004 00:16:24
-0000
@@ -43,6 +43,11 @@
* note that this is equivalent to apr_table_overlap except a new
* table is generated, which is required (otherwise we would
clobber
* the existing parent or child configurations)
+ *
+ * note that this is *not* equivalent to apr_table_overlap,
although
+ * I think it should be, because apr_table_overlap seems to clear
+ * its first argument when the tables have different pools. I think
+ * this is wrong -- rici
*/
apr_table_t *merge = apr_table_overlay(p, base, add);
@@ -59,54 +64,27 @@
#define merge_table_overlap_item(item) \
mrg->item = modperl_table_overlap(p, base->item, add->item)
-static apr_table_t *merge_table_config_vars(apr_pool_t *p,
- apr_table_t *configvars,
- apr_table_t *set,
- apr_table_t *add)
+static apr_table_t *merge_config_add_vars(apr_pool_t *p,
+ const apr_table_t *base,
+ const apr_table_t *unset,
+ const apr_table_t *add)
{
- apr_table_t *base = apr_table_copy(p, configvars);
- apr_table_t *merged_config_vars;
+ apr_table_t *temp = apr_table_copy(p, base);
const apr_array_header_t *arr;
apr_table_entry_t *entries;
int i;
- /* configvars already contains a properly merged
PerlSetVar/PerlAddVar
- * configuration for the base (parent), so all we need to do is
merge
- * the add (child) configuration into it properly.
- *
- * any PerlSetVar settings in the add (child) config need to reset
- * existing entries in the base (parent) config, or generate a
- * new entry where none existed previously. PerlAddVar settings
- * are merged into that.
- *
- * unfortunately, there is no set of apr functions to do this for
us -
- * apr_compress_table would be ok, except it always merges
mulit-valued
- * keys into one, regardless of the APR_OVERLAP_TABLES flag. that
is,
- * regardless of whether newer entries are set or merged into
existing
- * entries, the entire table is _always_ compressed. this is no
good -
- * we need separate entries for existing keys, not a single
(compressed)
- * entry.
- *
- * fortunately, the logic here is simple. first, (re)set the base
(parent)
- * table where a PerlSetVar entry exists in the add (child)
configuration.
- * then, just overlay the PerlAddVar configuration into it.
- */
-
- arr = apr_table_elts(set);
+ /* for each key in unset do apr_table_unset(temp, key); */
+ arr = apr_table_elts(unset);
entries = (apr_table_entry_t *)arr->elts;
/* hopefully this is faster than using apr_table_do */
for (i = 0; i < arr->nelts; i++) {
- apr_table_setn(base, entries[i].key, entries[i].val);
+ if (entries[i].key) apr_table_unset(temp, entries[i].key);
}
-
- /* at this point, all the PerlSetVar merging has happened. add in
the
- * add (child) PerlAddVar entries and we're done
- */
- merged_config_vars = apr_table_overlay(p, base, add);
-
- return merged_config_vars;
+
+ return apr_table_overlay(p, temp, add);
}
#define merge_handlers(merge_flag, array) \
@@ -141,18 +119,10 @@
merge_table_overlap_item(SetEnv);
/* this is where we merge PerlSetVar and PerlAddVar together */
- mrg->configvars = merge_table_config_vars(p,
- base->configvars,
- add->setvars,
add->addvars);
-
- /* note we don't care about merging dcfg->setvars or dcfg->addvars
- * specifically - what is important to merge is dfcg->configvars.
- * but we need to keep track of the entries for this config, so
- * the merged values are simply the values for the add (current)
- * configuration.
- */
- mrg->setvars = add->setvars;
- mrg->addvars = add->addvars;
+ mrg->configvars = merge_config_add_vars(p,
+ base->configvars,
+ add->setvars,
add->configvars);
+ merge_table_overlap_item(setvars);
/* XXX: check if Perl*Handler is disabled */
for (i=0; i < MP_HANDLER_NUM_PER_DIR; i++) {
@@ -187,7 +157,6 @@
scfg->argv = apr_array_make(p, 2, sizeof(char *));
scfg->setvars = apr_table_make(p, 2);
- scfg->addvars = apr_table_make(p, 2);
scfg->configvars = apr_table_make(p, 2);
scfg->PassEnv = apr_table_make(p, 2);
@@ -223,7 +192,6 @@
dcfg->flags = modperl_options_new(p, MpDirType);
dcfg->setvars = apr_table_make(p, 2);
- dcfg->addvars = apr_table_make(p, 2);
dcfg->configvars = apr_table_make(p, 2);
dcfg->SetEnv = apr_table_make(p, 2);
@@ -322,18 +290,10 @@
merge_table_overlap_item(PassEnv);
/* this is where we merge PerlSetVar and PerlAddVar together */
- mrg->configvars = merge_table_config_vars(p,
- base->configvars,
- add->setvars,
add->addvars);
-
- /* note we don't care about merging dcfg->setvars or dcfg->addvars
- * specifically - what is important to merge is dfcg->configvars.
- * but we need to keep track of the entries for this config, so
- * the merged values are simply the values for the add (current)
- * configuration.
- */
- mrg->setvars = add->setvars;
- mrg->addvars = add->addvars;
+ mrg->configvars = merge_config_add_vars(p,
+ base->configvars,
+ add->setvars,
add->configvars);
+ merge_table_overlap_item(setvars);
merge_item(threaded_mpm);
merge_item(server);
Index: modperl-2.0/src/modules/perl/modperl_types.h
===================================================================
RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/modperl_types.h,v
retrieving revision 1.76
diff -u -r1.76 modperl_types.h
--- modperl-2.0/src/modules/perl/modperl_types.h 18 Aug 2004 22:05:16
-0000 1.76
+++ modperl-2.0/src/modules/perl/modperl_types.h 18 Sep 2004 00:16:25
-0000
@@ -127,7 +127,6 @@
typedef struct {
MpHV *setvars;
- MpHV *addvars;
MpHV *configvars;
MpHV *SetEnv;
MpHV *PassEnv;
@@ -160,7 +159,6 @@
MpAV *handlers_per_dir[MP_HANDLER_NUM_PER_DIR];
MpHV *SetEnv;
MpHV *setvars;
- MpHV *addvars;
MpHV *configvars;
modperl_options_t *flags;
#ifdef USE_ITHREADS
Index: modperl-2.0/t/response/TestModperl/merge.pm
===================================================================
RCS file: /home/cvspublic/modperl-2.0/t/response/TestModperl/merge.pm,v
retrieving revision 1.7
diff -u -r1.7 merge.pm
--- modperl-2.0/t/response/TestModperl/merge.pm 9 Jul 2004 18:53:01
-0000 1.7
+++ modperl-2.0/t/response/TestModperl/merge.pm 18 Sep 2004 00:16:27
-0000
@@ -90,7 +90,7 @@
$hash = $1;
# skip .htaccess merges for now - they are still broken
- plan tests => 10, under_construction;
+ # plan tests => 10, under_construction;
} elsif ($uri =~ m/(merge2)/) {
$hash = $1;
} else {
--
Report problems: http://perl.apache.org/bugs/
Mail list info: http://perl.apache.org/maillist/modperl.html
List etiquette: http://perl.apache.org/maillist/email-etiquette.html
Re: [mp2] patch for addvar/setvar merging
Posted by Rici Lake <ri...@ricilake.net>.
On 17-Sep-04, at 4:55 PM, Stas Bekman wrote:
> Geoffrey Young wrote:
>> Rici Lake wrote:
> [...]
>>> As a little bonus, it adds the
>>>
>>>> directive PerlUnsetVar
>> hmm... I guess this would be useful. but what does it do, unset the
>> last
>> value (of multiple adds) or clear the table entry entirely? if the
>> latter
>> maybe PerlResetVar or PerlClearVar instead? also, I'm not entirely
>> sure
>> about ITERATE - maybe TAKE1 is better? others can weigh in, too :)
>
> I'd rather not add new directives, but have:
>
> PerlSetVar Foo
>
> set the value to an empty value, which internally could be
> impelemented as: remove that table entry completely. We already use
> this technique in more than one place (e.g. set_handlers,
> add_handlers), so it'd be nice to be consistent.
That would be easy enough... The use case I had in mind for the ITERATE
setting was:
# clear the environment
PerlUnSetVar secret1 secret2 secret3 thisOneAsWell
Although now that I think of it, unset within some scope only unsets
directory variables; the server variables (if any) continue to exist.
So maybe it's too confusing to be useful.
It could just get eliminated altogether, I don't care enough to make a
case one way or the other. Just because something is easy to implement
doesn't mean it's a good idea.
>> oh, and a new test for the new dorective much appreciated :)
>
> seconded. any new features or bug fixes *must* come with a test
> reproducing the problem/exercising the new feature, *before* they will
> be committed. ah, and documented too if that's a new feature.
I'll see what I can do over the weekend, if anyone pops up on the list
to say, "Great, what a cool feature."
R.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: Fwd: [mp2] patch for addvar/setvar merging
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>
> Rici Lake wrote:
[...]
>>As a little bonus, it adds the
>>
>>>directive PerlUnsetVar
>
>
> hmm... I guess this would be useful. but what does it do, unset the last
> value (of multiple adds) or clear the table entry entirely? if the latter
> maybe PerlResetVar or PerlClearVar instead? also, I'm not entirely sure
> about ITERATE - maybe TAKE1 is better? others can weigh in, too :)
I'd rather not add new directives, but have:
PerlSetVar Foo
set the value to an empty value, which internally could be impelemented
as: remove that table entry completely. We already use this technique in
more than one place (e.g. set_handlers, add_handlers), so it'd be nice to
be consistent.
> oh, and a new test for the new dorective much appreciated :)
seconded. any new features or bug fixes *must* come with a test
reproducing the problem/exercising the new feature, *before* they will be
committed. ah, and documented too if that's a new feature.
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
--
Report problems: http://perl.apache.org/bugs/
Mail list info: http://perl.apache.org/maillist/modperl.html
List etiquette: http://perl.apache.org/maillist/email-etiquette.html
Re: Fwd: [mp2] patch for addvar/setvar merging
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>
> Rici Lake wrote:
[...]
>>As a little bonus, it adds the
>>
>>>directive PerlUnsetVar
>
>
> hmm... I guess this would be useful. but what does it do, unset the last
> value (of multiple adds) or clear the table entry entirely? if the latter
> maybe PerlResetVar or PerlClearVar instead? also, I'm not entirely sure
> about ITERATE - maybe TAKE1 is better? others can weigh in, too :)
I'd rather not add new directives, but have:
PerlSetVar Foo
set the value to an empty value, which internally could be impelemented
as: remove that table entry completely. We already use this technique in
more than one place (e.g. set_handlers, add_handlers), so it'd be nice to
be consistent.
> oh, and a new test for the new dorective much appreciated :)
seconded. any new features or bug fixes *must* come with a test
reproducing the problem/exercising the new feature, *before* they will be
committed. ah, and documented too if that's a new feature.
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: Fwd: [mp2] patch for addvar/setvar merging
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
Rici Lake wrote:
>
>
> Begin forwarded message:
>
>> From: Rici Lake <ri...@ricilake.net>
>> Date: September 17, 2004 3:26:00 PM GMT-05:00
>> To: dev@perl.apache.org
>> Subject: [mp2] patch for addvar/setvar merging
>>
>> Following is a patch which I believe fixes the mp2 outstanding issue
>> with PerlAddVar/PerlSetVar ordering.
cool! I'll review it on monday and report back.
> As a little bonus, it adds the
>> directive PerlUnsetVar
hmm... I guess this would be useful. but what does it do, unset the last
value (of multiple adds) or clear the table entry entirely? if the latter
maybe PerlResetVar or PerlClearVar instead? also, I'm not entirely sure
about ITERATE - maybe TAKE1 is better? others can weigh in, too :)
oh, and a new test for the new dorective much appreciated :)
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: Fwd: [mp2] patch for addvar/setvar merging
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
Rici Lake wrote:
>
>
> Begin forwarded message:
>
>> From: Rici Lake <ri...@ricilake.net>
>> Date: September 17, 2004 3:26:00 PM GMT-05:00
>> To: dev@perl.apache.org
>> Subject: [mp2] patch for addvar/setvar merging
>>
>> Following is a patch which I believe fixes the mp2 outstanding issue
>> with PerlAddVar/PerlSetVar ordering.
cool! I'll review it on monday and report back.
> As a little bonus, it adds the
>> directive PerlUnsetVar
hmm... I guess this would be useful. but what does it do, unset the last
value (of multiple adds) or clear the table entry entirely? if the latter
maybe PerlResetVar or PerlClearVar instead? also, I'm not entirely sure
about ITERATE - maybe TAKE1 is better? others can weigh in, too :)
oh, and a new test for the new dorective much appreciated :)
--Geoff
--
Report problems: http://perl.apache.org/bugs/
Mail list info: http://perl.apache.org/maillist/modperl.html
List etiquette: http://perl.apache.org/maillist/email-etiquette.html