refactor: extract shared utility sources into src/bin/common/

src/bin/Makefile

@@ -1,15 +1,19 @@
 # Copyright (c) Microsoft Corporation. All rights reserved.
 # Licensed under the PostgreSQL License.
 
-all: pg_autoctl ;
+all: common pg_autoctl ;
 
-pg_autoctl:
+common:
+	$(MAKE) -C common
+
+pg_autoctl: common
 	$(MAKE) -C pg_autoctl pg_autoctl
 
 clean:
+	$(MAKE) -C common clean
 	$(MAKE) -C pg_autoctl clean
 
-install: $(pg_autoctl)
+install: pg_autoctl
 	$(MAKE) -C pg_autoctl install
 
-.PHONY: all pg_autoctl install clean
+.PHONY: all common pg_autoctl install clean

src/bin/common/Makefile

@@ -0,0 +1,16 @@
+# Copyright (c) Microsoft Corporation. All rights reserved.
+# Licensed under the PostgreSQL License.
+
+SRC_DIR := $(dir $(abspath $(lastword $(MAKEFILE_LIST))))
+
+.DEFAULT_GOAL := $(SRC_DIR)libpgaf_common.a
+
+include $(SRC_DIR)Makefile.common
+
+override CFLAGS += -I$(SRC_DIR)../pg_autoctl
+
+clean:
+	rm -f $(COMMON_OBJ) $(COMMON_LIB)
+	rm -rf $(DEPDIR)
+
+.PHONY: clean

src/bin/common/Makefile.common

