core: allow split /usr/local/s?sbin with merged /usr/s?bin

Previously, we used either the fully split path or the fully merged path,
treating "split sbin" as a boolean condition. The idea was that conversion to
to merged bin would be a single event, so we don't need to care about the
details of the transition. But it turns out that some systems may be converted
in disparate steps. In https://bugzilla.redhat.com/show_bug.cgi?id=2400220,
there was a lengthy discussion about a coreos system where
/usr/local/{bin,sbin} were created as separate directories. Since /usr/local is
not part of the packaged system, it might remain split for a longer time. So
check /usr/local/s?bin separately and stop adding /usr/sbin to $PATH if only
/usr/local/s?bin is split. (I don't think it makes sense to handle the reverse
case, i.e. only /usr/s?bin being split, since that should be much rarer.)

Inspired by https://bugzilla.redhat.com/show_bug.cgi?id=2400220.

(cherry picked from commit e63917abe16c37c828f99710f1e9922093d9a2b9)

diff --git a/src/basic/path-util.c b/src/basic/path-util.c
index 36cf9a16491a53ff1c416a211b7999da5b63b05d..0972dd8de0c9715910bd86e0092eca17f0f0afa9 100644 (file)
--- a/src/basic/path-util.c
+++ b/src/basic/path-util.c
@@ -1518,30 +1518,40 @@ int path_glob_can_match(const char *pattern, const char *prefix, char **ret) {
         return false;
 }
 
-const char* default_PATH(void) {
 #if HAVE_SPLIT_BIN
-        static int split = -1;
+static bool dir_is_split(const char *a, const char *b) {
         int r;
 
-        /* Check whether /usr/sbin is not a symlink and return the appropriate $PATH.
-         * On error fall back to the safe value with both directories as configured… */
-
-        if (split < 0)
-                STRV_FOREACH_PAIR(bin, sbin, STRV_MAKE("/usr/bin", "/usr/sbin",
-                                                       "/usr/local/bin", "/usr/local/sbin")) {
-                        r = inode_same(*bin, *sbin, AT_NO_AUTOMOUNT);
-                        if (r > 0 || r == -ENOENT)
-                                continue;
-                        if (r < 0)
-                                log_debug_errno(r, "Failed to compare \"%s\" and \"%s\", using compat $PATH: %m",
-                                                *bin, *sbin);
-                        split = true;
-                        break;
-                }
-        if (split < 0)
-                split = false;
-        if (split)
-                return DEFAULT_PATH_WITH_SBIN;
+        r = inode_same(a, b, AT_NO_AUTOMOUNT);
+        if (r < 0 && r != -ENOENT) {
+                log_debug_errno(r, "Failed to compare \"%s\" and \"%s\", assuming split directories: %m", a, b);
+                return true;
+        }
+        return r == 0;
+}
 #endif
+
+const char* default_PATH(void) {
+#if HAVE_SPLIT_BIN
+        static const char *default_path = NULL;
+
+        /* Return one of the three sets of paths:
+         * a) split /usr/s?bin, /usr/local/sbin doesn't matter.
+         * b) merged /usr/s?bin, /usr/sbin is a symlink, but /usr/local/sbin is not,
+         * c) fully merged, neither /usr/sbin nor /usr/local/sbin are symlinks,
+         *
+         * On error the fallback to the safe value with both directories as configured is returned.
+         */
+
+        if (default_path)
+                return default_path;
+
+        if (dir_is_split("/usr/sbin", "/usr/bin"))
+                return (default_path = DEFAULT_PATH_WITH_FULL_SBIN);  /* a */
+        if (dir_is_split("/usr/local/sbin", "/usr/local/bin"))
+                return (default_path = DEFAULT_PATH_WITH_LOCAL_SBIN); /* b */
+        return (default_path = DEFAULT_PATH_WITHOUT_SBIN);            /* c */
+#else
         return DEFAULT_PATH_WITHOUT_SBIN;
+#endif
 }

diff --git a/src/basic/path-util.h b/src/basic/path-util.h
index 45518c07e50cefb19403d934bd962c3d46fbeec2..a3a82574ddb122c9b4284cf664450a5e60fd3421 100644 (file)
--- a/src/basic/path-util.h
+++ b/src/basic/path-util.h
@@ -9,10 +9,11 @@
 #define PATH_MERGED_BIN(x) x "bin"
 #define PATH_MERGED_BIN_NULSTR(x) x "bin\0"
 
-#define DEFAULT_PATH_WITH_SBIN PATH_SPLIT_BIN("/usr/local/") ":" PATH_SPLIT_BIN("/usr/")
+#define DEFAULT_PATH_WITH_FULL_SBIN PATH_SPLIT_BIN("/usr/local/") ":" PATH_SPLIT_BIN("/usr/")
+#define DEFAULT_PATH_WITH_LOCAL_SBIN PATH_SPLIT_BIN("/usr/local/") ":" PATH_MERGED_BIN("/usr/")
 #define DEFAULT_PATH_WITHOUT_SBIN PATH_MERGED_BIN("/usr/local/") ":" PATH_MERGED_BIN("/usr/")
 
-#define DEFAULT_PATH_COMPAT PATH_SPLIT_BIN("/usr/local/") ":" PATH_SPLIT_BIN("/usr/") ":" PATH_SPLIT_BIN("/")
+#define DEFAULT_PATH_COMPAT DEFAULT_PATH_WITH_FULL_SBIN ":" PATH_SPLIT_BIN("/")
 
 const char* default_PATH(void);