From 4a418164d3f4478e256b02d6ef38700b80eb7568 Mon Sep 17 00:00:00 2001 From: Marco Nenciarini Date: Mon, 10 Aug 2026 16:37:33 +0200 Subject: [PATCH v1 1/3] fe_utils: extract shared backup_manifest writer pg_combinebackup's own backup_manifest writer has no dependency on anything specific to that tool. Move it, unchanged in behavior, to fe_utils. Two things change along the way, not just the relocation. create_manifest_writer()'s directory argument becomes const char *, matching every other path argument in this API. And manifest_wal_range moves from pg_combinebackup's own private struct (declared in load_manifest.h, and only ever read back by that same file) into a real, complete type in write_manifest.h itself: finalize_manifest() now walks one shared manifest_wal_range chain, rather than reading a few leading fields through an incomplete type each caller's own struct had to independently match the shape of. pg_verifybackup had its own second, independent copy of the identical struct (same fields, same layout) for its own manifest-reading purposes, unrelated to writing one at all. Since the type is now a real, shared one, it uses that instead of its own copy too. --- src/bin/pg_combinebackup/Makefile | 3 +-- src/bin/pg_combinebackup/backup_label.c | 2 +- src/bin/pg_combinebackup/load_manifest.h | 15 +----------- src/bin/pg_combinebackup/meson.build | 1 - src/bin/pg_combinebackup/nls.mk | 4 ++-- src/bin/pg_combinebackup/pg_combinebackup.c | 2 +- src/bin/pg_verifybackup/pg_verifybackup.h | 14 +---------- src/fe_utils/Makefile | 3 ++- src/fe_utils/meson.build | 1 + .../write_manifest.c | 11 +++++---- .../fe_utils}/write_manifest.h | 24 +++++++++++++++---- 11 files changed, 37 insertions(+), 43 deletions(-) rename src/{bin/pg_combinebackup => fe_utils}/write_manifest.c (96%) rename src/{bin/pg_combinebackup => include/fe_utils}/write_manifest.h (50%) diff --git a/src/bin/pg_combinebackup/Makefile b/src/bin/pg_combinebackup/Makefile index 7a6c44d67c8..71b7cd5a5ff 100644 --- a/src/bin/pg_combinebackup/Makefile +++ b/src/bin/pg_combinebackup/Makefile @@ -27,8 +27,7 @@ OBJS = \ backup_label.o \ copy_file.o \ load_manifest.o \ - reconstruct.o \ - write_manifest.o + reconstruct.o all: pg_combinebackup diff --git a/src/bin/pg_combinebackup/backup_label.c b/src/bin/pg_combinebackup/backup_label.c index b757e772b92..bad174fc5b1 100644 --- a/src/bin/pg_combinebackup/backup_label.c +++ b/src/bin/pg_combinebackup/backup_label.c @@ -17,7 +17,7 @@ #include "backup_label.h" #include "common/file_perm.h" #include "common/logging.h" -#include "write_manifest.h" +#include "fe_utils/write_manifest.h" static int get_eol_offset(StringInfo buf); static bool line_starts_with(char *s, char *e, char *match, char **sout); diff --git a/src/bin/pg_combinebackup/load_manifest.h b/src/bin/pg_combinebackup/load_manifest.h index 25edda3d305..5cff40a8d7f 100644 --- a/src/bin/pg_combinebackup/load_manifest.h +++ b/src/bin/pg_combinebackup/load_manifest.h @@ -12,8 +12,8 @@ #ifndef LOAD_MANIFEST_H #define LOAD_MANIFEST_H -#include "access/xlogdefs.h" #include "common/checksum_helper.h" +#include "fe_utils/write_manifest.h" /* * Each file described by the manifest file is parsed to produce an object @@ -37,19 +37,6 @@ typedef struct manifest_file #define SH_DECLARE #include "lib/simplehash.h" -/* - * Each WAL range described by the manifest file is parsed to produce an - * object like this. - */ -typedef struct manifest_wal_range -{ - TimeLineID tli; - XLogRecPtr start_lsn; - XLogRecPtr end_lsn; - struct manifest_wal_range *next; - struct manifest_wal_range *prev; -} manifest_wal_range; - /* * All the data parsed from a backup_manifest file. */ diff --git a/src/bin/pg_combinebackup/meson.build b/src/bin/pg_combinebackup/meson.build index ba1c8cfa3d0..2d815db8f07 100644 --- a/src/bin/pg_combinebackup/meson.build +++ b/src/bin/pg_combinebackup/meson.build @@ -6,7 +6,6 @@ pg_combinebackup_sources = files( 'copy_file.c', 'load_manifest.c', 'reconstruct.c', - 'write_manifest.c', ) if host_system == 'windows' diff --git a/src/bin/pg_combinebackup/nls.mk b/src/bin/pg_combinebackup/nls.mk index ff56cee6aac..32d3a359cfe 100644 --- a/src/bin/pg_combinebackup/nls.mk +++ b/src/bin/pg_combinebackup/nls.mk @@ -6,7 +6,6 @@ GETTEXT_FILES = $(FRONTEND_COMMON_GETTEXT_FILES) \ load_manifest.c \ pg_combinebackup.c \ reconstruct.c \ - write_manifest.c \ ../../common/controldata_utils.c \ ../../common/cryptohash.c \ ../../common/cryptohash_openssl.c \ @@ -15,7 +14,8 @@ GETTEXT_FILES = $(FRONTEND_COMMON_GETTEXT_FILES) \ ../../common/jsonapi.c \ ../../common/parse_manifest.c \ ../../fe_utils/option_utils.c \ - ../../fe_utils/version.c + ../../fe_utils/version.c \ + ../../fe_utils/write_manifest.c GETTEXT_TRIGGERS = $(FRONTEND_COMMON_GETTEXT_TRIGGERS) \ json_token_error:2 \ json_manifest_parse_failure:2 \ diff --git a/src/bin/pg_combinebackup/pg_combinebackup.c b/src/bin/pg_combinebackup/pg_combinebackup.c index 86e2ee37c40..1dbc90a097e 100644 --- a/src/bin/pg_combinebackup/pg_combinebackup.c +++ b/src/bin/pg_combinebackup/pg_combinebackup.c @@ -35,11 +35,11 @@ #include "copy_file.h" #include "fe_utils/option_utils.h" #include "fe_utils/version.h" +#include "fe_utils/write_manifest.h" #include "getopt_long.h" #include "lib/stringinfo.h" #include "load_manifest.h" #include "reconstruct.h" -#include "write_manifest.h" /* Incremental file naming convention. */ #define INCREMENTAL_PREFIX "INCREMENTAL." diff --git a/src/bin/pg_verifybackup/pg_verifybackup.h b/src/bin/pg_verifybackup/pg_verifybackup.h index 8572c1433dc..0398d13677a 100644 --- a/src/bin/pg_verifybackup/pg_verifybackup.h +++ b/src/bin/pg_verifybackup/pg_verifybackup.h @@ -20,6 +20,7 @@ #include "common/parse_manifest.h" #include "fe_utils/astreamer.h" #include "fe_utils/simple_list.h" +#include "fe_utils/write_manifest.h" /* * Each file described by the manifest file is parsed to produce an object @@ -56,19 +57,6 @@ typedef struct manifest_file #define SH_DEFINE #include "lib/simplehash.h" -/* - * Each WAL range described by the manifest file is parsed to produce an - * object like this. - */ -typedef struct manifest_wal_range -{ - TimeLineID tli; - XLogRecPtr start_lsn; - XLogRecPtr end_lsn; - struct manifest_wal_range *next; - struct manifest_wal_range *prev; -} manifest_wal_range; - /* * All the data parsed from a backup_manifest file. */ diff --git a/src/fe_utils/Makefile b/src/fe_utils/Makefile index cbfbf93ac69..c12ae1f2693 100644 --- a/src/fe_utils/Makefile +++ b/src/fe_utils/Makefile @@ -38,7 +38,8 @@ OBJS = \ recovery_gen.o \ simple_list.o \ string_utils.o \ - version.o + version.o \ + write_manifest.o ifeq ($(PORTNAME), win32) override CPPFLAGS += -DFD_SETSIZE=1024 diff --git a/src/fe_utils/meson.build b/src/fe_utils/meson.build index 86befca192e..3feac188185 100644 --- a/src/fe_utils/meson.build +++ b/src/fe_utils/meson.build @@ -19,6 +19,7 @@ fe_utils_sources = files( 'simple_list.c', 'string_utils.c', 'version.c', + 'write_manifest.c', ) psqlscan = custom_target('psqlscan', diff --git a/src/bin/pg_combinebackup/write_manifest.c b/src/fe_utils/write_manifest.c similarity index 96% rename from src/bin/pg_combinebackup/write_manifest.c rename to src/fe_utils/write_manifest.c index c2ab7281266..6d92cb5e1a1 100644 --- a/src/bin/pg_combinebackup/write_manifest.c +++ b/src/fe_utils/write_manifest.c @@ -2,10 +2,13 @@ * * Write a new backup manifest. * + * The caller builds a manifest_wal_range list (fe_utils/write_manifest.h) + * and hands the head of that list to finalize_manifest(). + * * Portions Copyright (c) 1996-2026, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * src/bin/pg_combinebackup/write_manifest.c + * src/fe_utils/write_manifest.c * *------------------------------------------------------------------------- */ @@ -16,13 +19,13 @@ #include #include +#include "access/xlogdefs.h" #include "common/checksum_helper.h" #include "common/file_perm.h" #include "common/logging.h" +#include "fe_utils/write_manifest.h" #include "lib/stringinfo.h" -#include "load_manifest.h" #include "mb/pg_wchar.h" -#include "write_manifest.h" struct manifest_writer { @@ -45,7 +48,7 @@ static size_t hex_encode(const uint8 *src, size_t len, char *dst); * in the specified directory. */ manifest_writer * -create_manifest_writer(char *directory, uint64 system_identifier) +create_manifest_writer(const char *directory, uint64 system_identifier) { manifest_writer *mwriter = pg_malloc_object(manifest_writer); diff --git a/src/bin/pg_combinebackup/write_manifest.h b/src/include/fe_utils/write_manifest.h similarity index 50% rename from src/bin/pg_combinebackup/write_manifest.h rename to src/include/fe_utils/write_manifest.h index f5fa8fc7458..faf3165059a 100644 --- a/src/bin/pg_combinebackup/write_manifest.h +++ b/src/include/fe_utils/write_manifest.h @@ -2,24 +2,40 @@ * * Write a new backup manifest. * + * manifest_wal_range is defined here, rather than by each caller + * separately, so that finalize_manifest()'s single walk of the chain (via + * "next") is over one real, shared type rather than an informally + * documented shape multiple independently-declared structs happen to + * match. "prev" exists only for a caller that wants to build the chain + * back-to-front while parsing a manifest whose own entries arrive in + * forward order; finalize_manifest() itself never reads it. + * * Portions Copyright (c) 1996-2026, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * src/bin/pg_combinebackup/write_manifest.h + * src/include/fe_utils/write_manifest.h * *------------------------------------------------------------------------- */ #ifndef WRITE_MANIFEST_H #define WRITE_MANIFEST_H +#include "access/xlogdefs.h" #include "common/checksum_helper.h" -struct manifest_wal_range; +typedef struct manifest_wal_range +{ + TimeLineID tli; + XLogRecPtr start_lsn; + XLogRecPtr end_lsn; + struct manifest_wal_range *next; + struct manifest_wal_range *prev; +} manifest_wal_range; struct manifest_writer; typedef struct manifest_writer manifest_writer; -extern manifest_writer *create_manifest_writer(char *directory, +extern manifest_writer *create_manifest_writer(const char *directory, uint64 system_identifier); extern void add_file_to_manifest(manifest_writer *mwriter, const char *manifest_path, @@ -28,6 +44,6 @@ extern void add_file_to_manifest(manifest_writer *mwriter, int checksum_length, uint8 *checksum_payload); extern void finalize_manifest(manifest_writer *mwriter, - struct manifest_wal_range *first_wal_range); + manifest_wal_range *first_wal_range); #endif /* WRITE_MANIFEST_H */ -- 2.47.3