public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 05/10] (re)add wal_compression_method: lz4
6+ messages / 5 participants
[nested] [flat]
* [PATCH 05/10] (re)add wal_compression_method: lz4
@ 2021-03-12 20:32 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: Justin Pryzby @ 2021-03-12 20:32 UTC (permalink / raw)
---
configure | 176 ++++++++++++++++++
configure.ac | 26 +++
doc/src/sgml/config.sgml | 3 +-
src/backend/access/transam/xlog.c | 3 +
src/backend/access/transam/xloginsert.c | 12 ++
src/backend/access/transam/xlogreader.c | 13 ++
src/backend/utils/misc/postgresql.conf.sample | 2 +-
src/include/access/xlog_internal.h | 1 +
src/include/pg_config.h.in | 3 +
src/tools/msvc/Solution.pm | 1 +
10 files changed, 238 insertions(+), 2 deletions(-)
diff --git a/configure b/configure
index 3fd4cecbeb..fed440adcf 100755
--- a/configure
+++ b/configure
@@ -699,6 +699,9 @@ with_gnu_ld
LD
LDFLAGS_SL
LDFLAGS_EX
+LZ4_LIBS
+LZ4_CFLAGS
+with_lz4
with_zlib
with_system_tzdata
with_libxslt
@@ -864,6 +867,7 @@ with_libxml
with_libxslt
with_system_tzdata
with_zlib
+with_lz4
with_gnu_ld
with_ssl
with_openssl
@@ -891,6 +895,8 @@ ICU_LIBS
XML2_CONFIG
XML2_CFLAGS
XML2_LIBS
+LZ4_CFLAGS
+LZ4_LIBS
LDFLAGS_EX
LDFLAGS_SL
PERL
@@ -1569,6 +1575,7 @@ Optional Packages:
--with-system-tzdata=DIR
use system time zone data in DIR
--without-zlib do not use Zlib
+ --with-lz4 build with LZ4 support
--with-gnu-ld assume the C compiler uses GNU ld [default=no]
--with-ssl=LIB use LIB for SSL/TLS support (openssl)
--with-openssl obsolete spelling of --with-ssl=openssl
@@ -1596,6 +1603,8 @@ Some influential environment variables:
XML2_CONFIG path to xml2-config utility
XML2_CFLAGS C compiler flags for XML2, overriding pkg-config
XML2_LIBS linker flags for XML2, overriding pkg-config
+ LZ4_CFLAGS C compiler flags for LZ4, overriding pkg-config
+ LZ4_LIBS linker flags for LZ4, overriding pkg-config
LDFLAGS_EX extra linker flags for linking executables only
LDFLAGS_SL extra linker flags for linking shared libraries only
PERL Perl program
@@ -8563,6 +8572,137 @@ fi
+#
+# LZ4
+#
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether to build with LZ4 support" >&5
+$as_echo_n "checking whether to build with LZ4 support... " >&6; }
+
+
+
+# Check whether --with-lz4 was given.
+if test "${with_lz4+set}" = set; then :
+ withval=$with_lz4;
+ case $withval in
+ yes)
+
+$as_echo "#define USE_LZ4 1" >>confdefs.h
+
+ ;;
+ no)
+ :
+ ;;
+ *)
+ as_fn_error $? "no argument expected for --with-lz4 option" "$LINENO" 5
+ ;;
+ esac
+
+else
+ with_lz4=no
+
+fi
+
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $with_lz4" >&5
+$as_echo "$with_lz4" >&6; }
+
+
+if test "$with_lz4" = yes; then
+
+pkg_failed=no
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for liblz4" >&5
+$as_echo_n "checking for liblz4... " >&6; }
+
+if test -n "$LZ4_CFLAGS"; then
+ pkg_cv_LZ4_CFLAGS="$LZ4_CFLAGS"
+ elif test -n "$PKG_CONFIG"; then
+ if test -n "$PKG_CONFIG" && \
+ { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists --print-errors \"liblz4\""; } >&5
+ ($PKG_CONFIG --exists --print-errors "liblz4") 2>&5
+ ac_status=$?
+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+ test $ac_status = 0; }; then
+ pkg_cv_LZ4_CFLAGS=`$PKG_CONFIG --cflags "liblz4" 2>/dev/null`
+ test "x$?" != "x0" && pkg_failed=yes
+else
+ pkg_failed=yes
+fi
+ else
+ pkg_failed=untried
+fi
+if test -n "$LZ4_LIBS"; then
+ pkg_cv_LZ4_LIBS="$LZ4_LIBS"
+ elif test -n "$PKG_CONFIG"; then
+ if test -n "$PKG_CONFIG" && \
+ { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists --print-errors \"liblz4\""; } >&5
+ ($PKG_CONFIG --exists --print-errors "liblz4") 2>&5
+ ac_status=$?
+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+ test $ac_status = 0; }; then
+ pkg_cv_LZ4_LIBS=`$PKG_CONFIG --libs "liblz4" 2>/dev/null`
+ test "x$?" != "x0" && pkg_failed=yes
+else
+ pkg_failed=yes
+fi
+ else
+ pkg_failed=untried
+fi
+
+
+
+if test $pkg_failed = yes; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+
+if $PKG_CONFIG --atleast-pkgconfig-version 0.20; then
+ _pkg_short_errors_supported=yes
+else
+ _pkg_short_errors_supported=no
+fi
+ if test $_pkg_short_errors_supported = yes; then
+ LZ4_PKG_ERRORS=`$PKG_CONFIG --short-errors --print-errors --cflags --libs "liblz4" 2>&1`
+ else
+ LZ4_PKG_ERRORS=`$PKG_CONFIG --print-errors --cflags --libs "liblz4" 2>&1`
+ fi
+ # Put the nasty error message in config.log where it belongs
+ echo "$LZ4_PKG_ERRORS" >&5
+
+ as_fn_error $? "Package requirements (liblz4) were not met:
+
+$LZ4_PKG_ERRORS
+
+Consider adjusting the PKG_CONFIG_PATH environment variable if you
+installed software in a non-standard prefix.
+
+Alternatively, you may set the environment variables LZ4_CFLAGS
+and LZ4_LIBS to avoid the need to call pkg-config.
+See the pkg-config man page for more details." "$LINENO" 5
+elif test $pkg_failed = untried; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+ { { $as_echo "$as_me:${as_lineno-$LINENO}: error: in \`$ac_pwd':" >&5
+$as_echo "$as_me: error: in \`$ac_pwd':" >&2;}
+as_fn_error $? "The pkg-config script could not be found or is too old. Make sure it
+is in your PATH or set the PKG_CONFIG environment variable to the full
+path to pkg-config.
+
+Alternatively, you may set the environment variables LZ4_CFLAGS
+and LZ4_LIBS to avoid the need to call pkg-config.
+See the pkg-config man page for more details.
+
+To get pkg-config, see <http://pkg-config.freedesktop.org/;.
+See \`config.log' for more details" "$LINENO" 5; }
+else
+ LZ4_CFLAGS=$pkg_cv_LZ4_CFLAGS
+ LZ4_LIBS=$pkg_cv_LZ4_LIBS
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
+$as_echo "yes" >&6; }
+
+fi
+ LIBS="$LZ4_LIBS $LIBS"
+ CFLAGS="$LZ4_CFLAGS $CFLAGS"
+fi
+
#
# Assignments
#
@@ -13381,6 +13521,42 @@ fi
fi
+if test "$with_lz4" = yes; then
+ ac_save_CPPFLAGS=$CPPFLAGS
+ CPPFLAGS="$LZ4_CFLAGS $CPPFLAGS"
+
+ # Verify we have LZ4's header files
+ for ac_header in lz4/lz4.h
+do :
+ ac_fn_c_check_header_mongrel "$LINENO" "lz4/lz4.h" "ac_cv_header_lz4_lz4_h" "$ac_includes_default"
+if test "x$ac_cv_header_lz4_lz4_h" = xyes; then :
+ cat >>confdefs.h <<_ACEOF
+#define HAVE_LZ4_LZ4_H 1
+_ACEOF
+
+else
+ for ac_header in lz4.h
+do :
+ ac_fn_c_check_header_mongrel "$LINENO" "lz4.h" "ac_cv_header_lz4_h" "$ac_includes_default"
+if test "x$ac_cv_header_lz4_h" = xyes; then :
+ cat >>confdefs.h <<_ACEOF
+#define HAVE_LZ4_H 1
+_ACEOF
+
+else
+ as_fn_error $? "lz4.h header file is required for LZ4" "$LINENO" 5
+fi
+
+done
+
+fi
+
+done
+
+
+ CPPFLAGS=$ac_save_CPPFLAGS
+fi
+
if test "$with_gssapi" = yes ; then
for ac_header in gssapi/gssapi.h
do :
diff --git a/configure.ac b/configure.ac
index 2f1585adc0..8c454128bb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -986,6 +986,21 @@ PGAC_ARG_BOOL(with, zlib, yes,
[do not use Zlib])
AC_SUBST(with_zlib)
+#
+# LZ4
+#
+AC_MSG_CHECKING([whether to build with LZ4 support])
+PGAC_ARG_BOOL(with, lz4, no, [build with LZ4 support],
+ [AC_DEFINE([USE_LZ4], 1, [Define to 1 to build with LZ4 support. (--with-lz4)])])
+AC_MSG_RESULT([$with_lz4])
+AC_SUBST(with_lz4)
+
+if test "$with_lz4" = yes; then
+ PKG_CHECK_MODULES(LZ4, liblz4)
+ LIBS="$LZ4_LIBS $LIBS"
+ CFLAGS="$LZ4_CFLAGS $CFLAGS"
+fi
+
#
# Assignments
#
@@ -1410,6 +1425,17 @@ failure. It is possible the compiler isn't looking in the proper directory.
Use --without-zlib to disable zlib support.])])
fi
+if test "$with_lz4" = yes; then
+ ac_save_CPPFLAGS=$CPPFLAGS
+ CPPFLAGS="$LZ4_CFLAGS $CPPFLAGS"
+
+ # Verify we have LZ4's header files
+ AC_CHECK_HEADERS(lz4/lz4.h, [],
+ [AC_CHECK_HEADERS(lz4.h, [], [AC_MSG_ERROR([lz4.h header file is required for LZ4])])])
+
+ CPPFLAGS=$ac_save_CPPFLAGS
+fi
+
if test "$with_gssapi" = yes ; then
AC_CHECK_HEADERS(gssapi/gssapi.h, [],
[AC_CHECK_HEADERS(gssapi.h, [], [AC_MSG_ERROR([gssapi.h header file is required for GSSAPI])])])
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 7fb2a84626..257775c83b 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -3082,7 +3082,8 @@ include_dir 'conf.d'
<para>
This parameter selects the compression method used to compress WAL when
<varname>wal_compression</varname> is enabled.
- The supported methods are pglz and zlib.
+ The supported methods are pglz, zlib, and (if configured when
+ <productname>PostgreSQL</productname> was built) lz4.
The default value is <literal>pglz</literal>.
Only superusers can change this setting.
</para>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index f5d4450654..984ce39cc7 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -186,6 +186,9 @@ const struct config_enum_entry wal_compression_options[] = {
{"pglz", WAL_COMPRESSION_PGLZ, false},
#ifdef HAVE_LIBZ
{"zlib", WAL_COMPRESSION_ZLIB, false},
+#endif
+#ifdef USE_LZ4
+ {"lz4", WAL_COMPRESSION_LZ4, false},
#endif
{NULL, 0, false}
};
diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c
index 34e1227381..9fe5c30236 100644
--- a/src/backend/access/transam/xloginsert.c
+++ b/src/backend/access/transam/xloginsert.c
@@ -37,6 +37,10 @@
#include <zlib.h>
#endif
+#ifdef USE_LZ4
+#include "lz4.h"
+#endif
+
/* Buffer size required to store a compressed version of backup block image */
#define PGLZ_MAX_BLCKSZ PGLZ_MAX_OUTPUT(BLCKSZ)
@@ -884,6 +888,14 @@ XLogCompressBackupBlock(char *page, uint16 hole_offset, uint16 hole_length,
}
#endif
+#ifdef USE_LZ4
+ case WAL_COMPRESSION_LZ4:
+ len = LZ4_compress_fast(source, dest, orig_len, PGLZ_MAX_BLCKSZ, 1);
+ if (len == 0)
+ len = -1;
+ break;
+#endif
+
default:
/*
* It should be impossible to get here for unsupported algorithms,
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index afca22a26c..fa1e38a810 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -37,6 +37,10 @@
#include <zlib.h>
#endif
+#ifdef USE_LZ4
+#include "lz4.h"
+#endif
+
static void report_invalid_record(XLogReaderState *state, const char *fmt,...)
pg_attribute_printf(2, 3);
static bool allocate_recordbuf(XLogReaderState *state, uint32 reclength);
@@ -1558,6 +1562,8 @@ wal_compression_name(WalCompression compression)
return "pglz";
case WAL_COMPRESSION_ZLIB:
return "zlib";
+ case WAL_COMPRESSION_LZ4:
+ return "lz4";
default:
return "???";
}
@@ -1608,6 +1614,13 @@ RestoreBlockImage(XLogReaderState *record, uint8 block_id, char *page)
}
#endif
+#ifdef USE_LZ4
+ case WAL_COMPRESSION_LZ4:
+ decomp_result = LZ4_decompress_safe(ptr, tmp.data,
+ bkpb->bimg_len, BLCKSZ-bkpb->hole_length);
+ break;
+#endif
+
default:
report_invalid_record(record, "image at %X/%X is compressed with unsupported codec, block %d (%d/%s)",
(uint32) (record->ReadRecPtr >> 32),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index ef69a94492..76f494cb9b 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -213,7 +213,7 @@
# open_sync
#full_page_writes = on # recover from partial page writes
#wal_compression = off # enable compression of full-page writes
-#wal_compression_method = pglz # pglz, zlib
+#wal_compression_method = pglz # pglz, zlib, lz4
#wal_log_hints = off # also do full page writes of non-critical updates
# (change requires restart)
#wal_init_zero = on # zero-fill new WAL files
diff --git a/src/include/access/xlog_internal.h b/src/include/access/xlog_internal.h
index d653839b97..e70886b81c 100644
--- a/src/include/access/xlog_internal.h
+++ b/src/include/access/xlog_internal.h
@@ -328,6 +328,7 @@ typedef enum WalCompression
{
WAL_COMPRESSION_PGLZ,
WAL_COMPRESSION_ZLIB,
+ WAL_COMPRESSION_LZ4,
} WalCompression;
extern const char *wal_compression_name(WalCompression compression);
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 7a7cc21d8d..0a6422da4f 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -899,6 +899,9 @@
/* Define to 1 to build with LLVM based JIT support. (--with-llvm) */
#undef USE_LLVM
+/* Define to 1 to build with LZ4 support (--with-lz4) */
+#undef USE_LZ4
+
/* Define to select named POSIX semaphores. */
#undef USE_NAMED_POSIX_SEMAPHORES
diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index a4f5cc4bdb..14605371bb 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -485,6 +485,7 @@ sub GenerateFiles
USE_ICU => $self->{options}->{icu} ? 1 : undef,
USE_LIBXML => undef,
USE_LIBXSLT => undef,
+ USE_LZ4 => undef,
USE_LDAP => $self->{options}->{ldap} ? 1 : undef,
USE_LLVM => undef,
USE_NAMED_POSIX_SEMAPHORES => undef,
--
2.17.0
--XsQoSWH+UP9D9v3l
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="0006-Default-to-LZ4.patch"
^ permalink raw reply [nested|flat] 6+ messages in thread
* [PATCH] pg_regress.c: Fix "make check" on Mac OS X: Pass DYLD_LIBRARY_PATH
@ 2023-06-05 13:47 Evan Jones <[email protected]>
0 siblings, 2 replies; 6+ messages in thread
From: Evan Jones @ 2023-06-05 13:47 UTC (permalink / raw)
To: pgsql-hackers
This makes "make check" work on Mac OS X. Without this patch, on Mac OS X a
default "./configure; make; make check" fails with errors like:
dyld[65265]: Library not loaded: /usr/local/pgsql/lib/libpq.5.dylib
Referenced from: <59A2EAF9-6298-3112-BEDB-EA9A62A9DB53>
/Users/evan.jones/postgresql-clean/tmp_install/usr/local/pgsql/bin/initdb
Reason: tried: '/usr/local/pgsql/lib/libpq.5.dylib' (no such file),
'/System/Volumes/Preboot/Cryptexes/OS/usr/local/pgsql/lib/libpq.5.dylib'
(no such file), '/usr/local/pgsql/lib/libpq.5.dylib' (no such file),
'/usr/local/lib/libpq.5.dylib' (no such file), '/usr/lib/libpq.5.dylib' (no
such file, not in dyld cache)
The reason is that at some point, Mac OS X started removing the
DYLD_LIBRARY_PATH environment variable for "untrusted" executables [1]:
"Any dynamic linker (dyld) environment variables, such as
DYLD_LIBRARY_PATH, are purged when launching protected processes."
[1]
https://developer.apple.com/library/archive/documentation/Security/Conceptual/System_Integrity_Prote...
One solution is to explicitly pass the DYLD_LIBRARY_PATH environment
variable to to the sub-process shell scripts that are run by pg_regress. To
do this, I created an extra_envvars global variable which is set to the
empty string "", but on Mac OS X, is filled in with "DYLD_LIBRARY_PATH=%s",
where the %s is the current environment variable. The "make check" Makefile
sets this environment variable to the temporary install directory, so this
fixes the above errors.
I tested this on Mac OS X and on Linux (Ubuntu 23.04).
Thanks!
Evan Jones
Attachments:
[application/octet-stream] pg_regress_mac_os_x_dyld.patch (2.6K, ../../CA+HWA9Zhz+QqaKrWkBd4UVfxL1FANJvRyCTFY5J_hq3AhDBpJQ@mail.gmail.com/3-pg_regress_mac_os_x_dyld.patch)
download | inline diff:
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index a546fc3d34..61a62074cf 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -133,6 +133,16 @@ static bool postmaster_running = false;
static int success_count = 0;
static int fail_count = 0;
+/*
+ * Mac OS X removes DYLD_LIBRARY_PATH on exec(). Use this variable to explicitly pass it. See:
+ * https://developer.apple.com/library/archive/documentation/Security/Conceptual/System_Integrity_Protection_Guide/RuntimeProtections/RuntimeProtections.html
+ */
+#ifdef __darwin__
+static char extra_envvars[4096];
+#else
+static const char extra_envvars[] = "";
+#endif
+
static bool directory_exists(const char *dir);
static void make_directory(const char *dir);
@@ -897,6 +907,16 @@ initialize_environment(void)
note("using postmaster on Unix socket, default port");
}
+#ifdef __darwin__
+ /* Required to find shared libraries after exec(). See comment on extra_envars. */
+ int result = snprintf(extra_envvars, sizeof(extra_envvars), "DYLD_LIBRARY_PATH=%s",
+ getenv("DYLD_LIBRARY_PATH"));
+ if (result < 0 || result >= sizeof(extra_envvars))
+ {
+ bail("BUG: extra_envars too small for DYLD_LIBRARY_PATH");
+ }
+#endif
+
load_resultmap();
}
@@ -1106,7 +1126,8 @@ psql_start_command(void)
StringInfo buf = makeStringInfo();
appendStringInfo(buf,
- "\"%s%spsql\" -X -q",
+ "%s \"%s%spsql\" -X -q",
+ extra_envvars,
bindir ? bindir : "",
bindir ? "/" : "");
return buf;
@@ -1216,7 +1237,7 @@ spawn_process(const char *cmdline)
*/
char *cmdline2;
- cmdline2 = psprintf("exec %s", cmdline);
+ cmdline2 = psprintf("%s exec %s", extra_envvars, cmdline);
execl(shellprog, shellprog, "-c", cmdline2, (char *) NULL);
/* Not using the normal bail() here as we want _exit */
bail_noatexit("could not exec \"%s\": %s", shellprog, strerror(errno));
@@ -2319,7 +2340,8 @@ regression_main(int argc, char *argv[],
/* initdb */
snprintf(buf, sizeof(buf),
- "\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync%s%s > \"%s/log/initdb.log\" 2>&1",
+ "%s \"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync%s%s > \"%s/log/initdb.log\" 2>&1",
+ extra_envvars,
bindir ? bindir : "",
bindir ? "/" : "",
temp_instance,
@@ -2393,7 +2415,8 @@ regression_main(int argc, char *argv[],
* Check if there is a postmaster running already.
*/
snprintf(buf2, sizeof(buf2),
- "\"%s%spsql\" -X postgres <%s 2>%s",
+ "%s \"%s%spsql\" -X postgres <%s 2>%s",
+ extra_envvars,
bindir ? bindir : "",
bindir ? "/" : "",
DEVNULL, DEVNULL);
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [PATCH] pg_regress.c: Fix "make check" on Mac OS X: Pass DYLD_LIBRARY_PATH
@ 2023-06-06 02:25 Julien Rouhaud <[email protected]>
parent: Evan Jones <[email protected]>
1 sibling, 0 replies; 6+ messages in thread
From: Julien Rouhaud @ 2023-06-06 02:25 UTC (permalink / raw)
To: Evan Jones <[email protected]>; +Cc: pgsql-hackers
Hi,
On Mon, Jun 05, 2023 at 09:47:30AM -0400, Evan Jones wrote:
> This makes "make check" work on Mac OS X. Without this patch, on Mac OS X a
> default "./configure; make; make check" fails with errors like:
>
> dyld[65265]: Library not loaded: /usr/local/pgsql/lib/libpq.5.dylib
> Referenced from: <59A2EAF9-6298-3112-BEDB-EA9A62A9DB53>
> /Users/evan.jones/postgresql-clean/tmp_install/usr/local/pgsql/bin/initdb
> Reason: tried: '/usr/local/pgsql/lib/libpq.5.dylib' (no such file),
> '/System/Volumes/Preboot/Cryptexes/OS/usr/local/pgsql/lib/libpq.5.dylib'
> (no such file), '/usr/local/pgsql/lib/libpq.5.dylib' (no such file),
> '/usr/local/lib/libpq.5.dylib' (no such file), '/usr/lib/libpq.5.dylib' (no
> such file, not in dyld cache)
>
> The reason is that at some point, Mac OS X started removing the
> DYLD_LIBRARY_PATH environment variable for "untrusted" executables [1]:
> "Any dynamic linker (dyld) environment variables, such as
> DYLD_LIBRARY_PATH, are purged when launching protected processes."
>
> [1]
> https://developer.apple.com/library/archive/documentation/Security/Conceptual/System_Integrity_Prote...
>
> One solution is to explicitly pass the DYLD_LIBRARY_PATH environment
> variable to to the sub-process shell scripts that are run by pg_regress. To
> do this, I created an extra_envvars global variable which is set to the
> empty string "", but on Mac OS X, is filled in with "DYLD_LIBRARY_PATH=%s",
> where the %s is the current environment variable. The "make check" Makefile
> sets this environment variable to the temporary install directory, so this
> fixes the above errors.
Note that this is a known issue and a workaround is documented in the macos
specific notes at
https://www.postgresql.org/docs/current/installation-platform-notes.html#INSTALLATION-NOTES-MACOS:
> macOS's “System Integrity Protection” (SIP) feature breaks make check,
> because it prevents passing the needed setting of DYLD_LIBRARY_PATH down to
> the executables being tested. You can work around that by doing make install
> before make check. Most PostgreSQL developers just turn off SIP, though.
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [PATCH] pg_regress.c: Fix "make check" on Mac OS X: Pass DYLD_LIBRARY_PATH
@ 2023-06-16 21:25 David Zhang <[email protected]>
parent: Evan Jones <[email protected]>
1 sibling, 1 reply; 6+ messages in thread
From: David Zhang @ 2023-06-16 21:25 UTC (permalink / raw)
To: [email protected]
I have applied the patch to the latest master branch and successfully executed './configure && make && make check' on macOS Ventura. However, during the process, a warning was encountered: "mixing declarations and code is incompatible with standards before C99 [-Wdeclaration-after-statement]". Moving the declaration of 'result' to the beginning like below can resolve the warning, and it would be better to use a unique variable instead of 'result'.
#ifdef __darwin__
static char extra_envvars[4096];
+int result = -1;
... ...
-int result = snprintf(extra_envvars, sizeof(extra_envvars), "DYLD_LIBRARY_PATH=%s",
+result = snprintf(extra_envvars, sizeof(extra_envvars), "DYLD_LIBRARY_PATH=%s",
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [PATCH] pg_regress.c: Fix "make check" on Mac OS X: Pass DYLD_LIBRARY_PATH
@ 2023-06-22 19:08 David Zhang <[email protected]>
parent: David Zhang <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: David Zhang @ 2023-06-22 19:08 UTC (permalink / raw)
To: [email protected]
After conducting a further investigation into this issue, I have made
some discoveries. The previous patch successfully resolves the problem
when running the commands `./configure && make && make check` (without
any previous sudo make install or make install). However, it stops at
the 'isolation check' when using the commands `./configure
--enable-tap-tests && make && make check-world`.
To address this, I attempted to apply a similar approach as the previous
patch, resulting in an experimental patch (attached). This new patch
helps progress the 'make-world' process and passes the 'isolation
check', but there are still several remaining issues that need to be
addressed.
Currently, there is a description suggesting a workaround by running a
'make install' command first, but I find it to be somewhat inaccurate.
It would be better to update the existing description to provide more
precise instructions on how to overcome this issue. Here are the changes
I would suggest.
from:
"You can work around that by doing make install before make check. Most
PostgreSQL developers just turn off SIP, though."
to:
"You can execute sudo make install if you do not specify a prefix during
the configure step, or make install without sudo if you do specify a
prefix (assuming proper permissions) before make check. Most PostgreSQL
developers just turn off SIP, though."
Otherwise, following the current description, if you run `./configure
&& make install` you will get error: "mkdir: /usr/local/pgsql:
Permission denied"
Below are the steps I took that led to the discovery of additional issues.
git apply pg_regress_mac_os_x_dyld.patch
./configure
make
make check
... ...
# All 215 tests passed.
./configure --enable-tap-tests
make
make check-world
... ...
echo "# +++ isolation check in src/test/isolation +++"
... ...
dyld[32335]: Library not loaded: /usr/local/pgsql/lib/libpq.5.dylib
Referenced from: <EB3758C5-A87B-36C5-AA29-C1E31AD89E70>
/Users/david/hg/sandbox/postgres/src/test/isolation/isolationtester
Reason: tried: '/usr/local/pgsql/lib/libpq.5.dylib' (no such file),
'/System/Volumes/Preboot/Cryptexes/OS/usr/local/pgsql/lib/libpq.5.dylib'
(no such file), '/usr/local/pgsql/lib/libpq.5.dylib' (no such file),
'/usr/local/lib/libpq.5.dylib' (no such file), '/usr/lib/libpq.5.dylib'
(no such file, not in dyld cache)
no data was returned by command
""/Users/david/hg/sandbox/postgres/src/test/isolation/isolationtester" -V"
git apply pg_regress_mac_os_x_dyld_isolation_check_only.patch
./configure --enable-tap-tests
make
make check-world
... ...
# All 215 tests passed.
... ...
# +++ isolation check in src/test/isolation +++
... ...
# All 112 tests passed.
echo "# +++ tap check in src/test/modules/brin +++"
... ...
# +++ tap check in src/test/modules/brin +++
t/01_workitems.pl ........ Bailout called. Further testing stopped:
command "initdb -D
/Users/david/hg/sandbox/postgres/src/test/modules/brin/tmp_check/t_01_workitems_tango_data/pgdata
-A trust -N" died with signal 6
t/01_workitems.pl ........ Dubious, test returned 255 (wstat 65280, 0xff00)
No subtests run
Any thoughts ?
Thank you
David
On 2023-06-16 2:25 p.m., David Zhang wrote:
> I have applied the patch to the latest master branch and successfully executed './configure && make && make check' on macOS Ventura. However, during the process, a warning was encountered: "mixing declarations and code is incompatible with standards before C99 [-Wdeclaration-after-statement]". Moving the declaration of 'result' to the beginning like below can resolve the warning, and it would be better to use a unique variable instead of 'result'.
>
> #ifdef __darwin__
> static char extra_envvars[4096];
> +int result = -1;
> ... ...
> -int result = snprintf(extra_envvars, sizeof(extra_envvars), "DYLD_LIBRARY_PATH=%s",
> +result = snprintf(extra_envvars, sizeof(extra_envvars), "DYLD_LIBRARY_PATH=%s",
diff --git a/src/common/exec.c b/src/common/exec.c
index f209b934df..8cf2c21a66 100644
--- a/src/common/exec.c
+++ b/src/common/exec.c
@@ -74,6 +74,12 @@ static char *pg_realpath(const char *fname);
static BOOL GetTokenUser(HANDLE hToken, PTOKEN_USER *ppTokenUser);
#endif
+#ifdef __darwin__
+static char extra_envvars[4096];
+#else
+static const char extra_envvars[] = "";
+#endif
+
/*
* validate_exec -- validate "path" as an executable file
*
@@ -330,6 +336,15 @@ find_other_exec(const char *argv0, const char *target,
char cmd[MAXPGPATH];
char line[MAXPGPATH];
+#ifdef __darwin__
+ int result = 1;
+ result = snprintf(extra_envvars, sizeof(extra_envvars), "DYLD_LIBRARY_PATH=%s",
+ getenv("DYLD_LIBRARY_PATH"));
+ if (result < 0 || result >= sizeof(extra_envvars))
+ {
+ printf("extra_envars too small for DYLD_LIBRARY_PATH");
+ }
+#endif
if (find_my_exec(argv0, retpath) < 0)
return -1;
@@ -344,7 +359,7 @@ find_other_exec(const char *argv0, const char *target,
if (validate_exec(retpath) != 0)
return -1;
- snprintf(cmd, sizeof(cmd), "\"%s\" -V", retpath);
+ snprintf(cmd, sizeof(cmd), "%s \"%s\" -V", extra_envvars, retpath);
if (!pipe_read_line(cmd, line, sizeof(line)))
return -1;
Attachments:
[text/plain] pg_regress_mac_os_x_dyld_isolation_check_only.patch (1.3K, ../../[email protected]/2-pg_regress_mac_os_x_dyld_isolation_check_only.patch)
download | inline diff:
diff --git a/src/common/exec.c b/src/common/exec.c
index f209b934df..8cf2c21a66 100644
--- a/src/common/exec.c
+++ b/src/common/exec.c
@@ -74,6 +74,12 @@ static char *pg_realpath(const char *fname);
static BOOL GetTokenUser(HANDLE hToken, PTOKEN_USER *ppTokenUser);
#endif
+#ifdef __darwin__
+static char extra_envvars[4096];
+#else
+static const char extra_envvars[] = "";
+#endif
+
/*
* validate_exec -- validate "path" as an executable file
*
@@ -330,6 +336,15 @@ find_other_exec(const char *argv0, const char *target,
char cmd[MAXPGPATH];
char line[MAXPGPATH];
+#ifdef __darwin__
+ int result = 1;
+ result = snprintf(extra_envvars, sizeof(extra_envvars), "DYLD_LIBRARY_PATH=%s",
+ getenv("DYLD_LIBRARY_PATH"));
+ if (result < 0 || result >= sizeof(extra_envvars))
+ {
+ printf("extra_envars too small for DYLD_LIBRARY_PATH");
+ }
+#endif
if (find_my_exec(argv0, retpath) < 0)
return -1;
@@ -344,7 +359,7 @@ find_other_exec(const char *argv0, const char *target,
if (validate_exec(retpath) != 0)
return -1;
- snprintf(cmd, sizeof(cmd), "\"%s\" -V", retpath);
+ snprintf(cmd, sizeof(cmd), "%s \"%s\" -V", extra_envvars, retpath);
if (!pipe_read_line(cmd, line, sizeof(line)))
return -1;
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [PATCH] pg_regress.c: Fix "make check" on Mac OS X: Pass DYLD_LIBRARY_PATH
@ 2023-06-23 21:05 Peter Eisentraut <[email protected]>
parent: David Zhang <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: Peter Eisentraut @ 2023-06-23 21:05 UTC (permalink / raw)
To: David Zhang <[email protected]>; [email protected]
On 22.06.23 21:08, David Zhang wrote:
> Currently, there is a description suggesting a workaround by running a
> 'make install' command first, but I find it to be somewhat inaccurate.
> It would be better to update the existing description to provide more
> precise instructions on how to overcome this issue. Here are the changes
> I would suggest.
>
> from:
> "You can work around that by doing make install before make check. Most
> PostgreSQL developers just turn off SIP, though."
>
> to:
> "You can execute sudo make install if you do not specify a prefix during
> the configure step, or make install without sudo if you do specify a
> prefix (assuming proper permissions) before make check. Most PostgreSQL
> developers just turn off SIP, though."
>
> Otherwise, following the current description, if you run `./configure &&
> make install` you will get error: "mkdir: /usr/local/pgsql: Permission
> denied"
I think you should interpret "doing make install" as "running make
install or a similar command as described earlier in this chapter".
Note also that the installation instructions don't use "sudo" anywhere
right now, so throwing it in at this point would be weird.
> echo "# +++ tap check in src/test/modules/brin +++"
> ... ...
> # +++ tap check in src/test/modules/brin +++
> t/01_workitems.pl ........ Bailout called. Further testing stopped:
> command "initdb -D
> /Users/david/hg/sandbox/postgres/src/test/modules/brin/tmp_check/t_01_workitems_tango_data/pgdata -A trust -N" died with signal 6
> t/01_workitems.pl ........ Dubious, test returned 255 (wstat 65280, 0xff00)
> No subtests run
As I mentioned earlier, you would need to find all uses of system() in
the PostgreSQL source code and make your adjustments there. IIRC, the
TAP tests require pg_ctl, so maybe look there.
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2023-06-23 21:05 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-03-12 20:32 [PATCH 05/10] (re)add wal_compression_method: lz4 Justin Pryzby <[email protected]>
2023-06-05 13:47 [PATCH] pg_regress.c: Fix "make check" on Mac OS X: Pass DYLD_LIBRARY_PATH Evan Jones <[email protected]>
2023-06-06 02:25 ` Re: [PATCH] pg_regress.c: Fix "make check" on Mac OS X: Pass DYLD_LIBRARY_PATH Julien Rouhaud <[email protected]>
2023-06-16 21:25 ` Re: [PATCH] pg_regress.c: Fix "make check" on Mac OS X: Pass DYLD_LIBRARY_PATH David Zhang <[email protected]>
2023-06-22 19:08 ` Re: [PATCH] pg_regress.c: Fix "make check" on Mac OS X: Pass DYLD_LIBRARY_PATH David Zhang <[email protected]>
2023-06-23 21:05 ` Re: [PATCH] pg_regress.c: Fix "make check" on Mac OS X: Pass DYLD_LIBRARY_PATH Peter Eisentraut <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox