Feedback-ID: i131946ab:Fastmail
Message-ID: <031d212d-39cc-87e1-8149-c347c91eedb2@enterprisedb.com>
Date: Thu, 25 Aug 2022 15:29:30 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.12.0
Subject: Re: [RFC] building postgres with meson - v11
Content-Language: en-US
To: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers@postgresql.org, samay sharma <smilingsamay@gmail.com>,
 Nazir Bilal Yavuz <byavuz81@gmail.com>
References: <20211012083721.hvixq4pnh2pixr3j@alap3.anarazel.de>
 <20220811002012.ju3rrz47i2e5tdha@awork3.anarazel.de>
 <e0c44fb2-8b66-a4b9-b274-7ed3a1a0ab74@enterprisedb.com>
 <20220817215317.poeofidf7o7dy6hy@awork3.anarazel.de>
 <8992c637-0922-8cf8-9188-104153eb3561@enterprisedb.com>
 <20220820180139.6uj5etquyonz4otf@awork3.anarazel.de>
 <7dae5979-c6c0-cec5-7a36-76a85aa8053d@enterprisedb.com>
 <20220824153008.i7pnmh7xilqhsnej@awork3.anarazel.de>
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
In-Reply-To: <20220824153008.i7pnmh7xilqhsnej@awork3.anarazel.de>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/031d212d-39cc-87e1-8149-c347c91eedb2%40enterprisedb.com>
Precedence: bulk

On 24.08.22 17:30, Andres Freund wrote:
>> 258f6dc0a7 Don't hardcode tmp_check/ as test directory for tap tests
>> 8ecc33cf04 Split TESTDIR into TESTLOGDIR and TESTDATADIR
>>
>> I think these patches are split up a bit incorrectly.  If you apply
>> the first patch by itself, then the output appears in tab_comp_dir/
>> directly under the source directory.  And then the second patch moves
>> it to tmp_check/tap_comp_dir/.  If there is an intent to apply these
>> patches separately somehow, this should be cleaned up.
> 
> How is that happening with that version of the patch? The test puts
> tap_comp_dir under TESTDIR, and TESTDIR is $(CURDIR)/tmp_check. There was an
> earlier version of the patch that was split one more time that did have that
> problem, but I don't quite see how that version has it?

Ok, I see now how this works.  It's a bit weird since the meaning of 
TESTDIR is changed.  I'm not sure if this could create cross-branch 
confusion.

>> 97a0b096e8 meson: prereq: Add src/tools/gen_export.pl
>>
>> This produces leading whitespace in the output files that at least on
>> darwin wasn't there before.  See attached patch (0002).  This should
>> be checked again on other platforms as well.
> 
> Hm, to me the indentation as is makes more sense, but ...

Maybe for the 'gnu' format, but on darwin (and others) it's just a flat 
list, so indenting it is pointless.

> I wonder if we should rewrite this in python - I chose perl because I thought
> we could share it, but as you pointed out, that's not possible, because we
> don't want to depend on perl during the autoconf build from a tarball.

Given that the code is already written, I wouldn't do it.  Introducing 
Python into the toolchain would require considerations of minimum 
versions, finding the right binaries, formatting, style checking, etc., 
which would probably be a distraction right now.  I also think that this 
script, whose purpose is to read an input file line by line and print it 
back out slightly differently, is not going to be done better in Python.