sysupdate.d: Add way to drop binaries into $BOOT

[AdrianVovk]

As described in the BLS, we should place binaries into the XBOOTLDR directory if it is available, otherwise into the ESP. Thus, we might need to put binaries into /boot or into /efi depending on the existence of the XBOOTLDR partition.

With this change, if the Path= config option in the file starts with $BOOT, we replace that with either /boot or /efi, depending on what is available on the system.

[AdrianVovk]

I think this is ready for another look

[poettering]

anyway, I think we should do the PathRelativeTo= idea as per suggestions above. can you rework?

[AdrianVovk]

Done. I didn't implement the test as you suggested because it turns out to be rather finicky/annoying to get working (I'd have to figure out how to build sysupdate without the CLI so that I can graft a test onto it) and I feel isn't worth all that much (since it'd have to be manually updated and thus doesn't really prevent diverging...)

My work on the test before I scrapped it

diff --git a/src/sysupdate/meson.build b/src/sysupdate/meson.build
index 2f8c2305da..c42ca043c5 100644
--- a/src/sysupdate/meson.build
+++ b/src/sysupdate/meson.build
@@ -20,3 +20,22 @@ systemd_sysupdate_sources = files(
         'sysupdate.c',
         'sysupdate.h',
 )
+
+############################################################
+
+tests += [
+	{
+		'sources' : files(
+		        'sysupdate-resource.c',
+                        'sysupdate-resource.h',
+		        'test-resource.c',
+		 ),
+		'link_with' : [
+		        libshared,
+		        libshared_fdisk,
+		],
+		'dependencies': [
+		        libfdisk,
+		],
+	},
+]
diff --git a/src/sysupdate/sysupdate-resource.h b/src/sysupdate/sysupdate-resource.h
index 960f463c58..ee786311c2 100644
--- a/src/sysupdate/sysupdate-resource.h
+++ b/src/sysupdate/sysupdate-resource.h
@@ -68,7 +68,7 @@ static inline bool RESOURCE_IS_URL(ResourceType t) {
 
 typedef enum PathRelativeTo {
         /* Please make sure to folow the naming of the corresponding PartitionDesignator enum values,
-         * where this makes sense, like for the following three. */
+         * where this makes sense, like for the following cases. Make sure to update the test too! */
         PATH_RELATIVE_TO_ROOT,
         PATH_RELATIVE_TO_ESP,
         PATH_RELATIVE_TO_XBOOTLDR,
diff --git a/src/sysupdate/test-resource.c b/src/sysupdate/test-resource.c
new file mode 100644
index 0000000000..d9d5d1d967
--- /dev/null
+++ b/src/sysupdate/test-resource.c
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+
+/* Some unit tests for the helper functions in sysupdate-reousrce. */
+
+#include "gpt.h"
+#include "log.h"
+#include "sysupdate-resource.h"
+#include "tests.h"
+
+TEST(path_relative_to_to_string) {
+        /* Make sure that PathRelativeTo's to_string function matches
+         * those in PartitionDesignator */
+
+        struct {
+                PathRelativeTo relative_to;
+                PartitionDesignator designator;
+        } cases[] = {
+                { PATH_RELATIVE_TO_ROOT,     PARTITION_ROOT },
+                { PATH_RELATIVE_TO_ESP,      PARTITION_ESP },
+                { PATH_RELATIVE_TO_XBOOTLDR, PARTITION_XBOOTLDR },
+        };
+
+        for (size_t i = 0; i < ELEMENTSOF(cases); i++) {
+                assert(streq_ptr(path_relative_to_to_string(cases[i].relative_to),
+                                 partition_designator_to_string(cases[i].designator)));
+        }
+}
+
+DEFINE_TEST_MAIN(LOG_DEBUG);

[bluca]

There's an integration test that uses scripts, there is already one for sysupdate (TEST-72-SYSUPDATE) so it should be doable to update it to test this too

[poettering]

looks excellent, good to go if you address the oom issue. thanks!

[AdrianVovk]

Looks like CI failure is my fault. I'll investigate tomorrow

[AdrianVovk]

So CI was failing because the behavior of sysupdate changed in a way that breaks the pre-existing parts of the test. The test previously deleted /var/tmp/72-dirs, which is a destination that sysupdate is supposed to put files into. However, it would never recreate this directory, and instead sysupdate would do so

Prior to my changes, if --root was unset sysupdate wouldn't actually try to resolve the path in any way. Instead, it would just use the path as-specified by the user verbatim. It seems like this would eventually end up creating the destination directory. However, if you were to specify --root, sysupdate would chase the provided path and this could fail if the path doesn't exist.

With my changes, sysupdate chases the path even if --root is unspecified. This means that it will fail for a path that doesn't exist. I think this is reasonable behavior and I've updated the test to reflect (just added a mkdir call to create /var/tmp/72-dirs). However, if you intended the behavior where non-existing destination directories get created, I can rework the PR in another way if you prefer (probably by adding CHASE_NONEXISTENT and possibly even CHASE_MKDIR_0755 to the chase call?)

[poettering]

hmm, still ci failures

[AdrianVovk]

The test passed but then I had a silly oversight on cleanup :(

Anyway, should hopefully be fixed now 🤞. Thanks for your patience

[AdrianVovk]

Thanks!