On Mon, Apr 02, 2018 at 10:35:09AM -0400, Peter Eisentraut wrote: > I took a quick look at that part. It appears to be quite invasive, more > than I would have hoped. Basically, it imposes that from now on all > program invocations must observe the bindir setting, which someone is > surely going to forget. The pg_upgrade tests aren't going to exercise > all possible paths where programs are called, so this is going to lead > to omissions and inconsistencies -- which will then possibly only be > found much later by the extensions that Craig was talking about. I'd > like to see this more isolated, maybe via a path change, or something > inside system_or_bail or something less invasive and more maintainable.
Hm. PostgresNode.pm uses now a couple of different ways to trigger commands, which makes the problem harder than it seems: - system_or_bail - system_log - IPC::Run::timeout - IPC::Run::run Because of that, and if one wants to minimize the number of places where bindir is called, would be to modify the command strings themselves. What I could think of would be to have a wrapper like build_bin_path which adds $bindir on top of the binary, but that would still cause all the callers to use this wrapper. The use of this wrapper could still be forgotten. Enforcing PATH temporarily in a code path or another implies that the previous value of PATH needs to be added back, which could be forgotten as well. At the end, it seems to me that what I am proposing is themost readable approach, and with proper documentation things should be handled finely... Or there is an approach you have in mind I do not foresee? -- Michael
signature.asc
Description: PGP signature
