Skip to content

Fix make install for source builds - #345

Open
waldemort-auto[bot] wants to merge 1 commit into
mainfrom
waldemort/fix-make-install
Open

Fix make install for source builds#345
waldemort-auto[bot] wants to merge 1 commit into
mainfrom
waldemort/fix-make-install

Conversation

@waldemort-auto

@waldemort-auto waldemort-auto Bot commented Aug 13, 2026

Copy link
Copy Markdown

The bug

make install cannot install pg_durable. PGXS supplies an install recipe that copies only files named by its DATA and MODULES variables. Neither is set, so it installs only the uninterpolated pg_durable.control template: no shared library, no generated SQL, and an invalid @CARGO_VERSION@ default version.

This becomes user-facing when pg_durable is distributed through PGXN. pgxn install runs make as the invoking user, then runs make install through sudo when PostgreSQL's library directory is not writable. Compiling during that privileged phase depends on Cargo and the user's cargo-pgrx configuration surviving sudo environment filtering, which is not normally true. PGXN also invokes make uninstall directly, but the repository did not provide that target.

This is the companion build fix for #344.

The fix

Source installation now follows a conventional two-phase model:

$ make PG_CONFIG=/usr/lib/postgresql/17/bin/pg_config
cargo pgrx package --pg-config ... --out-dir ... \
    --no-default-features --features "pg17"

$ sudo make install PG_CONFIG=/usr/lib/postgresql/17/bin/pg_config
# Copies prebuilt artifacts only; Cargo is not invoked.

make uninstall is the copy-only inverse and does not require a package tree, so PGXN can remove an installed source distribution without rebuilding it.

Additional behavior:

  • PostgreSQL 17 and 18 are validated from pg_config; unsupported majors fail before building.
  • An explicit PG_CONFIG is honored. Otherwise Make first asks cargo-pgrx for PG_VERSION (default pg17) and then follows the conventional pg_config on PATH fallback.
  • EXTRA_FEATURES adds non-PostgreSQL Cargo features, for example EXTRA_FEATURES=http-allow-azure-domains.
  • DESTDIR stages install and uninstall operations for package builders.
  • PGXS is loaded only for installcheck, eliminating its conflicting install/uninstall recipes.
  • Custom nonempty package output directories are not replaced unless they were created by an earlier successful make package; the standard dedicated output remains rebuildable.
  • Installation rejects unrecognized files in the cargo-pgrx package tree, preventing source installs from silently diverging from Debian packages.
  • The default all target now builds release package artifacts. make build, pg-install, pg-clean, and PGXS installcheck retain their explicit roles.

Regression tests against PostgreSQL

make test-regress remains the recommended contributor command. It resets and configures the dedicated local cluster before running PGXS regressions.

Direct make installcheck remains available for an already-running disposable test server. PostgreSQL's pg_regress drops and recreates CONTRIB_TESTDB unless explicitly placed in --use-existing mode, and pg_durable's current regression initialization is not designed for a pre-populated database. The documentation now provides the required PGHOST, PGPORT, PGUSER, PG_CONFIG, preload, worker-database, privilege, and restart requirements and warns not to target valuable data.

Automated coverage

scripts/test-make-install.sh provides offline checks for:

  • PG17 and PG18 feature selection, including cargo-pgrx and PATH-based pg_config discovery
  • the documented default make target
  • unsupported-major rejection
  • copy-only installation and uninstall with Cargo unavailable
  • DESTDIR, paths containing spaces, installed file modes, and caller-supplied PGXS
  • missing and partial package artifacts
  • preservation of unrelated extension files during partial uninstall
  • refusal to replace unowned custom package directories
  • package-tree drift detection
  • PGXS install/installcheck/uninstall goal isolation

The release workflow builds through the Makefile for both PG17 and PG18, stages each real cargo-pgrx package through make install, and verifies the library, control file, install SQL, and upgrade SQL before Debian packaging. The source-install CI job uses the same full-length action SHA pinning convention as the rest of the repository.

Verification

Validated locally with PostgreSQL 17.10 and GNU make 4.3:

  1. Built a real release package with make package.
  2. Staged the real library, control file, install SQL, and upgrade SQL through make install DESTDIR=....
  3. Installed with Cargo absent from the install environment and loaded pg_durable:0.2.6 successfully.
  4. Ran all six PGXS regression tests successfully through make test-regress.
  5. Verified direct installcheck discovers a supported PG17 pg_config from PATH with cargo-pgrx unavailable.
  6. Passed source-install smoke checks, Rust formatting, workflow pin checks, patch whitespace checks, and editor diagnostics.

The branch is rebased onto current main and contains one commit authored by Pino de Candia (pinodeca).

Generated with the assistance of GitHub Copilot.

@pinodeca

Copy link
Copy Markdown
Contributor

Added two maintainer follow-up commits after reviewing the original cargo pgrx install approach:

  • 985ff1f — build source artifacts before privileged install
  • bb500f7 — test and enforce the source installation workflow

