Why do we have perl and sed versions of Gen_dummy_probes?

Andres Freund <andres@anarazel.de> writes:

I understand why we don't want to rely on sed because of windows - but
it's far from obvious why we can't just use the .pl variant all the
time?

Perl is not considered a hard build requirement on non-Windows.
We could dodge that by shipping a pre-built dummy probes.h,
but that doesn't really seem like a cleaner way than what's
there now.

Also, as I read it, Gen_dummy_probes.sed is useful in any case as
being the "source code" for Gen_dummy_probes.pl. You'd need some
other form of documentation if you removed it.

regards, tom lane

Hi,

On 2021-05-06 00:18:12 -0400, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

I understand why we don't want to rely on sed because of windows - but
it's far from obvious why we can't just use the .pl variant all the
time?

Perl is not considered a hard build requirement on non-Windows.

Oops, forgot that.

We could dodge that by shipping a pre-built dummy probes.h,
but that doesn't really seem like a cleaner way than what's
there now.

I tried to regenerate Gen_dummy_probes.pl using s2p - which doesn't seem
to exist for modern versions of perl anymore :(

Also, as I read it, Gen_dummy_probes.sed is useful in any case as
being the "source code" for Gen_dummy_probes.pl. You'd need some
other form of documentation if you removed it.

:/

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:

I tried to regenerate Gen_dummy_probes.pl using s2p - which doesn't seem
to exist for modern versions of perl anymore :(

It still exists, it's just not part of the core Perl distribution any
more (since 5.22, released in 2015):

https://metacpan.org/pod/perl5220delta#find2perl,-s2p-and-a2p-removal
https://metacpan.org/release/App-s2p.

You can install it with `cpan App::s2p`.

- ilmari
--
"The surreality of the universe tends towards a maximum" -- Skud's Law
"Never formulate a law or axiom that you're not prepared to live with
the consequences of." -- Skud's Meta-Law

On 5/6/21 12:59 AM, Andres Freund wrote:

Hi,

On 2021-05-06 00:18:12 -0400, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

I understand why we don't want to rely on sed because of windows - but
it's far from obvious why we can't just use the .pl variant all the
time?

Perl is not considered a hard build requirement on non-Windows.

Oops, forgot that.

We could dodge that by shipping a pre-built dummy probes.h,
but that doesn't really seem like a cleaner way than what's
there now.

I tried to regenerate Gen_dummy_probes.pl using s2p - which doesn't seem
to exist for modern versions of perl anymore :(

Also, as I read it, Gen_dummy_probes.sed is useful in any case as
being the "source code" for Gen_dummy_probes.pl. You'd need some
other form of documentation if you removed it.

I suggest we add a README that sets out

a) why we do things this way

b) that the sed script is what's authoritative

c) how to regenerate the perl script if you change the sed script,
including where to get s2p

I can do that.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

On 5/6/21 9:55 AM, Andrew Dunstan wrote:

On 5/6/21 12:59 AM, Andres Freund wrote:

Hi,

On 2021-05-06 00:18:12 -0400, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

I understand why we don't want to rely on sed because of windows - but
it's far from obvious why we can't just use the .pl variant all the
time?

Perl is not considered a hard build requirement on non-Windows.

Oops, forgot that.

We could dodge that by shipping a pre-built dummy probes.h,
but that doesn't really seem like a cleaner way than what's
there now.

I tried to regenerate Gen_dummy_probes.pl using s2p - which doesn't seem
to exist for modern versions of perl anymore :(

Also, as I read it, Gen_dummy_probes.sed is useful in any case as
being the "source code" for Gen_dummy_probes.pl. You'd need some
other form of documentation if you removed it.

I suggest we add a README that sets out

a) why we do things this way

b) that the sed script is what's authoritative

c) how to regenerate the perl script if you change the sed script,
including where to get s2p

I can do that.

Here's a patch that adds the README and also adds a Makefile recipe for
regenerating Gen_dummy_probes.pl after the sed script is changed. On my
system at least the recipe is idempotent.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Andrew Dunstan <andrew@dunslane.net> writes:

Here's a patch that adds the README and also adds a Makefile recipe for
regenerating Gen_dummy_probes.pl after the sed script is changed. On my
system at least the recipe is idempotent.

I've not tested the Makefile recipe, but the README looks good.

regards, tom lane

Hi,

On 2021-05-07 11:19:02 -0400, Andrew Dunstan wrote:

Here's a patch that adds the README and also adds a Makefile recipe for
regenerating Gen_dummy_probes.pl after the sed script is changed. On my
system at least the recipe is idempotent.

Nice! Thanks for this work.

Greetings,

Andres Freund

Hi,

On 2021-05-06 11:13:28 +0100, Dagfinn Ilmari Manns�ker wrote:

Andres Freund <andres@anarazel.de> writes:

I tried to regenerate Gen_dummy_probes.pl using s2p - which doesn't seem
to exist for modern versions of perl anymore :(

It still exists, it's just not part of the core Perl distribution any
more (since 5.22, released in 2015):

https://metacpan.org/pod/perl5220delta#find2perl,-s2p-and-a2p-removal
https://metacpan.org/release/App-s2p.

Oh, I got confused because the cpan link at the top of
https://perldoc.perl.org/5.6.2/s2p is dead, and because I forgot all I
knew about perl a long time ago.

Greetings,

Andres Freund

On 5/7/21 1:20 PM, Andres Freund wrote:

Hi,

On 2021-05-07 11:19:02 -0400, Andrew Dunstan wrote:

Here's a patch that adds the README and also adds a Makefile recipe for
regenerating Gen_dummy_probes.pl after the sed script is changed. On my
system at least the recipe is idempotent.

Nice! Thanks for this work.

de nada. pushed.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

On 07.05.21 20:31, Andrew Dunstan wrote:

On 5/7/21 1:20 PM, Andres Freund wrote:

On 2021-05-07 11:19:02 -0400, Andrew Dunstan wrote:

Here's a patch that adds the README and also adds a Makefile recipe for
regenerating Gen_dummy_probes.pl after the sed script is changed. On my
system at least the recipe is idempotent.

Nice! Thanks for this work.

de nada. pushed.

This recipe doesn't produce a Gen_dummy_probes.pl that matches exactly
the one that is there now. If this is going to be the preferred method,
then we should generate it once so that it matches going forward.

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

On 07.05.21 20:31, Andrew Dunstan wrote:

On 5/7/21 1:20 PM, Andres Freund wrote:

On 2021-05-07 11:19:02 -0400, Andrew Dunstan wrote:

Here's a patch that adds the README and also adds a Makefile recipe for
regenerating Gen_dummy_probes.pl after the sed script is changed. On my
system at least the recipe is idempotent.

Nice! Thanks for this work.

de nada. pushed.

This recipe doesn't produce a Gen_dummy_probes.pl that matches exactly
the one that is there now. If this is going to be the preferred method,
then we should generate it once so that it matches going forward.

Which version of perltidy do you have installed? For me it generates
identical versions using any of 20170521 (per src/tools/pgindent/README),
20201207 (what I happened to have installed before), and 20210402 (the
latest).

Also, what does the difference look like?

- ilmari
--
- Twitter seems more influential [than blogs] in the 'gets reported in
the mainstream press' sense at least. - Matt McLeod
- That'd be because the content of a tweet is easier to condense down
to a mainstream media article. - Calle Dybedahl

On 5/10/21 7:16 AM, Dagfinn Ilmari Mannsåker wrote:

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

On 07.05.21 20:31, Andrew Dunstan wrote:

On 5/7/21 1:20 PM, Andres Freund wrote:

On 2021-05-07 11:19:02 -0400, Andrew Dunstan wrote:

Here's a patch that adds the README and also adds a Makefile recipe for
regenerating Gen_dummy_probes.pl after the sed script is changed. On my
system at least the recipe is idempotent.

Nice! Thanks for this work.

de nada. pushed.

This recipe doesn't produce a Gen_dummy_probes.pl that matches exactly
the one that is there now. If this is going to be the preferred method,
then we should generate it once so that it matches going forward.

Which version of perltidy do you have installed? For me it generates
identical versions using any of 20170521 (per src/tools/pgindent/README),
20201207 (what I happened to have installed before), and 20210402 (the
latest).

Also, what does the difference look like?

Yep:

andrew@emma:utils $ touch Gen_dummy_probes.sed
andrew@emma:utils $ touch ../../../src/Makefile.global
andrew@emma:utils $ make top_srcdir=../../.. Gen_dummy_probes.pl
perl -ni -e ' print; exit if /^\$0/;' Gen_dummy_probes.pl
s2p -f Gen_dummy_probes.sed  | sed -e 1,4d -e '/# #/d' -e '$d' >>
Gen_dummy_probes.pl
perltidy --profile=../../tools/pgindent/perltidyrc Gen_dummy_probes.pl
perl -pi -e '!$lb && ( /^\t+#/  || /^# prototypes/ ) && print qq{\n};'\
    -e '$lb = m/^\n/; ' Gen_dummy_probes.pl
andrew@emma:utils $ git diff
andrew@emma:utils $ perltidy --version
This is perltidy, v20170521

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Andrew Dunstan <andrew@dunslane.net> writes:

On 5/10/21 7:16 AM, Dagfinn Ilmari Mannsåker wrote:

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

This recipe doesn't produce a Gen_dummy_probes.pl that matches exactly
the one that is there now. If this is going to be the preferred method,
then we should generate it once so that it matches going forward.

Which version of perltidy do you have installed? For me it generates
identical versions using any of 20170521 (per src/tools/pgindent/README),
20201207 (what I happened to have installed before), and 20210402 (the
latest).

Yep:

For me, using App-s2p-1.003 and perltidy v20170521, it works
as long as I start with the previous version of
Gen_dummy_probes.pl in place. I first tried to test this by
"rm Gen_dummy_probes.pl; make Gen_dummy_probes.pl", and what
I got was a script without all the initial commentary nor
the first line of actual Perl code.

I don't think this is good practice; it implies that any
accidental corruption of the commentary would be carried
forward. I think we should be extracting the commentary
from Gen_dummy_probes.sed.

regards, tom lane

On 5/10/21 12:07 PM, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

On 5/10/21 7:16 AM, Dagfinn Ilmari Mannsåker wrote:

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

This recipe doesn't produce a Gen_dummy_probes.pl that matches exactly
the one that is there now. If this is going to be the preferred method,
then we should generate it once so that it matches going forward.

Which version of perltidy do you have installed? For me it generates
identical versions using any of 20170521 (per src/tools/pgindent/README),
20201207 (what I happened to have installed before), and 20210402 (the
latest).

Yep:

For me, using App-s2p-1.003 and perltidy v20170521, it works
as long as I start with the previous version of
Gen_dummy_probes.pl in place. I first tried to test this by
"rm Gen_dummy_probes.pl; make Gen_dummy_probes.pl", and what
I got was a script without all the initial commentary nor
the first line of actual Perl code.

I don't think this is good practice; it implies that any
accidental corruption of the commentary would be carried
forward. I think we should be extracting the commentary
from Gen_dummy_probes.sed.

I don't know how likely accidental corruption is, but OK, let's not make
the next generation dependent on the current generation of the file. The
simplest way around that seems to me to cache the perl prolog, as in the
attached patch Is that more to your liking? I also adjusted it so we
pick up the first line of code from s2p rather than from the prolog,
which is now just comments and the #! line.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Andrew Dunstan <andrew@dunslane.net> writes:

On 5/10/21 12:07 PM, Tom Lane wrote:

I don't think this is good practice; it implies that any
accidental corruption of the commentary would be carried
forward. I think we should be extracting the commentary
from Gen_dummy_probes.sed.

I don't know how likely accidental corruption is, but OK, let's not make
the next generation dependent on the current generation of the file. The
simplest way around that seems to me to cache the perl prolog, as in the
attached patch Is that more to your liking? I also adjusted it so we
pick up the first line of code from s2p rather than from the prolog,
which is now just comments and the #! line.

Works for me. One other thought --- do we care whether this works
in a VPATH build, and if so does it? The $< and $@ references should
be OK, but I'm betting you need $(srcdir)/Gen_dummy_probes.pl.prolog
or the like.

regards, tom lane

On 5/11/21 10:52 AM, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

On 5/10/21 12:07 PM, Tom Lane wrote:

I don't think this is good practice; it implies that any
accidental corruption of the commentary would be carried
forward. I think we should be extracting the commentary
from Gen_dummy_probes.sed.

I don't know how likely accidental corruption is, but OK, let's not make
the next generation dependent on the current generation of the file. The
simplest way around that seems to me to cache the perl prolog, as in the
attached patch Is that more to your liking? I also adjusted it so we
pick up the first line of code from s2p rather than from the prolog,
which is now just comments and the #! line.

Works for me. One other thought --- do we care whether this works
in a VPATH build, and if so does it? The $< and $@ references should
be OK, but I'm betting you need $(srcdir)/Gen_dummy_probes.pl.prolog
or the like.

Why would we? It's only used in Windows builds, and there's no VPATH
there (sadly). In fact, building the file isn't part of any standard
build procedure. I think this is probably in the same boat as the SSL
certs we make in src/test/ssl - I don't think those recipes are meant
for use in VPATH builds either.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Hi,

On 2021-05-11 10:52:22 -0400, Tom Lane wrote:

Works for me. One other thought --- do we care whether this works
in a VPATH build, and if so does it? The $< and $@ references should
be OK, but I'm betting you need $(srcdir)/Gen_dummy_probes.pl.prolog
or the like.

It doesn't work in a VPATH build right now, FWIW. $@, $< will point to a
local file in the build directory, right now. And the path to perltidyrc
doesn't work either. It seems to work after the following modifications

diff --git i/src/backend/utils/Makefile w/src/backend/utils/Makefile
index bcf9dd41adf..ca733d12dce 100644
--- i/src/backend/utils/Makefile
+++ w/src/backend/utils/Makefile
@@ -92,10 +92,10 @@ $(top_builddir)/src/include/utils/probes.h: probes.h
 # Nothing depends on it, so it will never be called unless explicitly requested
 # The last two lines of the recipe format the script according to  our
 # standard and put back some blank lines for improved readability.
-Gen_dummy_probes.pl: Gen_dummy_probes.sed
+$(top_srcdir)/src/backend/utils/Gen_dummy_probes.pl: $(top_srcdir)/src/backend/utils/Gen_dummy_probes.sed
     perl -ni -e ' print; exit if /^\$$0/;' $@
     s2p -f $<  | sed -e 1,4d -e '/# #/d' -e '$$d' >> $@
-    perltidy --profile=../../tools/pgindent/perltidyrc $@
+    perltidy --profile=$(top_srcdir)/src/tools/pgindent/perltidyrc $@
     perl -pi -e '!$$lb && ( /^\t+#/  || /^# prototypes/ ) && print qq{\n};'\
         -e '$$lb = m/^\n/; ' $@

diff --git i/src/test/regress/parallel_schedule w/src/test/regress/parallel_schedule

Greetings,

Andres Freund

Hi,

On 2021-05-11 11:44:22 -0400, Andrew Dunstan wrote:

Why would we? It's only used in Windows builds, and there's no VPATH
there (sadly).

Is that really relevant? We'll need to update the file on any platform
when modifying the .sed, not just in windows.

Greetings,

Andres Freund

On 5/11/21 1:21 PM, Andres Freund wrote:

Hi,

On 2021-05-11 10:52:22 -0400, Tom Lane wrote:

Works for me. One other thought --- do we care whether this works
in a VPATH build, and if so does it? The $< and $@ references should
be OK, but I'm betting you need $(srcdir)/Gen_dummy_probes.pl.prolog
or the like.

It doesn't work in a VPATH build right now, FWIW. $@, $< will point to a
local file in the build directory, right now. And the path to perltidyrc
doesn't work either. It seems to work after the following modifications

diff --git i/src/backend/utils/Makefile w/src/backend/utils/Makefile
index bcf9dd41adf..ca733d12dce 100644
--- i/src/backend/utils/Makefile
+++ w/src/backend/utils/Makefile
@@ -92,10 +92,10 @@ $(top_builddir)/src/include/utils/probes.h: probes.h
# Nothing depends on it, so it will never be called unless explicitly requested
# The last two lines of the recipe format the script according to  our
# standard and put back some blank lines for improved readability.
-Gen_dummy_probes.pl: Gen_dummy_probes.sed
+$(top_srcdir)/src/backend/utils/Gen_dummy_probes.pl: $(top_srcdir)/src/backend/utils/Gen_dummy_probes.sed
perl -ni -e ' print; exit if /^\$$0/;' $@
s2p -f $<  | sed -e 1,4d -e '/# #/d' -e '$$d' >> $@
-    perltidy --profile=../../tools/pgindent/perltidyrc $@
+    perltidy --profile=$(top_srcdir)/src/tools/pgindent/perltidyrc $@
perl -pi -e '!$$lb && ( /^\t+#/  || /^# prototypes/ ) && print qq{\n};'\
-e '$$lb = m/^\n/; ' $@

Yeah, but this will create the perl file in the vpath directory where it
won't ever be used anyway. You really want this back in the source
directory where you can check it in etc.

I came up with this:

Gen_dummy_probes.pl: $(top_srcdir)/$(subdir)/Gen_dummy_probes.sed $(top_srcdir)/$(subdir)/Gen_dummy_probes.pl.prolog
    cp $(top_srcdir)/$(subdir)/Gen_dummy_probes.pl.prolog $(top_srcdir)/$(subdir)/$@
    s2p -f $<  | sed -e 1,3d -e '/# #/ d' -e '$$d' >> $(top_srcdir)/$(subdir)/$@
    perltidy --profile=$(top_srcdir)/$(subdir)/../../tools/pgindent/perltidyrc $(top_srcdir)/$(subdir)/$@
    perl -pi -e '!$$lb && ( /^\t+#/  || /^# prototypes/ ) && print qq{\n};'\
        -e '$$lb = m/^\n/; ' $(top_srcdir)/$(subdir)/$@

I'm not aware of any other case where we generate an in-tree file from a
vpath, which is why it feels strange.

cheers

andrew