From myon@debian.org

The change "properly define perl_parse() return value" with git hash
0301e89 breaks PostgreSQL's pl/perl procedural language, because the
return value isn't 0 anymore.

Perl change by Zefram​:
https://perl5.git.perl.org/perl.git/commitdiff/0301e899536a22752f40481d8a1d141b7a7dda82

PostgreSQL problem analysis by Tom Lane​:
https://www.postgresql.org/message-id/23260.1527026547%40sss.pgh.pa.us

Perl analysis by Matt S Trout​:
https://www.nntp.perl.org/group/perl.perl5.porters/2018/05/msg251029.html

Please consider reverting this, or clarify how users should handle the
return value.

Thanks,
Christoph

From @jmdh

Quick side-note in Debian we also notice breakage of barnowl (https://barnowl.mit.edu/) at least in the test suite, which is caused by perl_run returning 256​:

http​://perl.debian.net/rebuild-logs/perl-5.27/barnowl_1.9-4/barnowl_1.9-4+b3_amd64-2018-05-19T10​:29​:01Z.build

Not sure if there is runtime breakage too.

The RT System itself - Status changed from 'new' to 'open'

From @iabyn

On Wed, 23 May 2018 13​:38​:25 -0700, Christoph Berg via RT wrote​:

The change "properly define perl_parse() return value" with git hash
0301e89 breaks PostgreSQL's pl/perl procedural language, because the
return value isn't 0 anymore.

On Wed, May 23, 2018 at 01​:44​:34PM -0700, Dominic Hargreaves via RT wrote​:

Quick side-note in Debian we also notice breakage of barnowl
(https://barnowl.mit.edu/) at least in the test suite, which is caused
by perl_run returning 256​:

Just to clarify something here​:

Zefram's commit changed the behaviour of the return value of both
perl_parse() and perl_run(), but from the various linked-to discussions
and an initial inspection, I think that only the perl_run() change is
potentially problematical (due to the fact that S_run_body() calls
my_exit(0) rather than just returning).

However, this ticket's subject line claims that the issue is with
perl_parse(), which doesn't seem to be the case. I would like
clarification that perl_parse() isn't an issue.

My gut feeling is that the change to perl_run should be reverted, but the
perl_parse() change be kept.

--
"You may not work around any technical limitations in the software"
  -- Windows Vista license

From @jkeenan

On Wed, 23 May 2018 20​:44​:34 GMT, dom wrote​:

Quick side-note in Debian we also notice breakage of barnowl
(https://barnowl.mit.edu/) at least in the test suite, which is caused
by perl_run returning 256​:

http​://perl.debian.net/rebuild-logs/perl-5.27/barnowl_1.9-
4/barnowl_1.9-4+b3_amd64-2018-05-19T10​:29​:01Z.build

Not sure if there is runtime breakage too.

Dom​: Is there any way the tests found in the link you posted above could be incorporated into the core test suite so that, once this problem is addressed, we get early warnings of problems in the future?

Thank you very much.
Jim Keenan
--
James E Keenan (jkeenan@​cpan.org)

From @iabyn

On Wed, May 23, 2018 at 02​:36​:52PM -0700, James E Keenan via RT wrote​:

Dom​: Is there any way the tests found in the link you posted above
could be incorporated into the core test suite so that, once this
problem is addressed, we get early warnings of problems in the future?

This bug is related to how the perl interpreter is embedded in, and called
by, a third-party application. So there's nothing directly from those
tests which could be incorporated into the perl distribution.

Instead, we (p5p) would have to code up an embedded interpreter - probably
within XS​::APItest - then test various combos of running to completion or
doing exit(0) / exit(non-zero).

--
A walk of a thousand miles begins with a single step...
then continues for another 1,999,999 or so.

From @df7cb

On Wed, 23 May 2018 14​:35​:34 -0700, davem wrote​:

However, this ticket's subject line claims that the issue is with
perl_parse(), which doesn't seem to be the case. I would like
clarification that perl_parse() isn't an issue.

plperl.c is failing on perl_run in line 860​:
https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/pl/plperl/plperl.c;hb=HEAD#l860

Sorry for the bad title, that was just me sloppily copying from the git commit message.

From @jkeenan

On Wed, 23 May 2018 21​:35​:34 GMT, davem wrote​:

On Wed, 23 May 2018 13​:38​:25 -0700, Christoph Berg via RT wrote​:

The change "properly define perl_parse() return value" with git hash
0301e89 breaks PostgreSQL's pl/perl procedural language, because the
return value isn't 0 anymore.

On Wed, May 23, 2018 at 01​:44​:34PM -0700, Dominic Hargreaves via RT wrote​:

Quick side-note in Debian we also notice breakage of barnowl
(https://barnowl.mit.edu/) at least in the test suite, which is caused
by perl_run returning 256​:

Just to clarify something here​:

Zefram's commit changed the behaviour of the return value of both
perl_parse() and perl_run(), but from the various linked-to discussions
and an initial inspection, I think that only the perl_run() change is
potentially problematical (due to the fact that S_run_body() calls
my_exit(0) rather than just returning).

However, this ticket's subject line claims that the issue is with
perl_parse(), which doesn't seem to be the case. I would like
clarification that perl_parse() isn't an issue.

My gut feeling is that the change to perl_run should be reverted, but the
perl_parse() change be kept.

On the basis of this comment from Dave Mitchell, I'm designating this ticket as a blocker, meaning that some decision will have to be made on it before 5.28.0 can be released.

Thank you very much.
Jim Keenan

--
James E Keenan (jkeenan@​cpan.org)

From @df7cb

Reverting perl_run and leaving perl_parse alone is, I think, what mst was getting at in his email.

From @jkeenan

On Wed, 23 May 2018 21​:35​:34 GMT, davem wrote​:

On Wed, 23 May 2018 13​:38​:25 -0700, Christoph Berg via RT wrote​:

The change "properly define perl_parse() return value" with git hash
0301e89 breaks PostgreSQL's pl/perl procedural language, because the
return value isn't 0 anymore.

On Wed, May 23, 2018 at 01​:44​:34PM -0700, Dominic Hargreaves via RT wrote​:

Quick side-note in Debian we also notice breakage of barnowl
(https://barnowl.mit.edu/) at least in the test suite, which is caused
by perl_run returning 256​:

Just to clarify something here​:

Zefram's commit changed the behaviour of the return value of both
perl_parse() and perl_run(), but from the various linked-to discussions
and an initial inspection, I think that only the perl_run() change is
potentially problematical (due to the fact that S_run_body() calls
my_exit(0) rather than just returning).

However, this ticket's subject line claims that the issue is with
perl_parse(), which doesn't seem to be the case. I would like
clarification that perl_parse() isn't an issue.

My gut feeling is that the change to perl_run should be reverted, but the
perl_parse() change be kept.

Would it be possible to get a branch which we could use for smoke-testing and which the Debian folks could use for 3rd-party application testing?

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

From @iabyn

On Wed, May 23, 2018 at 10​:35​:18PM +0100, Dave Mitchell wrote​:

My gut feeling is that the change to perl_run should be reverted, but the
perl_parse() change be kept.

Which I've now done in blead with v5.28.0-RC1-15-g9f9606382c.

I've written some some tests, but not pushed them, because they
potentially run the risk of spuriously failing due to platform or build
combos - i.e. I think the revert is solid, but the tests are fragile, and
it's too late into the RC cycle to make the tests solid. I'll add them to
blead in 5.29.

--
Spock (or Data) is fired from his high-ranking position for not being able
to understand the most basic nuances of about one in three sentences that
anyone says to him.
  -- Things That Never Happen in "Star Trek" #19

From @khwilliamson

On Mon, 28 May 2018 08​:16​:26 -0700, davem wrote​:

On Wed, May 23, 2018 at 10​:35​:18PM +0100, Dave Mitchell wrote​:

My gut feeling is that the change to perl_run should be reverted, but the
perl_parse() change be kept.

Which I've now done in blead with v5.28.0-RC1-15-g9f9606382c.

I've written some some tests, but not pushed them, because they
potentially run the risk of spuriously failing due to platform or build
combos - i.e. I think the revert is solid, but the tests are fragile, and
it's too late into the RC cycle to make the tests solid. I'll add them to
blead in 5.29.

Did these tests get added?

--
Karl Williamson

From @iabyn

On Thu, Mar 28, 2019 at 07​:29​:50PM -0700, Karl Williamson via RT wrote​:

On Mon, 28 May 2018 08​:16​:26 -0700, davem wrote​:

On Wed, May 23, 2018 at 10​:35​:18PM +0100, Dave Mitchell wrote​:

My gut feeling is that the change to perl_run should be reverted, but the
perl_parse() change be kept.

Which I've now done in blead with v5.28.0-RC1-15-g9f9606382c.

I've written some some tests, but not pushed them, because they
potentially run the risk of spuriously failing due to platform or build
combos - i.e. I think the revert is solid, but the tests are fragile, and
it's too late into the RC cycle to make the tests solid. I'll add them to
blead in 5.29.

Did these tests get added?

No. Not yet.

--
My get-up-and-go just got up and went.

From @iabyn

I've moved this ticket from 5.30.0 blocker to 5.32.0 blocker

From @jkeenan

On Mon, 28 May 2018 15​:16​:26 GMT, davem wrote​:

On Wed, May 23, 2018 at 10​:35​:18PM +0100, Dave Mitchell wrote​:

My gut feeling is that the change to perl_run should be reverted, but the
perl_parse() change be kept.

Which I've now done in blead with v5.28.0-RC1-15-g9f9606382c.

I've written some some tests, but not pushed them, because they
potentially run the risk of spuriously failing due to platform or build
combos - i.e. I think the revert is solid, but the tests are fragile, and
it's too late into the RC cycle to make the tests solid. I'll add them to
blead in 5.29.

Since we're early in the perl-5.31 dev cycle, could we get these tests added now?

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@iabyn If your fix was added to blead in 5.29, can we close this ticket already? We can open a separate ticket for the tests (with the code branch).