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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions