On Wed, 6 Jun 2018, Steve Singer wrote:

> On Fri, 25 May 2018, Tignor, Tom wrote:
>
> Maybe this switch shouldn't be based on a compile-time conditional?
>
> Should this switch really be a command line argument instead of an 
> environment variable such as
>
> slonik --pg-share-dir=/usr/local/pg95/share
>
> And if the argument is set we use that path ?

Hearing no objections or better ideas I've done what I've described above.
(However the option is -s not a long form).

See 
https://github.com/ssinger/slony1-engine/commit/60266f0dec3f0e88c2741ec37320bdd66b74314d

My plan is to merge this branch into REL_2_2_STABLE and master.

I hope to release 2.2.7 early next week, or the following week with this and 
the other patches included.




>
>
>> 
>>
>> 	Tom    (
>> 
>> 
>> On 5/24/18, 7:27 AM, "Tignor, Tom" <ttignor at akamai.com> wrote:
>> 
>>
>>    	Great! Thank you. I can write up something for the options. I'll find 
>> config options in the existing doc and come up with something similar. Let 
>> me know if there's anything else I should do.
>>    	What version do you expect these changes to first appear in?
>>
>>    	Tom    (
>> 
>>
>>    On 5/23/18, 10:15 PM, "Steve Singer" <steve at ssinger.info> wrote:
>>
>>        On Thu, 26 Apr 2018, Tignor, Tom wrote:
>>
>>        >
>>        > 	Hello Steve and Slony-I,
>>        > 	After much discussion, Akamai has decided to assign copyright 
>> for the changes discussed here to me. I'm now contributing the changes as 
>> myself. Please find the work for the previous changes consolidated and 
>> attached as one "options" patch. The other issues discussed were also 
>> addressed. Please take another look when time allows. What would be the 
>> next steps here?
>>        > 	Thanks,
>>        >
>>        > 	Tom    (
>>        >
>>
>>        I've looked over the patch.
>>        It looks fine (I've changed some indentation)
>>
>>        I am okay merging this 
>> https://github.com/ssinger/slony1-engine/tree/akamai
>>        into master.
>>
>>        However we do need to also update the admin guide to describe the 
>> new
>>        options.
>> 
>> 
>>
>>        >
>>        > On 2/3/18, 4:15 PM, "Steve Singer" <steve at ssinger.info> wrote:
>>        >
>>        >    On Tue, 23 Jan 2018, Tignor, Tom wrote:
>>        >
>>        >    >    compatibility_v2.2.patch
>>        >    >
>>        >    >    My understanding of this patch is so that you slon for any 
>> 2.2.x version to
>>        >    >    be able to connect and replicate against a backend 
>> database with any other
>>        >    >    2.2.y version (where x and y can be, but are not required 
>> to be equal).
>>        >    >
>>        >    > ttignor – Yes, that’s the intention and I believe the exact 
>> effect.
>>        >    > Specifically, we’re upgrading our slony1-dependent services 
>> from
>>        >    > slony1-2.2.4 to slony1-2.2.6. The upgrade process we’ve 
>> verified requires
>>        >    > a mixed mode environment for a limited period of time (a few 
>> days). I
>>        >    > chose “2.2.x” arbitrarily. We could achieve what we need with 
>> more
>>        >    > specific criteria, or a config option, or a combination of 
>> the two.
>>        >    > --------
>>        >
>>        >    How do you feel about instead having an option to disable the 
>> slony version
>>        >    check.
>>        >
>>        >    My concern with saying all 2.2.x versions are compatible or 
>> even that
>>        >    certain minor/patch versions are compatible is the cross 
>> version testing
>>        >    load it puts on slony maintainers and how it might restrict 
>> things in the
>>        >    future.
>>        >
>>        >    If instead we just provide a option to disable the version 
>> check users can
>>        >    use this option as they see fit.
>>        >
>>        >
>>        >
>>        >    >
>>        >    > A few things on the specific patch
>>        >    >
>>        >    >    +++ slony1-2.2.6/src/slon/dbutils.c     2017-12-22 
>> 08:08:33.027322631 -0500
>>        >    >    @@ -5,6 +5,7 @@
>>        >    >       *
>>        >    >       *     Copyright (c) 2003-2009, PostgreSQL Global 
>> Development Group
>>        >    >       *     Author: Jan Wieck, Afilias USA INC.
>>        >    >    + *      Copyright (C) 2017 - Akamai Technologies, Inc
>>        >    >       *
>>        >    >       *
>>        >    >       * 
>> ----------------------------------------------------------------------
>>        >    >    @@ -418,7 +419,8 @@
>>        >    >
>>        >    >    We don't normally list contributors in the copyright 
>> section. The copyright
>>        >    >    to PostgreSQL global development group is intended to 
>> cover all
>>        >    >    contributors.
>>        >    >
>>        >    > ttignor – I’ve gone a few rounds on this with our opensource 
>> group. As I understand the issues, there is a choice to either maintain or 
>> assign away the Akamai copyright for the code I wrote.  Per our previous 
>> discussion, we’re maintaining the Akamai copyright. This is accomplished by 
>> header comments and/or some meta-data in the patch. If the header comment 
>> is a problem, then the patch meta-data becomes essential. If slony1-hackers 
>> can agree on exactly what copyright changes are needed, I can take them 
>> back to Akamai opensource.
>>        >    > --------
>>        >    >
>>        >    > So for the pg_home patch.
>>        >    > …
>>        >    >  #else
>>        >    > +       char *pgHome = getenv("PG_HOME");
>>        >    > +       if (pgHome) {
>>        >    > +         strncpy(share_path, pgHome, MAXPGPATH-1);
>>        >    > +         share_path[MAXPGPATH-1] = '\0';
>>        >    > +         strncat(share_path, "/share", 
>> MAXPGPATH-1-strlen(pgHome));
>>        >    > +       } else {
>>        >    >         strcpy(share_path, PGSHARE);
>>        >    > +       }
>>        >    >
>>        >    >
>>        >    > The above code only gets compiled in if PGPORT is not 
>> defined/present at
>>        >    > build time.  Is that your intention,  or do you want PG_HOME 
>> to take
>>        >    > precendence even if pgport is present?
>>        >    >
>>        >    > snprintf(share_path,"%s/share",pgHome,MAXPGPATH-1);
>>        >    >
>>        >    > Would the above code be clearer? (I haven't tested/tried to 
>> compile above)
>>        >    > but trying to do this in one line?
>>        >    >
>>        >    > ttignor – Reviewing the changes, the HAVE_PGPORT clause is 
>> marked with a comment “We need to find a share directory like PostgreSQL.”. 
>> That seemed like something I shouldn’t override with a customization. Re: 
>> snprintf, that seems like a good improvement. I see snprintf can also 
>> truncate output, so I’ll need to account for that as well with a rewrite.
>>        >    > --------
>>        >    >
>>        >    > 	More thoughts from Steve or others? Keep them coming.
>>        >    >
>>        >    > 	Tom    (
>>        >    >
>>        >    >
>>        >    >
>>        >    >
>>        >    >
>>        >    >
>>        >    >
>>        >
>>        >
>>        >
>> 
>> 
>> 
>> 
>> 
>