Re: lz4 --rm on Ubuntu 18.04 (Add LZ4 compression to pg_dump)

From: Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Daniel Gustafsson <daniel(at)yesql(dot)se>, Christoph Berg <myon(at)debian(dot)org>, Tomas Vondra <tomas(dot)vondra(at)postgresql(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: lz4 --rm on Ubuntu 18.04 (Add LZ4 compression to pg_dump)
Date: 2023-03-13 23:16:16
Message-ID: 74f02417-d18a-71dd-a406-8af41cbeb6c4@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On 3/9/23 19:00, Tomas Vondra wrote:
>
>
> On 3/9/23 01:30, Michael Paquier wrote:
>> On Thu, Mar 09, 2023 at 12:39:08AM +0100, Tomas Vondra wrote:
>>> IMO we should fix that. We have a bunch of buildfarm members running on
>>> Ubuntu 18.04 (or older) - it's true none of them seems to be running TAP
>>> tests. But considering how trivial the fix is ...
>>>
>>> Barring objections, I'll push a fix early next week.
>>
>> +1, better to change that, thanks. Actually, would --rm be OK even on
>> Windows? As far as I can see, the CI detects a LZ4 command for the
>> VS2019 environment but not the liblz4 libraries that would be needed
>> to trigger the set of tests.
>
> Thanks for noticing that. I'll investigate next week.
>

So, here's a fix that should (I think) replace the 'lz4 --rm' with a
simple 'rm'. I have two doubts about this, though:

1) I haven't found a simple way to inject additional command into the
test. The pg_dump runs have a predefined list of "steps" to run:

-- compress_cmd
-- glob_patterns
-- command_like
-- restore_cmd

and I don't think there's a good place to inject the 'rm' so I ended up
adding a 'cleanup_cmd' right after 'compress_cmd'. But it seems a bit
strange / hacky. Maybe there's a better way?

2) I wonder if Windows will know what 'rm' means. I haven't found any
TAP test doing 'rm' and don't see 'rm' in any $ENV either.

That being said, I have no idea how to make this work on our Windows CI.
As mentioned, the environment is missing the lz4 library - there's a

setup_additional_packages_script: |
REM choco install -y --no-progress ...

in the .yml file, but AFAICS the chocolatey does not have lz4 :-/

regards

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachment Content-Type Size
lz4-rm-tap-test.patch text/x-patch 1.4 KB

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Michael Paquier 2023-03-14 05:00:45 pgsql: libpq: Add support for require_auth to control authorized auth m
Previous Message Andres Freund 2023-03-13 21:44:58 pgsql: meson: fix openssl detection issues in 6a30027

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2023-03-13 23:24:48 Re: Testing autovacuum wraparound (including failsafe)
Previous Message Nathan Bossart 2023-03-13 23:10:08 Re: Sub-millisecond [autovacuum_]vacuum_cost_delay broken