Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

custom pg_config does not work #13

Closed
markokr opened this Issue · 17 comments

3 participants

@markokr

I have postgres installed under my user and it's pg_config in PATH. Problems:

1) pgxn does sudo although its not needed
2) after sudo it loses original pg_config it complies under and uses pg_config in root's PATH

potential fixes:

1) check if installed file user is current user? then don't sudo.
It seems better fix would be to stop sudoing by default (what other install tool does that?) and give maybe extra switch for that.
Or expect that user calls pgxn with sudo. It seems this is usual way and gives user full control for sudoing.

2) if using pg_config from PATH, remember full path for it and use it afterwards.

@theory

I think #2, since some folks are having the opposite problem: They build something with PGXN using a pg_config in the path, and then PGXN sudo installs and it is no longer in the path.

@markokr

Thats the same problem but for me it was nastier - there was different pg_config in path for root, from different postgres version, to the module was installed under incompatible postgres, without any errors.

automatic sudo is evil...

@dvarrazzo
Owner
@dvarrazzo
Owner

automatic sudo is evil...

sorry about the trouble: fixing the app so that only the pg_config found in the user's PATH is used should fix this problem.

@dvarrazzo dvarrazzo was assigned
@markokr

you still have problems if any other piece that depends on user environment. and the pgxn will do wrong thing if the postgres install is not owned by root.

i think sudo should be dropped. instead you can encourage users to do

$ pgxn build
$ sudo pgxn install

to avoid doing really unrelated things under root.

another angle - installing stuff for root-owned things should be special. using pgxn / pip / cpan stuff for user private owned installation should be default mode of operation. currently pgxn has things in reverse

@dvarrazzo
Owner
@markokr

Well, if "sudo pgxs foo" is really unsatisfactory, then you can require --sudo[=user] switch. But you must not do it by default. sudo+env switch is really complex and dangerous operation. Only build system that is allowed to do it BSD ports-like things that manage full system. Product-specific build system have no knowledge to do it safely by default.

Especially annoying is that it does not even have knowledge to know when it does not need to do it - instead it can corrupt private postgres installaltion that has no need for sudo.

@theory

I just cannot remember now why I've added the sudo/nosudo options now:
there was a reason for which I found "sudo pgxn install"
unsatisfactory. If looking at the code will bring it to mind I'll let
you know.

I like the single command for installing. The way CPAN.pm deals with this is that it has a configuration file, and one can edit the make_install setting. Its default is make install, of course, but one can set it to sudo make install. OTOH, that will still result in potential environment problems. :-(

@markokr

Well, no - if the user clearly intends to install under root user then root's env is probably fine. In any case it up to user, and pgxn has no need to worry.

The case that is completely broken now is when user has customized env and no need for sudo - to make such case work with sudo-by-default, you need to bypass sudo protections and inject users customizations into root's env...

Do I need to go on? ;)

@dvarrazzo
Owner

I'd like to fix the current design in a version 1.0.4 and think about dropping the sudo thing in a version 1.1, as it would be a change to the program behaviour.

Thinking about how it should work, I think install/uninstall (the only commands sudoing) should check if some system dir, such as --libdir, is writable, and fail early in this case. This is how some python installer work (easy_install, but not pip). If root is required, the user should invoke "sudo pgxn".

I think I would leave the --sudo option anyway, but without a default: if the commands executed are "configure, make, make install", I may still want only the last one to be run under sudo. This seems a secondary concern in most of the cases though: mostly useful when calling the client on an existing dir, not with a network spec or a zipped file.

@theory

I think it makes sense to drop the default use of sudo, but leave the --sudo option. You unpack and install everything in the temp directory, right? Why not just do everything using sudo in that case (when --sudo, of course).

Might also be useful to have a configuration file, so one can tell it to always use sudo by default. Wish list item, I guess, not super important.

@dvarrazzo dvarrazzo referenced this issue from a commit
@dvarrazzo Make sure the same pg_config is used both by the current user and by …
…sudo

Partial solution of the problem discussed in the ticket #13.
2be8d7f
@dvarrazzo dvarrazzo referenced this issue from a commit
@dvarrazzo Don't use a default sudo program
--sudo's argument is now optional, defaulting to 'sudo'. If not specified,
and the target directory is not writable, fail.

Fix issue #13

This is a behavioural change from previous version, so bumping the
minor version number.
4a77483
@dvarrazzo
Owner

Hello,

I've changed the program behavior by making --sudo not defaulting to 'sudo': it now takes an optional argument and no sudo is performed if not specified. An help excerpt is:

$ pgxn install --help 
usage: pgxn install [--help] [--mirror URL] [--verbose] [--yes]
                    [--stable | --testing | --unstable] [--pg_config PATH]
                    [--sudo [PROG] | --nosudo]
                    SPEC
[...]
  --sudo [PROG]     run PROG to elevate privileges when required [default:
                    sudo]
  --nosudo          never elevate privileges (no more needed: for backward
                    compatibility)

The libdir is tested beforehand and, if found not writable, the command fails with a polite message:

$ ./bin/pgxn install semver
ERROR: PostgreSQL library directory (/usr/lib) not writable: you should run the program as superuser, or specify a 'sudo' program

I think this closes the bug. I've bumped the version number to 1.1 as the command line usage is slightly changed.

Is everything ok for you? Care about testing to see if everything's ok? Thank you.

@theory

Makes sense to me. Does --sudo sudo apply sudo to all commands, or just make install?

@dvarrazzo
Owner

Just make install.

@dvarrazzo
Owner

David, are the docs (cmd line help and html) clean enough that "--sudo" defaults to "--sudo sudo" and that not using --sudo doesn't try to elevate privilege? the docs markup change is at 4a77483#diff-1 Thank you.

@theory

I think rather than

An optional privilege elevation program :samp:{PROG} can be specified.

I would say something like:

If your system doesn't have program:sudo but some other program to elevate privileges, pass the name of that program to the -sudo option.

But yeah, it's clear enough.

@dvarrazzo
Owner

Closed with the release of PGXN Client 1.1. Thank you Marko and David.

@dvarrazzo dvarrazzo closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.