From 71d48a5033995864ae1a960c8a66aae2bb1df122 Mon Sep 17 00:00:00 2001 From: Castulo Martinez Date: Mon, 26 Nov 2018 22:27:55 +0000 Subject: [PATCH] Reporting of deleted files in bundle-remove When running bundle-remove to remove a bundle the number in "Total deleted files" is often wrong. This is happening because when deleting the files from a bundle sometimes we start by deleting a directory (with all the files within it), so all the files that got deleted are not accounted for, only one file will be counted as deleted (the directory), this will yield to an incorrect count of deleted files. This commit fixes this issue by printing the total number of files that should be deleted, minus those that failed to be deleted, instead of counting those that succeeded. Closes #690 Signed-off-by: Castulo Martinez --- src/manifest.c | 13 +++++++---- .../bundleremove/remove-basics.bats | 23 ++++++++++--------- .../bundleremove/remove-boot-file.bats | 3 +-- 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/src/manifest.c b/src/manifest.c index dbbedaac..da23fba9 100644 --- a/src/manifest.c +++ b/src/manifest.c @@ -1108,19 +1108,22 @@ void remove_files_in_manifest_from_fs(struct manifest *m) struct list *iter = NULL; struct file *file = NULL; char *fullfile = NULL; - int count = 0; + int count = list_len(m->files); iter = list_head(m->files); while (iter) { file = iter->data; iter = iter->next; string_or_die(&fullfile, "%s/%s", path_prefix, file->filename); - if (swupd_rm(fullfile) != 0) { - free_string(&fullfile); - continue; + if (swupd_rm(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. + * Note: If a file didn't exist it will still be counted as deleted, + * this is a limitation */ + count--; } free_string(&fullfile); - count++; } fprintf(stderr, "Total deleted files: %i\n", count); } diff --git a/test/functional/bundleremove/remove-basics.bats b/test/functional/bundleremove/remove-basics.bats index 9090ae2d..fa9f254d 100755 --- a/test/functional/bundleremove/remove-basics.bats +++ b/test/functional/bundleremove/remove-basics.bats @@ -5,8 +5,8 @@ load "../testlib" global_setup() { create_test_environment "$TEST_NAME" - create_bundle -L -n test-bundle1 -d /foo -f /test-file1,/bar/test-file2,/bat/test-file3 "$TEST_NAME" - create_bundle -L -n test-bundle2 -f /bat/test-file4 "$TEST_NAME" + create_bundle -L -n test-bundle1 -d /foo -f /test-file1,/bar/test-file2,/bat/test-file3,/bat/common "$TEST_NAME" + create_bundle -L -n test-bundle2 -f /bat/test-file4,/bat/common "$TEST_NAME" create_bundle -n test-bundle3 -f /baz/test-file5 "$TEST_NAME" } @@ -51,14 +51,14 @@ global_teardown() { # bundle2 was not removed assert_file_exists "$TARGETDIR"/usr/share/clear/bundles/test-bundle2 assert_file_exists "$TARGETDIR"/bat/test-file4 + assert_file_exists "$TARGETDIR"/bat/common expected_output=$(cat <<-EOM Deleting bundle files... - Total deleted files: 5 + Total deleted files: 6 Successfully removed 1 bundle EOM ) assert_is_output --identical "$expected_output" - # TODO(castulo): The total deleted files should have been 6, change once bug #690 is fixed } @@ -73,21 +73,22 @@ global_teardown() { assert_file_not_exists "$TARGETDIR"/test-file1 assert_file_not_exists "$TARGETDIR"/bar/test-file2 assert_file_not_exists "$TARGETDIR"/bat/test-file3 + assert_file_not_exists "$TARGETDIR"/bat/test-file4 + assert_file_not_exists "$TARGETDIR"/bat/common assert_dir_not_exists "$TARGETDIR"/foo assert_dir_not_exists "$TARGETDIR"/bar assert_dir_not_exists "$TARGETDIR"/bat expected_output=$(cat <<-EOM Removing bundle: test-bundle1 Deleting bundle files... - Total deleted files: 5 + Total deleted files: 6 Removing bundle: test-bundle2 Deleting bundle files... - Total deleted files: 2 + Total deleted files: 4 Successfully removed 2 bundles EOM ) assert_is_output --identical "$expected_output" - # TODO(castulo): The total deleted files should have been 7 + 2, change once bug #690 is fixed } @@ -156,16 +157,16 @@ global_teardown() { # bundle2 was not removed assert_file_exists "$TARGETDIR"/usr/share/clear/bundles/test-bundle2 assert_file_exists "$TARGETDIR"/bat/test-file4 + assert_file_exists "$TARGETDIR"/bat/common expected_output=$(cat <<-EOM Warning: Bundle "test-bundle3" is not installed, skipping it... Removing bundle: test-bundle1 Deleting bundle files... - Total deleted files: 5 + Total deleted files: 6 Failed to remove 1 of 2 bundles EOM ) assert_is_output --identical "$expected_output" - # TODO(castulo): The total deleted files should have been 6, change once bug #690 is fixed } @@ -183,15 +184,15 @@ global_teardown() { # bundle2 was not removed assert_file_exists "$TARGETDIR"/usr/share/clear/bundles/test-bundle2 assert_file_exists "$TARGETDIR"/bat/test-file4 + assert_file_exists "$TARGETDIR"/bat/common expected_output=$(cat <<-EOM Warning: Bundle "fake-bundle" is not installed, skipping it... Removing bundle: test-bundle1 Deleting bundle files... - Total deleted files: 5 + Total deleted files: 6 Failed to remove 1 of 2 bundles EOM ) assert_is_output --identical "$expected_output" - # TODO(castulo): The total deleted files should have been 6, change once bug #690 is fixed } diff --git a/test/functional/bundleremove/remove-boot-file.bats b/test/functional/bundleremove/remove-boot-file.bats index ea60555c..4e524003 100755 --- a/test/functional/bundleremove/remove-boot-file.bats +++ b/test/functional/bundleremove/remove-boot-file.bats @@ -22,11 +22,10 @@ test_setup() { assert_dir_exists "$TARGETDIR"/usr/ expected_output=$(cat <<-EOM Deleting bundle files... - Total deleted files: 2 + Total deleted files: 3 Successfully removed 1 bundle EOM ) assert_is_output "$expected_output" - # TODO(castulo): The total deleted files should have been 3, change once bug #690 is fixed }