docs/PORTABLE_SERVICES.md

@@ -284,12 +284,9 @@ following must be also be observed:
 4. The upper extension(s) image(s) must at least contain one matching unit file each,
    with the right name prefix and suffix (see above).
 
-5. As with the base/OS image, the upper extension(s) image(s) must be a plain
-   sub-directory, a btrfs subvolume or a raw disk image.
-
 ```
 # portablectl attach --extension foobar_0.7.23.raw debian-runtime_11.1.raw foobar
-# portablectl attach --extension barbaz_7.0.23/ debian-runtime_11.1.raw barbaz
+# portablectl attach --extension barbaz_7.0.23.raw debian-runtime_11.1.raw barbaz
 ```
 
 ## Execution Environment

man/portablectl.xml

@@ -365,9 +365,8 @@
         The image(s) must contain an <filename>extension-release</filename> file with metadata that matches
         what is defined in the <filename>os-release</filename> of <replaceable>IMAGE</replaceable>. See:
         <citerefentry><refentrytitle>os-release</refentrytitle><manvolnum>5</manvolnum></citerefentry>.
-        Images can be block images, btrfs subvolumes or directories. For more information on portable
+        For more information on portable services with extensions, see the <literal>Extension Images</literal>
-        services with extensions, see the <literal>Extension Images</literal> paragraph on
+        paragraph on <ulink url="https://systemd.io/PORTABLE_SERVICES">Portable Services</ulink>.
-        <ulink url="https://systemd.io/PORTABLE_SERVICES">Portable Services</ulink>.
         </para>
 
         <para>Note that the same extensions have to be specified, in the same order, when attaching

man/systemd.exec.xml

@@ -2513,39 +2513,18 @@ SystemCallErrorNumber=EPERM</programlisting>
       <varlistentry>
         <term><varname>EnvironmentFile=</varname></term>
 
