Stas Bekman <[EMAIL PROTECTED]> writes:

> Joe Schaefer wrote:

[...]

> > I didn't notice the 5.6.x issue until after I submitted the patch,
> > however the fix for that is very simple, so I'd prefer to wait until
> > the patch is added before adding the 2-3 neded #ifdef 's to
> > modperl_common_util.c. 
> 
> If it's knowingly doesn't work with 5.6 it shouldn't be committed then
> until it's resolved. We try hard to keep cvs HEAD buildable at all times.

No, I don't "know" that this breaks the 5.6 build of mp2, since I never
tried it on 5.6.  The *only* issue is that the size of MGVTBL pointer
has changed from 5.6 to 5.8, so this assignment

  static const MGVTBL modperl_table_magic_prefetch = {0, 0, 0, 0, 0,
                                              modperl_table_magic_copy};

may or may not be tolerable to your favorite compiler. MGVTBL has 
only 5 slots in 5.6, so when say gcc encounters this line, it will
generate something like 

  warning: excess elements in struct initializer

Strictly speaking it is a perfectly legal C expression. However it will 
cause compilation to fail when -Werror is set, so the fix is to add an

  #if PERL_VERSION >= 8 

wrapper here, and do the same thing for its invocation magic in
modperl_hash_tie. That's the entire 4-line fix, and here was 
the apreq2 counterpart to it (top portion only- ignore MAGIC
KEYS): 

  http://marc.theaimsgroup.com/?l=apreq-cvs&m=108970253506645&w=2


> In which case we first need to find out how to clearly indicate to a
> user that values() are broken. 

values() has *never, ever* worked for multivalued keys.
This is a bugfix for values(), and it requires 5.8.0 or better.

The remainder of the patch fixes each() (which again, has 
*never, ever* worked for multivalued keys) so it returns the 
current value in list context().  The each() bugfix applies to
both 5.6.x and 5.8.x.

Beyond this, I have no idea what you are asking for.

> We don't want obscure bug reports from 5.6 users. Can we arrange
> for values() to croak, explaining that it won't work

That would be silly IMO, since values() works fine for tables 
which are not multivalued.  It's only the multivalued case
which is broken.

>  and requires 5.8.1 (or is it 5.8.0)?
                                ^^^^^

5.8.0 - please do not confuse this with the MAGIC KEYS 
experiment going on in apreq2.

>  And of course this needs to be documented.

Good luck with that.  I'm just a lowly user trying to fix
a bug in mp2 that was reported here, on modperl-dev@ a very
long time ago.  I've done my best to provide additional tests 
and documentation to accompany the patch.  If need be I will
gladly submit another patch that includes the missing
ifdefs for 5.6, but much beyond that I'm not willing to go.

-- 
Joe Schaefer


---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to