Separate Postgres build dir from installation dir.

Previously, Postgres was built in 'tmp_install/build', and installed
into 'tmp_install'. In the CI, the 'build' directory was included in
the final neon.tar.zst artifact that includes all the necessary
binaries. That was unnecessary, the intermediate build results are not
needed, only the final binaries. Separate the build directory so that
the Postgres build happens in 'build', and it is installed into
'tmp_install'. That makes the final neon.tar.zst artifact smaller.

The changes to the python tests are needed to find the 'pg_regress'
binary in the installation directory. Previously, they would use the
'pg_regress' binary from the build directory, not the final
installation location.

.github/actions/run-python-test-set/action.yml

@@ -31,6 +31,13 @@ inputs:
 runs:
   using: "composite"
   steps:
+    - name: Checkout
+      if: inputs.needs_postgres_source == 'true'
+      uses: actions/checkout@v3
+      with:
+        submodules: true
+        fetch-depth: 1
+
     - name: Get Neon artifact for restoration
       uses: actions/download-artifact@v3
       with:
@@ -44,12 +51,11 @@ runs:
         tar -xf ./neon-artifact/neon.tar.zst -C /tmp/neon/
         rm -rf ./neon-artifact/
 
-    - name: Checkout
-      if: inputs.needs_postgres_source == 'true'
-      uses: actions/checkout@v3
-      with:
-        submodules: true
-        fetch-depth: 1
+        # Restore the parts of the 'build' directory that were included in the
+        # tarball. This includes the regression test modules in
+        # src/test/regress/*.so.
+        mkdir -p build/
+        cp -a /tmp/neon/pg_build/* build/
 
     - name: Cache poetry deps
       id: cache_poetry

.github/workflows/build_and_test.yml

@@ -88,7 +88,9 @@ jobs:
         id: cache_pg
         uses: actions/cache@v3
         with:
-          path: tmp_install/
+          path: |
+            tmp_install/
+            build/src/test/regress/*.so
           key: v1-${{ runner.os }}-${{ matrix.build_type }}-pg-${{ steps.pg_ver.outputs.pg_rev }}-${{ hashFiles('Makefile') }}
 
       - name: Build postgres
@@ -143,7 +145,12 @@ jobs:
           fi
 
       - name: Install postgres binaries
-        run: cp -a tmp_install /tmp/neon/pg_install
+        run: |
+          cp -a tmp_install /tmp/neon/pg_install
+
+          # Include modules needed by the Postgres regression tests
+          mkdir -p /tmp/neon/pg_build/src/test/regress
+          cp -a build/src/test/regress/*.so /tmp/neon/pg_build/src/test/regress
 
       - name: Prepare neon artifact
         run: ZSTD_NBTHREADS=0 tar -C /tmp/neon/ -cf ./neon.tar.zst --zstd .

Makefile

@@ -60,10 +60,14 @@ neon: postgres-headers
 	$(CARGO_CMD_PREFIX) cargo build $(CARGO_BUILD_FLAGS)
 
 ### PostgreSQL parts
-$(POSTGRES_INSTALL_DIR)/build/config.status:
+#
+# Postgres is built in the 'build' directory, and installed into
+# $(POSTGRES_INSTALL_DIR), which defaults to 'tmp_install'
+#
+build/config.status:
 	+@echo "Configuring postgres build"
-	mkdir -p $(POSTGRES_INSTALL_DIR)/build
-	(cd $(POSTGRES_INSTALL_DIR)/build && \
+	mkdir -p build
+	(cd build && \
 	$(ROOT_PROJECT_DIR)/vendor/postgres/configure CFLAGS='$(PG_CFLAGS)' \
 		$(PG_CONFIGURE_OPTS) \
 		$(SECCOMP) \
@@ -71,44 +75,44 @@ $(POSTGRES_INSTALL_DIR)/build/config.status:
 
 # nicer alias for running 'configure'
 .PHONY: postgres-configure
-postgres-configure: $(POSTGRES_INSTALL_DIR)/build/config.status
+postgres-configure: build/config.status
 
 # Install the PostgreSQL header files into $(POSTGRES_INSTALL_DIR)/include
 .PHONY: postgres-headers
 postgres-headers: postgres-configure
 	+@echo "Installing PostgreSQL headers"
-	$(MAKE) -C $(POSTGRES_INSTALL_DIR)/build/src/include MAKELEVEL=0 install
+	$(MAKE) -C build/src/include MAKELEVEL=0 install
 
 # Compile and install PostgreSQL and contrib/neon
 .PHONY: postgres
 postgres: postgres-configure \
-		  postgres-headers # to prevent `make install` conflicts with zenith's `postgres-headers`
+		  postgres-headers # to prevent `make install` conflicts with neon's `postgres-headers`
 	+@echo "Compiling PostgreSQL"
-	$(MAKE) -C $(POSTGRES_INSTALL_DIR)/build MAKELEVEL=0 install
+	$(MAKE) -C build MAKELEVEL=0 install
 	+@echo "Compiling contrib/neon"
-	$(MAKE) -C $(POSTGRES_INSTALL_DIR)/build/contrib/neon install
+	$(MAKE) -C build/contrib/neon install
 	+@echo "Compiling contrib/neon_test_utils"
-	$(MAKE) -C $(POSTGRES_INSTALL_DIR)/build/contrib/neon_test_utils install
+	$(MAKE) -C build/contrib/neon_test_utils install
 	+@echo "Compiling pg_buffercache"
-	$(MAKE) -C $(POSTGRES_INSTALL_DIR)/build/contrib/pg_buffercache install
+	$(MAKE) -C build/contrib/pg_buffercache install
 	+@echo "Compiling pageinspect"
-	$(MAKE) -C $(POSTGRES_INSTALL_DIR)/build/contrib/pageinspect install
+	$(MAKE) -C build/contrib/pageinspect install
 
 
 .PHONY: postgres-clean
 postgres-clean:
-	$(MAKE) -C $(POSTGRES_INSTALL_DIR)/build MAKELEVEL=0 clean
+	$(MAKE) -C build MAKELEVEL=0 clean
 
 # This doesn't remove the effects of 'configure'.
 .PHONY: clean
 clean:
-	cd $(POSTGRES_INSTALL_DIR)/build && $(MAKE) clean
+	cd build && $(MAKE) clean
 	$(CARGO_CMD_PREFIX) cargo clean
 
 # This removes everything
 .PHONY: distclean
 distclean:
-	rm -rf $(POSTGRES_INSTALL_DIR)
+	rm -rf build $(POSTGRES_INSTALL_DIR)
 	$(CARGO_CMD_PREFIX) cargo clean
 
 .PHONY: fmt

test_runner/batch_pg_regress/test_isolation.py

@@ -20,18 +20,22 @@ def test_isolation(neon_simple_env: NeonEnv, test_output_dir: Path, pg_bin, caps
     runpath = test_output_dir / 'regress'
     (runpath / 'testtablespace').mkdir(parents=True)
 
+    # Find the pg_isolation_regress binary
+    proc = pg_bin.run(['pg_config', '--libdir'], capture_output=True)
+    libdir = proc.stdout.decode().strip()
+    proc = pg_bin.run(['pg_config', '--bindir'], capture_output=True)
+    bindir = proc.stdout.decode().strip()
+    pg_isolation_regress = os.path.join(libdir,
+                                        'postgresql/pgxs/src/test/isolation/pg_isolation_regress')
+
     # Compute all the file locations that pg_isolation_regress will need.
-    build_path = os.path.join(pg_distrib_dir, 'build/src/test/isolation')
     src_path = os.path.join(base_dir, 'vendor/postgres/src/test/isolation')
-    bindir = os.path.join(pg_distrib_dir, 'bin')
     schedule = os.path.join(src_path, 'isolation_schedule')
-    pg_isolation_regress = os.path.join(build_path, 'pg_isolation_regress')
 
     pg_isolation_regress_command = [
         pg_isolation_regress,
         '--use-existing',
         '--bindir={}'.format(bindir),
-        '--dlpath={}'.format(build_path),
         '--inputdir={}'.format(src_path),
         '--schedule={}'.format(schedule),
     ]

test_runner/batch_pg_regress/test_neon_regress.py

@@ -20,19 +20,22 @@ def test_neon_regress(neon_simple_env: NeonEnv, test_output_dir: Path, pg_bin, c
     runpath = test_output_dir / 'regress'
     (runpath / 'testtablespace').mkdir(parents=True)
 
+    # Find the pg_regress binary and --bindir option to pass to it.
+    proc = pg_bin.run(['pg_config', '--libdir'], capture_output=True)
+    libdir = proc.stdout.decode().strip()
+    proc = pg_bin.run(['pg_config', '--bindir'], capture_output=True)
+    bindir = proc.stdout.decode().strip()
+    pg_regress = os.path.join(libdir, 'postgresql/pgxs/src/test/regress/pg_regress')
+
     # Compute all the file locations that pg_regress will need.
     # This test runs neon specific tests
-    build_path = os.path.join(pg_distrib_dir, 'build/src/test/regress')
     src_path = os.path.join(base_dir, 'test_runner/neon_regress')
-    bindir = os.path.join(pg_distrib_dir, 'bin')
     schedule = os.path.join(src_path, 'parallel_schedule')
-    pg_regress = os.path.join(build_path, 'pg_regress')
 
     pg_regress_command = [
         pg_regress,
         '--use-existing',
         '--bindir={}'.format(bindir),
-        '--dlpath={}'.format(build_path),
         '--schedule={}'.format(schedule),
         '--inputdir={}'.format(src_path),
     ]

test_runner/batch_pg_regress/test_pg_regress.py

@@ -19,19 +19,23 @@ def test_pg_regress(neon_simple_env: NeonEnv, test_output_dir: pathlib.Path, pg_
     runpath = test_output_dir / 'regress'
     (runpath / 'testtablespace').mkdir(parents=True)
 
+    # Find the pg_regress binary and --bindir option to pass to it.
+    proc = pg_bin.run(['pg_config', '--libdir'], capture_output=True)
+    libdir = proc.stdout.decode().strip()
+    proc = pg_bin.run(['pg_config', '--bindir'], capture_output=True)
+    bindir = proc.stdout.decode().strip()
+    pg_regress = os.path.join(libdir, 'postgresql/pgxs/src/test/regress/pg_regress')
+
     # Compute all the file locations that pg_regress will need.
-    build_path = os.path.join(pg_distrib_dir, 'build/src/test/regress')
     src_path = os.path.join(base_dir, 'vendor/postgres/src/test/regress')
-    bindir = os.path.join(pg_distrib_dir, 'bin')
     schedule = os.path.join(src_path, 'parallel_schedule')
-    pg_regress = os.path.join(build_path, 'pg_regress')
+    dlpath = os.path.join(base_dir, 'build/src/test/regress')
 
     pg_regress_command = [
         pg_regress,
-        '--bindir=""',
         '--use-existing',
         '--bindir={}'.format(bindir),
-        '--dlpath={}'.format(build_path),
+        '--dlpath={}'.format(dlpath),
         '--schedule={}'.format(schedule),
         '--inputdir={}'.format(src_path),
     ]

test_runner/fixtures/neon_fixtures.py

@@ -1372,7 +1372,10 @@ class PgBin:
         env.update(env_add)
         return env
 
-    def run(self, command: List[str], env: Optional[Env] = None, cwd: Optional[str] = None):
+    def run(self,
+            command: List[str],
+            env: Optional[Env] = None,
+            **kwargs) -> 'subprocess.CompletedProcess[str]':
         """
         Run one of the postgres binaries.
 
@@ -1389,7 +1392,7 @@ class PgBin:
         self._fixpath(command)
         log.info('Running command "{}"'.format(' '.join(command)))
         env = self._build_env(env)
-        subprocess.run(command, env=env, cwd=cwd, check=True)
+        return subprocess.run(command, env=env, check=True, **kwargs)
 
     def run_capture(self,
                     command: List[str],