Basic conformance tests create test source directories with files and sub folders, execute robocopy on it, checking whether the resulting destination directory, the remaining source directory and the exit code is as expected
Signed-off-by: Florian Eder others.meder@gmail.com --- v8: Added helper functions to check the source folder to reduce amount of duplicated code, use only the relative path for DeleteFileW / RemoveDirectoryW and used string arrays to reduce the amount of code necessary for the tests.
The remaining tests that are not using an array / loop structure either break not completly identical with the other tests or will not be fixed with the same patch (which would in turn break the tests when implementing the feature, as the test would succeed for most cmd strings, but not for all).
For the cmd return value, this could be fixed with a todo_wine_if and hard coding the array entries that fail, but then the check_*_test functions would still fail as there are also differences in the resulting files.
Another solution to this problem would be to also give the id of the specific array entry that is checked as an argument to the check_*_test functions, so that todo_wine_if could be used, but this would
a) require hardcoding array entry ids, which could easily change if tests are added from the top instead of the end of the array, b) make the tests more complex, as an additional function is linked to the content of the cmd string array, c) deviate even further from other tests that are (AFAIK) seen as the "gold standard".
However, if you prefer this solution, feel free to tell me. :-)
Also, basic_copy_tests and absolute_copy_tests could be theoretically merged, as it should not hurt to use the strings in basic_copy_tests as format strings for swprintf (they don't contain any format specifiers, causing swprintf to do nothing). I think it's reasonable to seperate those tests anyway, as it's not directly visible what strings are added to the strings otherwise (it's IMO much more clear when talking about a test for absolute paths, which requires information only available at runtime), but if you think otherwise, I'll change it immediately. :-) --- configure | 1 + configure.ac | 1 + programs/robocopy/tests/Makefile.in | 4 + programs/robocopy/tests/robocopy.c | 342 ++++++++++++++++++++++++++++ 4 files changed, 348 insertions(+) create mode 100644 programs/robocopy/tests/Makefile.in create mode 100644 programs/robocopy/tests/robocopy.c
diff --git a/configure b/configure index 0156a507439..542d0fb7233 100755 --- a/configure +++ b/configure @@ -19782,6 +19782,7 @@ wine_fn_config_makefile programs/regini enable_regini wine_fn_config_makefile programs/regsvcs enable_regsvcs wine_fn_config_makefile programs/regsvr32 enable_regsvr32 wine_fn_config_makefile programs/robocopy enable_robocopy +wine_fn_config_makefile programs/robocopy/tests enable_tests wine_fn_config_makefile programs/rpcss enable_rpcss wine_fn_config_makefile programs/rundll.exe16 enable_win16 wine_fn_config_makefile programs/rundll32 enable_rundll32 diff --git a/configure.ac b/configure.ac index ab55c72a238..c5913721a3a 100644 --- a/configure.ac +++ b/configure.ac @@ -3679,6 +3679,7 @@ WINE_CONFIG_MAKEFILE(programs/regini) WINE_CONFIG_MAKEFILE(programs/regsvcs) WINE_CONFIG_MAKEFILE(programs/regsvr32) WINE_CONFIG_MAKEFILE(programs/robocopy) +WINE_CONFIG_MAKEFILE(programs/robocopy/tests) WINE_CONFIG_MAKEFILE(programs/rpcss) WINE_CONFIG_MAKEFILE(programs/rundll.exe16,enable_win16) WINE_CONFIG_MAKEFILE(programs/rundll32) diff --git a/programs/robocopy/tests/Makefile.in b/programs/robocopy/tests/Makefile.in new file mode 100644 index 00000000000..d86e70f4c43 --- /dev/null +++ b/programs/robocopy/tests/Makefile.in @@ -0,0 +1,4 @@ +TESTDLL = robocopy.exe + +C_SRCS = \ + robocopy.c diff --git a/programs/robocopy/tests/robocopy.c b/programs/robocopy/tests/robocopy.c new file mode 100644 index 00000000000..48075709cf5 --- /dev/null +++ b/programs/robocopy/tests/robocopy.c @@ -0,0 +1,342 @@ +/* + * Copyright 2021 Florian Eder + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#define WIN32_LEAN_AND_MEAN +#include <windows.h> +#include <wchar.h> +#include <wine/test.h> + +WCHAR temp_path[MAX_PATH]; + +static BOOL run_cmd(const WCHAR *cmd, DWORD *exit_code) +{ + STARTUPINFOW startup_info; + PROCESS_INFORMATION process_info; + WCHAR cmd_copy[256]; + + memset(&startup_info, 0, sizeof(STARTUPINFOW)); + startup_info.dwFlags = STARTF_USESTDHANDLES; + startup_info.hStdInput = INVALID_HANDLE_VALUE; + startup_info.hStdOutput = INVALID_HANDLE_VALUE; + startup_info.hStdError = INVALID_HANDLE_VALUE; + + swprintf(cmd_copy, ARRAY_SIZE(cmd_copy), L"%s", cmd); + + if (!CreateProcessW(NULL, cmd_copy, NULL, NULL, TRUE, 0, NULL, NULL, &startup_info, &process_info)) + return FALSE; + + if (WaitForSingleObject(process_info.hProcess, 30000) == WAIT_TIMEOUT) + return FALSE; + + GetExitCodeProcess(process_info.hProcess, exit_code); + + CloseHandle(process_info.hThread); + CloseHandle(process_info.hProcess); + + return TRUE; +} + +static void create_test_file(const WCHAR *relative_path, size_t size) +{ + HANDLE handle; + WCHAR path[MAX_PATH]; + swprintf(path, ARRAY_SIZE(path), L"%s%s", temp_path, relative_path); + handle = CreateFileW(path, FILE_GENERIC_WRITE | FILE_GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_ALWAYS, 0, NULL); + ok(handle != INVALID_HANDLE_VALUE, "creation of %s failed (0x%08x)\n", debugstr_w(path), GetLastError()); + if (size != 0) + { + BYTE *data; + DWORD bytes_written; + data = calloc(size, sizeof(BYTE)); + ok(WriteFile(handle, data, size, &bytes_written, NULL), "writing to %s failed (%d)\n", debugstr_w(path), GetLastError()); + } + CloseHandle(handle); +} + +static void create_test_folder(const WCHAR *relative_path) +{ + WCHAR path[MAX_PATH]; + swprintf(path, ARRAY_SIZE(path), L"%s%s", temp_path, relative_path); + + CreateDirectoryW(path, NULL); +} + +static void create_test_source_folder(void) +{ + create_test_folder(L"source"); + create_test_folder(L"source\folderA"); + create_test_folder(L"source\folderB"); + create_test_folder(L"source\folderC"); + create_test_folder(L"source\folderA\folderD"); + create_test_folder(L"source\folderA\folderE"); + create_test_file(L"source\fileA.a", 4000); + create_test_file(L"source\fileB.b", 8000); + create_test_file(L"source\folderA\fileC.c", 60); + create_test_file(L"source\folderA\fileD.d", 80); + create_test_file(L"source\folderA\folderD\fileE.e", 10000); + create_test_file(L"source\folderB\fileF.f", 10000); + create_test_file(L"source\folderB\fileG.g", 200); +} + +static void check_file(const WCHAR *relative_path, BOOL should_exist) +{ + ok (DeleteFileW(relative_path) == should_exist, "file %s expected exist to be %d, but is %d\n", debugstr_w(relative_path), should_exist, !should_exist); + if (!should_exist) + ok(GetLastError() == ERROR_FILE_NOT_FOUND || GetLastError() == ERROR_PATH_NOT_FOUND, + "file %s DeleteFileW returned error %d, should not exist\n", + debugstr_w(relative_path), + GetLastError()); +} + +static void check_folder(const WCHAR *relative_path, BOOL should_exist) +{ + ok (RemoveDirectoryW(relative_path) == should_exist, "folder %s expected exist to be %d, but is %d\n", debugstr_w(relative_path), should_exist, !should_exist); + if (!should_exist) + ok(GetLastError() == ERROR_FILE_NOT_FOUND || GetLastError() == ERROR_PATH_NOT_FOUND, + "folder %s RemoveDirectoryW returned error %d, should not exist\n", + debugstr_w(relative_path), + GetLastError()); +} + +static void check_source_folder_unchanged(void) +{ + check_file(L"source\fileA.a", TRUE); + check_file(L"source\fileB.b", TRUE); + check_file(L"source\folderA\fileC.c", TRUE); + check_file(L"source\folderA\fileD.d", TRUE); + check_file(L"source\folderA\folderD\fileE.e", TRUE); + check_file(L"source\folderB\fileF.f", TRUE); + check_file(L"source\folderB\fileG.g", TRUE); + check_folder(L"source\folderA\folderD", TRUE); + check_folder(L"source\folderA\folderE", TRUE); + check_folder(L"source\folderA", TRUE); + check_folder(L"source\folderB", TRUE); + check_folder(L"source\folderC", TRUE); + check_folder(L"source", TRUE); +} + +static void check_basic_copy_1_test(void) +{ + check_source_folder_unchanged(); + todo_wine check_file(L"destination\fileA.a", TRUE); + todo_wine check_file(L"destination\fileB.b", TRUE); + check_file(L"destination\folderA\fileC.c", FALSE); + check_file(L"destination\folderA\fileD.d", FALSE); + check_file(L"destination\folderA\folderD\fileE.e", FALSE); + check_file(L"destination\folderB\fileF.f", FALSE); + check_file(L"destination\folderB\fileG.g", FALSE); + check_folder(L"destination\folderA\folderD", FALSE); + check_folder(L"destination\folderA\folderE", FALSE); + check_folder(L"destination\folderA", FALSE); + check_folder(L"destination\folderB", FALSE); + check_folder(L"destination\folderC", FALSE); + todo_wine check_folder(L"destination", TRUE); +} + +static void check_basic_copy_2_test(void) +{ + check_source_folder_unchanged(); + check_file(L"destination\fileA.a", TRUE); + todo_wine check_file(L"destination\fileB.b", TRUE); + check_file(L"destination\folderA\fileC.c", FALSE); + check_file(L"destination\folderA\fileD.d", FALSE); + check_file(L"destination\folderA\folderD\fileE.e", FALSE); + check_file(L"destination\folderB\fileF.f", FALSE); + check_file(L"destination\folderB\fileG.g", FALSE); + check_folder(L"destination\folderA\folderD", FALSE); + check_folder(L"destination\folderA\folderE", FALSE); + check_folder(L"destination\folderA", FALSE); + check_folder(L"destination\folderB", FALSE); + check_folder(L"destination\folderC", FALSE); + check_folder(L"destination", TRUE); +} + +static void check_no_copy_test(void) +{ + check_source_folder_unchanged(); + check_file(L"destination\fileA.a", FALSE); + check_file(L"destination\fileB.b", FALSE); + check_file(L"destination\folderA\fileC.c", FALSE); + check_file(L"destination\folderA\fileD.d", FALSE); + check_file(L"destination\folderA\folderD\fileE.e", FALSE); + check_file(L"destination\folderB\fileF.f", FALSE); + check_file(L"destination\folderB\fileG.g", FALSE); + check_folder(L"destination\folderA\folderD", FALSE); + check_folder(L"destination\folderA\folderE", FALSE); + check_folder(L"destination\folderA", FALSE); + check_folder(L"destination\folderB", FALSE); + check_folder(L"destination\folderC", FALSE); + check_folder(L"destination", FALSE); +} + +static void check_wildcard_1_test(void) +{ + check_source_folder_unchanged(); + todo_wine check_file(L"destination\fileA.a", TRUE); + check_file(L"destination\fileB.b", FALSE); + check_file(L"destination\folderA\fileC.c", FALSE); + check_file(L"destination\folderA\fileD.d", FALSE); + check_file(L"destination\folderA\folderD\fileE.e", FALSE); + check_file(L"destination\folderB\fileF.f", FALSE); + check_file(L"destination\folderB\fileG.g", FALSE); + check_folder(L"destination\folderA\folderD", FALSE); + check_folder(L"destination\folderA\folderE", FALSE); + check_folder(L"destination\folderA", FALSE); + check_folder(L"destination\folderB", FALSE); + check_folder(L"destination\folderC", FALSE); + todo_wine check_folder(L"destination", TRUE); +} + +START_TEST(robocopy) +{ + DWORD exit_code; + WCHAR temp_cmd[512]; + int i; + static const WCHAR *invalid_syntax_tests[] = + { + L"robocopy", + L"robocopy invalid_folder", + L"robocopy -flag invalid_folder", + L"robocopy invalid_folder destination", + L"robocopy -?", + L"robocopy invalid_folder -?", + L"robocopy invalid_folder /?", + }; + static const WCHAR *basic_copy_tests[] = + { + L"robocopy source destination /r:1 /w:0", + L"robocopy ./source destination /r:1 /w:0", + L"robocopy source ./destination /r:1 /w:0", + L"robocopy ./source ./destination /r:1 /w:0", + L"robocopy source third_folder/../destination /r:1 /w:0", + L"robocopy ./source third_folder/../destination /r:1 /w:0", + L"robocopy ./third_folder/../source third_folder/../destination /r:1 /w:0", + L"robocopy ./third_folder/../source ./destination /r:1 /w:0", + L"robocopy source destination *le?.? /r:1 /w:0", + L"robocopy source destination file?.? /r:1 /w:0", + L"robocopy source destination ?ile?.* /r:1 /w:0", + L"robocopy source destination f??e* /r:1 /w:0", + L"robocopy source destination file* /r:1 /w:0", + }; + static const WCHAR *absolute_copy_tests[] = + { + L"robocopy %s\source %s\destination /r:1 /w:0", + L"robocopy %s\source destination /r:1 /w:0", + L"robocopy source %s\destination /r:1 /w:0", + L"robocopy %s\third_folder\..\source %s\third_folder\..\destination /r:1 /w:0", + }; + static const WCHAR *parser_tests[] = + { + L"robocopy source /r:1 destination /r:1 /w:0", + L"robocopy /r:1 source destination /r:1 /w:0", + L"robocopy /r:1 /r:1 source destination /r:1 /w:0", + L"robocopy /r:1 source /r:1 destination /r:1 /w:0", + }; + + /* robocopy is only available from Vista onwards, abort test if not available */ + if (!run_cmd(L"robocopy", &exit_code)) return; + + ok(GetTempPathW(ARRAY_SIZE(temp_path), temp_path) != 0, "couldn't get temp folder path\n"); + wcscat(temp_path, L"robocopy_test\"); + ok(CreateDirectoryW(temp_path, NULL), "couldn't create temp test folder %s, error %d\n", debugstr_w(temp_path), GetLastError()); + ok(SetCurrentDirectoryW(temp_path), "couldn't set CWD to temp folder %s\n", debugstr_w(temp_path)); + + for (i = 0; i < ARRAY_SIZE(invalid_syntax_tests); i++) + { + run_cmd(invalid_syntax_tests[i], &exit_code); + ok(exit_code == 16, "unexpected exit code %d from command %s\n", exit_code, debugstr_w(invalid_syntax_tests[i])); + } + + for (i = 0; i < ARRAY_SIZE(basic_copy_tests); i++) + { + winetest_push_context("basic copy test %d", i + 1); + create_test_source_folder(); + run_cmd(basic_copy_tests[i], &exit_code); + todo_wine ok(exit_code == 1, "unexpected exit code %d from command %s\n", exit_code, debugstr_w(basic_copy_tests[i])); + check_basic_copy_1_test(); + winetest_pop_context(); + } + + winetest_push_context("basic copy test %d", ARRAY_SIZE(basic_copy_tests) + 1); + create_test_source_folder(); + create_test_folder(L"destination"); + create_test_file(L"destination\fileA.a", 9000); + run_cmd(L"robocopy ./source source/../destination /r:1 /w:0", &exit_code); + todo_wine ok(exit_code == 1, "unexpected exit code %d\n", exit_code); + check_basic_copy_2_test(); + winetest_pop_context(); + + for (i = 0; i < ARRAY_SIZE(absolute_copy_tests); i++) + { + winetest_push_context("absolute copy test %d", i + 1); + create_test_source_folder(); + swprintf(temp_cmd, ARRAY_SIZE(temp_cmd), absolute_copy_tests[i], temp_path, temp_path); + run_cmd(temp_cmd, &exit_code); + todo_wine ok(exit_code == 1, "unexpected exit code %d from command %s\n", exit_code, debugstr_w(absolute_copy_tests[i])); + check_basic_copy_1_test(); + winetest_pop_context(); + } + + for (i = 0; i < ARRAY_SIZE(parser_tests); i++) + { + winetest_push_context("parser test %d", i + 1); + create_test_source_folder(); + run_cmd(parser_tests[i], &exit_code); + todo_wine ok(exit_code == 1, "unexpected exit code %d from command %s\n", exit_code, debugstr_w(parser_tests[i])); + check_basic_copy_1_test(); + winetest_pop_context(); + } + + winetest_push_context("invalid copy test 1"); + create_test_source_folder(); + run_cmd(L"robocopy source source /r:1 /w:0", &exit_code); + todo_wine ok(exit_code == 0, "unexpected exit code %d\n", exit_code); + check_no_copy_test(); + winetest_pop_context(); + + winetest_push_context("invalid copy test 2"); + create_test_source_folder(); + run_cmd(L"robocopy invalid_folder destination /r:1 /w:0", &exit_code); + ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + check_no_copy_test(); + winetest_pop_context(); + + winetest_push_context("wildcard test 1"); + create_test_source_folder(); + run_cmd(L"robocopy source destination *A.? /r:1 /w:0", &exit_code); + todo_wine ok(exit_code == 1, "unexpected exit code %d\n", exit_code); + check_wildcard_1_test(); + winetest_pop_context(); + + winetest_push_context("wildcard test 2"); + create_test_source_folder(); + run_cmd(L"robocopy sour?e destination /r:1 /w:0", &exit_code); + ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + check_no_copy_test(); + winetest_pop_context(); + + winetest_push_context("wildcard test 3"); + create_test_source_folder(); + run_cmd(L"robocopy s* destination /r:1 /w:0", &exit_code); + ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + check_no_copy_test(); + winetest_pop_context(); + + SetCurrentDirectoryW(L".."); + ok(RemoveDirectoryW(temp_path), "could not remove temp path %s, error %d\n", debugstr_w(temp_path), GetLastError()); +}
Parses path arguments as source, destination and files to include, reads all files in the source folder that match any of the files to include and copies them to the destination, creating necessary folders in the process
Signed-off-by: Florian Eder others.meder@gmail.com --- v8: Made some arguments constant, added helper function to replace / remove duplicates of very long flags for PathAllocCombine and simplified logic in append_matching_directory_content
This patch still converts both source and destination path to absolute paths, as doing otherwise would break the already existing support for paths > MAX_PATH. It would also not really save many LoCs / much logic, as it would still be required to make sure the paths both end with a backslash, to simplify the copy logic (as we then can just append the relative paths). --- programs/robocopy/Makefile.in | 1 + programs/robocopy/main.c | 192 ++++++++++++++++++++++++++++- programs/robocopy/tests/robocopy.c | 22 ++-- 3 files changed, 202 insertions(+), 13 deletions(-)
diff --git a/programs/robocopy/Makefile.in b/programs/robocopy/Makefile.in index f5d9dff6e4d..d1f5db665df 100644 --- a/programs/robocopy/Makefile.in +++ b/programs/robocopy/Makefile.in @@ -1,6 +1,7 @@ MODULE = robocopy.exe
EXTRADLLFLAGS = -mconsole -municode +IMPORTS = kernelbase shlwapi
C_SRCS = \ main.c diff --git a/programs/robocopy/main.c b/programs/robocopy/main.c index 5e2ca5c0f01..b58111c7bff 100644 --- a/programs/robocopy/main.c +++ b/programs/robocopy/main.c @@ -18,13 +18,201 @@
#define WIN32_LEAN_AND_MEAN #include <windows.h> +#include <stdlib.h> +#include <pathcch.h> +#include <shlwapi.h>
#include "wine/debug.h"
WINE_DEFAULT_DEBUG_CHANNEL(robocopy);
+#include <wine/list.h> + +struct path_array +{ + UINT size; + WCHAR *array[1]; +}; + +struct path +{ + struct list entry; + WCHAR *name; +}; + +struct robocopy_options +{ + WCHAR *destination; + WCHAR *source; + struct path_array *files; +}; + +#define ROBOCOPY_SUCCESS_FILES_COPIED 1 +#define ROBOCOPY_ERROR_NO_FILES_COPIED 16 + +struct robocopy_options options; + +static WCHAR *get_absolute_path(const WCHAR *path) +{ + DWORD size; + WCHAR *absolute_path; + + size = GetFullPathNameW(path, 0, NULL, NULL) + 2; + absolute_path = calloc(size, sizeof(WCHAR)); + GetFullPathNameW(path, size, absolute_path, NULL); + PathCchAddBackslashEx(absolute_path, size, NULL, NULL); + return absolute_path; +} + +static BOOL path_in_array(const WCHAR *name, const struct path_array *names) +{ + int i; + for (i = 0; i < names->size; i++) + { + if (PathMatchSpecW(name, names->array[i])) return TRUE; + } + return FALSE; +} + +static BOOL create_directory_path(const WCHAR *path) +{ + WCHAR *pointer, *current_folder; + current_folder = calloc(wcslen(path) + 1, sizeof(WCHAR)); + pointer = wcschr(path, L'\'); + while (pointer != NULL) + { + if (!lstrcpynW(current_folder, path, pointer - path + 2)) return FALSE; + if (!CreateDirectoryW(current_folder, NULL) && GetLastError() != ERROR_ALREADY_EXISTS) return FALSE; + pointer = wcschr(pointer + 1, L'\'); + } + return TRUE; +} + +static void combine_path(const WCHAR *path1, const WCHAR *path2, WCHAR **out) +{ + PathAllocCombine(path1, path2, + PATHCCH_ALLOW_LONG_PATHS | PATHCCH_FORCE_ENABLE_LONG_NAME_PROCESS, + out); +} + +static void append_matching_directory_content(struct list *paths, const WCHAR *search_path, const WCHAR *root, struct path_array *file_names) +{ + WIN32_FIND_DATAW entry_data; + HANDLE handle; + WCHAR *current_absolute_path; + struct path *new_path; + + handle = FindFirstFileExW(search_path, FindExInfoStandard, &entry_data, FindExSearchNameMatch, NULL, 0); + if (handle == INVALID_HANDLE_VALUE) return; + do + { + if (!wcscmp(L".", entry_data.cFileName) || !wcscmp(L"..", entry_data.cFileName)) continue; + + combine_path(root, entry_data.cFileName, ¤t_absolute_path); + + if (PathIsDirectoryW(current_absolute_path) || path_in_array(entry_data.cFileName, file_names)) + { + new_path = calloc(1, sizeof(struct path)); + new_path->name = wcsdup(entry_data.cFileName); + list_add_tail(paths, &new_path->entry); + } + } + while (FindNextFileW(handle, &entry_data)); +} + +static void get_file_paths_in_folder(const WCHAR *root, struct list *paths) +{ + struct path *new_path; + WCHAR *current_search_path; + + list_init(paths); + new_path = calloc(1, sizeof(struct path)); + new_path->name = calloc(2, sizeof(WCHAR)); + list_add_tail(paths, &new_path->entry); + + combine_path(root, L"*", ¤t_search_path); + append_matching_directory_content(paths, current_search_path, root, options.files); +} + +static BOOL perform_copy(void) +{ + struct list paths_source; + struct path *current_path; + WCHAR *current_absolute_path, *target_path; + + list_init(&paths_source); + + if (!PathIsDirectoryW(options.source)) return FALSE; + + create_directory_path(options.destination); + + /* get files in the source folder */ + get_file_paths_in_folder(options.source, &paths_source); + + /* walk through files in the source folder */ + LIST_FOR_EACH_ENTRY(current_path, &paths_source, struct path, entry) + { + combine_path(options.source, current_path->name, ¤t_absolute_path); + combine_path(options.destination, current_path->name, &target_path); + + if (!PathIsDirectoryW(current_absolute_path)) + { + create_directory_path(target_path); + CopyFileW(current_absolute_path, target_path, FALSE); + } + } + return TRUE; +} + +static void parse_arguments(int argc, WCHAR *argv[]) +{ + int i; + + memset(&options, 0, sizeof(options)); + options.files = calloc(1, offsetof(struct path_array, array[argc])); + + for (i = 1; i < argc; i++) + { + /* + * Robocopy switches contain one (and only one) backslash at the start + * /xf => valid flag + * /r:1 => valid flag + * /r:1aö => valid flag + * /r:1aö/ => not a valid flag, is interpreted as a filename + */ + if ((argv[i][0] == '/') && !wcschr(argv[i] + 1, '/')) + WINE_FIXME("encountered an unknown robocopy flag: %s\n", debugstr_w(argv[i])); + else + { + if (!options.source) + options.source = get_absolute_path(argv[i]); + else if (!options.destination) + options.destination = get_absolute_path(argv[i]); + else + { + options.files->array[options.files->size] = wcsdup(argv[i]); + options.files->size++; + } + } + } +} + int __cdecl wmain(int argc, WCHAR *argv[]) { - FIXME("robocopy stub\n"); - return 16; + parse_arguments(argc, argv); + + /* If no file filters are set, set *.* to include all files */ + if (options.files->size == 0) + { + options.files->array[options.files->size] = wcsdup(L"*.*"); + options.files->size++; + } + + if (!options.destination || !options.source) + return ROBOCOPY_ERROR_NO_FILES_COPIED; + + if (!perform_copy()) + return ROBOCOPY_ERROR_NO_FILES_COPIED; + + return ROBOCOPY_SUCCESS_FILES_COPIED; } diff --git a/programs/robocopy/tests/robocopy.c b/programs/robocopy/tests/robocopy.c index 48075709cf5..3e268f1cb30 100644 --- a/programs/robocopy/tests/robocopy.c +++ b/programs/robocopy/tests/robocopy.c @@ -133,8 +133,8 @@ static void check_source_folder_unchanged(void) static void check_basic_copy_1_test(void) { check_source_folder_unchanged(); - todo_wine check_file(L"destination\fileA.a", TRUE); - todo_wine check_file(L"destination\fileB.b", TRUE); + check_file(L"destination\fileA.a", TRUE); + check_file(L"destination\fileB.b", TRUE); check_file(L"destination\folderA\fileC.c", FALSE); check_file(L"destination\folderA\fileD.d", FALSE); check_file(L"destination\folderA\folderD\fileE.e", FALSE); @@ -145,14 +145,14 @@ static void check_basic_copy_1_test(void) check_folder(L"destination\folderA", FALSE); check_folder(L"destination\folderB", FALSE); check_folder(L"destination\folderC", FALSE); - todo_wine check_folder(L"destination", TRUE); + check_folder(L"destination", TRUE); }
static void check_basic_copy_2_test(void) { check_source_folder_unchanged(); check_file(L"destination\fileA.a", TRUE); - todo_wine check_file(L"destination\fileB.b", TRUE); + check_file(L"destination\fileB.b", TRUE); check_file(L"destination\folderA\fileC.c", FALSE); check_file(L"destination\folderA\fileD.d", FALSE); check_file(L"destination\folderA\folderD\fileE.e", FALSE); @@ -187,7 +187,7 @@ static void check_no_copy_test(void) static void check_wildcard_1_test(void) { check_source_folder_unchanged(); - todo_wine check_file(L"destination\fileA.a", TRUE); + check_file(L"destination\fileA.a", TRUE); check_file(L"destination\fileB.b", FALSE); check_file(L"destination\folderA\fileC.c", FALSE); check_file(L"destination\folderA\fileD.d", FALSE); @@ -199,7 +199,7 @@ static void check_wildcard_1_test(void) check_folder(L"destination\folderA", FALSE); check_folder(L"destination\folderB", FALSE); check_folder(L"destination\folderC", FALSE); - todo_wine check_folder(L"destination", TRUE); + check_folder(L"destination", TRUE); }
START_TEST(robocopy) @@ -267,7 +267,7 @@ START_TEST(robocopy) winetest_push_context("basic copy test %d", i + 1); create_test_source_folder(); run_cmd(basic_copy_tests[i], &exit_code); - todo_wine ok(exit_code == 1, "unexpected exit code %d from command %s\n", exit_code, debugstr_w(basic_copy_tests[i])); + ok(exit_code == 1, "unexpected exit code %d from command %s\n", exit_code, debugstr_w(basic_copy_tests[i])); check_basic_copy_1_test(); winetest_pop_context(); } @@ -277,7 +277,7 @@ START_TEST(robocopy) create_test_folder(L"destination"); create_test_file(L"destination\fileA.a", 9000); run_cmd(L"robocopy ./source source/../destination /r:1 /w:0", &exit_code); - todo_wine ok(exit_code == 1, "unexpected exit code %d\n", exit_code); + ok(exit_code == 1, "unexpected exit code %d\n", exit_code); check_basic_copy_2_test(); winetest_pop_context();
@@ -287,7 +287,7 @@ START_TEST(robocopy) create_test_source_folder(); swprintf(temp_cmd, ARRAY_SIZE(temp_cmd), absolute_copy_tests[i], temp_path, temp_path); run_cmd(temp_cmd, &exit_code); - todo_wine ok(exit_code == 1, "unexpected exit code %d from command %s\n", exit_code, debugstr_w(absolute_copy_tests[i])); + ok(exit_code == 1, "unexpected exit code %d from command %s\n", exit_code, debugstr_w(absolute_copy_tests[i])); check_basic_copy_1_test(); winetest_pop_context(); } @@ -297,7 +297,7 @@ START_TEST(robocopy) winetest_push_context("parser test %d", i + 1); create_test_source_folder(); run_cmd(parser_tests[i], &exit_code); - todo_wine ok(exit_code == 1, "unexpected exit code %d from command %s\n", exit_code, debugstr_w(parser_tests[i])); + ok(exit_code == 1, "unexpected exit code %d from command %s\n", exit_code, debugstr_w(parser_tests[i])); check_basic_copy_1_test(); winetest_pop_context(); } @@ -319,7 +319,7 @@ START_TEST(robocopy) winetest_push_context("wildcard test 1"); create_test_source_folder(); run_cmd(L"robocopy source destination *A.? /r:1 /w:0", &exit_code); - todo_wine ok(exit_code == 1, "unexpected exit code %d\n", exit_code); + ok(exit_code == 1, "unexpected exit code %d\n", exit_code); check_wildcard_1_test(); winetest_pop_context();
On 10/31/21 15:34, Florian Eder wrote:
Parses path arguments as source, destination and files to include, reads all files in the source folder that match any of the files to include and copies them to the destination, creating necessary folders in the process
Signed-off-by: Florian Eder others.meder@gmail.com
v8: Made some arguments constant, added helper function to replace / remove duplicates of very long flags for PathAllocCombine and simplified logic in append_matching_directory_content
This patch still converts both source and destination path to absolute paths, as doing otherwise would break the already existing support for paths > MAX_PATH. It would also not really save many LoCs / much logic, as it would still be required to make sure the paths both end with a backslash, to simplify the copy logic (as we then can just append the relative paths).
Right, thanks, that makes sense.
Just a few more nitpicks below...
+struct robocopy_options options;
Missing "static".
+static void combine_path(const WCHAR *path1, const WCHAR *path2, WCHAR **out) +{
- PathAllocCombine(path1, path2,
PATHCCH_ALLOW_LONG_PATHS | PATHCCH_FORCE_ENABLE_LONG_NAME_PROCESS,out);+}
You might consider making the combined path the return value. Food for thought.
+static void append_matching_directory_content(struct list *paths, const WCHAR *search_path, const WCHAR *root, struct path_array *file_names)
"file_names" could be const. This line is also kind of long—we don't really have a hard limit, but 120 characters is pushing it.
+{
- WIN32_FIND_DATAW entry_data;
- HANDLE handle;
- WCHAR *current_absolute_path;
- struct path *new_path;
- handle = FindFirstFileExW(search_path, FindExInfoStandard, &entry_data, FindExSearchNameMatch, NULL, 0);
- if (handle == INVALID_HANDLE_VALUE) return;
- do
- {
if (!wcscmp(L".", entry_data.cFileName) || !wcscmp(L"..", entry_data.cFileName)) continue;combine_path(root, entry_data.cFileName, ¤t_absolute_path);if (PathIsDirectoryW(current_absolute_path) || path_in_array(entry_data.cFileName, file_names)){new_path = calloc(1, sizeof(struct path));new_path->name = wcsdup(entry_data.cFileName);list_add_tail(paths, &new_path->entry);}- }
- while (FindNextFileW(handle, &entry_data));
+}
+static void get_file_paths_in_folder(const WCHAR *root, struct list *paths) +{
- struct path *new_path;
- WCHAR *current_search_path;
- list_init(paths);
This list is initialized both here and in the caller. Not sure which makes more sense given following patches, but once is enough ;-)
- new_path = calloc(1, sizeof(struct path));
- new_path->name = calloc(2, sizeof(WCHAR));
- list_add_tail(paths, &new_path->entry);
- combine_path(root, L"*", ¤t_search_path);
- append_matching_directory_content(paths, current_search_path, root, options.files);
+}
Adds output methods to output copied files and errors
Signed-off-by: Florian Eder others.meder@gmail.com --- v8: Identical to v7 --- programs/robocopy/main.c | 86 +++++++++++++++++++++++++++++++++-- programs/robocopy/robocopy.h | 24 ++++++++++ programs/robocopy/robocopy.rc | 13 ++++++ 3 files changed, 119 insertions(+), 4 deletions(-) create mode 100644 programs/robocopy/robocopy.h
diff --git a/programs/robocopy/main.c b/programs/robocopy/main.c index b58111c7bff..6aa5f102da7 100644 --- a/programs/robocopy/main.c +++ b/programs/robocopy/main.c @@ -27,6 +27,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(robocopy);
#include <wine/list.h> +#include "robocopy.h"
struct path_array { @@ -52,6 +53,62 @@ struct robocopy_options
struct robocopy_options options;
+static const WCHAR *format_string(UINT format_string_id) +{ + WCHAR format_string[2048]; + if (!LoadStringW(GetModuleHandleW(NULL), format_string_id, format_string, ARRAY_SIZE(format_string))) + { + WINE_ERR("invalid string loaded"); + return L""; + } + return wcsdup(format_string); +} + +static void output_message(const WCHAR *format_string, ...) +{ + __ms_va_list va_args; + WCHAR *string; + DWORD length, bytes_written; + + __ms_va_start(va_args, format_string); + length = FormatMessageW(FORMAT_MESSAGE_FROM_STRING | FORMAT_MESSAGE_ALLOCATE_BUFFER, + format_string, 0, 0, (WCHAR *)&string, 0, &va_args); + __ms_va_end(va_args); + if (!length) + { + WINE_ERR("string formation failed"); + return; + } + + /* If WriteConsole fails, the output is being redirected to a file */ + if (!WriteConsoleW(GetStdHandle(STD_OUTPUT_HANDLE), string, wcslen(string), &bytes_written, NULL)) + { + CHAR *string_multibyte; + DWORD length_multibyte; + + length_multibyte = WideCharToMultiByte(GetConsoleOutputCP(), 0, string, wcslen(string), NULL, 0, NULL, NULL); + string_multibyte = malloc(length_multibyte); + + WideCharToMultiByte(GetConsoleOutputCP(), 0, string, wcslen(string), string_multibyte, length_multibyte, NULL, NULL); + WriteFile(GetStdHandle(STD_OUTPUT_HANDLE), string_multibyte, length_multibyte, &bytes_written, NULL); + free(string_multibyte); + } + + LocalFree(string); +} + +static void output_error(UINT format_string_id, HRESULT error_code, WCHAR* path) +{ + WCHAR *error_string; + + FormatMessageW(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS | FORMAT_MESSAGE_ALLOCATE_BUFFER, + NULL, error_code, 0, (WCHAR *)&error_string, 0, NULL); + + output_message(format_string(format_string_id), path, error_string); + + LocalFree(error_string); +} + static WCHAR *get_absolute_path(const WCHAR *path) { DWORD size; @@ -82,7 +139,16 @@ static BOOL create_directory_path(const WCHAR *path) while (pointer != NULL) { if (!lstrcpynW(current_folder, path, pointer - path + 2)) return FALSE; - if (!CreateDirectoryW(current_folder, NULL) && GetLastError() != ERROR_ALREADY_EXISTS) return FALSE; + if (!CreateDirectoryW(current_folder, NULL)) + { + if (GetLastError() != ERROR_ALREADY_EXISTS) + { + output_error(STRING_ERROR_WRITE_DIRECTORY, GetLastError(), current_folder); + return FALSE; + } + } + else + output_message(format_string(STRING_CREATE_DIRECTORY), current_folder); pointer = wcschr(pointer + 1, L'\'); } return TRUE; @@ -142,7 +208,11 @@ static BOOL perform_copy(void)
list_init(&paths_source);
- if (!PathIsDirectoryW(options.source)) return FALSE; + if (!PathIsDirectoryW(options.source)) + { + output_error(STRING_ERROR_READ_DIRECTORY, ERROR_FILE_NOT_FOUND, options.source); + return FALSE; + }
create_directory_path(options.destination);
@@ -155,10 +225,18 @@ static BOOL perform_copy(void) combine_path(options.source, current_path->name, ¤t_absolute_path); combine_path(options.destination, current_path->name, &target_path);
- if (!PathIsDirectoryW(current_absolute_path)) + if (PathIsDirectoryW(current_absolute_path)) + { + if (!create_directory_path(target_path)) + output_error(STRING_ERROR_WRITE_DIRECTORY, GetLastError(), target_path); + } + else { create_directory_path(target_path); - CopyFileW(current_absolute_path, target_path, FALSE); + if (!CopyFileW(current_absolute_path, target_path, FALSE)) + output_error(STRING_ERROR_WRITE_FILE, GetLastError(), target_path); + else + output_message(format_string(STRING_CREATE_FILE), target_path); } } return TRUE; diff --git a/programs/robocopy/robocopy.h b/programs/robocopy/robocopy.h new file mode 100644 index 00000000000..4add81dd208 --- /dev/null +++ b/programs/robocopy/robocopy.h @@ -0,0 +1,24 @@ +/* + * Copyright 2021 Florian Eder + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#define STRING_MISSING_DESTINATION_OR_SOURCE 1010 +#define STRING_ERROR_READ_DIRECTORY 1011 +#define STRING_ERROR_WRITE_DIRECTORY 1012 +#define STRING_ERROR_WRITE_FILE 1014 +#define STRING_CREATE_DIRECTORY 1019 +#define STRING_CREATE_FILE 1022 diff --git a/programs/robocopy/robocopy.rc b/programs/robocopy/robocopy.rc index ffd5f770b1b..70dc11aab34 100644 --- a/programs/robocopy/robocopy.rc +++ b/programs/robocopy/robocopy.rc @@ -17,9 +17,22 @@ */
#include <windef.h> +#include "robocopy.h"
#pragma makedep po
+LANGUAGE LANG_ENGLISH, SUBLANG_DEFAULT + +STRINGTABLE +{ + STRING_MISSING_DESTINATION_OR_SOURCE, "No destination or source specified, can't copy anything\n" + STRING_ERROR_READ_DIRECTORY, "Error occurred reading directory "%1":\n%2\n" + STRING_ERROR_WRITE_DIRECTORY, "Error occurred writing directory "%1":\n%2\n" + STRING_ERROR_WRITE_FILE, "Error occurred writing file "%1":\n%2\n" + STRING_CREATE_DIRECTORY, " Created Dir: %1\n" + STRING_CREATE_FILE, " Copied File: %1\n" +} + LANGUAGE LANG_NEUTRAL, SUBLANG_NEUTRAL
#define WINE_FILEDESCRIPTION_STR "Wine Robocopy"
On 10/31/21 15:34, Florian Eder wrote:
Basic conformance tests create test source directories with files and sub folders, execute robocopy on it, checking whether the resulting destination directory, the remaining source directory and the exit code is as expected
Signed-off-by: Florian Eder others.meder@gmail.com
v8: Added helper functions to check the source folder to reduce amount of duplicated code, use only the relative path for DeleteFileW / RemoveDirectoryW and used string arrays to reduce the amount of code necessary for the tests.
The remaining tests that are not using an array / loop structure either break not completly identical with the other tests or will not be fixed with the same patch (which would in turn break the tests when implementing the feature, as the test would succeed for most cmd strings, but not for all).
For the cmd return value, this could be fixed with a todo_wine_if and hard coding the array entries that fail, but then the check_*_test functions would still fail as there are also differences in the resulting files.
Another solution to this problem would be to also give the id of the specific array entry that is checked as an argument to the check_*_test functions, so that todo_wine_if could be used, but this would
a) require hardcoding array entry ids, which could easily change if tests are added from the top instead of the end of the array, b) make the tests more complex, as an additional function is linked to the content of the cmd string array, c) deviate even further from other tests that are (AFAIK) seen as the "gold standard".
However, if you prefer this solution, feel free to tell me. :-)
Also, basic_copy_tests and absolute_copy_tests could be theoretically merged, as it should not hurt to use the strings in basic_copy_tests as format strings for swprintf (they don't contain any format specifiers, causing swprintf to do nothing). I think it's reasonable to seperate those tests anyway, as it's not directly visible what strings are added to the strings otherwise (it's IMO much more clear when talking about a test for absolute paths, which requires information only available at runtime), but if you think otherwise, I'll change it immediately. :-)
In general I personally don't think it's worth going to extremes to accommodate incremental removal of todo_wine. My rule of thumb is "order the tests before the patch unless it's hard to do so".
What you have right now already looks a lot easier to parse, I think, so it's fine as-is. What you might do to improve it even further is to do what most other tests do and separate each test into helper functions, e.g. test_basic_copy(), test_absolute_path(), test_wildcards().
Just a few other nitpicks below (sorry I keep missing things... the bigger a patch is, the harder it gets to find everything the first time...)
+static void create_test_file(const WCHAR *relative_path, size_t size) +{
- HANDLE handle;
- WCHAR path[MAX_PATH];
- swprintf(path, ARRAY_SIZE(path), L"%s%s", temp_path, relative_path);
- handle = CreateFileW(path, FILE_GENERIC_WRITE | FILE_GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_ALWAYS, 0, NULL);
- ok(handle != INVALID_HANDLE_VALUE, "creation of %s failed (0x%08x)\n", debugstr_w(path), GetLastError());
- if (size != 0)
- {
BYTE *data;DWORD bytes_written;data = calloc(size, sizeof(BYTE));ok(WriteFile(handle, data, size, &bytes_written, NULL), "writing to %s failed (%d)\n", debugstr_w(path), GetLastError());- }
- CloseHandle(handle);
+}
+static void create_test_folder(const WCHAR *relative_path) +{
- WCHAR path[MAX_PATH];
- swprintf(path, ARRAY_SIZE(path), L"%s%s", temp_path, relative_path);
- CreateDirectoryW(path, NULL);
+}
I know I only said DeleteFile() and RemoveDirectory(), but, well, these ones too. Most Win32 functions are capable of taking relative paths; there's no reasons not to make use of that in places like this.
+static void check_file(const WCHAR *relative_path, BOOL should_exist)
"relative_path" is somewhat redundant when you're not dealing with any other kind of path anymore.
+{
- ok (DeleteFileW(relative_path) == should_exist, "file %s expected exist to be %d, but is %d\n", debugstr_w(relative_path), should_exist, !should_exist);
This ok message strikes me as a little more verbose than it needs to be. I'd suggest just "DeleteFile returned %d"; you could append "expected %d" to the end but it's pretty much implicit.
- if (!should_exist)
ok(GetLastError() == ERROR_FILE_NOT_FOUND || GetLastError() == ERROR_PATH_NOT_FOUND,"file %s DeleteFileW returned error %d, should not exist\n",debugstr_w(relative_path),GetLastError());+}
The way these lines wrap is very inconsistent. The first line in this function is very long; the last line wraps even where not particularly necessary.
The two ok() calls have inconsistent spacing before the parenthesis as well.
Same for check_folder() below of course.
+static void check_folder(const WCHAR *relative_path, BOOL should_exist) +{
- ok (RemoveDirectoryW(relative_path) == should_exist, "folder %s expected exist to be %d, but is %d\n", debugstr_w(relative_path), should_exist, !should_exist);
- if (!should_exist)
ok(GetLastError() == ERROR_FILE_NOT_FOUND || GetLastError() == ERROR_PATH_NOT_FOUND,"folder %s RemoveDirectoryW returned error %d, should not exist\n",debugstr_w(relative_path),GetLastError());+}
...
+START_TEST(robocopy) +{
- DWORD exit_code;
- WCHAR temp_cmd[512];
- int i;
- static const WCHAR *invalid_syntax_tests[] =
"static const WCHAR *const invalid_syntax_tests[]" etc.
-
Florian Eder -
Zebediah Figura (she/her)