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

[Tomas Vondra]

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
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 9c354213ce..b3dcf6ff6d 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -178,11 +178,18 @@ my %pgdump_runs = (
 		compress_cmd => {
 			program => $ENV{'LZ4'},
 			args    => [
-				'-z', '-f', '--rm',
+				'-z', '-f',
 				"$tempdir/compression_lz4_dir/blobs.toc",
 				"$tempdir/compression_lz4_dir/blobs.toc.lz4",
 			],
 		},
+		# remove the source (uncompressed) .toc file
+		cleanup_cmd => {
+			program => 'rm',
+			args    => [
+				"$tempdir/compression_lz4_dir/blobs.toc",
+			],
+		},
 		# Verify that data files were compressed
 		glob_patterns => [
 			"$tempdir/compression_lz4_dir/toc.dat",
@@ -4274,6 +4281,20 @@ foreach my $run (sort keys %pgdump_runs)
 		command_ok(\@full_compress_cmd, "$run: compression commands");
 	}
 
+	if ($pgdump_runs{$run}->{cleanup_cmd})
+	{
+		my ($cleanup_cmd) = $pgdump_runs{$run}->{cleanup_cmd};
+		my $cleanup_program = $cleanup_cmd->{program};
+
+		# Skip the rest of the test if the compression program is
+		# not defined.
+		next if (!defined($cleanup_cmd) || $cleanup_cmd eq '');
+
+		my @full_cleanup_cmd =
+		  ($cleanup_cmd->{program}, @{ $cleanup_cmd->{args} });
+		command_ok(\@full_cleanup_cmd, "$run: cleanup commands");
+	}
+
 	if ($pgdump_runs{$run}->{glob_patterns})
 	{
 		my $glob_patterns = $pgdump_runs{$run}->{glob_patterns};