PostgreSQL 9.3 beta breaks some extensions "make install"
Hi list,
While testing out PostgreSQL 9.3beta1, I stumbled upon a problem
installing some extensions (pgTAP and semver among others):
% make
[...]
% make DESTDIR=/tmp/foo install
[...]
/usr/bin/install -c -m 644 ./sql/semver--unpackaged--0.2.1.sql
./sql/semver--0.2.4--0.3.0.sql ./sql/semver--0.2.1--0.2.4.sql
./sql/semver--0.3.0.sql ./sql/semver--0.3.0.sql
'/tmp/foo/usr/share/postgresql/extension/'
/usr/bin/install: will not overwrite just-created
‘/tmp/foo/usr/share/postgresql/extension/semver--0.3.0.sql’ with
‘./sql/semver--0.3.0.sql’
make: *** [install] Error 1
I traced the problem down to commit
9db7ccae2000524b72a4052352cbb5407fb53b02 "Use system install program
when available and usable". It turns out that 'install' from coreutils
8.21 complains when it's told to install the same source file twice.
It's caused by this common pattern in extension makefiles:
DATA = $(wildcard sql/*--*.sql) sql/$(EXTENSION)--$(EXTVERSION).sql
The wildcard will match semver--0.3.0.sql too, in addition to the
direct reference, causing it to be duplicated (unless "make install"
is called from a clean directory).
The attached 1st patch fixes this case by putting $(sort ...) around
the DATA install command, which drops duplicate filenames. While there
are other install commands, the chances of them containing duplicate
names seem lower.
I'm not sure if this is worth worrying about, but there's still a
problem when DATA and DATA_built are separated, e.g:
DATA = $(wildcard sql/*--*.sql)
DATA_built = sql/$(EXTENSION)--$(EXTVERSION).sql
This can't be solved with a sort since DATA_built doesn't use
$(addprefix). But the install could be split into 2 separate commands
(2nd patch)
Regards,
Marti
Marti Raudsepp <marti@juffo.org> writes:
While testing out PostgreSQL 9.3beta1, I stumbled upon a problem
installing some extensions (pgTAP and semver among others):
...
I traced the problem down to commit
9db7ccae2000524b72a4052352cbb5407fb53b02 "Use system install program
when available and usable". It turns out that 'install' from coreutils
8.21 complains when it's told to install the same source file twice.
TBH, I thought that was a dangerous idea from the get-go. My vote for
fixing this would be to revert that change, not try to hack all the
makefiles to work around it.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, 2013-05-14 at 04:12 +0300, Marti Raudsepp wrote:
It's caused by this common pattern in extension makefiles:
DATA = $(wildcard sql/*--*.sql) sql/$(EXTENSION)--$(EXTVERSION).sql
What is the point of this? Why have the wildcard and then the
non-wildcard term?
I think using wildcard is bad style and you should fix it.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 05/13/2013 10:27 PM, Peter Eisentraut wrote:
On Tue, 2013-05-14 at 04:12 +0300, Marti Raudsepp wrote:
It's caused by this common pattern in extension makefiles:
DATA = $(wildcard sql/*--*.sql) sql/$(EXTENSION)--$(EXTVERSION).sqlWhat is the point of this? Why have the wildcard and then the
non-wildcard term?I think using wildcard is bad style and you should fix it.
I've been caught by it. I'm not sure why the wildcard is a bad idea -
don't we want any present update sql files to be installed? . I don't
see any reason to have the redundant addition of the current version's
sql file, though.
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, May 14, 2013 at 5:27 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
On Tue, 2013-05-14 at 04:12 +0300, Marti Raudsepp wrote:
It's caused by this common pattern in extension makefiles:
DATA = $(wildcard sql/*--*.sql) sql/$(EXTENSION)--$(EXTVERSION).sql
What is the point of this? Why have the wildcard and then the
non-wildcard term?
Because the non-wildcard file is built by the same Makefile (it's
copied from the sql/$(EXTENSION).sql file). If it wasn't there, a
"make install" from a clean checkout would miss this file.
all: sql/$(EXTENSION)--$(EXTVERSION).sql
sql/$(EXTENSION)--$(EXTVERSION).sql: sql/$(EXTENSION).sql
cp $< $@
I think using wildcard is bad style and you should fix it.
Perhaps, but fixing the extensions is not a solution at this point. A
large number of extensions use this exact code (it comes from David
Wheeler's template AFAIK). We might stand a chance in fixing the
public extensions on PGXN, but this would presumably still break
non-public extensions that people have written.
For example:
David Wheeler's pgTAP https://github.com/theory/pgTAP/blob/master/Makefile#L91
Jan Urbański's first_last_agg
https://github.com/wulczer/first_last_agg/blob/master/Makefile
Andrew Dunstan's json_build
https://github.com/pgexperts/json_build/blob/master/Makefile
Theo Schlossnagle's pg_amqp
http://api.pgxn.org/src/pg_amqp/pg_amqp-0.3.0/Makefile
OmniTI's pg_jobmon https://github.com/omniti-labs/pg_jobmon/blob/master/Makefile
Regards,
Marti
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Le mardi 14 mai 2013 10:17:13, Marti Raudsepp a écrit :
On Tue, May 14, 2013 at 5:27 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
On Tue, 2013-05-14 at 04:12 +0300, Marti Raudsepp wrote:
It's caused by this common pattern in extension makefiles:
DATA = $(wildcard sql/*--*.sql) sql/$(EXTENSION)--$(EXTVERSION).sqlWhat is the point of this? Why have the wildcard and then the
non-wildcard term?Because the non-wildcard file is built by the same Makefile (it's
copied from the sql/$(EXTENSION).sql file). If it wasn't there, a
"make install" from a clean checkout would miss this file.all: sql/$(EXTENSION)--$(EXTVERSION).sql
sql/$(EXTENSION)--$(EXTVERSION).sql: sql/$(EXTENSION).sql
cp $< $@I think using wildcard is bad style and you should fix it.
Perhaps, but fixing the extensions is not a solution at this point. A
large number of extensions use this exact code (it comes from David
Wheeler's template AFAIK). We might stand a chance in fixing the
public extensions on PGXN, but this would presumably still break
non-public extensions that people have written.
My understanding is that the offending commit reveals possible bad written
instruction in some Makefile. What's wrong ?
--
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation
Marti Raudsepp <marti@juffo.org> writes:
all: sql/$(EXTENSION)--$(EXTVERSION).sql
sql/$(EXTENSION)--$(EXTVERSION).sql: sql/$(EXTENSION).sql
cp $< $@
That's a recipe for problems. Each time I meet with such a construct in
an extension's Makefile I propose an hard coded alternative. We're
speaking of maintaining less than half a dozen file names here, with one
or two additions per year, in most cases.
Really, that trick is a recipe for problems. Use at your own risk.
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
PS: yes I've been saying that all along, ever since David first cooked
that, and I've blogged about it, and I've worked on alternatives and
tools, in particular pg_buildext, part of the postgresql-common
debian package.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 5/13/13 10:58 PM, Andrew Dunstan wrote:
I'm not sure why the wildcard is a bad idea - don't we want any present
update sql files to be installed?
Generally, wildcards in makefiles are a bad idea because you will then
not discover any broken tarballs and checkouts. The users will just
install nothing or a subset of the files you had intended.
It might also break vpath builds. That can probably be worked around,
though.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 5/14/13 4:17 AM, Marti Raudsepp wrote:
On Tue, May 14, 2013 at 5:27 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
On Tue, 2013-05-14 at 04:12 +0300, Marti Raudsepp wrote:
It's caused by this common pattern in extension makefiles:
DATA = $(wildcard sql/*--*.sql) sql/$(EXTENSION)--$(EXTVERSION).sqlWhat is the point of this? Why have the wildcard and then the
non-wildcard term?Because the non-wildcard file is built by the same Makefile (it's
copied from the sql/$(EXTENSION).sql file). If it wasn't there, a
"make install" from a clean checkout would miss this file.
If it's built, then it should be listed in DATA_built.
Perhaps, but fixing the extensions is not a solution at this point. A
large number of extensions use this exact code (it comes from David
Wheeler's template AFAIK).
So far, the number is still less than the number of extensions broken by
the htup header refactoring, so I'm not worried about it.
Also, substituting a custom install program has always been supported,
so this was already broken anyway, now we just know about it earlier.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 5/14/13 7:50 AM, Dimitri Fontaine wrote:
Marti Raudsepp <marti@juffo.org> writes:
all: sql/$(EXTENSION)--$(EXTVERSION).sql
sql/$(EXTENSION)--$(EXTVERSION).sql: sql/$(EXTENSION).sql
cp $< $@That's a recipe for problems. Each time I meet with such a construct in
an extension's Makefile I propose an hard coded alternative. We're
speaking of maintaining less than half a dozen file names here, with one
or two additions per year, in most cases.
I don't think this in particular is a problem. I use something like
that myself. But you should list all the older extension upgrade files
explicitly (to which your point applies).
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 05/14/2013 07:59 AM, Peter Eisentraut wrote:
On 5/14/13 4:17 AM, Marti Raudsepp wrote:
On Tue, May 14, 2013 at 5:27 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
On Tue, 2013-05-14 at 04:12 +0300, Marti Raudsepp wrote:
It's caused by this common pattern in extension makefiles:
DATA = $(wildcard sql/*--*.sql) sql/$(EXTENSION)--$(EXTVERSION).sqlWhat is the point of this? Why have the wildcard and then the
non-wildcard term?Because the non-wildcard file is built by the same Makefile (it's
copied from the sql/$(EXTENSION).sql file). If it wasn't there, a
"make install" from a clean checkout would miss this file.If it's built, then it should be listed in DATA_built.
So, AIUI, leaving aside stylistic arguments about use of wildcards, one
solution could be:
DATA_built = sql/$(EXTENSION)--$(EXTVERSION).sql
DATA = $(filter-out sql/$(EXTENSION)--$(EXTVERSION).sql, $(wildcard
sql/*--*.sql))
Is that right?
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Le mardi 14 mai 2013 10:17:13, Marti Raudsepp a écrit :
On Tue, May 14, 2013 at 5:27 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
On Tue, 2013-05-14 at 04:12 +0300, Marti Raudsepp wrote:
It's caused by this common pattern in extension makefiles:
DATA = $(wildcard sql/*--*.sql) sql/$(EXTENSION)--$(EXTVERSION).sqlWhat is the point of this? Why have the wildcard and then the
non-wildcard term?Because the non-wildcard file is built by the same Makefile (it's
copied from the sql/$(EXTENSION).sql file). If it wasn't there, a
"make install" from a clean checkout would miss this file.all: sql/$(EXTENSION)--$(EXTVERSION).sql
sql/$(EXTENSION)--$(EXTVERSION).sql: sql/$(EXTENSION).sql
cp $< $@
There is also something else here, the addition of the all:.... target.
I believe it should be removed and the added target(s) be written after the
include of pgxs makefile.
If I follow your example, then I would rewrite http://manager.pgxn.org/howto
From
=====
PG91 = $(shell $(PG_CONFIG) --version | grep -qE " 8\.| 9\.0" && echo no ||
echo yes)
ifeq ($(PG91),yes)
all: sql/$(EXTENSION)--$(EXTVERSION).sql
sql/$(EXTENSION)--$(EXTVERSION).sql: sql/$(EXTENSION).sql
cp $< $@
DATA = $(wildcard sql/*--*.sql) sql/$(EXTENSION)--$(EXTVERSION).sql
EXTRA_CLEAN = sql/$(EXTENSION)--$(EXTVERSION).sql
endif
PGXS := $(shell $(PG_CONFIG) --pgxs)
include $(PGXS)
====
To
====
PG91 = $(shell $(PG_CONFIG) --version | grep -qE " 8\.| 9\.0" && echo no ||
echo yes)
ifeq ($(PG91),yes)
DATA = $(wildcard sql/*--*.sql) sql/$(EXTENSION)--$(EXTVERSION).sql
EXTRA_CLEAN = sql/$(EXTENSION)--$(EXTVERSION).sql
endif
PGXS := $(shell $(PG_CONFIG) --pgxs)
include $(PGXS)
ifeq ($(PG91),yes)
sql/$(EXTENSION)--$(EXTVERSION).sql: sql/$(EXTENSION).sql
cp $< $@
endif
====
because the default target from the PostgreSQL Makefile is «all:» and the
addition of a target before inclusion of PostgreSQL makefile change the default
(see DEFAULT_GOAL variable)
Thought ?
--
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation
On Tue, May 14, 2013 at 2:59 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
Perhaps, but fixing the extensions is not a solution at this point. A
large number of extensions use this exact code (it comes from David
Wheeler's template AFAIK).So far, the number is still less than the number of extensions broken by
the htup header refactoring, so I'm not worried about it.
I think that's fair. The C API has always been in flux and there are
no stability guarantees. We expect C extension writers to know what
they're doing.
But I think we should be more forgiving to extensions written in
"stable" languages like PL/pgSQL, especially if they break *because*
the writer was following PGXN packaging best practices
(http://manager.pgxn.org/howto) and not a fault of their own.
I did a quick and dirty survey of extensions on PGXN and found that
the install change causes problems for (at least) 22% of extensions
there. I think it's well worth the time to implement a workaround,
rather than hassle extension writers.
---
I downloaded and tried to build the latest version of each package on
PGXN (103 total). Now obviously I can't build everything because I
don't have all the necessary dependencies -- which means it's probably
skewed against C extensions. So don't take these results too
seriously. But a it's data point regardless:
9.2 9.3beta1
70 61 <-- "make" succeeds
65 33 <-- "make install" succeeds
0 23 <-- outputs "install: will not overwrite just-created"
0 7 <-- outputs "implicit declaration of function 'heap_"
Note that the htup change causes just a compile-time warning (it will
fail at extension load time), so some of these are counted as
successful builds.
Script and some instructions to replicate this test are attached.
Regards,
Marti
Attachments:
Marti Raudsepp <marti@juffo.org> writes:
I did a quick and dirty survey of extensions on PGXN and found that
the install change causes problems for (at least) 22% of extensions
there. I think it's well worth the time to implement a workaround,
rather than hassle extension writers.
What's really worrying me about this is that beta1 has been out for
less than 48 hours and we've already found one way in which a lot
of system-provided install scripts vary from the one we provide.
How many more compatibility problems will there be?
We changed to using install-sh unconditionally back in 2001 because
we had too many problems with system-provided scripts that didn't do
what we expected. I see very little reason to believe that the
compatibility problems have disappeared since then, and in fact this
complaint seems to me to be sufficient to refute that thesis.
I still think we should revert 9db7ccae2000524b72a4052352cbb5407fb53b02.
The argument that the system-provided program might be faster carries
very little weight for me --- "make install" is fast enough already.
It's not worth making a bunch of extension authors jump through hoops,
whether their style was bad or not.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
I still think we should revert 9db7ccae2000524b72a4052352cbb5407fb53b02.
The argument that the system-provided program might be faster carries
very little weight for me --- "make install" is fast enough already.
It's not worth making a bunch of extension authors jump through hoops,
whether their style was bad or not.
+1.
Thanks,
Stephen
On Tue, May 14, 2013 at 10:30 PM, Stephen Frost <sfrost@snowman.net> wrote:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
I still think we should revert 9db7ccae2000524b72a4052352cbb5407fb53b02.
The argument that the system-provided program might be faster carries
very little weight for me --- "make install" is fast enough already.
It's not worth making a bunch of extension authors jump through hoops,
whether their style was bad or not.+1.
+1.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 5/14/13 5:45 PM, Tom Lane wrote:
We changed to using install-sh unconditionally back in 2001 because
we had too many problems with system-provided scripts that didn't do
what we expected. I see very little reason to believe that the
compatibility problems have disappeared since then, and in fact this
complaint seems to me to be sufficient to refute that thesis.
The compatibility issues in 2001 were completely different ones and were
explicitly resolved with a new configure check (which is used worldwide
by thousands of packages, note). Let's not confuse the issue.
The argument that the system-provided program might be faster carries
very little weight for me --- "make install" is fast enough already.
It is not for me. Note also that make install is part of test runs.
It's not worth making a bunch of extension authors jump through hoops,
whether their style was bad or not.
Well, I consider that this is not a style issue. It's an issue of
wide-spread bugginess caused by uninformed copy-and-paste, and I'm glad
we found it. Considering the widespread crappiness in PostgreSQL
extension makefiles, I don't consider it a problem that a few things
need to be fixed.
That said, I'm obviously outnumbered here. What about the following
compromise: Use the configure-selected install program inside
PostgreSQL (which we can test easily), and use install-sh under
USE_PGXS? Admittedly, the make install time of extensions is probably
not an issue.
(The affected extensions will still be buggy because users can still
substitute their own install programs. We're just hiding the issue for
a while.)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 5/14/13 10:38 AM, Cédric Villemain wrote:
If I follow your example, then I would rewrite http://manager.pgxn.org/howto
Oh that's where that is coming from? Well that example has all kinds of
problems.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Peter Eisentraut <peter_e@gmx.net> writes:
That said, I'm obviously outnumbered here. What about the following
compromise: Use the configure-selected install program inside
PostgreSQL (which we can test easily), and use install-sh under
USE_PGXS? Admittedly, the make install time of extensions is probably
not an issue.
That works for me, since as you say we can easily fix any such bugs
in the core code. The scary thing about this for extension authors
is that they may very well see no bug in their own testing, only to
have their packages fall over in the wild. We shouldn't make each
author who's copied that code rediscover the problem for themselves
that expensively.
(The affected extensions will still be buggy because users can still
substitute their own install programs. We're just hiding the issue for
a while.)
I'm not following this argument. The old makefile coding explicitly set
INSTALL = $(SHELL) $(top_srcdir)/config/install-sh -c
The only way I can see that that would be overridden is an explicit
make INSTALL=/my/random/script install
and surely we cannot guarantee that things don't break when you do
something like that. There are hundreds of ways you can break
the build if you mess with makefile variables. So I'm not prepared
to call it a bug if that doesn't work.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Le mardi 14 mai 2013 15:08:42, Andrew Dunstan a écrit :
On 05/14/2013 07:59 AM, Peter Eisentraut wrote:
On 5/14/13 4:17 AM, Marti Raudsepp wrote:
On Tue, May 14, 2013 at 5:27 AM, Peter Eisentraut <peter_e@gmx.net>
wrote:
On Tue, 2013-05-14 at 04:12 +0300, Marti Raudsepp wrote:
It's caused by this common pattern in extension makefiles:
DATA = $(wildcard sql/*--*.sql) sql/$(EXTENSION)--$(EXTVERSION).sqlWhat is the point of this? Why have the wildcard and then the
non-wildcard term?Because the non-wildcard file is built by the same Makefile (it's
copied from the sql/$(EXTENSION).sql file). If it wasn't there, a
"make install" from a clean checkout would miss this file.If it's built, then it should be listed in DATA_built.
So, AIUI, leaving aside stylistic arguments about use of wildcards, one
solution could be:DATA_built = sql/$(EXTENSION)--$(EXTVERSION).sql
DATA = $(filter-out sql/$(EXTENSION)--$(EXTVERSION).sql, $(wildcard
sql/*--*.sql))Is that right?
Yes.
--
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation
