Re: backup manifests - Mailing list pgsql-hackers

On Mon, Apr 13, 2020 at 11:09:34AM +0900, Fujii Masao wrote:
> -    while ((c = getopt_long(argc, argv, "CD:F:r:RS:T:X:l:nNzZ:d:c:h:p:U:s:wWkvP",
> +    while ((c = getopt_long(argc, argv, "CD:F:r:RS:T:X:l:nNzZ:d:c:h:p:U:s:wWkvPm:",
>
> "m:" seems unnecessary, so should be removed?
> Patch attached.

Smells like some remnant diff from a previous version.

> +    if (strcmp(basedir, "-") == 0)
> +    {
> +        char        header[512];
> +        PQExpBufferData    buf;
> +
> +        initPQExpBuffer(&buf);
> +        ReceiveBackupManifestInMemory(conn, &buf);
>
> backup_manifest should be received only when the manifest is enabled,
> so ISTM that the flag "manifest" should be checked in the above if-condition.
> Thought? Patch attached.
>
> -    if (strcmp(basedir, "-") == 0)
> +    if (strcmp(basedir, "-") == 0 && manifest)
>      {
>          char        header[512];
>          PQExpBufferData    buf;

Indeed.  Using the tar format with --no-manifest causes a failure:
pg_basebackup -D - --format=t --wal-method=none \
    --no-manifest > /dev/null

The doc changes look right to me.  Nice catches.
--
Michael