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 |
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 |
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 |