Thanks Steve. Sorry this fell off my queue. I'll take a look at the revised patch.

	Tom    (


On 8/1/18, 4:08 PM, "Steve Singer" <steve at ssinger.info> wrote:

    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    (
    >>        >    >
    >>        >    >
    >>        >    >
    >>        >    >
    >>        >    >
    >>        >    >
    >>        >    >
    >>        >
    >>        >
    >>        >
    >> 
    >> 
    >> 
    >> 
    >> 
    >