Guarantee test/install/*.sql schedule ordering - #120
Open
jnasbyupgrade wants to merge 1 commit into
Open
jnasbyupgrade wants to merge 1 commit into
jnasbyupgrade wants to merge 1 commit into
Conversation
`TEST_INSTALL_SQL_FILES` fed the test/install schedule straight from `$(wildcard)`, whose order depended entirely on the underlying filesystem/libc `glob()` behavior -- alphabetical in practice almost everywhere, but never a documented guarantee. A real consumer hit this: two install files had an implicit dependency (one provisioning roles the other's `CREATE EXTENSION ... CASCADE` requires), named such that alphabetical order ran them backwards. Wrap the wildcard in `$(sort ...)`. GNU Make's `sort` is a plain byte-value comparison, independent of locale (`LC_COLLATE` etc. have no effect on it), so ordering is now a guarantee pgxntool itself enforces rather than an accident of the platform. Documented in `README.asc` and `base.mk`, along with the numeric-prefix convention for consumers with an inter-file dependency between install scripts. Related changes in pgxntool-test: - Regression test creating install files out of sorted order and asserting the generated schedule lists them correctly anyway - Direct check that `base.mk` wires `TEST_INSTALL_SQL_FILES` through `$(sort ...)`, since this container's libc already returns sorted `glob()` results by default and the behavioral test alone can't distinguish the fix from a revert here Fixes Postgres-Extensions#111. Co-Authored-By: Claude Sonnet 5 <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: Advanced 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 |
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.
TEST_INSTALL_SQL_FILESfed the test/install schedule straight from$(wildcard), whose order depended entirely on the underlying filesystem/libcglob()behavior -- alphabetical in practice almost everywhere, but never a documented guarantee. A real consumer hit this: two install files had an implicit dependency (one provisioning roles the other'sCREATE EXTENSION ... CASCADErequires), named such that alphabetical order ran them backwards. It "worked" for a long time only because an unrelated step elsewhere in the same test run happened to provision the same roles as a side effect.Wraps the wildcard in
$(sort ...). GNU Make'ssortis a plain byte-value comparison, independent of locale (LC_COLLATEetc. have no effect on it), so ordering is now a guarantee pgxntool itself enforces rather than an accident of the platform. Documented inREADME.ascandbase.mk, along with the numeric-prefix convention for consumers with an inter-file dependency between install scripts.Fixes #111.
Related pgxntool-test PR: Postgres-Extensions/pgxntool-test#84