The key reason for the change is PGXN's execution model: it runs make as the invoking user, then runs make install through sudo when PostgreSQL's library directory is not writable. Compiling inside make install therefore depends on Cargo and the user's cargo-pgrx configuration surviving sudo's environment filtering, which is not normally true.

The revised flow is deliberately two-phase:

  1. make / make package runs cargo pgrx package without elevated privileges.
  2. sudo make install only validates and copies the prebuilt .so, control file, install SQL, and upgrade SQL. It never invokes Cargo.

The follow-up also:

  • validates that the selected pg_config is PostgreSQL 17 or 18;
  • honors explicit PG_CONFIG and supports cargo-pgrx PG_VERSION resolution;
  • supports DESTDIR staging and EXTRA_FEATURES;
  • isolates PGXS to installcheck, avoiding its conflicting install recipe;
  • adds fast PG17/PG18 Makefile contract tests, including Cargo-free install, partial packages, caller-supplied PGXS, paths containing spaces, file modes, and unsupported versions;
  • routes the real PG17/PG18 release package builds through make package and stages them through make install before Debian packaging.

Local validation completed:

  • real PG17 release package generated successfully;
  • install succeeded with Cargo absent from PATH;
  • PostgreSQL loaded pg_durable:0.2.6 from the copied artifacts;
  • all six pg_regress tests passed;
  • source-install smoke checks, formatting, shell syntax, whitespace, and editor diagnostics passed.

The dedicated Source Install Checks CI job is green. The longer PG17 test and PG17/PG18 package jobs are still running at the time of this comment.

@waldemort-auto waldemort-auto Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Permission probe — checking whether the App can post via the pull-request reviews endpoint rather than the issue-comments endpoint.

@waldemort-auto waldemort-auto Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a fourth commit, 4dd59a6, covering the uninstall gap and the packaging-drift risk raised in review.

(Apologies for the preceding one-line review — the App cannot post through the issue-comments endpoint, so that was a probe to find a path that works. It cannot be edited or deleted through the API available to me.)

pgxn uninstall pg_durable currently fails. pgxnclient's Uninstall command runs make uninstall under the same sudo rules as install, and no such target exists. The new target is the inverse of install and follows the same two-phase reasoning: pgxnclient unpacks a distribution and runs uninstall directly, without building first, so it must not read the package tree or invoke Cargo. It is idempotent, honors DESTDIR, and is excluded alongside install when installcheck is also requested, since PGXS defines its own uninstall recipe too.

On .deb versus source-install divergence. Checked against cargo-pgrx's install_extension, which emits exactly three things: the control file, the shared library, and the SQL files. So the packaged tree and what install copies agree today, and no restructuring is warranted. What was missing is anything holding that true: package-deb.sh copies the whole usr/ tree, so a future cargo-pgrx emitting anything more would ship it in the .deb and silently drop it from source installs.

install now fails when the packaged tree contains a file it does not install, naming the file. That turns a silent divergence into a build failure where the decision belongs. Since the release workflow already stages through make install, it is enforced on every release with no extra wiring.

Test coverage, including two invocation shapes that were previously untested:

Case Why
uninstall removes exactly what install placed with Cargo off PATH, caller-supplied PGXS, missing package dir, DESTDIR staging
uninstall is idempotent a partial install must always be cleanable
install rejects an unrecognized packaged file the drift guard, using a stray .bc
make installcheck invoked directly pgxn check calls it with PG_CONFIG on the command line and no wrapper; CI previously reached installcheck only through make test-regress
uninstall + installcheck refused together matches the existing install rule

The stale assertion for the goal-conflict message was updated to match its new wording.

Verified with PostgreSQL 17.10 and make 4.3: scripts/test-make-install.sh passes, and build, all, package, install, uninstall, installcheck and help each produce their expected recipe with no warnings.

One finding outside this commit: macOS source installs cannot work. cargo-pgrx names the library using DLL_SUFFIX, which is .dylib on macOS for PostgreSQL 16 and later, while install looks for pg_durable.so. Reasonable given the release targets Debian/Linux, but a PGXN listing reaches macOS users, so it may deserve either support or an explicit statement.

Generated with the assistance of GitHub Copilot.

Build cargo-pgrx package artifacts before the privileged install phase so PGXN installs do not depend on Cargo surviving sudo environment filtering. Add copy-only install and uninstall targets with PG17/PG18 validation, DESTDIR support, package drift checks, and conventional pg_config discovery.

Exercise the source workflow in CI and release packaging, pin the new action reference, and document direct installcheck as an advanced operation against a disposable configured server.
@microsoft-github-policy-service

Copy link
Copy Markdown

@waldemort-auto[bot] please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"
Contributor License Agreement

Contribution License Agreement

This Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
and conveys certain license rights to Microsoft Corporation and its affiliates (“Microsoft”) for Your
contributions to Microsoft open source projects. This Agreement is effective as of the latest signature
date below.

  1. Definitions.
    “Code” means the computer software code, whether in human-readable or machine-executable form,
    that is delivered by You to Microsoft under this Agreement.
    “Project” means any of the projects owned or managed by Microsoft and offered under a license
    approved by the Open Source Initiative (www.opensource.org).
    “Submit” is the act of uploading, submitting, transmitting, or distributing code or other content to any
    Project, including but not limited to communication on electronic mailing lists, source code control
    systems, and issue tracking systems that are managed by, or on behalf of, the Project for the purpose of
    discussing and improving that Project, but excluding communication that is conspicuously marked or
    otherwise designated in writing by You as “Not a Submission.”
    “Submission” means the Code and any other copyrightable material Submitted by You, including any
    associated comments and documentation.
  2. Your Submission. You must agree to the terms of this Agreement before making a Submission to any
    Project. This Agreement covers any and all Submissions that You, now or in the future (except as
    described in Section 4 below), Submit to any Project.
  3. Originality of Work. You represent that each of Your Submissions is entirely Your original work.
    Should You wish to Submit materials that are not Your original work, You may Submit them separately
    to the Project if You (a) retain all copyright and license information that was in the materials as You
    received them, (b) in the description accompanying Your Submission, include the phrase “Submission
    containing materials of a third party:” followed by the names of the third party and any licenses or other
    restrictions of which You are aware, and (c) follow any other instructions in the Project’s written
    guidelines concerning Submissions.
  4. Your Employer. References to “employer” in this Agreement include Your employer or anyone else
    for whom You are acting in making Your Submission, e.g. as a contractor, vendor, or agent. If Your
    Submission is made in the course of Your work for an employer or Your employer has intellectual
    property rights in Your Submission by contract or applicable law, You must secure permission from Your
    employer to make the Submission before signing this Agreement. In that case, the term “You” in this
    Agreement will refer to You and the employer collectively. If You change employers in the future and
    desire to Submit additional Submissions for the new employer, then You agree to sign a new Agreement
    and secure permission from the new employer before Submitting those Submissions.
  5. Licenses.
  • Copyright License. You grant Microsoft, and those who receive the Submission directly or
    indirectly from Microsoft, a perpetual, worldwide, non-exclusive, royalty-free, irrevocable license in the
    Submission to reproduce, prepare derivative works of, publicly display, publicly perform, and distribute
    the Submission and such derivative works, and to sublicense any or all of the foregoing rights to third
    parties.
  • Patent License. You grant Microsoft, and those who receive the Submission directly or
    indirectly from Microsoft, a perpetual, worldwide, non-exclusive, royalty-free, irrevocable license under
    Your patent claims that are necessarily infringed by the Submission or the combination of the
    Submission with the Project to which it was Submitted to make, have made, use, offer to sell, sell and
    import or otherwise dispose of the Submission alone or with the Project.
  • Other Rights Reserved. Each party reserves all rights not expressly granted in this Agreement.
    No additional licenses or rights whatsoever (including, without limitation, any implied licenses) are
    granted by implication, exhaustion, estoppel or otherwise.
  1. Representations and Warranties. You represent that You are legally entitled to grant the above
    licenses. You represent that each of Your Submissions is entirely Your original work (except as You may
    have disclosed under Section 3). You represent that You have secured permission from Your employer to
    make the Submission in cases where Your Submission is made in the course of Your work for Your
    employer or Your employer has intellectual property rights in Your Submission by contract or applicable
    law. If You are signing this Agreement on behalf of Your employer, You represent and warrant that You
    have the necessary authority to bind the listed employer to the obligations contained in this Agreement.
    You are not expected to provide support for Your Submission, unless You choose to do so. UNLESS
    REQUIRED BY APPLICABLE LAW OR AGREED TO IN WRITING, AND EXCEPT FOR THE WARRANTIES
    EXPRESSLY STATED IN SECTIONS 3, 4, AND 6, THE SUBMISSION PROVIDED UNDER THIS AGREEMENT IS
    PROVIDED WITHOUT WARRANTY OF ANY KIND, INCLUDING, BUT NOT LIMITED TO, ANY WARRANTY OF
    NONINFRINGEMENT, MERCHANTABILITY, OR FITNESS FOR A PARTICULAR PURPOSE.
  2. Notice to Microsoft. You agree to notify Microsoft in writing of any facts or circumstances of which
    You later become aware that would make Your representations in this Agreement inaccurate in any
    respect.
  3. Information about Submissions. You agree that contributions to Projects and information about
    contributions may be maintained indefinitely and disclosed publicly, including Your name and other
    information that You submit with Your Submission.
  4. Governing Law/Jurisdiction. This Agreement is governed by the laws of the State of Washington, and
    the parties consent to exclusive jurisdiction and venue in the federal courts sitting in King County,
    Washington, unless no federal subject matter jurisdiction exists, in which case the parties consent to
    exclusive jurisdiction and venue in the Superior Court of King County, Washington. The parties waive all
    defenses of lack of personal jurisdiction and forum non-conveniens.
  5. Entire Agreement/Assignment. This Agreement is the entire agreement between the parties, and
    supersedes any and all prior agreements, understandings or communications, written or oral, between
    the parties relating to the subject matter hereof. This Agreement may be assigned by Microsoft.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant