Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compilation, installation and tests without postgres bin directory in PATH #21

Closed
wants to merge 2 commits into from

Conversation

Green-Chan
Copy link

@Green-Chan Green-Chan commented Nov 16, 2023

The following instruction in README makes no sense:

Compile/Installation:

To compile pg_filedump, you will need to have a properly configured
PostgreSQL source tree or the devel packages (with include files)
of the appropriate PostgreSQL major version.

make PG_CONFIG=/path/to/postgresql/bin/pg_config
make install PG_CONFIG=/path/to/postgresql/bin/pg_config

PG_CONFIG is always set to "pg_config" in Makefile, and the value "/path/to/postgresql/bin/pg_config" is ignored:
PG_CONFIG = pg_config

I suggest (the first commit) changing this line to:
PG_CONFIG ?= pg_config

This allows building and installing pg_filedump without postgres bin directory in PATH. But bin directory in PATH is still needed to run make installcheck. The second commit allows running tests without it:

make installcheck PG_CONFIG=/path/to/postgresql/bin/pg_config

@df7cb
Copy link
Owner

df7cb commented Jun 4, 2024

Hi,

PG_CONFIG = pg_config is overridable by make PG_CONFIG=/elsewhere, but your suggestion is still an improvement since it makes the slightly different PG_CONFIG=/elsewhere make also work, so I picked that commit.

The second change is unfortunately not good - Debian installs pg_filedump into /usr/bin since it's cross-version compatible. Looking for it in /usr/lib/postgresql/NN/bin will fail. I'll adjust the Makefile to set PATH instead.

Thanks!

@df7cb df7cb closed this in 888f5f4 Jun 4, 2024
@Green-Chan
Copy link
Author

I'll adjust the Makefile to set PATH instead.

Thank you! It works perfectly.

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.

2 participants