Re: Clean up find_typedefs and add support for Mac
On 2023-12-15 Fr 11:06, Tom Lane wrote: Andrew Dunstan writes: Here's more or less what I had in mind. Do we really need the dependency on an install tree? Can't we just find the executables (or .o files for Darwin) in the build tree? Seems like that would simplify usage, and reduce the possibility for version-skew errors. Sure, I just picked up the buildfarm code more or less without any except necessary modifications. I don't remember the history - we've done it that way for a good long while. We'll need a way to identify the files to analyze, e.g. different library and exe extensions. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Re: Clean up find_typedefs and add support for Mac
On Fri Dec 15, 2023 at 10:06 AM CST, Tom Lane wrote: Andrew Dunstan writes: > Here's more or less what I had in mind. Do we really need the dependency on an install tree? Can't we just find the executables (or .o files for Darwin) in the build tree? Seems like that would simplify usage, and reduce the possibility for version-skew errors. Seems like you would be forcing an extension author to keep a Postgres source tree around if you went this route. Perhaps supporting either the build tree or an install tree would get you the best of both worlds. -- Tristan Partin Neon (https://neon.tech)
Re: Clean up find_typedefs and add support for Mac
if ($using_osx)
{
# On OS X, we need to examine the .o files
# exclude ecpg/test, which pgindent does too
my $obj_wanted = sub {
/^.*\.o\z/s
&& !($File::Find::name =~ m!/ecpg/test/!s)
&& push(@testfiles, $File::Find::name);
};
File::Find::find($obj_wanted, $bindir);
}
I think we should use dsymutil --flat to assemble .dwarf files on Mac
instead of inspecting the plain object files. This would allow you to
use the script in the same way on presumably any system that Postgres
supports, meaning we can drop this distinction:
# The second argument is a directory. For everything except OSX it should be
# a directory where postgres is installed (e.g. $installdir for the buildfarm).
# It should have bin and lib subdirectories. On OSX it should instead be the
# top of the build tree, as we need to examine the individual object files.
my @err = `$objdump -W 2>&1`;
my @readelferr = `readelf -w 2>&1`;
my $using_osx = (`uname` eq "Darwin\n");
Is there any reason we can't use uname -s to detect the system instead
of relying on error condition heuristics of readelf and objdump?
# Note that this assumes there is not a platform-specific subdirectory of
# lib like meson likes to use. (The buildfarm avoids this by specifying
# --libdir=lib to meson setup.)
Should we just default the Meson build to libdir=lib in
project(default_options:)? This assumes that you don't think what Tom
said about running it on the build tree is better.
--
Tristan Partin
Neon (https://neon.tech)
Re: Clean up find_typedefs and add support for Mac
Andrew Dunstan writes: > Here's more or less what I had in mind. Do we really need the dependency on an install tree? Can't we just find the executables (or .o files for Darwin) in the build tree? Seems like that would simplify usage, and reduce the possibility for version-skew errors. regards, tom lane
Re: Clean up find_typedefs and add support for Mac
On 2023-12-14 Th 10:36, Tristan Partin wrote:
On Thu Dec 14, 2023 at 9:16 AM CST, Andrew Dunstan wrote:
On 2023-12-13 We 15:59, Tristan Partin wrote:
> On Wed Dec 13, 2023 at 2:35 PM CST, Andrew Dunstan wrote:
>>
>> On 2023-12-12 Tu 18:02, Tom Lane wrote:
>> > "Tristan Partin" writes:
>> >> The big patch here is adding support for Mac. objdump -W
doesn't >> work on
>> >> Mac. So, I used dsymutil and dwarfdump to achieve the same result.
>> > We should probably nuke the current version of
src/tools/find_typedef
>> > altogether in favor of copying the current buildfarm code for that.
>> > We know that the buildfarm's version works, whereas I'm not real
sure
>> > that src/tools/find_typedef is being used in anger by anyone.
Also,
>> > we're long past the point where developers can avoid having Perl
>> > installed.
>> >
>> > Ideally, the buildfarm client would then start to use find_typedef
>> > from the tree rather than have its own copy, both to reduce
>> > duplication and to ensure that the in-tree copy keeps working.
>> >
>> >
>>
>>
>> +1. I'd be more than happy to be rid of maintaining the code. We
>> already performed a rather more complicated operation along these
>> lines with the PostgreSQL::Test::AdjustUpgrade stuff.
>
> I'll work with you on GitHub to help make the transition. I've
already > made a draft PR in the client-code repo, but I am sure I am
missing > some stuff.
I think we're putting the cart before the horse here. I think we need
a perl module in core that can be loaded by the buildfarm code, and
could also be used by a standalone find_typedef (c.f.
src/test/PostgreSQL/Test/AdjustUpgrade.pm). To be useful that would
have to be backported.
I'll have a go at that.
Here is an adaptation of the find_typedefs from run_build.pl. Maybe it
will help you.
Here's more or less what I had in mind.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
FindTypedefs.pm
Description: Perl program
#!/usr/bin/perl
use strict;
use warnings;
use FindBin;
use lib "$FindBin::Bin";
use FindTypedefs;
## assume we're processing the source tree we're in
# @ARGV should be top of install tree (or build tree on OSX)
# and optional name of objdump program (for --host= etc builds)
print_typedefs("$FindBin::Bin/../..", @ARGV);
# XXX probably should have a help subroutine here.
Re: Clean up find_typedefs and add support for Mac
On Thu Dec 14, 2023 at 9:16 AM CST, Andrew Dunstan wrote: On 2023-12-13 We 15:59, Tristan Partin wrote: > On Wed Dec 13, 2023 at 2:35 PM CST, Andrew Dunstan wrote: >> >> On 2023-12-12 Tu 18:02, Tom Lane wrote: >> > "Tristan Partin" writes: >> >> The big patch here is adding support for Mac. objdump -W doesn't >> work on >> >> Mac. So, I used dsymutil and dwarfdump to achieve the same result. >> > We should probably nuke the current version of src/tools/find_typedef >> > altogether in favor of copying the current buildfarm code for that. >> > We know that the buildfarm's version works, whereas I'm not real sure >> > that src/tools/find_typedef is being used in anger by anyone. Also, >> > we're long past the point where developers can avoid having Perl >> > installed. >> > >> > Ideally, the buildfarm client would then start to use find_typedef >> > from the tree rather than have its own copy, both to reduce >> > duplication and to ensure that the in-tree copy keeps working. >> > >> > >> >> >> +1. I'd be more than happy to be rid of maintaining the code. We >> already performed a rather more complicated operation along these >> lines with the PostgreSQL::Test::AdjustUpgrade stuff. > > I'll work with you on GitHub to help make the transition. I've already > made a draft PR in the client-code repo, but I am sure I am missing > some stuff. I think we're putting the cart before the horse here. I think we need a perl module in core that can be loaded by the buildfarm code, and could also be used by a standalone find_typedef (c.f. src/test/PostgreSQL/Test/AdjustUpgrade.pm). To be useful that would have to be backported. I'll have a go at that. Here is an adaptation of the find_typedefs from run_build.pl. Maybe it will help you. -- Tristan Partin Neon (https://neon.tech) find_typedefs.pl Description: Perl program
Re: Clean up find_typedefs and add support for Mac
On 2023-12-13 We 15:59, Tristan Partin wrote: On Wed Dec 13, 2023 at 2:35 PM CST, Andrew Dunstan wrote: On 2023-12-12 Tu 18:02, Tom Lane wrote: > "Tristan Partin" writes: >> The big patch here is adding support for Mac. objdump -W doesn't work on >> Mac. So, I used dsymutil and dwarfdump to achieve the same result. > We should probably nuke the current version of src/tools/find_typedef > altogether in favor of copying the current buildfarm code for that. > We know that the buildfarm's version works, whereas I'm not real sure > that src/tools/find_typedef is being used in anger by anyone. Also, > we're long past the point where developers can avoid having Perl > installed. > > Ideally, the buildfarm client would then start to use find_typedef > from the tree rather than have its own copy, both to reduce > duplication and to ensure that the in-tree copy keeps working. > > +1. I'd be more than happy to be rid of maintaining the code. We already performed a rather more complicated operation along these lines with the PostgreSQL::Test::AdjustUpgrade stuff. I'll work with you on GitHub to help make the transition. I've already made a draft PR in the client-code repo, but I am sure I am missing some stuff. I think we're putting the cart before the horse here. I think we need a perl module in core that can be loaded by the buildfarm code, and could also be used by a standalone find_typedef (c.f. src/test/PostgreSQL/Test/AdjustUpgrade.pm). To be useful that would have to be backported. I'll have a go at that. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Re: Clean up find_typedefs and add support for Mac
On Wed Dec 13, 2023 at 2:35 PM CST, Andrew Dunstan wrote: On 2023-12-12 Tu 18:02, Tom Lane wrote: > "Tristan Partin" writes: >> The big patch here is adding support for Mac. objdump -W doesn't work on >> Mac. So, I used dsymutil and dwarfdump to achieve the same result. > We should probably nuke the current version of src/tools/find_typedef > altogether in favor of copying the current buildfarm code for that. > We know that the buildfarm's version works, whereas I'm not real sure > that src/tools/find_typedef is being used in anger by anyone. Also, > we're long past the point where developers can avoid having Perl > installed. > > Ideally, the buildfarm client would then start to use find_typedef > from the tree rather than have its own copy, both to reduce > duplication and to ensure that the in-tree copy keeps working. > > +1. I'd be more than happy to be rid of maintaining the code. We already performed a rather more complicated operation along these lines with the PostgreSQL::Test::AdjustUpgrade stuff. I'll work with you on GitHub to help make the transition. I've already made a draft PR in the client-code repo, but I am sure I am missing some stuff. -- Tristan Partin Neon (https://neon.tech)
Re: Clean up find_typedefs and add support for Mac
On 2023-12-12 Tu 18:02, Tom Lane wrote: "Tristan Partin" writes: The big patch here is adding support for Mac. objdump -W doesn't work on Mac. So, I used dsymutil and dwarfdump to achieve the same result. We should probably nuke the current version of src/tools/find_typedef altogether in favor of copying the current buildfarm code for that. We know that the buildfarm's version works, whereas I'm not real sure that src/tools/find_typedef is being used in anger by anyone. Also, we're long past the point where developers can avoid having Perl installed. Ideally, the buildfarm client would then start to use find_typedef from the tree rather than have its own copy, both to reduce duplication and to ensure that the in-tree copy keeps working. +1. I'd be more than happy to be rid of maintaining the code. We already performed a rather more complicated operation along these lines with the PostgreSQL::Test::AdjustUpgrade stuff. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Re: Clean up find_typedefs and add support for Mac
On Wed Dec 13, 2023 at 11:27 AM CST, Tom Lane wrote: "Tristan Partin" writes: > That makes sense to me. Where can I find the buildfarm code to propose > a different patch, at least pulling in the current state of the > buildfarm script? Or perhaps Andrew is the best person for this job. I think this is the authoritative repo: https://github.com/PGBuildFarm/client-code.git Cool. I'll reach out to Andrew off list to work with him. Maybe I'll gain a little bit more knowledge of how the buildfarm works :). -- Tristan Partin Neon (https://neon.tech)
Re: Clean up find_typedefs and add support for Mac
"Tristan Partin" writes: > That makes sense to me. Where can I find the buildfarm code to propose > a different patch, at least pulling in the current state of the > buildfarm script? Or perhaps Andrew is the best person for this job. I think this is the authoritative repo: https://github.com/PGBuildFarm/client-code.git regards, tom lane
Re: Clean up find_typedefs and add support for Mac
On Tue Dec 12, 2023 at 5:02 PM CST, Tom Lane wrote: "Tristan Partin" writes: > The big patch here is adding support for Mac. objdump -W doesn't work on > Mac. So, I used dsymutil and dwarfdump to achieve the same result. We should probably nuke the current version of src/tools/find_typedef altogether in favor of copying the current buildfarm code for that. We know that the buildfarm's version works, whereas I'm not real sure that src/tools/find_typedef is being used in anger by anyone. Also, we're long past the point where developers can avoid having Perl installed. Ideally, the buildfarm client would then start to use find_typedef from the tree rather than have its own copy, both to reduce duplication and to ensure that the in-tree copy keeps working. That makes sense to me. Where can I find the buildfarm code to propose a different patch, at least pulling in the current state of the buildfarm script? Or perhaps Andrew is the best person for this job. -- Tristan Partin Neon (https://neon.tech)
Re: Clean up find_typedefs and add support for Mac
"Tristan Partin" writes: > The big patch here is adding support for Mac. objdump -W doesn't work on > Mac. So, I used dsymutil and dwarfdump to achieve the same result. We should probably nuke the current version of src/tools/find_typedef altogether in favor of copying the current buildfarm code for that. We know that the buildfarm's version works, whereas I'm not real sure that src/tools/find_typedef is being used in anger by anyone. Also, we're long past the point where developers can avoid having Perl installed. Ideally, the buildfarm client would then start to use find_typedef from the tree rather than have its own copy, both to reduce duplication and to ensure that the in-tree copy keeps working. regards, tom lane
Clean up find_typedefs and add support for Mac
The script was using a few deprecated things according to POSIX:
- -o instead of ||
- egrep
- `` instead of $()
I removed those for their "modern" equivalents. Hopefully no buildfarm
member complains. I can remove any of those patches though. I did go
ahead and remove egrep usage from the entire codebase while I was at it.
There is still a configure check though. I'm thinking that could also be
removed?
I moved system detection to use uname -s. I hope that isn't a big deal.
Not sure the best way to identify Mac otherwise.
The big patch here is adding support for Mac. objdump -W doesn't work on
Mac. So, I used dsymutil and dwarfdump to achieve the same result. I am
not someone who ever uses awk, so someone should definitely check my
work there. I can only confirm this works on the latest version of Mac,
and have no clue how backward compatible it is. I also wrote this
without having a Mac. I had to ping a coworker with a Mac for help.
My goal with the Mac support is to enable use of find_typedef for
extension developers, where using a Mac might be more prominent than
upstream Postgres development, but that is just a guess.
--
Tristan Partin
Neon (https://neon.tech)
From e717a14a171c0226921ffed003dedd104bf3cf99 Mon Sep 17 00:00:00 2001
From: Tristan Partin
Date: Tue, 12 Dec 2023 10:13:13 -0600
Subject: [PATCH v1 1/6] Replace egrep with grep -E
GNU grep has been warning about grep -E since the 3.8 series. Further
GNU commentary:
> 7th Edition Unix had commands egrep and fgrep that were the counterparts
> of the modern grep -E and grep -F. Although breaking up grep into three
> programs was perhaps useful on the small computers of the 1970s, egrep
> and fgrep were not standardized by POSIX and are no longer needed. In
> the current GNU implementation, egrep and fgrep issue a warning and then
> act like their modern counterparts; eventually, they are planned to be
> removed entirely.
Further man page documentation:
> This grep has been enhanced in an upwards-compatible way to provide the
> exact functionality of the historical egrep and fgrep commands as well.
> It was the clear intention of the standard developers to consolidate the
> three greps into a single command.
---
src/backend/port/aix/mkldexport.sh | 4 ++--
src/tools/find_typedef | 6 +++---
src/tools/perlcheck/find_perl_files | 2 +-
src/tools/pginclude/pgrminclude | 6 +++---
4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/backend/port/aix/mkldexport.sh b/src/backend/port/aix/mkldexport.sh
index adf3793e86..41405eba02 100755
--- a/src/backend/port/aix/mkldexport.sh
+++ b/src/backend/port/aix/mkldexport.sh
@@ -53,9 +53,9 @@ else
fi
fi
$NM -BCg $1 | \
- egrep ' [TDB] ' | \
+ grep -E ' [TDB] ' | \
sed -e 's/.* //' | \
- egrep -v '\$' | \
+ grep -E -v '\$' | \
sed -e 's/^[.]//' | \
sort | \
uniq
diff --git a/src/tools/find_typedef b/src/tools/find_typedef
index 24e9b76651..fec0520c32 100755
--- a/src/tools/find_typedef
+++ b/src/tools/find_typedef
@@ -36,12 +36,12 @@ do # if objdump -W is recognized, only one line of error should appear
if [ `objdump -W 2>&1 | wc -l` -eq 1 ]
then # Linux
objdump -W "$DIR"/* |
- egrep -A3 '\(DW_TAG_typedef\)' |
+ grep -E -A3 '\(DW_TAG_typedef\)' |
awk ' $2 == "DW_AT_name" {print $NF}'
elif [ `readelf -w 2>&1 | wc -l` -gt 1 ]
then # FreeBSD, similar output to Linux
readelf -w "$DIR"/* |
- egrep -A3 '\(DW_TAG_typedef\)' |
+ grep -E -A3 '\(DW_TAG_typedef\)' |
awk ' $1 == "DW_AT_name" {print $NF}'
fi
done |
@@ -50,4 +50,4 @@ sort |
uniq |
# these are used both for typedefs and variable names
# so do not include them
-egrep -v '^(date|interval|timestamp|ANY)$'
+grep -E -v '^(date|interval|timestamp|ANY)$'
diff --git a/src/tools/perlcheck/find_perl_files b/src/tools/perlcheck/find_perl_files
index 20dceb800d..406ec7f3a0 100644
--- a/src/tools/perlcheck/find_perl_files
+++ b/src/tools/perlcheck/find_perl_files
@@ -12,7 +12,7 @@ find_perl_files () {
find "$@" -type f -name '*.p[lm]' -print
# take executable files that file(1) thinks are perl files
find "$@" -type f -perm -100 -exec file {} \; -print |
- egrep -i ':.*perl[0-9]*\>' |
+ grep -E -i ':.*perl[0-9]*\>' |
cut -d: -f1
} | sort -u | grep -v '^\./\.git/'
}
diff --git a/src/tools/pginclude/pgrminclude b/src/tools/pginclude/pgrminclude
index 7cbd2e7c9c..27c6c5bfb5 100755
--- a/src/tools/pginclude/pgrminclude
+++ b/src/tools/pginclude/pgrminclude
@@ -64,14 +64,14 @@ compile_file() {
[ "$INCLUDE" = "pg_config.h" ] && continue
[ "$INCLUDE" = "c.h" ] && continue
# Stringify macros will expand undefined identifiers, so skip files that use it
- egrep -q '\<(CppAsString2?|CppConcat)\>' "$FILE" && continue
+ grep -E -q '\<(CppAsString2?|CppConcat)\>' "$FILE" && continue
# preserve configure-specific includes
# these includes are surrounded by #ifdef's
grep -B1 '^#include[ ][ ]*[<"]'"$INCLUDE"'[>"]' "$FILE" |
- egrep -q
