Mailing List Archive: Devel-PPPort-3.59 drops PERL_BCDVERSION from ppport.h and breaks Text::CSV

Devel-PPPort-3.59 drops PERL_BCDVERSION from ppport.h and breaks Text::CSV_XS

Aug 11, 2020, 4:36 AM

Post #1 of 12 (720 views)

The diff in generated .i for 3.58 vs 3.59, showing that $\ will now
fail being dealt with:

@@ -3,20 +3,13 @@ static int cx_xsCombine (PerlInterpreter
csv_t csv;
int result;

- SV *ors = (my_perl->Iors_sv);
-
cx_SetupCsv (my_perl, &csv, hv, self);
csv.useIO = useIO;

- if (*csv.eol)
- (my_perl->Iors_sv) = ((void *)0);
-
if (useIO && csv.has_hooks & 0x0004)
(void)hook (my_perl, hv, "before_print", av);
result = cx_Combine (my_perl, &csv, io, av);

- (my_perl->Iors_sv) = ors;
-
if (result && !useIO && csv.utf8)
Perl_sv_utf8_upgrade_flags_grow (my_perl, io, 2, 0);
return result;

Looking at the source code, shows to me that PERL_BCDVERSION is not
correctly computed anymore:

#define xsCombine(self,hv,av,io,useIO) cx_xsCombine (aTHX_ self, hv, av, io, useIO)
static int cx_xsCombine (pTHX_ SV *self, HV *hv, AV *av, SV *io, bool useIO) {
csv_t csv;
int result;
#if (PERL_BCDVERSION >= 0x5008000)
SV *ors = PL_ors_sv;
#endif

SetupCsv (&csv, hv, self);
csv.useIO = useIO;
#if (PERL_BCDVERSION >= 0x5008000)
if (*csv.eol)
PL_ors_sv = NULL;
#endif
if (useIO && csv.has_hooks & HOOK_BEFORE_PRINT)
(void)hook (aTHX_ hv, "before_print", av);
result = Combine (&csv, io, av);
#if (PERL_BCDVERSION >= 0x5008000)
PL_ors_sv = ors;
#endif
if (result && !useIO && csv.utf8)
sv_utf8_upgrade (io);
return result;
} /* xsCombine */

--
H.Merijn Brand http://tux.nl Perl Monger http://amsterdam.pm.org/
using perl5.00307 .. 5.31 porting perl5 on HP-UX, AIX, and Linux
https://useplaintext.email https://tux.nl http://www.test-smoke.org
http://qa.perl.org http://www.goldmark.org/jeff/stupid-disclaimers/

Re: Devel-PPPort-3.59 drops PERL_BCDVERSION from ppport.h and breaks Text::CSV_XS [ In reply to ]

atoomic at cpan

Aug 11, 2020, 6:16 AM

Post #2 of 12 (720 views)

Permalink

Nice catch Tux, PERL_BCDVERSION was renamed and is now available as
D_PPP_BCDVERSION in the last version
otherwise you can still use the PERL_VERSION_GE or similar helpers to
replace
the PERL_BCDVERSION check.

nicolas

On Tue, Aug 11, 2020 at 5:36 AM H.Merijn Brand <perl5@tux.freedom.nl> wrote:

> The diff in generated .i for 3.58 vs 3.59, showing that $\ will now
> fail being dealt with:
>
> @@ -3,20 +3,13 @@ static int cx_xsCombine (PerlInterpreter
> csv_t csv;
> int result;
>
> - SV *ors = (my_perl->Iors_sv);
> -
> cx_SetupCsv (my_perl, &csv, hv, self);
> csv.useIO = useIO;
>
> - if (*csv.eol)
> - (my_perl->Iors_sv) = ((void *)0);
> -
> if (useIO && csv.has_hooks & 0x0004)
> (void)hook (my_perl, hv, "before_print", av);
> result = cx_Combine (my_perl, &csv, io, av);
>
> - (my_perl->Iors_sv) = ors;
> -
> if (result && !useIO && csv.utf8)
> Perl_sv_utf8_upgrade_flags_grow (my_perl, io, 2, 0);
> return result;
>
> Looking at the source code, shows to me that PERL_BCDVERSION is not
> correctly computed anymore:
>
> #define xsCombine(self,hv,av,io,useIO) cx_xsCombine (aTHX_ self, hv, av,
> io, useIO)
> static int cx_xsCombine (pTHX_ SV *self, HV *hv, AV *av, SV *io, bool
> useIO) {
> csv_t csv;
> int result;
> #if (PERL_BCDVERSION >= 0x5008000)
> SV *ors = PL_ors_sv;
> #endif
>
> SetupCsv (&csv, hv, self);
> csv.useIO = useIO;
> #if (PERL_BCDVERSION >= 0x5008000)
> if (*csv.eol)
> PL_ors_sv = NULL;
> #endif
> if (useIO && csv.has_hooks & HOOK_BEFORE_PRINT)
> (void)hook (aTHX_ hv, "before_print", av);
> result = Combine (&csv, io, av);
> #if (PERL_BCDVERSION >= 0x5008000)
> PL_ors_sv = ors;
> #endif
> if (result && !useIO && csv.utf8)
> sv_utf8_upgrade (io);
> return result;
> } /* xsCombine */
>
> --
> H.Merijn Brand http://tux.nl Perl Monger http://amsterdam.pm.org/
> using perl5.00307 .. 5.31 porting perl5 on HP-UX, AIX, and Linux
> https://useplaintext.email https://tux.nl http://www.test-smoke.org
> http://qa.perl.org http://www.goldmark.org/jeff/stupid-disclaimers/
>

Re: Devel-PPPort-3.59 drops PERL_BCDVERSION from ppport.h and breaks Text::CSV_XS [ In reply to ]

ilmari at ilmari

Aug 11, 2020, 6:36 AM

Post #3 of 12 (720 views)

Permalink

"Nicolas R." <atoomic@cpan.org> writes:

> Nice catch Tux, PERL_BCDVERSION was renamed and is now available as
> D_PPP_BCDVERSION in the last version
> otherwise you can still use the PERL_VERSION_GE or similar helpers to
> replace
> the PERL_BCDVERSION check.

What's the point of this gratuitous renaming? PERL_BCDVERSION is just
as easy to use correctly as PERL_VERSION_GE() is, and it would
completely unnecessarily break at least 30 CPAN modules:

https://grep.cpanauthors.org/search?q=PERL_BCDVERSION&extension=xs

- ilmari
--
"A disappointingly low fraction of the human race is,
at any given time, on fire." - Stig Sandbeck Mathisen

Re: Devel-PPPort-3.59 drops PERL_BCDVERSION from ppport.h and breaks Text::CSV_XS [ In reply to ]

perl5 at tux

Aug 11, 2020, 6:36 AM

Post #4 of 12 (720 views)

Permalink

On Tue, 11 Aug 2020 07:16:26 -0600, "Nicolas R." <atoomic@cpan.org> wrote:

> Nice catch Tux, PERL_BCDVERSION was renamed and is now available as
> D_PPP_BCDVERSION in the last version

Why? There are (according to Ilmari) 30 distributions that use
PERL_BCDVERSION, which is the one that is least likely to get wrong.

Furthermore, the GNU gcc compiler does, ate least in my cases, *NOT*
warn about PERL_BCDVERSION not being defined (anymore), so I am not
pointed at this change by the compiler.

I disagree with this rename (or I am opposed to this rename), for it
is likely to cause unneeded grief. Ilmari agrees with me.

Instead of just update/upgrade Devel::PPPort and get bug fixes (which
is what XS authors are used to) suddenly stuff might start breaking.

This is not a bugfix.

> otherwise you can still use the PERL_VERSION_GE or similar helpers to
> replace the PERL_BCDVERSION check.

Both require the XS authors to make unneeded changes to the source.

> nicolas
>
> On Tue, Aug 11, 2020 at 5:36 AM H.Merijn Brand <perl5@tux.freedom.nl>
> wrote:
>
> > The diff in generated .i for 3.58 vs 3.59, showing that $\ will now
> > fail being dealt with:
> >
> > @@ -3,20 +3,13 @@ static int cx_xsCombine (PerlInterpreter
> > csv_t csv;
> > int result;
> >
> > - SV *ors = (my_perl->Iors_sv);
> > -
> > cx_SetupCsv (my_perl, &csv, hv, self);
> > csv.useIO = useIO;
> >
> > - if (*csv.eol)
> > - (my_perl->Iors_sv) = ((void *)0);
> > -
> > if (useIO && csv.has_hooks & 0x0004)
> > (void)hook (my_perl, hv, "before_print", av);
> > result = cx_Combine (my_perl, &csv, io, av);
> >
> > - (my_perl->Iors_sv) = ors;
> > -
> > if (result && !useIO && csv.utf8)
> > Perl_sv_utf8_upgrade_flags_grow (my_perl, io, 2, 0);
> > return result;
> >
> > Looking at the source code, shows to me that PERL_BCDVERSION is not
> > correctly computed anymore:
> >
> > #define xsCombine(self,hv,av,io,useIO) cx_xsCombine (aTHX_ self,
> > hv, av, io, useIO)
> > static int cx_xsCombine (pTHX_ SV *self, HV *hv, AV *av, SV *io,
> > bool useIO) {
> > csv_t csv;
> > int result;
> > #if (PERL_BCDVERSION >= 0x5008000)
> > SV *ors = PL_ors_sv;
> > #endif
> >
> > SetupCsv (&csv, hv, self);
> > csv.useIO = useIO;
> > #if (PERL_BCDVERSION >= 0x5008000)
> > if (*csv.eol)
> > PL_ors_sv = NULL;
> > #endif
> > if (useIO && csv.has_hooks & HOOK_BEFORE_PRINT)
> > (void)hook (aTHX_ hv, "before_print", av);
> > result = Combine (&csv, io, av);
> > #if (PERL_BCDVERSION >= 0x5008000)
> > PL_ors_sv = ors;
> > #endif
> > if (result && !useIO && csv.utf8)
> > sv_utf8_upgrade (io);
> > return result;
> > } /* xsCombine */

--
H.Merijn Brand http://tux.nl Perl Monger http://amsterdam.pm.org/
using perl5.00307 .. 5.31 porting perl5 on HP-UX, AIX, and Linux
https://useplaintext.email https://tux.nl http://www.test-smoke.org
http://qa.perl.org http://www.goldmark.org/jeff/stupid-disclaimers/

Re: Devel-PPPort-3.59 drops PERL_BCDVERSION from ppport.h and breaks Text::CSV_XS [ In reply to ]

fawaka at gmail

Aug 11, 2020, 8:01 AM

Post #5 of 12 (720 views)

Permalink

On Tue, Aug 11, 2020 at 3:36 PM Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
wrote:

> "Nicolas R." <atoomic@cpan.org> writes:
>
> > Nice catch Tux, PERL_BCDVERSION was renamed and is now available as
> > D_PPP_BCDVERSION in the last version
> > otherwise you can still use the PERL_VERSION_GE or similar helpers to
> > replace
> > the PERL_BCDVERSION check.
>
> What's the point of this gratuitous renaming? PERL_BCDVERSION is just
> as easy to use correctly as PERL_VERSION_GE() is, and it would
> completely unnecessarily break at least 30 CPAN modules:
>
> https://grep.cpanauthors.org/search?q=PERL_BCDVERSION&extension=xs
>

Agreed. No need to remove it after 20 years of service, even if we now
recommend doing another new thing.

Leon

Re: Devel-PPPort-3.59 drops PERL_BCDVERSION from ppport.h and breaks Text::CSV_XS [ In reply to ]

public at khwilliamson

Aug 11, 2020, 9:15 AM

Post #6 of 12 (718 views)

Permalink

Top posted

The reason I made this change was

1) This appeared to be an internal implementation detail usable only
from D:P, and conflicts with a similar implementation detail furnished
by core perl. This can cause confounding bugs.
2) Hence, I presumed it was not in use by any code, without checking
3) I wanted to stop anyone from using it in the future. Variables named
D_PPP_foo are reserved for D:P itself. Nicolas should not have
suggested you to use any such variable

