Обсуждение: 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)
Вложения
- v1-0001-Replace-egrep-with-grep-E.patch
- v1-0002-Remove-use-of-test-o-in-find_typedef.patch
- v1-0003-Remove-use-of-backticks-for-running-processes-in-.patch
- v1-0004-Use-uname-s-for-determining-the-system-in-find_ty.patch
- v1-0005-Add-Mac-support-to-find_typedef.patch
- v1-0006-Use-grep-F-instead-of-E-in-find_typedef.patch
"Tristan Partin" <tristan@neon.tech> 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
On Tue Dec 12, 2023 at 5:02 PM CST, Tom Lane wrote: > "Tristan Partin" <tristan@neon.tech> 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)
"Tristan Partin" <tristan@neon.tech> 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
On Wed Dec 13, 2023 at 11:27 AM CST, Tom Lane wrote: > "Tristan Partin" <tristan@neon.tech> 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)
On 2023-12-12 Tu 18:02, Tom Lane wrote: > "Tristan Partin" <tristan@neon.tech> 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
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" <tristan@neon.tech> 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)
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" <tristan@neon.tech> 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
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" <tristan@neon.tech> 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)
Вложения
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" <tristan@neon.tech> 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
Вложения
Andrew Dunstan <andrew@dunslane.net> 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
> 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)
On Fri Dec 15, 2023 at 10:06 AM CST, Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> 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)
On 2023-12-15 Fr 11:06, Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> 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