-        <listitem><para>Similar to <varname>Environment=</varname> but reads the environment variables from a text file.
+        <listitem><para>Similar to <varname>Environment=</varname> but reads the environment variables from a text
-        The text file should contain newline-separated variable assignments.  Empty lines, lines without an
+        file. The text file should contain new-line-separated variable assignments.  Empty lines, lines without an
-        <literal>=</literal> separator, or lines starting with <literal>;</literal> or <literal>#</literal> will be
+        <literal>=</literal> separator, or lines starting with ; or # will be ignored, which may be used for
-        ignored, which may be used for commenting. The file must be UTF-8 encoded. Valid characters are <ulink
+        commenting. A line ending with a backslash will be concatenated with the following one, allowing multiline
-        url="https://www.unicode.org/glossary/#unicode_scalar_value">unicode scalar values</ulink> other than <ulink
+        variable definitions. The parser strips leading and trailing whitespace from the values of assignments, unless
-        url="https://www.unicode.org/glossary/#noncharacter">noncharacters</ulink>, U+0000 NUL, and U+FEFF <ulink
+        you use double quotes (").</para>
-        url="https://www.unicode.org/glossary/#byte_order_mark">byte order mark</ulink>. Control codes other than NUL
-        are allowed.</para>
 
-        <para>In the file, an unquoted value after the <literal>=</literal> is parsed with the same backslash-escape
+        <para><ulink url="https://en.wikipedia.org/wiki/Escape_sequences_in_C#Table_of_escape_sequences">C escapes</ulink>
-        rules as <ulink
+        are supported, but not
-        url="https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_02_01">unquoted
+        <ulink url="https://en.wikipedia.org/wiki/Control_character#In_ASCII">most control characters</ulink>.
-        text</ulink> in a POSIX shell, but unlike in a shell, interior whitespace is preserved and quotes after the
+        <literal>\t</literal> and <literal>\n</literal> can be used to insert tabs and newlines within
-        first non-whitespace character are preserved. Leading and trailing whitespace (space, tab, carriage return) is
+        <varname>EnvironmentFile=</varname>.</para>
-        discarded, but interior whitespace within the line is preserved verbatim. A line ending with a backslash will be
-        continued to the following one, with the newline itself discarded. A backslash
-        <literal>\</literal> followed by any character other than newline will preserve the following character, so that
-        <literal>\\</literal> will become the value <literal>\</literal>.</para>
-
-        <para>In the file, a <literal>'</literal>-quoted value after the <literal>=</literal> can span multiple lines
-        and contain any character verbatim other than single quote, like <ulink
-        url="https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_02_02">single-quoted
-        text</ulink> in a POSIX shell. No backslash-escape sequences are recognized. Leading and trailing whitespace
-        outside of the single quotes is discarded.</para>
-
-        <para>In the file, a <literal>"</literal>-quoted value after the <literal>=</literal> can span multiple lines,
-        and the same escape sequences are recognized as in <ulink
-        url="https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_02_03">double-quoted
-        text</ulink> of a POSIX shell. Backslash (<literal>\</literal>) followed by any of <literal>"\`$</literal> will
-        preserve that character. A backslash followed by newline is a line continuation, and the newline itself is
-        discarded. A backslash followed by any other character is ignored; both the backslash and the following
-        character are preserved verbatim. Leading and trailing whitespace outside of the double quotes is
-        discarded.</para>
 
         <para>The argument passed should be an absolute filename or wildcard expression, optionally prefixed with
         <literal>-</literal>, which indicates that if the file does not exist, it will not be read and no error or
@@ -2578,6 +2557,12 @@ SystemCallErrorNumber=EPERM</programlisting>
         <para>Variables set for invoked processes due to this setting are subject to being overridden by those
         configured with <varname>Environment=</varname> or <varname>EnvironmentFile=</varname>.</para>
 
+        <para><ulink url="https://en.wikipedia.org/wiki/Escape_sequences_in_C#Table_of_escape_sequences">C escapes</ulink>
+        are supported, but not
+        <ulink url="https://en.wikipedia.org/wiki/Control_character#In_ASCII">most control characters</ulink>.
+        <literal>\t</literal> and <literal>\n</literal> can be used to insert tabs and newlines within
+        <varname>EnvironmentFile=</varname>.</para>
+
         <para>Example:
         <programlisting>PassEnvironment=VAR1 VAR2 VAR3</programlisting>
         passes three variables <literal>VAR1</literal>,

src/core/execute.c

@@ -5501,18 +5501,20 @@ static int exec_context_named_iofds(
         return targets == 0 ? 0 : -ENOENT;
 }
 
-static int exec_context_load_environment(const Unit *unit, const ExecContext *c, char ***ret) {
+static int exec_context_load_environment(const Unit *unit, const ExecContext *c, char ***l) {
-        _cleanup_strv_free_ char **v = NULL;
+        char **i, **r = NULL;
-        char **i;
-        int r;
 
         assert(c);
-        assert(ret);
+        assert(l);
 
         STRV_FOREACH(i, c->environment_files) {
-                _cleanup_globfree_ glob_t pglob = {};
+                char *fn;
+                int k;
                 bool ignore = false;
-                char *fn = *i;
+                char **p;
+                _cleanup_globfree_ glob_t pglob = {};
+
+                fn = *i;
 
                 if (fn[0] == '-') {
                         ignore = true;
@@ -5522,30 +5524,33 @@ static int exec_context_load_environment(const Unit *unit, const ExecContext *c,
                 if (!path_is_absolute(fn)) {
                         if (ignore)
                                 continue;
+
+                        strv_free(r);
                         return -EINVAL;
                 }
 
                 /* Filename supports globbing, take all matching files */
-                r = safe_glob(fn, 0, &pglob);
+                k = safe_glob(fn, 0, &pglob);
-                if (r < 0) {
+                if (k < 0) {
                         if (ignore)
                                 continue;
-                        return r;
+
+                        strv_free(r);
+                        return k;
                 }
 
                 /* When we don't match anything, -ENOENT should be returned */
                 assert(pglob.gl_pathc > 0);
 
                 for (unsigned n = 0; n < pglob.gl_pathc; n++) {
-                        _cleanup_strv_free_ char **p = NULL;
+                        k = load_env_file(NULL, pglob.gl_pathv[n], &p);
-
+                        if (k < 0) {
-                        r = load_env_file(NULL, pglob.gl_pathv[n], &p);
-                        if (r < 0) {
                                 if (ignore)
                                         continue;
-                                return r;
-                        }
 
+                                strv_free(r);
+                                return k;
+                        }
                         /* Log invalid environment variables with filename */
                         if (p) {
                                 InvalidEnvInfo info = {
@@ -5556,19 +5561,23 @@ static int exec_context_load_environment(const Unit *unit, const ExecContext *c,
                                 p = strv_env_clean_with_callback(p, invalid_env, &info);
                         }
 
-                        if (!v)
+                        if (!r)
-                                v = TAKE_PTR(p);
+                                r = p;
                         else {
-                                char **m = strv_env_merge(v, p);
+                                char **m;
+
+                                m = strv_env_merge(r, p);
+                                strv_free(r);
+                                strv_free(p);
                                 if (!m)
                                         return -ENOMEM;
 
-                                strv_free_and_replace(v, m);
+                                r = m;
                         }
                 }
         }
 
-        *ret = TAKE_PTR(v);
+        *l = r;
 
         return 0;
 }

src/portable/portable.c

@@ -253,7 +253,6 @@ static int extract_now(
 
                 FOREACH_DIRENT(de, d, return log_debug_errno(errno, "Failed to read directory: %m")) {
                         _cleanup_(portable_metadata_unrefp) PortableMetadata *m = NULL;
-                        _cleanup_(mac_selinux_freep) char *con = NULL;
                         _cleanup_close_ int fd = -1;
 
                         if (!unit_name_is_valid(de->d_name, UNIT_NAME_ANY))
@@ -275,16 +274,16 @@ static int extract_now(
                                 continue;
                         }
 
-#if HAVE_SELINUX
-                        /* The units will be copied on the host's filesystem, so if they had a SELinux label
-                         * we have to preserve it. Copy it out so that it can be applied later. */
-
-                        r = fgetfilecon_raw(fd, &con);
-                        if (r < 0 && errno != ENODATA)
-                                log_debug_errno(errno, "Failed to get SELinux file context from '%s', ignoring: %m", de->d_name);
-#endif
-
                         if (socket_fd >= 0) {
+                                _cleanup_(mac_selinux_freep) char *con = NULL;
+#if HAVE_SELINUX
+                                /* The units will be copied on the host's filesystem, so if they had a SELinux label
+                                 * we have to preserve it. Copy it out so that it can be applied later. */
+
+                                r = fgetfilecon_raw(fd, &con);
+                                if (r < 0 && errno != ENODATA)
+                                        log_debug_errno(errno, "Failed to get SELinux file context from '%s', ignoring: %m", de->d_name);
+#endif
                                 struct iovec iov[] = {
                                         IOVEC_MAKE_STRING(de->d_name),
                                         IOVEC_MAKE((char *)"\0", sizeof(char)),
@@ -296,7 +295,7 @@ static int extract_now(
                                         return log_debug_errno(r, "Failed to send unit metadata to parent: %m");
                         }
 
-                        m = portable_metadata_new(de->d_name, where, con, fd);
+                        m = portable_metadata_new(de->d_name, NULL, NULL, fd);
                         if (!m)
                                 return -ENOMEM;
                         fd = -1;
@@ -337,16 +336,10 @@ static int portable_extract_by_path(
 
         r = loop_device_make_by_path(path, O_RDONLY, LO_FLAGS_PARTSCAN, &d);
         if (r == -EISDIR) {
-                _cleanup_free_ char *image_name = NULL;
-
                 /* We can't turn this into a loop-back block device, and this returns EISDIR? Then this is a directory
                  * tree and not a raw device. It's easy then. */
 
-                r = path_extract_filename(path, &image_name);
+                r = extract_now(path, matches, NULL, path_is_extension, -1, &os_release, &unit_files);
-                if (r < 0)
-                        return log_error_errno(r, "Failed to extract image name from path '%s': %m", path);
-
-                r = extract_now(path, matches, image_name, path_is_extension, -1, &os_release, &unit_files);
                 if (r < 0)
                         return r;
 
@@ -879,10 +872,6 @@ static const char *root_setting_from_image(ImageType type) {
         return IN_SET(type, IMAGE_DIRECTORY, IMAGE_SUBVOLUME) ? "RootDirectory=" : "RootImage=";
 }
 
-static const char *extension_setting_from_image(ImageType type) {
-        return IN_SET(type, IMAGE_DIRECTORY, IMAGE_SUBVOLUME) ? "ExtensionDirectories=" : "ExtensionImages=";
-}
-
 static int make_marker_text(const char *image_path, OrderedHashmap *extension_images, char **ret_text) {
         _cleanup_free_ char *text = NULL, *escaped_image_path = NULL;
         Image *ext;
@@ -929,6 +918,7 @@ static int install_chroot_dropin(
                 size_t *n_changes) {
 
         _cleanup_free_ char *text = NULL, *dropin = NULL;
+        Image *ext;
         int r;
 
         assert(image_path);
@@ -946,7 +936,6 @@ static int install_chroot_dropin(
         if (endswith(m->name, ".service")) {
                 const char *os_release_source, *root_type;
                 _cleanup_free_ char *base_name = NULL;
-                Image *ext;
 
                 root_type = root_setting_from_image(type);
 
@@ -973,7 +962,7 @@ static int install_chroot_dropin(
 
                 if (m->image_path && !path_equal(m->image_path, image_path))
                         ORDERED_HASHMAP_FOREACH(ext, extension_images)
-                                if (!strextend(&text, extension_setting_from_image(ext->type), ext->path, "\n"))
+                                if (!strextend(&text, "ExtensionImages=", ext->path, "\n"))
                                         return -ENOMEM;
         }
 

src/test/test-env-file.c

@@ -13,11 +13,11 @@
         "a=a\n"                                 \
         "b=b\\\n"                               \
         "c\n"                                   \
-        "d= d\\\n"                              \
+        "d=d\\\n"                               \
-        "e  \\\n"                               \
+        "e\\\n"                                 \
-        "f  \n"                                 \
+        "f\n"                                   \
         "g=g\\ \n"                              \
-        "h= ąęół\\ śćńźżµ \n"                   \
+        "h=h\n"                                 \
         "i=i\\"
 
 #define env_file_2                              \
@@ -26,34 +26,22 @@
 #define env_file_3 \
         "#SPAMD_ARGS=\"-d --socketpath=/var/lib/bulwark/spamd \\\n" \
         "#--nouser-config                                     \\\n" \
-        "normal=line                                          \\\n" \
+        "normal=line"
-        ";normal=ignored                                      \\\n" \
-        "normal_ignored                                       \\\n" \
-        "normal ignored                                       \\\n"
 
-#define env_file_4                              \
+#define env_file_4 \
-        "# Generated\n"                         \
+       "# Generated\n" \
-        "\n"                                    \
+       "\n" \
-        "HWMON_MODULES=\"coretemp f71882fg\"\n" \
+       "HWMON_MODULES=\"coretemp f71882fg\"\n" \
-        "\n"                                    \
+       "\n" \
-        "# For compatibility reasons\n"         \
+       "# For compatibility reasons\n" \
-        "\n"                                    \
+       "\n" \
-        "MODULE_0=coretemp\n"                   \
+       "MODULE_0=coretemp\n" \
-        "MODULE_1=f71882fg"
+       "MODULE_1=f71882fg"
 
 #define env_file_5                              \
-        "a=\n"                                  \
+        "a=\n"                                 \
         "b="
 
-#define env_file_6                              \
-        "a=\\ \\n \\t \\x \\y \\' \n"           \
-        "b= \\$'                  \n"           \
-        "c= ' \\n\\t\\$\\`\\\\\n"               \
-        "'   \n"                                \
-        "d= \" \\n\\t\\$\\`\\\\\n"              \
-        "\"   \n"
-
-
 TEST(load_env_file_1) {
         _cleanup_strv_free_ char **data = NULL;
         int r;
@@ -69,9 +57,9 @@ TEST(load_env_file_1) {
         assert_se(r == 0);
         assert_se(streq(data[0], "a=a"));
         assert_se(streq(data[1], "b=bc"));
-        assert_se(streq(data[2], "d=de  f"));
+        assert_se(streq(data[2], "d=def"));
         assert_se(streq(data[3], "g=g "));
-        assert_se(streq(data[4], "h=ąęół śćńźżµ"));
+        assert_se(streq(data[4], "h=h"));
         assert_se(streq(data[5], "i=i"));
         assert_se(data[6] == NULL);
 }
@@ -145,26 +133,6 @@ TEST(load_env_file_5) {
         assert_se(data[2] == NULL);
 }
 
-TEST(load_env_file_6) {
-        _cleanup_strv_free_ char **data = NULL;
-        int r;
-
-        _cleanup_(unlink_tempfilep) char name[] = "/tmp/test-load-env-file.XXXXXX";
-        _cleanup_close_ int fd;
-
-        fd = mkostemp_safe(name);
-        assert_se(fd >= 0);
-        assert_se(write(fd, env_file_6, strlen(env_file_6)) == strlen(env_file_6));
-
-        r = load_env_file(NULL, name, &data);
-        assert_se(r == 0);
-        assert_se(streq(data[0], "a= n t x y '"));
-        assert_se(streq(data[1], "b=$'"));
-        assert_se(streq(data[2], "c= \\n\\t\\$\\`\\\\\n"));
-        assert_se(streq(data[3], "d= \\n\\t$`\\\n"));
-        assert_se(data[4] == NULL);
-}
-
 TEST(write_and_load_env_file) {
         const char *v;
 

src/test/test-load-fragment.c

@@ -769,70 +769,6 @@ TEST(config_parse_pass_environ) {
         assert_se(streq(passenv[0], "normal_name"));
 }
 
-TEST(config_parse_unit_env_file) {
-        /* int config_parse_unit_env_file(
-                 const char *unit,
-                 const char *filename,
-                 unsigned line,
-                 const char *section,
-                 unsigned section_line,
-                 const char *lvalue,
-                 int ltype,
-                 const char *rvalue,
-                 void *data,
-                 void *userdata) */
-
-        _cleanup_(manager_freep) Manager *m = NULL;
-        Unit *u;
-        _cleanup_strv_free_ char **files = NULL;
-        int r;
-
-        r = manager_new(UNIT_FILE_USER, MANAGER_TEST_RUN_MINIMAL, &m);
-        if (manager_errno_skip_test(r)) {
-                log_notice_errno(r, "Skipping test: manager_new: %m");
-                return;
-        }
-
-        assert_se(r >= 0);
-        assert_se(manager_startup(m, NULL, NULL, NULL) >= 0);
-
-        assert_se(u = unit_new(m, sizeof(Service)));
-        assert_se(unit_add_name(u, "foobar.service") == 0);
-
-        r = config_parse_unit_env_file(u->id, "fake", 1, "section", 1,
-                                      "EnvironmentFile", 0, "not-absolute",
-                                       &files, u);
-        assert_se(r == 0);
-        assert_se(strv_length(files) == 0);
-
-        r = config_parse_unit_env_file(u->id, "fake", 1, "section", 1,
-                                      "EnvironmentFile", 0, "/absolute1",
-                                       &files, u);
-        assert_se(r == 0);
-        assert_se(strv_length(files) == 1);
-
-        r = config_parse_unit_env_file(u->id, "fake", 1, "section", 1,
-                                      "EnvironmentFile", 0, "/absolute2",
-                                       &files, u);
-        assert_se(r == 0);
-        assert_se(strv_length(files) == 2);
-        assert_se(streq(files[0], "/absolute1"));
-        assert_se(streq(files[1], "/absolute2"));
-
-        r = config_parse_unit_env_file(u->id, "fake", 1, "section", 1,
-                                       "EnvironmentFile", 0, "",
-                                       &files, u);
-        assert_se(r == 0);
-        assert_se(strv_isempty(files));
-
-        r = config_parse_unit_env_file(u->id, "fake", 1, "section", 1,
-                                       "EnvironmentFile", 0, "/path/%n.conf",
-                                       &files, u);
-        assert_se(r == 0);
-        assert_se(strv_length(files) == 1);
-        assert_se(streq(files[0], "/path/foobar.service.conf"));
-}
-
 TEST(unit_dump_config_items) {
         unit_dump_config_items(stdout);
 }

test/units/testsuite-29.sh

@@ -103,8 +103,7 @@ portablectl detach --now --runtime --extension /usr/share/app1.raw /usr/share/mi
 
 # portablectl also works with directory paths rather than images
 
-mkdir /tmp/rootdir /tmp/app0 /tmp/app1 /tmp/overlay /tmp/os-release-fix /tmp/os-release-fix/etc
+mkdir /tmp/rootdir /tmp/app1 /tmp/overlay /tmp/os-release-fix /tmp/os-release-fix/etc
-mount /usr/share/app0.raw /tmp/app0
 mount /usr/share/app1.raw /tmp/app1
 mount /usr/share/minimal_0.raw /tmp/rootdir
 
@@ -125,16 +124,7 @@ systemctl is-active app1.service
 portablectl detach --now --runtime overlay app1
 
 umount /tmp/overlay
-
-portablectl "${ARGS[@]}" attach --copy=symlink --now --runtime --extension /tmp/app0 --extension /tmp/app1 /tmp/rootdir app0 app1
-
-systemctl is-active app0.service
-systemctl is-active app1.service
-
-portablectl detach --now --runtime --extension /tmp/app0 --extension /tmp/app1 /tmp/rootdir app0 app1
-
 umount /tmp/rootdir
-umount /tmp/app0
 umount /tmp/app1
 
 echo OK >/testok