diff --git a/src/swupd_lib/target_root.c b/src/swupd_lib/target_root.c index 14d8fca1..1cdd132f 100644 --- a/src/swupd_lib/target_root.c +++ b/src/swupd_lib/target_root.c @@ -21,6 +21,7 @@ * */ +#include #include #include #include @@ -29,6 +30,7 @@ #include #include #include +#include #include #include "swupd.h" @@ -36,6 +38,7 @@ #include "xattrs.h" #define STAGE_FILE_PREFIX ".update." +#define BACKUP_FILE_PREFIX ".deleted." static bool verify_fix_path(char *dir, struct manifest *mom); @@ -330,6 +333,63 @@ static enum swupd_code install_dir(const char *fullfile_path, const char *target return install_dir_using_tar(fullfile_path, target_file); } +/* Backup target directory to a backup and timestamp prefixed name. */ +/* This function should only be used after the deletes are processed. */ +static enum swupd_code move_to_timestamp(const char *dirname) +{ + struct tm *timeinfo; + time_t now; + char time_str[50] = { 0 }; + char *backup_dir = NULL; + int ret = SWUPD_OK; + char *target_basename; + char *dir = NULL; + char *target_dir = NULL; + char *target_path = NULL; + + dir = sys_dirname(dirname); + target_basename = sys_basename(dirname); + target_path = sys_path_join("%s/%s", globals.path_prefix, dir); + target_dir = sys_path_join("%s/%s", globals.path_prefix, dirname); + + time(&now); + timeinfo = localtime(&now); + if (!timeinfo) { + ret = SWUPD_TIME_UNKNOWN; + goto out; + } + + strftime(time_str, sizeof(time_str), "%s", timeinfo); + backup_dir = sys_path_join("%s/%s%s.%s", target_path, BACKUP_FILE_PREFIX, time_str, target_basename); + + /* It shouldn't be possible to have an already created backup_dir + but just in case, try and remove it and error out if unable to. */ + if (sys_file_exists(backup_dir)) { + ret = sys_rm_recursive(backup_dir); + if (ret != 0 && ret != -ENOENT) { + ret = SWUPD_COULDNT_REMOVE_FILE; + error("Failed to remove backup dir %s\n", backup_dir); + goto out; + } + } + + ret = rename(target_dir, backup_dir); + if (ret != 0) { + ret = SWUPD_COULDNT_RENAME_DIR; + goto out; + } + +out: + FREE(dir); + FREE(target_dir); + FREE(target_path); + + if (backup_dir) { + FREE(backup_dir); + } + return ret; +} + /* Do the staging of new files into the filesystem */ // TODO: "stage_single_file is currently not able to be run in parallel" /* Consider adding a remove_leftovers() that runs in verify/fix in order to @@ -389,11 +449,13 @@ static enum swupd_code stage_single_file(struct file *file, struct manifest *mom if (file_type_changed(target_file, file)) { // file type changed, move old out of the way for new debug("The file type changed for file %s, removing old file\n", target_file); - ret = sys_rm_recursive(target_file); + ret = sys_rm(target_file); if (ret != 0 && ret != -ENOENT) { - error("Target has different file type but could not be removed: %s\n", target_file); - ret = SWUPD_COULDNT_REMOVE_FILE; - goto out; + ret = move_to_timestamp(file->filename); + if (ret != 0) { + error("Target has different file type but could not be moved to backup location: %s\n", target_file); + goto out; + } } } @@ -554,11 +616,11 @@ int rename_staged_file_to_final(struct file *file) * or we might end up deleting something else */ target_path = sys_dirname(target); if (sys_path_is_absolute(target_path)) { - ret = sys_rm_recursive(target); + ret = sys_rm(target); /* don't count missing ones as errors... * if somebody already deleted them for us then all is well */ - if ((ret == -ENOENT) || (ret == -ENOTDIR)) { + if (ret == -ENOENT) { ret = 0; } } @@ -566,42 +628,12 @@ int rename_staged_file_to_final(struct file *file) } else if (file->is_dir || file->is_ghosted) { ret = 0; } else { - /* If the file was previously a directory but no longer, then - * we need to move it out of the way. - * This should not happen because the server side complains - * when creating update content that includes such a state - * change. But...you never know. */ - - if (sys_is_dir(target)) { - char *lostnfound; - char *base; - - lostnfound = sys_path_join("%s/lost+found", globals.path_prefix); - ret = mkdir(lostnfound, S_IRWXU); - if ((ret != 0) && (errno != EEXIST)) { - FREE(lostnfound); - FREE(target); - return ret; - } - FREE(lostnfound); - - base = sys_basename(file->filename); - lostnfound = sys_path_join("%s/lost+found/%s", globals.path_prefix, base); - /* this will fail if the directory was not already emptied */ - ret = rename(target, lostnfound); - if (ret < 0 && errno != ENOTEMPTY && errno != EEXIST) { - error("failed to move %s to lost+found: %s\n", - base, strerror(errno)); - } - FREE(lostnfound); - } else { - ret = rename(file->staging, target); - if (ret < 0) { - error("failed to rename staged %s to final: %s\n", - file->hash, strerror(errno)); - } - unlink(file->staging); + ret = rename(file->staging, target); + if (ret < 0) { + error("failed to rename staged %s to final: %s\n", + file->hash, strerror(errno)); } + unlink(file->staging); } FREE(target); @@ -611,7 +643,8 @@ int rename_staged_file_to_final(struct file *file) static int rename_all_files_to_final(struct list *updates) { int ret, update_errs = 0; - struct list *list; + struct list *dirs_to_remove = NULL; + struct list *list, *dirs_list; int complete = 0; int list_length = list_len(updates); @@ -623,7 +656,9 @@ static int rename_all_files_to_final(struct list *updates) if (!file->do_not_update) { ret = rename_staged_file_to_final(file); - if (ret != 0) { + if (ret == -ENOTEMPTY) { + dirs_to_remove = list_prepend_data(dirs_to_remove, file); + } else if (ret != 0) { update_errs++; } } @@ -632,6 +667,25 @@ static int rename_all_files_to_final(struct list *updates) progress_report(list_length + complete, list_length * 2); } + dirs_list = list_head(dirs_to_remove); + while (dirs_list) { + struct file *file; + file = dirs_list->data; + dirs_list = dirs_list->next; + + ret = rename_staged_file_to_final(file); + if (ret != 0) { + ret = move_to_timestamp(file->filename); + if (ret != 0) { + error("Could not move file to backup location: %s\n", file->filename); + } + } + if (ret != 0) { + update_errs++; + } + } + list_free_list(dirs_to_remove); + return -update_errs; } @@ -732,21 +786,30 @@ enum swupd_code target_root_install_files(struct list *files, struct manifest *m int target_root_remove_files(struct list *files) { struct list *iter = NULL; + struct list *dirs_to_remove = NULL; + struct list *dirs_iter = NULL; struct file *file = NULL; char *fullfile = NULL; int total = list_len(files); int deleted = total; int count = 0; + int ret; iter = list_head(files); while (iter) { file = iter->data; iter = iter->next; fullfile = sys_path_join("%s/%s", globals.path_prefix, file->filename); - if (sys_rm_recursive(fullfile) == -1) { - /* if a -1 is returned it means there was an issue deleting the - * file or directory, in that case decrease the counter of deleted - * files. + ret = sys_rm(fullfile); + if (ret == -ENOTEMPTY) { + /* The directory couldn't be deleted right away so add it to + * the directory specific remove list to try again, and update + * count to indicate processing is still needed reporting. */ + count--; + dirs_to_remove = list_prepend_data(dirs_to_remove, file); + } else if (ret != 0) { + /* There was an issue deleting the file or directory, in that + * case decrease the counter of deleted files. * Note: If a file didn't exist it will still be counted as deleted, * this is a limitation */ deleted--; @@ -756,5 +819,28 @@ int target_root_remove_files(struct list *files) progress_report(count, total); } + dirs_iter = list_head(dirs_to_remove); + while (dirs_iter) { + file = dirs_iter->data; + dirs_iter = dirs_iter->next; + fullfile = sys_path_join("%s/%s", globals.path_prefix, file->filename); + ret = sys_rm(fullfile); + if (ret == -ENOTEMPTY) { + ret = move_to_timestamp(file->filename); + if (ret != 0) { + /* As above, an issue was encountered deleting the directory, so + * decrease the counter of deleted files. */ + error("Could not move file to backup location: %s\n", file->filename); + deleted--; + } + } else if (ret != 0) { + deleted--; + } + FREE(fullfile); + count++; + progress_report(count, total); + } + list_free_list(dirs_to_remove); + return deleted; } diff --git a/test/functional/bundleremove/remove-basics.bats b/test/functional/bundleremove/remove-basics.bats index ad7f3446..38ffb9f5 100755 --- a/test/functional/bundleremove/remove-basics.bats +++ b/test/functional/bundleremove/remove-basics.bats @@ -31,6 +31,7 @@ test_teardown() { @test "REM001: Removing one bundle" { + sudo mkdir "$TARGET_DIR"/bar/keep1 run sudo sh -c "$SWUPD bundle-remove $SWUPD_OPTS test-bundle1" assert_status_is 0 @@ -42,6 +43,8 @@ test_teardown() { assert_dir_not_exists "$TARGET_DIR"/foo assert_dir_not_exists "$TARGET_DIR"/bar assert_file_not_exists "$STATE_DIR"/bundles/test-bundle1 + # keep file isn't removed + assert_file_exists "$TARGET_DIR"/.deleted.*.bar/keep1 # bundle2 was not removed assert_file_exists "$TARGET_DIR"/usr/share/clear/bundles/test-bundle2 assert_file_exists "$TARGET_DIR"/bat/test-file4 @@ -55,7 +58,6 @@ test_teardown() { Successfully removed 1 bundle EOM ) - assert_is_output "$expected_output" } diff --git a/test/functional/repair/repair-error.bats b/test/functional/repair/repair-error.bats index 13b084c0..c258a013 100755 --- a/test/functional/repair/repair-error.bats +++ b/test/functional/repair/repair-error.bats @@ -47,12 +47,12 @@ test_teardown() { Validate downloaded files Starting download of remaining update content. This may take a while... Adding any missing files - Error: Target has different file type but could not be removed: $ABS_TARGET_DIR/baz + Error: Target has different file type but could not be moved to backup location: $ABS_TARGET_DIR/baz -> Missing file: $ABS_TARGET_DIR/baz/bat -> not fixed - Error: Target has different file type but could not be removed: $ABS_TARGET_DIR/baz + Error: Target has different file type but could not be moved to backup location: $ABS_TARGET_DIR/baz -> Missing file: $ABS_TARGET_DIR/baz/bat/file_3 -> not fixed Repairing corrupt files - Error: Target has different file type but could not be removed: $ABS_TARGET_DIR/baz + Error: Target has different file type but could not be moved to backup location: $ABS_TARGET_DIR/baz -> Hash mismatch for file: $ABS_TARGET_DIR/baz -> not fixed -> Hash mismatch for file: $ABS_TARGET_DIR/foo/file_1 -> fixed -> Hash mismatch for file: $ABS_TARGET_DIR/usr/lib/os-release -> fixed @@ -83,11 +83,11 @@ test_teardown() { assert_status_is_not "$SWUPD_OK" expected_output=$(cat <<-EOM - Error: Target has different file type but could not be removed: $ABS_TARGET_DIR/baz + Error: Target has different file type but could not be moved to backup location: $ABS_TARGET_DIR/baz $ABS_TARGET_DIR/baz/bat -> not fixed - Error: Target has different file type but could not be removed: $ABS_TARGET_DIR/baz + Error: Target has different file type but could not be moved to backup location: $ABS_TARGET_DIR/baz $ABS_TARGET_DIR/baz/bat/file_3 -> not fixed - Error: Target has different file type but could not be removed: $ABS_TARGET_DIR/baz + Error: Target has different file type but could not be moved to backup location: $ABS_TARGET_DIR/baz $ABS_TARGET_DIR/baz -> not fixed $ABS_TARGET_DIR/foo/file_1 -> fixed $ABS_TARGET_DIR/usr/lib/os-release -> fixed diff --git a/test/functional/repair/repair-type-changed-system.bats b/test/functional/repair/repair-type-changed-system.bats index a3890483..fe9289e2 100755 --- a/test/functional/repair/repair-type-changed-system.bats +++ b/test/functional/repair/repair-type-changed-system.bats @@ -94,6 +94,7 @@ test_setup() { assert_regular_file_exists "$TARGET_DIR"/file_removed assert_regular_file_exists "$TARGET_DIR"/file_to_dir + assert_file_exists "$TARGET_DIR"/.deleted.*.symlink_to_dir/dir/file assert_regular_file_exists "$TARGET_DIR"/file_to_symlink assert_regular_file_exists "$TARGET_DIR"/file_to_symlink_dir assert_regular_file_exists "$TARGET_DIR"/file_to_symlink_broken @@ -105,6 +106,7 @@ test_setup() { assert_symlink_exists "$TARGET_DIR"/symlink_to_dir run stat --printf "%N" "$TARGET_DIR"/symlink_to_dir assert_is_output "'$TARGET_DIR/symlink_to_dir' -> 'broken'" + assert_file_exists "$TARGET_DIR"/.deleted.*.symlink_to_dir/dir/file assert_symlink_exists "$TARGET_DIR"/symlink_to_symlink run stat --printf "%N" "$TARGET_DIR"/symlink_to_symlink diff --git a/test/functional/testlib.bash b/test/functional/testlib.bash index d77315de..ffe92e6d 100644 --- a/test/functional/testlib.bash +++ b/test/functional/testlib.bash @@ -304,7 +304,7 @@ show_target() { # swupd_function EOM )" "$@" - print "\n$(tree "$TARGET_DIR")\n" + print "\n$(tree -a "$TARGET_DIR")\n" } @@ -4050,7 +4050,7 @@ update_bundle() { # swupd_function sudo sed -i "/\\t$filename$/s/./d/2" "$bundle_manifest" sudo sed -i "/\\t$filename\\t/s/./d/2" "$bundle_manifest" # remove the related file(s) from the version dir (if there) - sudo rm -f "$version_path"/files/"$fhash" + sudo rm -f "$version_path"/files/"$fhash" || sudo rmdir "$version_path"/files/"$fhash" sudo rm -f "$version_path"/files/"$fhash".tar else # replace the second character of the line that matches with "g" diff --git a/test/functional/update/update-newest-deleted.bats b/test/functional/update/update-newest-deleted.bats index d3cd1289..1b894383 100755 --- a/test/functional/update/update-newest-deleted.bats +++ b/test/functional/update/update-newest-deleted.bats @@ -6,11 +6,14 @@ test_setup() { create_test_environment "$TEST_NAME" create_bundle -L -n test-bundle1 -f /testfile "$TEST_NAME" - create_bundle -L -n test-bundle2 -d /foo "$TEST_NAME" + create_bundle -L -n test-bundle2 -d /foo -d /foo/bar -f /foo/bar/baz "$TEST_NAME" create_version -p "$TEST_NAME" 20 10 update_bundle "$TEST_NAME" os-core --update /core update_bundle "$TEST_NAME" test-bundle1 --delete /testfile update_bundle "$TEST_NAME" test-bundle2 --add /testfile + update_bundle "$TEST_NAME" test-bundle2 --delete /foo/bar/baz + update_bundle "$TEST_NAME" test-bundle2 --delete /foo/bar + update_bundle "$TEST_NAME" test-bundle2 --delete /foo create_version -p "$TEST_NAME" 30 20 update_bundle "$TEST_NAME" os-core --update /core update_bundle "$TEST_NAME" test-bundle2 --delete /testfile @@ -20,6 +23,8 @@ test_setup() { @test "UPD003: Updating a system where a file was deleted in the newer version" { # NOTE: we don't create delta packs when a file is deleted in an update with the test library + sudo mkdir "$TARGET_DIR"/foo/bar/keep1 + sudo mkdir "$TARGET_DIR"/foo/keep2 run sudo sh -c "$SWUPD update $SWUPD_OPTS" assert_status_is 0 @@ -37,7 +42,7 @@ test_setup() { deleted bundles : 0 changed files : 1 new files : 0 - deleted files : 1 + deleted files : 4 Validate downloaded files No extra files need to be downloaded Installing files... @@ -48,7 +53,12 @@ test_setup() { ) assert_is_output "$expected_output" assert_file_not_exists "$TARGET_DIR"/testfile - + assert_dir_not_exists "$TARGET_DIR"/foo + assert_dir_not_exists "$TARGET_DIR"/foo/bar + assert_file_not_exists "$TARGET_DIR"/foo/bar/baz + assert_dir_exists "$TARGET_DIR"/.deleted.*.foo/keep2 + assert_dir_exists "$TARGET_DIR"/.deleted.*.foo/.deleted.*.bar/keep1 + show_target } #WEIGHT=7 diff --git a/test/functional/update/update-type-changes-dir-to-file.bats b/test/functional/update/update-type-changes-dir-to-file.bats index 2d1667b3..25d7c0fb 100755 --- a/test/functional/update/update-type-changes-dir-to-file.bats +++ b/test/functional/update/update-type-changes-dir-to-file.bats @@ -44,6 +44,8 @@ test_setup() { assert_file_exists "$TARGET_DIR"/file1 assert_file_exists "$TARGET_DIR"/common_file assert_dir_exists "$TARGET_DIR"/dir1 + sudo mkdir "$TARGET_DIR"/dir1/dir2 + show_target run sudo sh -c "$SWUPD update $SWUPD_OPTS" @@ -75,6 +77,7 @@ test_setup() { assert_dir_exists "$TARGET_DIR"/file1 assert_dir_not_exists "$TARGET_DIR"/dir1 assert_file_exists "$TARGET_DIR"/dir1 + assert_dir_exists "$TARGET_DIR"/.deleted.*.dir1/dir2 assert_file_exists "$TARGET_DIR"/common_file show_target