Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cORkN-0007EM-Mr for pgsql-pkg-yum@arkaria.postgresql.org; Tue, 03 Jan 2017 16:15:35 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1cORkN-0004Lh-9y for pgsql-pkg-yum@arkaria.postgresql.org; Tue, 03 Jan 2017 16:15:35 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1cORkJ-0004Hs-JW for pgsql-pkg-yum@postgresql.org; Tue, 03 Jan 2017 16:15:31 +0000 Received: from mail-qt0-x245.google.com ([2607:f8b0:400d:c0d::245]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1cORk7-0002uB-Tx for pgsql-pkg-yum@postgresql.org; Tue, 03 Jan 2017 16:15:31 +0000 Received: by mail-qt0-x245.google.com with SMTP id p16so360826477qta.5 for ; Tue, 03 Jan 2017 08:15:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dyn.com; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=6wA6Mi2h7dp4lVG9mwHKV3PlNxnUVLCI/Hq9AAbQ/gU=; b=Dx2S1+w1eCjQ1/PaKrLpwrfvgSS+zKd6gCc9oWGucAsBpsDDU646Vk1PBvySB4vJxh HAbke12kgQtXDSYVChYZ80w4sM/+v2INd3C8BAs/Mrri/XuZdjJYPRCzQcuGzJA6gf6b 3u5Az4AaUiHodH3lywPgxh5klTfuBUEwVgtRQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=6wA6Mi2h7dp4lVG9mwHKV3PlNxnUVLCI/Hq9AAbQ/gU=; b=pYOha+abAQv1A0dj6WaZlGjNoHbbNS2zmIzdsfe25jknIssh3uhvzVlGVMmbmN5Ir3 aq/f2KWqCVsvAGxqUk/5aCxhFItPdYmT2heVa/TeR+OH/y6d8ArfVvOOcp8ELJ+g4ccw YornQh+GPwJ1dm6yx3H03UTP+rEbIfzyzEylErTML0LaW+QlqessdC0bvT6rBZUbMFzz kTp4eEVpheIAZuHunXNBfsAljrFXJAmqGFs8Gro6cFwypZX6krziamAOPqqWaMXBbsrd zi5ZZ6z3VhDJsJmYUHwt2CRkf8rjlIajv3Twi1DaYk0H1CWbNZgIJBph4lh8TDzh+8IU OQtg== X-Gm-Message-State: AIkVDXKnUf28+19zjtZH61HQejwmhny78aDX85tGxUHasmfdY665MJVbkqqdD6LUiAe5PiBgippqBCDsAT9Xto34 X-Received: by 10.200.51.199 with SMTP id d7mr52580544qtb.67.1483460118031; Tue, 03 Jan 2017 08:15:18 -0800 (PST) MIME-Version: 1.0 Received: by 10.237.46.71 with HTTP; Tue, 3 Jan 2017 08:14:57 -0800 (PST) In-Reply-To: <1481308873.16175.24.camel@gunduz.org> References: <1481308873.16175.24.camel@gunduz.org> From: Jonathon Nelson Date: Tue, 3 Jan 2017 10:14:57 -0600 Message-ID: Subject: Re: a bunch of potential improvements to the postgresql spec files To: =?UTF-8?B?RGV2cmltIEfDvG5kw7x6?= Cc: pgsql-pkg-yum@postgresql.org Content-Type: multipart/alternative; boundary=001a114541de5fe9fc054532f911 X-Pg-Spam-Score: -2.7 (--) List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgsql-pkg-yum Precedence: bulk Sender: pgsql-pkg-yum-owner@postgresql.org --001a114541de5fe9fc054532f911 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On Fri, Dec 9, 2016 at 12:41 PM, Devrim G=C3=BCnd=C3=BCz wrote: > > Hi Jonathon, > > First of all, thank your for working on this. Comments inline: > > On Fri, 2016-12-02 at 14:06 -0600, Jonathon Nelson wrote: > > > > 0. globally replace /usr/sbin with %{_sbindir}, /usr/bin with %{_bindir= }, > > and /usr/share/man with %{_mandir} > > Looks good. > > > 1. Remove the redundant use of 0%{?rhel} in tests for RHEL-specific > values. > > Turns: > > %if 0%{?rhel} && 0%{rhel} > 6 > > into: > > %if 0%{rhel} > 6 > > If we remove that one, then Fedora builds should fail. > Ah! I think I meant to suggest this: %if 0%{rhel} > 6 (note the addition of the question mark). That should be logically equivalent to %if 0%{?rhel} && 0%{rhel} > 6 > ... > 8. Replace all repetitive blocks for update-alternatives with for loop > > *AND* use %{mandir} and %{bindir} (per item 0): > > > > %{_sbindir}/update-alternatives --install %{bindir}/psql pgsql-psql > > %{pgbaseinstdir}/bin/psql %{packageversion}0 > > %{_sbindir}/update-alternatives --install %{bindir}/clusterdb > > pgsql-clusterdb %{pgbaseinstdir}/bin/clusterdb %{packageversion}0 > > ... > > %{_sbindir}/update-alternatives --install %{bindir}/pg_basebackup > > pgsql-pg_basebackup %{pgbaseinstdir}/bin/pg_basebackup > %{packageversion}0 > > ... > > > > with: > > > > for i in \ > > pgbench pg_test_timing pg_upgrade pg_xlogdump pg_archivecleanup \ > > pg_config pg_isready pg_test_fsync pg_receivexlog \ > > psql clusterdb \ > > createdb createlang createuser \ > > dropdb droplang dropuser \ > > pg_basebackup pg_dump pg_dumpall pg_restore \ > > reindexdb vacuumdb; do > > %{_sbindir}/update-alternatives --install %{bindir}/${i} > > pgsql-${i} %{pgbaseinstdir}/bin/${i} %{packageversion}0 > > %{_sbindir}/update-alternatives --install %{mandir}/man1/${i}.1 > > pgsql-${i}man %{pgbaseinstdir}/share/man/man1/${i}.1 > %{packageversion}0 > > done > > This line creates alternatives for *all* binaries, but we don't do this. = We > only create alternatives entries for the binaries that can work across > major > releases -- but I liked the approach. Will apply with a few changes. > Thanks! I found that not having ready access to the latest versions of many of those binaries (without having to specify the full path) was a pain. Plus, isn't that why alternatives exists? I will try to push these changes in next minor release set. > Thanks! --001a114541de5fe9fc054532f911 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


On Fri, Dec 9, 2016 at 12:41 PM, Devrim G=C3=BCnd=C3=BCz <devrim@gundu= z.org> wrote:

Hi Jonathon,

First of all, thank your for working on this. Comments inline:

On Fri, 2016-12-02 at 14:06 -0600, Jonathon Nelson wrote:
>
> 0. globally replace /usr/sbin with %{_sbindir}, /usr/bin with %{_bindi= r},
> and /usr/share/man with %{_mandir}

Looks good.

> 1. Remove the redundant use of 0%{?rhel} in tests for RHEL-specific va= lues.
> Turns:
> %if 0%{?rhel} && 0%{rhel} > 6
> into:
> %if 0%{rhel} > 6

If we remove that one, then Fedora builds should fail.


Ah!=C2=A0 I think I meant to suggest this:
%if 0%{rhel} > 6
(note the addition of the question mark).=C2=A0 That shou= ld be logically equivalent to
%if 0%{?rhe= l} && 0%{rhel} > 6
...

> 8. Replace all repetitive blocks for update-alternatives with for loop=
> *AND* use %{mandir} and %{bindir} (per item 0):
>
> %{_sbindir}/update-alternatives --install %{bindir}/psql=C2=A0=C2= =A0=C2=A0pgsql-psql
> %{pgbaseinstdir}/bin/psql %{packageversion}0
> %{_sbindir}/update-alternatives --install %{bindir}/clusterdb
> pgsql-clusterdb=C2=A0=C2=A0%{pgbaseinstdir}/bin/clusterdb %{packa= geversion}0
> ...
> %{_sbindir}/update-alternatives --install %{bindir}/pg_basebackup=
> pgsql-pg_basebackup=C2=A0=C2=A0=C2=A0=C2=A0%{pgbaseinstdir}/bin/p= g_basebackup %{packageversion}0
> ...
>
> with:
>
> for i in \
> =C2=A0=C2=A0=C2=A0=C2=A0pgbench pg_test_timing pg_upgrade pg_xlogdump = pg_archivecleanup \
> =C2=A0=C2=A0=C2=A0=C2=A0pg_config pg_isready pg_test_fsync pg_receivex= log \
> =C2=A0=C2=A0=C2=A0=C2=A0psql clusterdb \
> =C2=A0=C2=A0=C2=A0=C2=A0createdb createlang createuser \
> =C2=A0=C2=A0=C2=A0=C2=A0dropdb droplang dropuser \
> =C2=A0=C2=A0=C2=A0=C2=A0pg_basebackup pg_dump pg_dumpall pg_restore \<= br> > =C2=A0=C2=A0=C2=A0=C2=A0reindexdb vacuumdb; do
> =C2=A0=C2=A0=C2=A0=C2=A0%{_sbindir}/update-alternatives --install= %{bindir}/${i}
> pgsql-${i} %{pgbaseinstdir}/bin/${i} %{packageversion}0
> =C2=A0=C2=A0=C2=A0=C2=A0%{_sbindir}/update-alternatives --install= %{mandir}/man1/${i}.1
> pgsql-${i}man=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0%{pgbaseinstdir}/share= /man/man1/${i}.1 %{packageversion}0
> done

This line creates alternatives for *all* binaries, but we don't = do this. We
only create alternatives entries for the binaries that can work across majo= r
releases -- but I liked the approach. Will apply with a few changes.

Thanks!=C2=A0 I found that not having ready a= ccess to the latest versions of many of those binaries (without having to s= pecify the full path) was a pain. Plus, isn't that why alternatives exi= sts?

I will try to push these changes in next minor release set.

Thanks!

--001a114541de5fe9fc054532f911--