The renaming needs to be reverted, given that modules use it. I predict
unnecessary bugs as a result however. I will add a warning for its use
that pops up when ppport.h is asked for advice.

On 8/11/20 7:36 AM, H.Merijn Brand wrote:
> On Tue, 11 Aug 2020 07:16:26 -0600, "Nicolas R." <atoomic@cpan.org> wrote:
>
>> Nice catch Tux, PERL_BCDVERSION was renamed and is now available as
>> D_PPP_BCDVERSION in the last version
>
> Why? There are (according to Ilmari) 30 distributions that use
> PERL_BCDVERSION, which is the one that is least likely to get wrong.
>
> Furthermore, the GNU gcc compiler does, ate least in my cases, *NOT*
> warn about PERL_BCDVERSION not being defined (anymore), so I am not
> pointed at this change by the compiler.
>
> I disagree with this rename (or I am opposed to this rename), for it
> is likely to cause unneeded grief. Ilmari agrees with me.
>
> Instead of just update/upgrade Devel::PPPort and get bug fixes (which
> is what XS authors are used to) suddenly stuff might start breaking.
>
> This is not a bugfix.
>
>> otherwise you can still use the PERL_VERSION_GE or similar helpers to
>> replace the PERL_BCDVERSION check.
>
> Both require the XS authors to make unneeded changes to the source.
>
>> nicolas
>>
>> On Tue, Aug 11, 2020 at 5:36 AM H.Merijn Brand <perl5@tux.freedom.nl>
>> wrote:
>>
>>> The diff in generated .i for 3.58 vs 3.59, showing that $\ will now
>>> fail being dealt with:
>>>
>>> @@ -3,20 +3,13 @@ static int cx_xsCombine (PerlInterpreter
>>> csv_t csv;
>>> int result;
>>>
>>> - SV *ors = (my_perl->Iors_sv);
>>> -
>>> cx_SetupCsv (my_perl, &csv, hv, self);
>>> csv.useIO = useIO;
>>>
>>> - if (*csv.eol)
>>> - (my_perl->Iors_sv) = ((void *)0);
>>> -
>>> if (useIO && csv.has_hooks & 0x0004)
>>> (void)hook (my_perl, hv, "before_print", av);
>>> result = cx_Combine (my_perl, &csv, io, av);
>>>
>>> - (my_perl->Iors_sv) = ors;
>>> -
>>> if (result && !useIO && csv.utf8)
>>> Perl_sv_utf8_upgrade_flags_grow (my_perl, io, 2, 0);
>>> return result;
>>>
>>> Looking at the source code, shows to me that PERL_BCDVERSION is not
>>> correctly computed anymore:
>>>
>>> #define xsCombine(self,hv,av,io,useIO) cx_xsCombine (aTHX_ self,
>>> hv, av, io, useIO)
>>> static int cx_xsCombine (pTHX_ SV *self, HV *hv, AV *av, SV *io,
>>> bool useIO) {
>>> csv_t csv;
>>> int result;
>>> #if (PERL_BCDVERSION >= 0x5008000)
>>> SV *ors = PL_ors_sv;
>>> #endif
>>>
>>> SetupCsv (&csv, hv, self);
>>> csv.useIO = useIO;
>>> #if (PERL_BCDVERSION >= 0x5008000)
>>> if (*csv.eol)
>>> PL_ors_sv = NULL;
>>> #endif
>>> if (useIO && csv.has_hooks & HOOK_BEFORE_PRINT)
>>> (void)hook (aTHX_ hv, "before_print", av);
>>> result = Combine (&csv, io, av);
>>> #if (PERL_BCDVERSION >= 0x5008000)
>>> PL_ors_sv = ors;
>>> #endif
>>> if (result && !useIO && csv.utf8)
>>> sv_utf8_upgrade (io);
>>> return result;
>>> } /* xsCombine */
>

