Thread: Re: pgsql: Unbreak the build.

Hi,

On March 23, 2022 7:46:10 AM PDT, Andres Freund <andres@anarazel.de> wrote:
>Hi,
>
>On March 23, 2022 7:24:13 AM PDT, Robert Haas <rhaas@postgresql.org> wrote:
>>Unbreak the build.
>>
>>Commit ffd53659c46a54a6978bcb8c4424c1e157a2c0f1 broke the build for
>>anyone not compiling with LZ4 and ZSTD enabled. Woops.
>
>There's new warnings that sound reasonable introduced in the prior commit that didn't get removed in this one:
>
>https://cirrus-ci.com/task/5259487073271808?logs=mingw_cross_warning#L392

And windows still fails tests after this commit: https://cirrus-ci.com/task/6424123323711488?logs=test_bin#L22

Andres

--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Andres Freund <andres@anarazel.de> writes:
> And windows still fails tests after this commit: https://cirrus-ci.com/task/6424123323711488?logs=test_bin#L22

Yeah.  drongo is reporting

# Running: pg_basebackup --no-sync -cfast -D
C:\\prog\\bf\\root\\HEAD\\pgsql.build\\src\\bin\\pg_basebackup\\tmp_check\\tmp_test_vv4i/tarbackup-Ft 
Assertion failed: 0, file c:\\prog\\bf\\root\\HEAD\\pgsql.build\\src\\bin\\pg_basebackup\\walmethods.c, line 953
not ok 82 - tar format

#   Failed test 'tar format'
#   at t/010_pg_basebackup.pl line 261.

which is pointing at

        /* not reachable */
        Assert(false);

so it's not so unreachable after all.

            regards, tom lane

On Wed, Mar 23, 2022 at 11:55 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Andres Freund <andres@anarazel.de> writes:
> > And windows still fails tests after this commit: https://cirrus-ci.com/task/6424123323711488?logs=test_bin#L22
>
> Yeah.  drongo is reporting
>
> # Running: pg_basebackup --no-sync -cfast -D
C:\\prog\\bf\\root\\HEAD\\pgsql.build\\src\\bin\\pg_basebackup\\tmp_check\\tmp_test_vv4i/tarbackup-Ft
 
> Assertion failed: 0, file c:\\prog\\bf\\root\\HEAD\\pgsql.build\\src\\bin\\pg_basebackup\\walmethods.c, line 953
> not ok 82 - tar format
>
> #   Failed test 'tar format'
> #   at t/010_pg_basebackup.pl line 261.
>
> which is pointing at
>
>                 /* not reachable */
>                 Assert(false);
>
> so it's not so unreachable after all.

I'm looking into this now, but that's not the same Assert(false). The
one Andres is talking about is at the end of get_bc_algorithm_name().
This one is in tar_open_for_write(). AFAIK, the first of these is
actually unreachable or at least I see no evidence that we are
reaching it. The second is clearly reachable because we're failing the
assertion. I thought that might be because I didn't test
--without-zlib locally, and indeed in testing that just now, I found
another unused variable warning which I need to fix. But, that doesn't
account for this failure, because when I correct the problem with the
unused variable, all the tests pass.

I think what likely happened here is that in reorganizing some of the
logic in basebackup.c, I caused COMPRESSION_GZIP to get passed to
tar_open_for_write() even when HAVE_LIBZ is not defined. But I don't
yet understand why it only happens on Windows. I am suspicious that
the problem is in basebackup.c's main() function, but I haven't
pinpointed it yet.

-- 
Robert Haas
EDB: http://www.enterprisedb.com