Re: macOS SIP, next try - Mailing list pgsql-hackers

Mark Dilger <mark.dilger@enterprisedb.com> writes:
> See also https://www.postgresql.org/message-id/flat/18012hGLG6HJ9pQDkHAMYuwQKg%40sparkpost.com

Yeah.  As I recall from that thread and prior ones, the bottleneck is
really just /bin/sh: something, either the kernel or sh itself, is
clearing out DYLD_LIBRARY_PATH when a shell script starts.  (Since
PATH is not reset at the same time, this seems the height of
stupidity/uselessness; but I digress.)  But if "make" has a setting
for that variable, the setting *is* successfully passed down to
executed programs.  Which implies that make doesn't use the shell while
running stuff, which seems odd.  But it gives us a possible escape
hatch.

I experimented a bit, and found that the attached very-quick-hack
patch is enough to get through "make check" and also the isolation
tests.  Unsurprisingly, none of the TAP tests work.  I experimented
a bit and it looks like "prove" may be failing to pass down
DYLD_LIBRARY_PATH to the test scripts, which would be annoying but
could be worked around (say, by getting TestLib.pm to pull the
value from somewhere).

Based on what I've got here, this avenue probably ends up messier
and more invasive than the install_name_tool avenue, since we'd
likely have to teach quite a few places about explicitly passing
down DYLD_LIBRARY_PATH.  It does have the advantage that "make
check" doesn't have to test executables that are different from
the bits we'll really install; but that's probably not worth all
that much.  Unless someone can think of a cute way to centralize
the changes some more, I'm inclined to think this isn't the way
forward.  But it seemed worth posting the results anyway.

            regards, tom lane

diff --git a/src/common/exec.c b/src/common/exec.c
index 6cbc820042..4de79f94ef 100644
--- a/src/common/exec.c
+++ b/src/common/exec.c
@@ -323,8 +323,9 @@ int
 find_other_exec(const char *argv0, const char *target,
                 const char *versionstr, char *retpath)
 {
-    char        cmd[MAXPGPATH];
+    char        cmd[MAXPGPATH * 2];
     char        line[MAXPGPATH];
+    char       *dlp = getenv("DYLD_LIBRARY_PATH");

     if (find_my_exec(argv0, retpath) < 0)
         return -1;
@@ -340,7 +341,11 @@ find_other_exec(const char *argv0, const char *target,
     if (validate_exec(retpath) != 0)
         return -1;

-    snprintf(cmd, sizeof(cmd), "\"%s\" -V", retpath);
+    if (dlp)
+        snprintf(cmd, sizeof(cmd), "DYLD_LIBRARY_PATH=\"%s\" \"%s\" -V",
+                 dlp, retpath);
+    else
+        snprintf(cmd, sizeof(cmd), "\"%s\" -V", retpath);

     if (!pipe_read_line(cmd, line, sizeof(line)))
         return -1;
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 5cfb4c4a49..dbde78608a 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -77,6 +77,7 @@ bool        debug = false;
 char       *inputdir = ".";
 char       *outputdir = ".";
 char       *bindir = PGBINDIR;
+char       *setlibpath = NULL;
 char       *launcher = NULL;
 static _stringlist *loadextension = NULL;
 static int    max_connections = 0;
@@ -270,7 +271,8 @@ stop_postmaster(void)
         fflush(stderr);

         snprintf(buf, sizeof(buf),
-                 "\"%s%spg_ctl\" stop -D \"%s/data\" -s",
+                 "%s\"%s%spg_ctl\" stop -D \"%s/data\" -s",
+                 setlibpath,
                  bindir ? bindir : "",
                  bindir ? "/" : "",
                  temp_instance);
@@ -1135,7 +1137,8 @@ psql_command(const char *database, const char *query,...)

     /* And now we can build and execute the shell command */
     snprintf(psql_cmd, sizeof(psql_cmd),
-             "\"%s%spsql\" -X -c \"%s\" \"%s\"",
+             "%s\"%s%spsql\" -X -c \"%s\" \"%s\"",
+             setlibpath,
              bindir ? bindir : "",
              bindir ? "/" : "",
              query_escaped,
@@ -1187,7 +1190,7 @@ spawn_process(const char *cmdline)
          */
         char       *cmdline2;

-        cmdline2 = psprintf("exec %s", cmdline);
+        cmdline2 = psprintf("%sexec %s", setlibpath, cmdline);
         execl(shellprog, shellprog, "-c", cmdline2, (char *) NULL);
         fprintf(stderr, _("%s: could not exec \"%s\": %s\n"),
                 progname, shellprog, strerror(errno));
@@ -2290,6 +2293,15 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
     unlimit_core_size();
 #endif

+    {
+        char   *dlp = getenv("DYLD_LIBRARY_PATH");
+
+        if (dlp)
+            setlibpath = psprintf("DYLD_LIBRARY_PATH=\"%s\" ", dlp);
+        else
+            setlibpath = "";
+    }
+
     if (temp_instance)
     {
         FILE       *pg_conf;
@@ -2324,7 +2336,8 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
         /* initdb */
         header(_("initializing database system"));
         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",
+                 setlibpath,
                  bindir ? bindir : "",
                  bindir ? "/" : "",
                  temp_instance,
@@ -2397,7 +2410,8 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
          * 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",
+                 setlibpath,
                  bindir ? bindir : "",
                  bindir ? "/" : "",
                  DEVNULL, DEVNULL);