diff --git a/TODO b/TODO index 44fe53f68f..c6c00a5e9b 100644 --- a/TODO +++ b/TODO @@ -813,9 +813,6 @@ Features: * systemctl: if some operation fails, show log output? -* systemctl edit: use equivalent of cat() to insert existing config as a comment, prepended with #. - Upon editor exit, lines with one # are removed, lines with two # are left with one #, etc. - * exponential backoff in timesyncd when we cannot reach a server * timesyncd: add ugly bus calls to set NTP servers per-interface, for usage by NM diff --git a/src/basic/fileio.c b/src/basic/fileio.c index c5a093a857..050c8709f8 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -252,7 +252,8 @@ int write_string_file_ts( /* We manually build our own version of fopen(..., "we") that works without O_CREAT and with O_NOFOLLOW if needed. */ fd = open(fn, O_WRONLY|O_CLOEXEC|O_NOCTTY | (FLAGS_SET(flags, WRITE_STRING_FILE_NOFOLLOW) ? O_NOFOLLOW : 0) | - (FLAGS_SET(flags, WRITE_STRING_FILE_CREATE) ? O_CREAT : 0), + (FLAGS_SET(flags, WRITE_STRING_FILE_CREATE) ? O_CREAT : 0) | + (FLAGS_SET(flags, WRITE_STRING_FILE_TRUNCATE) ? O_TRUNC : 0), (FLAGS_SET(flags, WRITE_STRING_FILE_MODE_0600) ? 0600 : 0666)); if (fd < 0) { r = -errno; diff --git a/src/basic/fileio.h b/src/basic/fileio.h index 9cba5a90e3..963d7d08fc 100644 --- a/src/basic/fileio.h +++ b/src/basic/fileio.h @@ -16,14 +16,15 @@ typedef enum { WRITE_STRING_FILE_CREATE = 1 << 0, - WRITE_STRING_FILE_ATOMIC = 1 << 1, - WRITE_STRING_FILE_AVOID_NEWLINE = 1 << 2, - WRITE_STRING_FILE_VERIFY_ON_FAILURE = 1 << 3, - WRITE_STRING_FILE_SYNC = 1 << 4, - WRITE_STRING_FILE_DISABLE_BUFFER = 1 << 5, - WRITE_STRING_FILE_NOFOLLOW = 1 << 6, - WRITE_STRING_FILE_MKDIR_0755 = 1 << 7, - WRITE_STRING_FILE_MODE_0600 = 1 << 8, + WRITE_STRING_FILE_TRUNCATE = 1 << 1, + WRITE_STRING_FILE_ATOMIC = 1 << 2, + WRITE_STRING_FILE_AVOID_NEWLINE = 1 << 3, + WRITE_STRING_FILE_VERIFY_ON_FAILURE = 1 << 4, + WRITE_STRING_FILE_SYNC = 1 << 5, + WRITE_STRING_FILE_DISABLE_BUFFER = 1 << 6, + WRITE_STRING_FILE_NOFOLLOW = 1 << 7, + WRITE_STRING_FILE_MKDIR_0755 = 1 << 8, + WRITE_STRING_FILE_MODE_0600 = 1 << 9, /* And before you wonder, why write_string_file_atomic_label_ts() is a separate function instead of just one more flag here: it's about linking: we don't want to pull -lselinux into all users of write_string_file() diff --git a/src/basic/strv.c b/src/basic/strv.c index b2b6de388a..c5e6dd5f21 100644 --- a/src/basic/strv.c +++ b/src/basic/strv.c @@ -537,6 +537,19 @@ int strv_consume_prepend(char ***l, char *value) { return r; } +int strv_prepend(char ***l, const char *value) { + char *v; + + if (!value) + return 0; + + v = strdup(value); + if (!v) + return -ENOMEM; + + return strv_consume_prepend(l, v); +} + int strv_extend(char ***l, const char *value) { char *v; diff --git a/src/basic/strv.h b/src/basic/strv.h index 195dbe4ca9..a85c289d73 100644 --- a/src/basic/strv.h +++ b/src/basic/strv.h @@ -34,6 +34,7 @@ size_t strv_length(char * const *l) _pure_; int strv_extend_strv(char ***a, char * const *b, bool filter_duplicates); int strv_extend_strv_concat(char ***a, char * const *b, const char *suffix); +int strv_prepend(char ***l, const char *value); int strv_extend(char ***l, const char *value); int strv_extendf(char ***l, const char *format, ...) _printf_(2,0); int strv_extend_front(char ***l, const char *value); diff --git a/src/systemctl/systemctl-edit.c b/src/systemctl/systemctl-edit.c index f4b256335a..d6f595d477 100644 --- a/src/systemctl/systemctl-edit.c +++ b/src/systemctl/systemctl-edit.c @@ -2,6 +2,8 @@ #include "bus-error.h" #include "copy.h" +#include "fd-util.h" +#include "fileio.h" #include "fs-util.h" #include "mkdir.h" #include "pager.h" @@ -17,6 +19,9 @@ #include "terminal-util.h" #include "tmpfile-util.h" +#define EDIT_MARKER_START "### Anything between here and the comment below will become the new contents of the file" +#define EDIT_MARKER_END "### Lines below this comment will be discarded" + int cat(int argc, char *argv[], void *userdata) { _cleanup_(hashmap_freep) Hashmap *cached_name_map = NULL, *cached_id_map = NULL; _cleanup_(lookup_paths_free) LookupPaths lp = {}; @@ -106,12 +111,11 @@ int cat(int argc, char *argv[], void *userdata) { return rc; } -static int create_edit_temp_file(const char *new_path, const char *original_path, char **ret_tmp_fn) { +static int create_edit_temp_file(const char *new_path, const char *original_path, char ** const original_unit_paths, char **ret_tmp_fn) { _cleanup_free_ char *t = NULL; int r; assert(new_path); - assert(original_path); assert(ret_tmp_fn); r = tempfn_random(new_path, NULL, &t); @@ -122,25 +126,78 @@ static int create_edit_temp_file(const char *new_path, const char *original_path if (r < 0) return log_error_errno(r, "Failed to create directories for \"%s\": %m", new_path); - r = mac_selinux_create_file_prepare(original_path, S_IFREG); - if (r < 0) - return r; + if (original_path) { + r = mac_selinux_create_file_prepare(new_path, S_IFREG); + if (r < 0) + return r; - r = copy_file(original_path, t, 0, 0644, 0, 0, COPY_REFLINK); - if (r == -ENOENT) { + r = copy_file(original_path, t, 0, 0644, 0, 0, COPY_REFLINK); + if (r == -ENOENT) { + r = touch(t); + mac_selinux_create_file_clear(); + if (r < 0) + return log_error_errno(r, "Failed to create temporary file \"%s\": %m", t); + } else { + mac_selinux_create_file_clear(); + if (r < 0) + return log_error_errno(r, "Failed to create temporary file for \"%s\": %m", new_path); + } + } else if (original_unit_paths) { + _cleanup_free_ char *new_contents = NULL; + _cleanup_fclose_ FILE *f = NULL; + char **path; + size_t size; - r = touch(t); + r = mac_selinux_create_file_prepare(new_path, S_IFREG); + if (r < 0) + return r; + f = fopen(t, "we"); mac_selinux_create_file_clear(); + if (!f) + return log_error_errno(errno, "Failed to open \"%s\": %m", t); + r = fchmod(fileno(f), 0644); + if (r < 0) + return log_error_errno(errno, "Failed to change mode of \"%s\": %m", t); + + r = read_full_file(new_path, &new_contents, &size); + if (r < 0 && r != -ENOENT) + return log_error_errno(r, "Failed to read \"%s\": %m", new_path); + + fprintf(f, + "### Editing %s\n" + EDIT_MARKER_START + "\n\n%s%s\n" + EDIT_MARKER_END, + new_path, + strempty(new_contents), + new_contents && endswith(new_contents, "\n") ? "" : "\n"); + + /* Add a comment with the contents of the original unit files */ + STRV_FOREACH(path, original_unit_paths) { + _cleanup_free_ char *contents = NULL; + + /* Skip the file that's being edited */ + if (path_equal(*path, new_path)) + continue; + + r = read_full_file(*path, &contents, &size); + if (r < 0) + return log_error_errno(r, "Failed to read \"%s\": %m", *path); + + fprintf(f, "\n\n### %s", *path); + if (!isempty(contents)) { + contents = strreplace(strstrip(contents), "\n", "\n# "); + if (!contents) + return log_oom(); + fprintf(f, "\n# %s", contents); + } + } + + r = fflush_and_check(f); if (r < 0) return log_error_errno(r, "Failed to create temporary file \"%s\": %m", t); - - } else { - mac_selinux_create_file_clear(); - - if (r < 0) - return log_error_errno(r, "Failed to create temporary file for \"%s\": %m", new_path); } *ret_tmp_fn = TAKE_PTR(t); @@ -185,6 +242,7 @@ static int unit_file_create_new( const LookupPaths *paths, const char *unit_name, const char *suffix, + char ** const original_unit_paths, char **ret_new_path, char **ret_tmp_path) { @@ -201,7 +259,7 @@ static int unit_file_create_new( if (r < 0) return r; - r = create_edit_temp_file(new_path, new_path, &tmp_path); + r = create_edit_temp_file(new_path, NULL, original_unit_paths, &tmp_path); if (r < 0) return r; @@ -240,7 +298,7 @@ static int unit_file_create_copy( return log_warning_errno(SYNTHETIC_ERRNO(EKEYREJECTED), "%s skipped.", unit_name); } - r = create_edit_temp_file(new_path, fragment_path, &tmp_path); + r = create_edit_temp_file(new_path, fragment_path, NULL, &tmp_path); if (r < 0) return r; @@ -332,9 +390,10 @@ static int find_paths_to_edit(sd_bus *bus, char **names, char ***paths) { STRV_FOREACH(name, names) { _cleanup_free_ char *path = NULL, *new_path = NULL, *tmp_path = NULL, *tmp_name = NULL; + _cleanup_strv_free_ char **unit_paths = NULL; const char *unit_name; - r = unit_find_paths(bus, *name, &lp, false, &cached_name_map, &cached_id_map, &path, NULL); + r = unit_find_paths(bus, *name, &lp, false, &cached_name_map, &cached_id_map, &path, &unit_paths); if (r == -EKEYREJECTED) { /* If loading of the unit failed server side complete, then the server won't tell us * the unit file path. In that case, find the file client side. */ @@ -361,7 +420,7 @@ static int find_paths_to_edit(sd_bus *bus, char **names, char ***paths) { unit_name = *name; r = unit_file_create_new(&lp, unit_name, arg_full ? NULL : ".d/override.conf", - &new_path, &tmp_path); + NULL, &new_path, &tmp_path); } else { assert(path); @@ -384,8 +443,13 @@ static int find_paths_to_edit(sd_bus *bus, char **names, char ***paths) { if (arg_full) r = unit_file_create_copy(&lp, unit_name, path, &new_path, &tmp_path); - else - r = unit_file_create_new(&lp, unit_name, ".d/override.conf", &new_path, &tmp_path); + else { + r = strv_prepend(&unit_paths, path); + if (r < 0) + return log_oom(); + + r = unit_file_create_new(&lp, unit_name, ".d/override.conf", unit_paths, &new_path, &tmp_path); + } } if (r < 0) return r; @@ -400,6 +464,40 @@ static int find_paths_to_edit(sd_bus *bus, char **names, char ***paths) { return 0; } +static int trim_edit_markers(const char *path) { + _cleanup_free_ char *contents = NULL; + char *contents_start = NULL; + const char *contents_end = NULL; + size_t size; + int r; + + /* Trim out the lines between the two markers */ + r = read_full_file(path, &contents, &size); + if (r < 0) + return log_error_errno(r, "Failed to read temporary file \"%s\": %m", path); + + contents_start = strstr(contents, EDIT_MARKER_START); + if (contents_start) + contents_start += strlen(EDIT_MARKER_START); + else + contents_start = contents; + + contents_end = strstr(contents_start, EDIT_MARKER_END); + if (contents_end) + strshorten(contents_start, contents_end - contents_start); + + contents_start = strstrip(contents_start); + + /* Write new contents if the trimming actually changed anything */ + if (strlen(contents) != size) { + r = write_string_file(path, contents_start, WRITE_STRING_FILE_CREATE | WRITE_STRING_FILE_TRUNCATE | WRITE_STRING_FILE_AVOID_NEWLINE); + if (r < 0) + return log_error_errno(r, "Failed to modify temporary file \"%s\": %m", path); + } + + return 0; +} + int edit(int argc, char *argv[], void *userdata) { _cleanup_(lookup_paths_free) LookupPaths lp = {}; _cleanup_strv_free_ char **names = NULL; @@ -452,6 +550,10 @@ int edit(int argc, char *argv[], void *userdata) { STRV_FOREACH_PAIR(original, tmp, paths) { /* If the temporary file is empty we ignore it. This allows the user to cancel the * modification. */ + r = trim_edit_markers(*tmp); + if (r < 0) + continue; + if (null_or_empty_path(*tmp)) { log_warning("Editing \"%s\" canceled: temporary file is empty.", *original); continue;