Feedback-ID: ie0a040ee:Fastmail
Message-ID: <7f7e0760-2c27-4e94-b3de-e8c141a94e98@eisentraut.org>
Date: Wed, 22 May 2024 13:32:03 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: tydedef extraction - back to the future
To: Andrew Dunstan <andrew@dunslane.net>,
 PostgreSQL-development <pgsql-hackers@postgresql.org>
References: <43a819bb-8c92-4a0d-a8ce-01221a5ab20f@dunslane.net>
Content-Language: en-US
From: Peter Eisentraut <peter@eisentraut.org>
In-Reply-To: <43a819bb-8c92-4a0d-a8ce-01221a5ab20f@dunslane.net>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/7f7e0760-2c27-4e94-b3de-e8c141a94e98%40eisentraut.org>
Precedence: bulk

On 20.05.24 23:11, Andrew Dunstan wrote:
> Attached is an attempt to thread this needle. The core is a new perl 
> module that imports the current buildfarm client logic. The intention is 
> that once we have this, the buildfarm client will switch to using the 
> module (if found) rather than its own built-in logic. There is precedent 
> for this sort of arrangement (AdjustUpgrade.pm). Accompanying the new 
> module is a standalone perl script that uses the new module, and 
> replaces the current shell script (thus making it more portable).

It looks like this code could use a bit of work to modernize and clean 
up cruft, such as

+       my $sep = $using_msvc ? ';' : ':';

This can be done with File::Spec.

+               next if $bin =~ m!bin/(ipcclean|pltcl_)!;

Those are long gone.

+               next if $bin =~ m!/postmaster.exe$!;    # sometimes a 
copy not a link

Also gone.

+       elsif ($using_osx)
+       {
+               # On OS X, we need to examine the .o files

Update the name.

+               # 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, $binloc);
+       }

Not clear why this is specific to that platform.

Also, some instructions should be provided.  It looks like this is meant 
to be run on the installation tree?  A README and/or a build target 
would be good.

The code distinguishes between srcdir and bindir, but it's not clear 
what the latter is.  It rather looks like the installation prefix.  Does 
this code support out of tree builds?  This should be cleared up.