@@ -0,0 +1,129 @@
+# Copyright (c) Microsoft Corporation. All rights reserved.
+# Licensed under the PostgreSQL License.
+#
+# Makefile.common — shared build variables and common source library.
+# Included by both pg_autoctl and pgaftest Makefiles via:
+#   include ../common/Makefile.common
+
+PG_CONFIG ?= pg_config
+
+COMMON_DIR := $(dir $(abspath $(lastword $(MAKEFILE_LIST))))
+LIB_DIR    := $(COMMON_DIR)../lib
+
+PG_SNPRINTF     = $(wildcard $(LIB_DIR)/pg/snprintf.*)
+LOG_SRC         = $(wildcard $(LIB_DIR)/log/src/log.*)
+COMMANDLINE_SRC = $(wildcard $(LIB_DIR)/subcommands.c/commandline.*)
+PARSON_SRC      = $(wildcard $(LIB_DIR)/parson/parson.*)
+
+COMMON_INCLUDES  = -I$(COMMON_DIR)
+COMMON_INCLUDES += -I$(LIB_DIR)/pg
+COMMON_INCLUDES += -I$(LIB_DIR)/log/src/
+COMMON_INCLUDES += -I$(LIB_DIR)/subcommands.c/
+COMMON_INCLUDES += -I$(LIB_DIR)/libs/
+COMMON_INCLUDES += -I$(LIB_DIR)/parson/
+
+CC = $(shell $(PG_CONFIG) --cc)
+
+DEFAULT_CFLAGS  = -std=c99 -D_GNU_SOURCE -g
+DEFAULT_CFLAGS += -I $(shell $(PG_CONFIG) --includedir)
+DEFAULT_CFLAGS += -I $(shell $(PG_CONFIG) --includedir-server)
+DEFAULT_CFLAGS += -I $(shell $(PG_CONFIG) --pkgincludedir)/internal
+DEFAULT_CFLAGS += $(shell $(PG_CONFIG) --cflags)
+DEFAULT_CFLAGS += -Wformat
+DEFAULT_CFLAGS += -Wall
+DEFAULT_CFLAGS += -Werror=implicit-int
+DEFAULT_CFLAGS += -Werror=implicit-function-declaration
+DEFAULT_CFLAGS += -Werror=return-type
+DEFAULT_CFLAGS += -Wno-declaration-after-statement
+DEFAULT_CFLAGS += -D_WANT_SEMUN
+DEFAULT_CFLAGS += -Wno-missing-braces
+DEFAULT_CFLAGS += $(COMMON_INCLUDES)
+
+# On macOS, gettext is keg-only and not in the default include path.
+# postgres_fe.h → c.h → libintl.h requires finding it explicitly.
+ifeq ($(shell uname -s),Darwin)
+  GETTEXT_PREFIX := $(shell brew --prefix gettext 2>/dev/null)
+  ifneq ($(GETTEXT_PREFIX),)
+    DEFAULT_CFLAGS += -I$(GETTEXT_PREFIX)/include
+  endif
+endif
+
+override CFLAGS := $(DEFAULT_CFLAGS) $(CFLAGS)
+
+BINDIR ?= $(shell $(PG_CONFIG) --bindir)
+
+# pg_config --ldflags from source builds can emit colon-joined paths such as
+# -L/path/a:/path/b as a single token, which the linker rejects.  Split each
+# -L token on ':' so every directory gets its own -L flag.
+PG_LDFLAGS_RAW := $(shell $(PG_CONFIG) --ldflags)
+PG_LDFLAGS := $(shell echo "$(PG_LDFLAGS_RAW)" | awk '{ \
+    for (i = 1; i <= NF; i++) { \
+        if (substr($$i, 1, 2) == "-L") { \
+            n = split(substr($$i, 3), p, ":"); \
+            for (j = 1; j <= n; j++) if (p[j] != "") printf "-L%s ", p[j]; \
+        } else { printf "%s ", $$i; } \
+    } \
+}')
+
+LIBS  = -L $(shell $(PG_CONFIG) --pkglibdir)
+LIBS += -L $(shell $(PG_CONFIG) --libdir)
+LIBS += $(PG_LDFLAGS)
+LIBS += $(shell $(PG_CONFIG) --libs)
+LIBS += -lpq
+
+DEPDIR = .deps
+
+# -----------------------------------------------------------------------
+# Common source library (src/bin/common/*.c → libpgaf_common.a)
+# Callers link with: $(COMMON_LIB) and add $(COMMON_LIB) to OBJS.
+# -----------------------------------------------------------------------
+
+COMMON_SRC = $(wildcard $(COMMON_DIR)*.c)
+COMMON_OBJ = $(patsubst $(COMMON_DIR)%.c,$(COMMON_DIR)%.o,$(COMMON_SRC))
+COMMON_LIB = $(COMMON_DIR)libpgaf_common.a
+
+# Compile rule for common objects (run from each caller's directory)
+$(COMMON_DIR)%.o: $(COMMON_DIR)%.c
+	@if test ! -d $(COMMON_DIR)$(DEPDIR); then mkdir -p $(COMMON_DIR)$(DEPDIR); fi
+	$(CC) $(CFLAGS) -c -MMD -MP -MF$(COMMON_DIR)$(DEPDIR)/$(*F).Po -o $@ $<
+
+$(COMMON_LIB): $(COMMON_OBJ)
+	$(AR) rcs $@ $^
+
+Po_common_files := $(wildcard $(COMMON_DIR)$(DEPDIR)/*.Po)
+ifneq (,$(Po_common_files))
+include $(Po_common_files)
+endif
+
+# -----------------------------------------------------------------------
+# Compile rule for caller's own .c → .o with auto-dependency tracking
+# -----------------------------------------------------------------------
+%.o : %.c
+	@if test ! -d $(DEPDIR); then mkdir -p $(DEPDIR); fi
+	$(CC) $(CFLAGS) -c -MMD -MP -MF$(DEPDIR)/$(*F).Po -o $@ $<
+
+Po_files := $(wildcard $(DEPDIR)/*.Po)
+ifneq (,$(Po_files))
+include $(Po_files)
+endif
+
+# Shared rules for vendored libs
+lib-snprintf.o: $(PG_SNPRINTF)
+	$(CC) $(CFLAGS) -c -MMD -MP -MF$(DEPDIR)/lib-snprintf.Po -MT$@ \
+	    -o $@ $(LIB_DIR)/pg/snprintf.c
+
+lib-strerror.o: $(PG_SNPRINTF)
+	$(CC) $(CFLAGS) -c -MMD -MP -MF$(DEPDIR)/lib-strerror.Po -MT$@ \
+	    -o $@ $(LIB_DIR)/pg/strerror.c
+
+lib-log.o: $(LOG_SRC)
+	$(CC) $(CFLAGS) -c -MMD -MP -MF$(DEPDIR)/lib-log.Po -MT$@ \
+	    -o $@ $(LIB_DIR)/log/src/log.c
+
+lib-commandline.o: $(COMMANDLINE_SRC)
+	$(CC) $(CFLAGS) -c -MMD -MP -MF$(DEPDIR)/lib-commandline.Po -MT$@ \
+	    -o $@ $(LIB_DIR)/subcommands.c/commandline.c
+
+lib-parson.o: $(PARSON_SRC)
+	$(CC) $(CFLAGS) -c -MMD -MP -MF$(DEPDIR)/lib-parson.Po -MT$@ \
+	    -o $@ $(LIB_DIR)/parson/parson.c

src/bin/pg_autoctl/ini_file.c → src/bin/common/ini_file.c

@@ -116,11 +116,12 @@ parse_ini_buffer(const char *filename,
 					}
 
 					default:
-
+					{
 						/* should never happen, or it's a development bug */
 						log_fatal("Unknown option type %d", option->type);
 						ini_destroy(ini);
 						return false;
+					}
 				}
 			}
 		}
