Skip to content

pgtle.sh: Makefile's PGTLE_VERSION variable silently collides with a consumer's same-named env var #78

Description

@jnasbyupgrade

`base.mk`'s `pgtle:` target reads a make variable called `PGTLE_VERSION` to optionally restrict SQL generation to one version range:

```make
pgtle: all control.mk $(PGXNTOOL_CONTROL_FILES)
@$(foreach ext,$(EXTENSIONS),
$(PGXNTOOL_DIR)/pgtle.sh --extension $(ext) $(if $(PGTLE_VERSION),--pgtle-version $(PGTLE_VERSION));)
```

Make automatically imports environment variables as make variables unless a Makefile explicitly overrides them. `PGTLE_VERSION` is also an extremely natural name for a CI job to use for "which pg_tle release to build from source and test against" — exactly the name `Postgres-Extensions/cat_tools` PR #47's `pg-tle-test` job used (`env: PGTLE_VERSION: "1.5.2"`), and what `Postgres-Extensions/pg_count_nulls` PR #16 copied from it.

The collision, confirmed reproduced

With that env var set, `make pgtle`/`make run-pgtle` silently does the wrong thing instead of erroring: it passes `--pgtle-version 1.5.2` (the literal detected/pinned version) straight through to `generate_pgtle_sql`, which uses it verbatim as the output directory name (`pg_tle/1.5.2/`) instead of resolving it to one of the three real range directories (`1.0.0-1.4.0`, `1.4.0-1.5.0`, `1.5.0+`). Then `run_pgtle_sql` (used by `--run`/`make run-pgtle`) independently re-derives the range directory from the live pg_tle version (correctly getting `1.5.0+` for 1.5.2) and fails because that directory was never generated:

```
Generating: pg_tle/1.5.2/count_nulls.sql (pg_tle 1.5.2)
✓ Generated: pg_tle/1.5.2/count_nulls.sql
Running pg_tle registration SQL files...
Using pg_tle files for version 1.5.2 (directory: pg_tle/1.5.0+)
ERROR: pg_tle directory pg_tle/1.5.0+ does not exist
Run 'make pgtle' first to generate files


Reproduced with a single command, no database or CI involved:
\`\`\`
PGTLE_VERSION=1.5.2 make pgtle    # generates pg_tle/1.5.2/, NOT pg_tle/1.5.0+/
make pgtle                       # (no env var) generates all three range dirs correctly
\`\`\`

## Why this is worth fixing here, not just in each consumer

count_nulls worked around it locally by renaming its own CI job's env var (\`PGTLE_VERSION\` → \`PG_TLE_BUILD_VERSION\`), which avoids the collision entirely — that's a reasonable immediate fix for that one repo. But \`PGTLE_VERSION\` is a name any pg_tle-testing consumer is likely to reach for independently (cat_tools' PR #47 already did, unprompted), and pgxntool's own variable of the same name changes behavior *silently* rather than erroring — no warning that an environment variable is being picked up and reinterpreted as a build-generation flag. Worth considering a more collision-resistant internal name (e.g. \`PGXNTOOL_PGTLE_VERSION\`), and/or a loud diagnostic when \`--pgtle-version\` is used with a value that doesn't correspond to a real generated range directory, instead of a downstream \`--run\` failure with the true cause several steps upstream and non-obvious.

## Where this was found

\`Postgres-Extensions/pg_count_nulls\` PR #16, immediately after fixing #57 (the same job's \`stable\`-version parsing issue) and rebasing onto pgxntool 2.2.0 — this is a distinct bug from #57, only visible once that one was fixed and the job actually ran against a live pg_tle.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions