Hi Oskari, thanks for the report.

> rpmbuild --rebuild postgresql-9.4.1-1.fc22.src.rpm fails due to missing
> BuildRequires for docs.  openjade and docbook-style-dsssl are needed to
> build documentation and to build the rpm.

Can you please give us the build logs?  Because I'm afraid this build fail
shouldn't happen.  I was able to build 9.4.1 in koji:

    http://koji.fedoraproject.org/koji/buildinfo?buildID=609311

We always pre-generate pdf documentation;  thats why it should be a part of
srpm (see generate-pdf.sh).  The pdf documentation is not worth the
BuildRequires bloat.

(In reply to Oskari Saarenmaa from comment #0).

> It would be nice if the .spec also used '--with-extra-version=-%{release}'
> flag for configure (new in 9.4), it'd add the RPM release to the displayed
> version string.

Hmm.  That sounds like we could break `pg_config --version` parsers (taking
into account %dist tag goes into %release.. and also in general %release tag
is hard to parse due to its unpredictable contents).

I'm not sure whether we should have it in our distribution default postgresql
package, opinions?

Pavel

(In reply to Pavel Raiskup from comment #1)
> Hmm.  That sounds like we could break `pg_config --version` parsers (taking
> into account %dist tag goes into %release.. and also in general %release tag
> is hard to parse due to its unpredictable contents).

Sorry, it is not "so" hard to parse if it really contains the leading dash.
But with RPM you always can do something like:

    rpm -qf "$(which pg_config)" --qf "%{VERSION}-%{RELEASE}\n"

Pavel

(In reply to Pavel Raiskup from comment #2)
> Sorry, it is not "so" hard to parse if it really contains the leading dash.
> But with RPM you always can do something like:
> 
>     rpm -qf "$(which pg_config)" --qf "%{VERSION}-%{RELEASE}\n"


Sure, but I'd like to see it when I connect to a remote server with psql.

(In reply to Pavel Raiskup from comment #1)
> > rpmbuild --rebuild postgresql-9.4.1-1.fc22.src.rpm fails due to missing
> > BuildRequires for docs.  openjade and docbook-style-dsssl are needed to
> > build documentation and to build the rpm.
> 
> Can you please give us the build logs?  Because I'm afraid this build fail
> shouldn't happen.  I was able to build 9.4.1 in koji:
> 
>     http://koji.fedoraproject.org/koji/buildinfo?buildID=609311
> 
> We always pre-generate pdf documentation;  thats why it should be a part of
> srpm (see generate-pdf.sh).  The pdf documentation is not worth the
> BuildRequires bloat.

Sorry - I built pg rpms a couple of times and didn't realize the doc build tools were only needed in the build where I included a patch that touched documentation.  Looks like docbook-style-dsssl isn't needed in unpatched builds.

(In reply to Oskari Saarenmaa from comment #3)
> Sure, but I'd like to see it when I connect to a remote server with psql.

Then your exact use-case would be 'select version();'?

That output could be probably enhanced upstream, somehow.  But I don't
think it's worth changing PG_VERSION globally.

TBH, I don't like --with-extra-version.  That option brings possibility
to mess the PostgreSQL versioning in the wild.  As a consumer of that variable
(on some places), I think for particular patch-version there should have been
separate API variable.  Have you tried to discuss something like that
upstream?

FWIW, this is exactly the use-case that upstream had in mind when inventing --with-extra-version.

You do need to insert some extra punctuation in the string to ensure that it's not seen as more digits in the minor release number, but Oskari's proposal includes a dash, which seems fine to me.

I don't have a lot of sympathy for the argument that it might break code looking at the output of version().  In the first place, there's already a lot of cruft there, because it's not especially meant as a program-readable value:

# select version();
                                                    version                             
---------------------------------------------------------------------------------------------------------------
 PostgreSQL 9.4.1 on x86_64-unknown-linux-gnu, compiled by gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-11), 64-bit
(1 row)

There are other APIs such as libpq's PQserverVersion() that are more meant for program consumption, and --with-extra-version doesn't affect those.

In the second place, other packagers certainly will be making use of this option, so any code that can't cope with extra stuff there is going to need work anyhow.

It's probably reasonable not to back-patch such a change into the stable releases, but I'm +1 for doing what Oskari suggests in rawhide.

(In reply to Tom Lane from comment #6)
> FWIW, this is exactly the use-case that upstream had in mind when inventing
> --with-extra-version.

Just to pair this bug with relevant mail link:
http://www.postgresql.org/message-id/20131105102226.GA26836@saarenmaa.fi

> You do need to insert some extra punctuation in the
> string to ensure that it's not seen as more digits in the minor release
> number, but Oskari's proposal includes a dash, which seems fine to me.

Ack.  As it was proposed, it still quite OK.  But there is not guard against
misuse in general.

> I don't have a lot of sympathy for the argument that it might break code
> looking at the output of version().  In the first place, there's already a
> lot of cruft there, because it's not especially meant as a program-readable
> value:
>
> # select version();
[....]
> There are other APIs such as libpq's PQserverVersion() that are more meant
> for program consumption, and --with-extra-version doesn't affect those.

I was not against changing the version() output, previously.  What I was
complaining about was the --version output of any binary distributed.  But as
long as those have to match, we can't do much.  Previously I was thinking
about separate ', build-info' string in version().

> In the second place, other packagers certainly will be making use of this
> option, so any code that can't cope with extra stuff there is going to need
> work anyhow.
>
> It's probably reasonable not to back-patch such a change into the stable
> releases, but I'm +1 for doing what Oskari suggests in rawhide.

I'm -0.1 for that change, at least as long as we are zero-motivated to do the
change.

Small (really small == -0.1) motivation to stay with the current PG_VERSION is
to not pioneer breakage in Fedora without benefits.  Also, I don't think that
seeing Fedora's RPM patch-level in version() would be useful, at least not
with honorable intentions.


About the request, Oskari.  As I understand it, you build RPMs from spec files
forked from Fedora's spec file(s), yes?  This RFE is more about to allow you
to change the PG_VERSION in your own builds, yes?  I could certainly add an
option to help you to do that.

(In reply to Pavel Raiskup from comment #7)
> About the request, Oskari.  As I understand it, you build RPMs from spec
> files forked from Fedora's spec file(s), yes?  This RFE is more about to
> allow you to change the PG_VERSION in your own builds, yes?  I could
> certainly add an option to help you to do that.

Oskari, do we actually need to change default Fedora's *binary* packages or is
it enough to give you way how to adjust PG_VERSION by some build option?