Add PGXNTOOL_ENABLE_FS_INSTALL and PGXNTOOL_ENABLE_PGXN_INSTALL - #91
Draft
jnasbyupgrade wants to merge 2 commits into
Draft
Add PGXNTOOL_ENABLE_FS_INSTALL and PGXNTOOL_ENABLE_PGXN_INSTALL#91jnasbyupgrade wants to merge 2 commits into
jnasbyupgrade wants to merge 2 commits into
Conversation
`test`/`verify-results`/`installcheck`/`test-build` always filesystem-installed the extension via PGXS's `install`, with no way to disable it. That defeated "existing mode" testing, where the extension under test was deployed some other way (e.g. a pg_tle registration, or a real pg_upgrade) -- the whole point is to prove that other deployment path works, not to filesystem-install as a side effect. `PGXNTOOL_ENABLE_FS_INSTALL` (default `yes`) now gates `install` out of all four. `installcheck` also unconditionally auto-installed the pgtap dependency via `pgxn install pgtap --sudo` -- itself a filesystem-install side effect. `PGXNTOOL_ENABLE_PGXN_INSTALL` independently gates that (defaulting to follow `PGXNTOOL_ENABLE_FS_INSTALL`, but overridable on its own). Documented both in README.asc's make-targets and environment-variable reference sections. Fixes Postgres-Extensions#55. Fixes Postgres-Extensions#90. Related changes in pgxntool-test: - Added BATS coverage in `test/standard/make-test.bats`: default/override behavior for both variables, structural proof (`make -p -n`) that `install`/`pgtap` prerequisites are genuinely removed, and end-to-end proof that `make test PGXNTOOL_ENABLE_FS_INSTALL=no` succeeds against an already-installed tree but fails against a genuinely uninstalled one. Co-Authored-By: Claude <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
2 tasks
testdeps' own `testdeps: pgtap` prerequisite is unconditional, so without an unconditional .PHONY declaration for pgtap, disabling PGXNTOOL_ENABLE_PGXN_INSTALL would leave `pgtap` completely undefined and fail with "No rule to make target 'pgtap'". Document that instead of leaving it looking like an inconsistency with check-stale-expected's own (intentionally different) .PHONY placement. Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
test/verify-results/installcheck/test-buildalways filesystem-installed the extension via PGXS'sinstall, with no way to disable it -- breaking "existing mode"/pg_tle-style testing, where the extension under test is deployed some other way and a filesystem install as a side effect defeats the point of the test.PGXNTOOL_ENABLE_FS_INSTALL(defaultyes) now gatesinstallout of all four.installcheckalso unconditionally auto-installed the pgtap dependency viapgxn install pgtap --sudo.PGXNTOOL_ENABLE_PGXN_INSTALLindependently gates that, defaulting to followPGXNTOOL_ENABLE_FS_INSTALLbut overridable on its own.Fixes #55.
Fixes #90.
Companion pgxntool-test PR: Postgres-Extensions/pgxntool-test#70
Test plan
test/standard/make-test.bats(35/35) and fulltest-allsuite (257/257) pass in pgxntool-test against this branch