Re: Devel-PPPort-3.59 drops PERL_BCDVERSION from ppport.h and breaks Text::CSV_XS [ In reply to ]

fawaka at gmail

Aug 11, 2020, 9:40 AM

Post #7 of 12 (718 views)

Permalink

On Tue, Aug 11, 2020 at 6:16 PM Karl Williamson <public@khwilliamson.com>
wrote:

> Top posted
>
> The reason I made this change was
>
> 1) This appeared to be an internal implementation detail usable only
> from D:P, and conflicts with a similar implementation detail furnished
> by core perl. This can cause confounding bugs.
> 2) Hence, I presumed it was not in use by any code, without checking
> 3) I wanted to stop anyone from using it in the future. Variables named
> D_PPP_foo are reserved for D:P itself. Nicolas should not have
> suggested you to use any such variable
>
> The renaming needs to be reverted, given that modules use it. I predict
> unnecessary bugs as a result however. I will add a warning for its use
> that pops up when ppport.h is asked for advice.
>

The perl script ppport.h is all about patching up your code. Letting it to
turn «PERL_BCDVERSION >= 0x5013006» into «PERL_VERSION_GE(5,13,6)» (and the
LT equivalent) seems entirely possible, though it's a good idea to leave
PERL_BCDVERSION in for any case it doesn't handle.

Leon

Re: Devel-PPPort-3.59 drops PERL_BCDVERSION from ppport.h and breaks Text::CSV_XS [ In reply to ]

atoomic at cpan

Aug 11, 2020, 10:13 AM

Post #8 of 12 (718 views)

Permalink

I agree with Karl, we are going to restore PERL_BCDVERSION and
perform another release to fix that issue.
The delta of changes was so big, that I missed it when working on the
Changelog, and when I reviewed this change

I was also incorrectly sharing the assumption this was D-P internal logic.

We are going to release a fix with it

nicolas

On Tue, Aug 11, 2020 at 10:41 AM Leon Timmermans <fawaka@gmail.com> wrote:

> On Tue, Aug 11, 2020 at 6:16 PM Karl Williamson <public@khwilliamson.com>
> wrote:
>
>> Top posted
>>
>> The reason I made this change was
>>
>> 1) This appeared to be an internal implementation detail usable only
>> from D:P, and conflicts with a similar implementation detail furnished
>> by core perl. This can cause confounding bugs.
>> 2) Hence, I presumed it was not in use by any code, without checking
>> 3) I wanted to stop anyone from using it in the future. Variables named
>> D_PPP_foo are reserved for D:P itself. Nicolas should not have
>> suggested you to use any such variable
>>
>> The renaming needs to be reverted, given that modules use it. I predict
>> unnecessary bugs as a result however. I will add a warning for its use
>> that pops up when ppport.h is asked for advice.
>>
>
> The perl script ppport.h is all about patching up your code. Letting it to
> turn «PERL_BCDVERSION >= 0x5013006» into «PERL_VERSION_GE(5,13,6)» (and the
> LT equivalent) seems entirely possible, though it's a good idea to leave
> PERL_BCDVERSION in for any case it doesn't handle.
>
> Leon
>