@@ -219,10 +220,11 @@ ini_validate_options(IniOption *optionList)
 			}
 
 			default:
-
+			{
 				/* should never happen, or it's a development bug */
 				log_fatal("Unknown option type %d", option->type);
 				return false;
+			}
 		}
 	}
 	return true;
@@ -660,10 +662,11 @@ ini_merge(IniOption *dstOptionList, IniOption *overrideOptionList)
 			}
 
 			default:
-
+			{
 				/* should never happen, or it's a development bug */
 				log_fatal("Unknown option type %d", option->type);
 				return false;
+			}
 		}
 	}
 	return true;

src/bin/pg_autoctl/ipaddr.c → src/bin/common/ipaddr.c

@@ -303,7 +303,9 @@ fetchLocalCIDR(const char *localIpAddress, char *localCIDR, int size)
 			}
 
 			default:
+			{
 				continue;
+			}
 		}
 
 		if (strcmp(address, localIpAddress) == 0)

src/bin/pg_autoctl/pgctl.c → src/bin/common/pgctl.c

@@ -1264,7 +1264,7 @@ pg_basebackup(const char *pgdata,
 	NodeAddress *primaryNode = &(replicationSource->primaryNode);
 	char primaryConnInfo[MAXCONNINFO] = { 0 };
 
-	char *args[16];
+	char *args[18];  /* enough for all pg_basebackup flags incl. --checkpoint=fast */
 	int argsIndex = 0;
 
 	char command[BUFSIZE];
@@ -1330,6 +1330,7 @@ pg_basebackup(const char *pgdata,
 	args[argsIndex++] = "--max-rate";
 	args[argsIndex++] = replicationSource->maximumBackupRate;
 	args[argsIndex++] = "--wal-method=stream";
+	args[argsIndex++] = "--checkpoint=fast";
 
 	/* we don't use a replication slot e.g. when upstream is a standby */
 	if (!IS_EMPTY_STRING_BUFFER(replicationSource->slotName))
@@ -1628,13 +1629,12 @@ pg_ctl_postgres(const char *pg_ctl, const char *pgdata, int pgport,
 	/* prepare startup.log file in PGDATA */
 	join_path_components(logfile, pgdata, "startup.log");
 
-	IntString pgportStr = intToString(pgport);
-
 	args[argsIndex++] = (char *) postgres;
 	args[argsIndex++] = "-D";
 	args[argsIndex++] = (char *) pgdata;
 	args[argsIndex++] = "-p";
-	args[argsIndex++] = pgportStr.strValue;
+	IntString pgportStr = intToString(pgport);
+	args[argsIndex++] = (char *) pgportStr.strValue;
 
 	if (listen)
 	{
@@ -2028,56 +2028,57 @@ pg_ctl_status(const char *pg_ctl, const char *pgdata, bool log_output)
 
 
 /*
- * pg_ctl_reload reloads PostgreSQL configuration by running "pg_ctl reload".
+ * pg_ctl_promote promotes a standby by running "pg_ctl promote"
  */
 bool
-pg_ctl_reload(const char *pg_ctl, const char *pgdata)
+pg_ctl_promote(const char *pg_ctl, const char *pgdata)
 {
-	Program program = run_program(pg_ctl, "-D", pgdata, "reload", NULL);
+	Program program =
+		run_program(pg_ctl, "-D", pgdata, "--no-wait", "promote", NULL);
 	int returnCode = program.returnCode;
 
+	log_debug("%s promote -D %s --no-wait", pg_ctl, pgdata);
+
 	if (program.stdErr != NULL)
 	{
-		log_debug("%s", program.stdErr);
+		log_error("%s", program.stdErr);
 	}
 
-	free_program(&program);
-
 	if (returnCode != 0)
 	{
-		log_error("pg_ctl reload -D %s failed (exit %d)", pgdata, returnCode);
+		/* pg_ctl promote will have logged errors */
+		free_program(&program);
 		return false;
 	}
 
+	free_program(&program);
 	return true;
 }
 
 
 /*
- * pg_ctl_promote promotes a standby by running "pg_ctl promote"
+ * pg_ctl_reload reloads Postgres configuration by running "pg_ctl reload".
+ * Does not require a libpq connection — useful when HBA hasn't been set up yet.
  */
 bool
-pg_ctl_promote(const char *pg_ctl, const char *pgdata)
+pg_ctl_reload(const char *pg_ctl, const char *pgdata)
 {
-	Program program =
-		run_program(pg_ctl, "-D", pgdata, "--no-wait", "promote", NULL);
+	Program program = run_program(pg_ctl, "-D", pgdata, "reload", NULL);
 	int returnCode = program.returnCode;
 
-	log_debug("%s promote -D %s --no-wait", pg_ctl, pgdata);
-
 	if (program.stdErr != NULL)
 	{
-		log_error("%s", program.stdErr);
+		log_debug("%s", program.stdErr);
 	}
 
+	free_program(&program);
+
 	if (returnCode != 0)
 	{
-		/* pg_ctl promote will have logged errors */
-		free_program(&program);
+		log_error("pg_ctl reload -D %s failed (exit %d)", pgdata, returnCode);
 		return false;
 	}
 
-	free_program(&program);
 	return true;
 }
 

src/bin/pg_autoctl/pgctl.h → src/bin/common/pgctl.h

@@ -57,9 +57,9 @@ bool pg_ctl_postgres(const char *pg_ctl, const char *pgdata, int pgport,
 bool pg_log_startup(const char *pgdata, int logLevel);
 bool pg_log_recovery_setup(const char *pgdata, int logLevel);
 bool pg_ctl_stop(const char *pg_ctl, const char *pgdata);
-bool pg_ctl_reload(const char *pg_ctl, const char *pgdata);
 int pg_ctl_status(const char *pg_ctl, const char *pgdata, bool log_output);
 bool pg_ctl_promote(const char *pg_ctl, const char *pgdata);
+bool pg_ctl_reload(const char *pg_ctl, const char *pgdata);
 
 bool pg_setup_standby_mode(uint32_t pg_control_version,
 						   const char *pgdata,

src/bin/pg_autoctl/pgsetup.c → src/bin/common/pgsetup.c

@@ -1491,8 +1491,10 @@ nodeKindToString(PgInstanceKind kind)
 		}
 
 		default:
+		{
 			log_fatal("nodeKindToString: unknown node kind %d", kind);
 			return NULL;
+		}
 	}
 
 	/* can't happen, keep compiler happy */
@@ -1563,7 +1565,9 @@ pmStatusToString(PostmasterStatus pm_status)
 		}
 
 		case POSTMASTER_STATUS_STANDBY:
+		{
 			return "standby";
+		}
 	}
 
 	/* keep compiler happy */
@@ -1842,7 +1846,9 @@ pgsetup_sslmode_to_string(SSLMode sslMode)
 		}
 
 		case SSL_MODE_VERIFY_FULL:
+		{
 			return "verify-full";
+		}
 	}
 
 	/* This is a huge bug */
@@ -1986,7 +1992,9 @@ pgsetup_hba_level_to_string(HBAEditLevel hbaLevel)
 		}
 
 		case HBA_EDIT_UNKNOWN:
+		{
 			return "unknown";
+		}
 	}
 
 	log_error("BUG: hbaLevel %d is unknown", hbaLevel);
@@ -2033,7 +2041,9 @@ dbstateToString(DBState state)
 		}
 
 		case DB_IN_PRODUCTION:
+		{
 			return "in production";
+		}
 	}
 	return "unrecognized status code";
 }