I'm proposing a way to make test runs a bit faster.
A fair amount of time is taken by creating the test installation. Not
huge compared to the whole test run, but still long enough to get
boring. The idea (that I stole from somewhere else) is that we make the
installation as symlinks pointing back to the source tree. So after the
first run, the installation is automatically up to date after each
build. Then we can skip the whole temp install run if we find that we
already have one (unless you change the set of installed files in a
major way, which is very rate). This also helps if you use make
installcheck: You just need to rebuild and restart the server, skipping
the make install run.
So that's what the patch does: make install INSTALL_SYMLINKS=1 installs
files by making relative symlinks. make check TEMP_INSTALL_SYMLINKS=1
creates the temporary installation using symlinks and skips the install
step if a tmp_install already exists. It requires the "ln" program from
GNU coreutils. So it would be optional.
Except ... this doesn't actually work. find_my_exec() resolves symlinks
to find the actual program installation, and so for example the
installed initdb will look for postgres in src/bin/initdb/. I would
like to revert this behavior, because it seems to do more harm than
good. The original commit message indicates that this would allow
symlinking a program to somewhere outside of the installation tree and
still allow it to work and find its friends. But that could just as
well be done with a shell script.
Reverting this behavior would also allow Homebrew-like installations to
work better. The actual installation is in a versioned directory like
/usr/local/Cellar/postgresql/10.1/... but symlinked to
/usr/local/opt/postgresql/. But because of the symlink resolution,
calling /usr/local/opt/postgresql/bin/pg_config returns paths under
Cellar, which then causes breakage of software built against that path
on postgresql upgrades the change the version number.
Thoughts?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From f101d14195d5bc26f09a84e823f06b6c422fe444 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Sun, 28 Jan 2018 12:24:14 -0500
Subject: [PATCH 1/3] Remove duplicate installation of some header files
In non-vpath builds, some header files such as pg_config.h were actually
installed twice (to the same target location). Rearrange the make rules
a bit to avoid that.
---
src/include/Makefile | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/src/include/Makefile b/src/include/Makefile
index a689d352b6..2d1baca04a 100644
--- a/src/include/Makefile
+++ b/src/include/Makefile
@@ -39,13 +39,7 @@ install: all installdirs
$(INSTALL_DATA) $(srcdir)/port.h
'$(DESTDIR)$(includedir_internal)'
$(INSTALL_DATA) $(srcdir)/postgres_fe.h
'$(DESTDIR)$(includedir_internal)'
$(INSTALL_DATA) $(srcdir)/libpq/pqcomm.h
'$(DESTDIR)$(includedir_internal)/libpq'
-# These headers are needed for server-side development
- $(INSTALL_DATA) pg_config.h '$(DESTDIR)$(includedir_server)'
- $(INSTALL_DATA) pg_config_ext.h '$(DESTDIR)$(includedir_server)'
- $(INSTALL_DATA) pg_config_os.h '$(DESTDIR)$(includedir_server)'
- $(INSTALL_DATA) utils/errcodes.h '$(DESTDIR)$(includedir_server)/utils'
- $(INSTALL_DATA) utils/fmgroids.h '$(DESTDIR)$(includedir_server)/utils'
- $(INSTALL_DATA) utils/fmgrprotos.h
'$(DESTDIR)$(includedir_server)/utils'
+# Headers for server-side development
# We don't use INSTALL_DATA for performance reasons --- there are a lot of
files
cp $(srcdir)/*.h '$(DESTDIR)$(includedir_server)'/ || exit; \
chmod $(INSTALL_DATA_MODE) '$(DESTDIR)$(includedir_server)'/*.h ||
exit; \
@@ -54,7 +48,8 @@ install: all installdirs
chmod $(INSTALL_DATA_MODE) '$(DESTDIR)$(includedir_server)'/$$dir/*.h
|| exit; \
done
ifeq ($(vpath_build),yes)
- for file in dynloader.h catalog/schemapg.h parser/gram.h
storage/lwlocknames.h utils/probes.h; do \
+ for file in pg_config.h pg_config_ext.h pg_config_os.h utils/errcodes.h
utils/fmgroids.h utils/fmgrprotos.h \
+ dynloader.h catalog/schemapg.h parser/gram.h storage/lwlocknames.h
utils/probes.h; do \
cp $$file '$(DESTDIR)$(includedir_server)'/$$file || exit; \
chmod $(INSTALL_DATA_MODE) '$(DESTDIR)$(includedir_server)'/$$file ||
exit; \
done
base-commit: 51057feaa6bd24b51e6a4715c2090491ef037534
--
2.16.2
From afc5738edc445f577d3b7da5503ff61d1debe953 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Sun, 28 Jan 2018 12:24:37 -0500
Subject: [PATCH 2/3] Don't resolve symlinks in find_my_exec()
This reverts 336969e490d71c316a42fabeccda87f798e562dd. That feature
allowed symlinking a binary to a location outside of the tree and still
be able to find the original installation tree by tracing the symlink.
That setup can still be done by writing a shell script wrapper.
Not resolving symlinks allows installation systems such as Homebrew
where the installation tree is in a version-specific directory but the
publicly visible directory is a symlink.
---
src/common/exec.c | 102 ++----------------------------------------------------
1 file changed, 3 insertions(+), 99 deletions(-)
diff --git a/src/common/exec.c b/src/common/exec.c
index e3e81c1db8..e48e597782 100644
--- a/src/common/exec.c
+++ b/src/common/exec.c
@@ -40,7 +40,6 @@
#endif
static int validate_exec(const char *path);
-static int resolve_symlinks(char *path);
static char *pipe_read_line(char *cmd, char *line, int maxsize);
#ifdef WIN32
@@ -141,7 +140,7 @@ find_my_exec(const char *argv0, char *retpath)
canonicalize_path(retpath);
if (validate_exec(retpath) == 0)
- return resolve_symlinks(retpath);
+ return 0;
log_error(_("invalid binary \"%s\""), retpath);
return -1;
@@ -151,7 +150,7 @@ find_my_exec(const char *argv0, char *retpath)
/* Win32 checks the current directory first for names without slashes */
join_path_components(retpath, cwd, argv0);
if (validate_exec(retpath) == 0)
- return resolve_symlinks(retpath);
+ return 0;
#endif
/*
@@ -188,7 +187,7 @@ find_my_exec(const char *argv0, char *retpath)
switch (validate_exec(retpath))
{
case 0: /* found ok */
- return resolve_symlinks(retpath);
+ return 0;
case -1: /* wasn't even a
candidate, keep looking */
break;
case -2: /* found but
disqualified */
@@ -204,101 +203,6 @@ find_my_exec(const char *argv0, char *retpath)
}
-/*
- * resolve_symlinks - resolve symlinks to the underlying file
- *
- * Replace "path" by the absolute path to the referenced file.
- *
- * Returns 0 if OK, -1 if error.
- *
- * Note: we are not particularly tense about producing nice error messages
- * because we are not really expecting error here; we just determined that
- * the symlink does point to a valid executable.
- */
-static int
-resolve_symlinks(char *path)
-{
-#ifdef HAVE_READLINK
- struct stat buf;
- char orig_wd[MAXPGPATH],
- link_buf[MAXPGPATH];
- char *fname;
-
- /*
- * To resolve a symlink properly, we have to chdir into its directory
and
- * then chdir to where the symlink points; otherwise we may fail to
- * resolve relative links correctly (consider cases involving mount
- * points, for example). After following the final symlink, we use
- * getcwd() to figure out where the heck we're at.
- *
- * One might think we could skip all this if path doesn't point to a
- * symlink to start with, but that's wrong. We also want to get rid of
- * any directory symlinks that are present in the given path. We expect
- * getcwd() to give us an accurate, symlink-free path.
- */
- if (!getcwd(orig_wd, MAXPGPATH))
- {
- log_error(_("could not identify current directory: %s"),
- strerror(errno));
- return -1;
- }
-
- for (;;)
- {
- char *lsep;
- int rllen;
-
- lsep = last_dir_separator(path);
- if (lsep)
- {
- *lsep = '\0';
- if (chdir(path) == -1)
- {
- log_error4(_("could not change directory to
\"%s\": %s"), path, strerror(errno));
- return -1;
- }
- fname = lsep + 1;
- }
- else
- fname = path;
-
- if (lstat(fname, &buf) < 0 ||
- !S_ISLNK(buf.st_mode))
- break;
-
- rllen = readlink(fname, link_buf, sizeof(link_buf));
- if (rllen < 0 || rllen >= sizeof(link_buf))
- {
- log_error(_("could not read symbolic link \"%s\""),
fname);
- return -1;
- }
- link_buf[rllen] = '\0';
- strcpy(path, link_buf);
- }
-
- /* must copy final component out of 'path' temporarily */
- strlcpy(link_buf, fname, sizeof(link_buf));
-
- if (!getcwd(path, MAXPGPATH))
- {
- log_error(_("could not identify current directory: %s"),
- strerror(errno));
- return -1;
- }
- join_path_components(path, path, link_buf);
- canonicalize_path(path);
-
- if (chdir(orig_wd) == -1)
- {
- log_error4(_("could not change directory to \"%s\": %s"),
orig_wd, strerror(errno));
- return -1;
- }
-#endif /* HAVE_READLINK */
-
- return 0;
-}
-
-
/*
* Find another program in our binary's directory,
* then make sure it is the proper version.
--
2.16.2
From 65ed4ac7d7101be1700c4427b57a2fceee14eb01 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Sun, 28 Jan 2018 12:25:02 -0500
Subject: [PATCH 3/3] Add TEMP_INSTALL_SYMLINKS mode
Running make install INSTALL_SYMLINKS=1 installs files by making
relative symlinks into the source tree instead of copying. That way,
the installation is automatically up to date after each build.
make check TEMP_INSTALL_SYMLINKS=1 creates the temporary installation
for the tests using symlinks, skipping the installation if the temporary
installation already exists. This speeds up testing.
This currently requires the "ln" program from GNU coreutils.
---
src/Makefile.global.in | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index d980f81046..78e7ef1f4f 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -326,15 +326,22 @@ BZIP2 = bzip2
check: temp-install
.PHONY: temp-install
+
+ifneq (,$(TEMP_INSTALL_SYMLINKS))
+temp-install: INSTALL_SYMLINKS=1
+endif
+
temp-install:
ifndef NO_TEMP_INSTALL
ifneq ($(abs_top_builddir),)
ifeq ($(MAKELEVEL),0)
+ifeq (,$(and $(TEMP_INSTALL_SYMLINKS),$(wildcard
$(abs_top_builddir)/tmp_install)))
rm -rf '$(abs_top_builddir)'/tmp_install
$(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log
- $(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install
install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1
+ $(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install
INSTALL_SYMLINKS=$(INSTALL_SYMLINKS) install
>'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1
endif
- $(if $(EXTRA_INSTALL),for extra in $(EXTRA_INSTALL); do $(MAKE) -C
'$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install install
>>'$(abs_top_builddir)'/tmp_install/log/install.log || exit; done)
+endif
+ $(if $(EXTRA_INSTALL),for extra in $(EXTRA_INSTALL); do $(MAKE) -C
'$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install
INSTALL_SYMLINKS=$(INSTALL_SYMLINKS) install
>>'$(abs_top_builddir)'/tmp_install/log/install.log || exit; done)
endif
endif
@@ -394,6 +401,15 @@ INSTALL_SHLIB = $(INSTALL_SHLIB_ENV) $(INSTALL)
$(INSTALL_SHLIB_OPTS) $(INSTALL_
# Override in Makefile.port if necessary
INSTALL_SHLIB_OPTS = -m 755
+ifneq ($(INSTALL_SYMLINKS),)
+INSTALL = ln -frs
+INSTALL_PROGRAM = $(INSTALL)
+INSTALL_SCRIPT = $(INSTALL)
+INSTALL_DATA = $(INSTALL)
+INSTALL_STLIB = $(INSTALL)
+INSTALL_SHLIB = $(INSTALL)
+endif
+
MKDIR_P = @MKDIR_P@
missing = $(SHELL) $(top_srcdir)/config/missing
--
2.16.2