Re: Devel-PPPort-3.59 drops PERL_BCDVERSION from ppport.h and breaks Text::CSV_XS [ In reply to ]

atoomic at cpan

Aug 11, 2020, 2:24 PM

Post #9 of 12 (718 views)

Permalink

Hi Tux, Devel-PPPort 3.60 was released and should fix your issue
thanks for confirming

nicolas

On Tue, Aug 11, 2020 at 11:13 AM Nicolas R. <atoomic@cpan.org> wrote:

> I agree with Karl, we are going to restore PERL_BCDVERSION and
> perform another release to fix that issue.
> The delta of changes was so big, that I missed it when working on the
> Changelog, and when I reviewed this change
>
> I was also incorrectly sharing the assumption this was D-P internal logic.
>
> We are going to release a fix with it
>
> nicolas
>
>
>
> On Tue, Aug 11, 2020 at 10:41 AM Leon Timmermans <fawaka@gmail.com> wrote:
>
>> On Tue, Aug 11, 2020 at 6:16 PM Karl Williamson <public@khwilliamson.com>
>> wrote:
>>
>>> Top posted
>>>
>>> The reason I made this change was
>>>
>>> 1) This appeared to be an internal implementation detail usable only
>>> from D:P, and conflicts with a similar implementation detail furnished
>>> by core perl. This can cause confounding bugs.
>>> 2) Hence, I presumed it was not in use by any code, without checking
>>> 3) I wanted to stop anyone from using it in the future. Variables named
>>> D_PPP_foo are reserved for D:P itself. Nicolas should not have
>>> suggested you to use any such variable
>>>
>>> The renaming needs to be reverted, given that modules use it. I predict
>>> unnecessary bugs as a result however. I will add a warning for its use
>>> that pops up when ppport.h is asked for advice.
>>>
>>
>> The perl script ppport.h is all about patching up your code. Letting it
>> to turn «PERL_BCDVERSION >= 0x5013006» into «PERL_VERSION_GE(5,13,6)» (and
>> the LT equivalent) seems entirely possible, though it's a good idea to
>> leave PERL_BCDVERSION in for any case it doesn't handle.
>>
>> Leon
>>
>

Re: Devel-PPPort-3.59 drops PERL_BCDVERSION from ppport.h and breaks Text::CSV_XS [ In reply to ]

perl5 at tux

Aug 12, 2020, 12:24 AM

Post #10 of 12 (718 views)

Permalink

On Tue, 11 Aug 2020 15:24:51 -0600, "Nicolas R." <atoomic@cpan.org>
wrote:

> Hi Tux, Devel-PPPort 3.60 was released and should fix your issue
> thanks for confirming

????

All tests successful.
Files=28, Tests=52131, 19 wallclock secs ( 3.54 usr 0.29 sys + 17.94 cusr 0.43 csys = 22.20 CPU)
Result: PASS

> nicolas
>
> On Tue, Aug 11, 2020 at 11:13 AM Nicolas R. <atoomic@cpan.org> wrote:
>
> > I agree with Karl, we are going to restore PERL_BCDVERSION and
> > perform another release to fix that issue.
> > The delta of changes was so big, that I missed it when working on
> > the Changelog, and when I reviewed this change
> >
> > I was also incorrectly sharing the assumption this was D-P internal
> > logic.
> >
> > We are going to release a fix with it
> >
> > nicolas

--
H.Merijn Brand http://tux.nl Perl Monger http://amsterdam.pm.org/
using perl5.00307 .. 5.31 porting perl5 on HP-UX, AIX, and Linux
https://useplaintext.email https://tux.nl http://www.test-smoke.org
http://qa.perl.org http://www.goldmark.org/jeff/stupid-disclaimers/

Re: Devel-PPPort-3.59 drops PERL_BCDVERSION from ppport.h and breaksText::CSV_XS [ In reply to ]

bulk88 at hotmail

Sep 16, 2020, 10:47 PM

Post #11 of 12 (626 views)

Permalink

H.Merijn Brand wrote:
> Looking at the source code, shows to me that PERL_BCDVERSION is not
> correctly computed anymore:

PERL_BCDVERSION is a piece of ppport.h unique API. IIRC, the ONLY API
that ppport adds instead of polyfills.

Re: Devel-PPPort-3.59 drops PERL_BCDVERSION from ppport.h and breaksText::CSV_XS [ In reply to ]

public at khwilliamson

Sep 22, 2020, 8:32 AM

Post #12 of 12 (621 views)

Permalink

On 9/16/20 11:47 PM, bulk88 wrote:
> H.Merijn Brand wrote:
>> Looking at the source code, shows to me that PERL_BCDVERSION is not
>> correctly computed anymore:
>
> PERL_BCDVERSION is a piece of ppport.h unique API. IIRC, the ONLY API
> that ppport adds instead of polyfills.

I'm trying to understand why you sent this now. 3.60 restored it, and
is working AFAIK.