[PATCH v6 1/6] robocopy: Add stub
Basic files and required changes to configure(.ac) to create a stub version of the robocopy utility Signed-off-by: Florian Eder <others.meder(a)gmail.com> --- v6: Identical to patch in v5 --- configure | 2 +- configure.ac | 1 + programs/robocopy/Makefile.in | 8 ++++++++ programs/robocopy/main.c | 29 +++++++++++++++++++++++++++++ programs/robocopy/robocopy.rc | 34 ++++++++++++++++++++++++++++++++++ 5 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 programs/robocopy/Makefile.in create mode 100644 programs/robocopy/main.c create mode 100644 programs/robocopy/robocopy.rc diff --git a/configure b/configure index db592f0868d..3a8dee81bbc 100755 --- a/configure +++ b/configure @@ -21235,6 +21235,7 @@ wine_fn_config_makefile programs/regedit/tests enable_tests 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/rpcss enable_rpcss wine_fn_config_makefile programs/rundll.exe16 enable_win16 wine_fn_config_makefile programs/rundll32 enable_rundll32 @@ -22768,4 +22769,3 @@ IFS="$ac_save_IFS" $as_echo " $as_me: Finished. Do '${ac_make}' to compile Wine. " >&6 - diff --git a/configure.ac b/configure.ac index 0bc46a1427b..5bd54cfbba7 100644 --- a/configure.ac +++ b/configure.ac @@ -3959,6 +3959,7 @@ WINE_CONFIG_MAKEFILE(programs/regedit/tests) WINE_CONFIG_MAKEFILE(programs/regini) WINE_CONFIG_MAKEFILE(programs/regsvcs) WINE_CONFIG_MAKEFILE(programs/regsvr32) +WINE_CONFIG_MAKEFILE(programs/robocopy) WINE_CONFIG_MAKEFILE(programs/rpcss) WINE_CONFIG_MAKEFILE(programs/rundll.exe16,enable_win16) WINE_CONFIG_MAKEFILE(programs/rundll32) diff --git a/programs/robocopy/Makefile.in b/programs/robocopy/Makefile.in new file mode 100644 index 00000000000..5a48be3725b --- /dev/null +++ b/programs/robocopy/Makefile.in @@ -0,0 +1,8 @@ +MODULE = robocopy.exe + +EXTRADLLFLAGS = -mconsole -municode -mno-cygwin + +C_SRCS = \ + main.c + +RC_SRCS = robocopy.rc diff --git a/programs/robocopy/main.c b/programs/robocopy/main.c new file mode 100644 index 00000000000..bbda15f573a --- /dev/null +++ b/programs/robocopy/main.c @@ -0,0 +1,29 @@ +/* + * 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 + */ + +#include "wine/debug.h" +WINE_DEFAULT_DEBUG_CHANNEL(robocopy); + +#define WIN32_LEAN_AND_MEAN +#include <windows.h> + +int __cdecl wmain(int argc, WCHAR *argv[]) +{ + WINE_FIXME("robocopy stub"); + return 1; +} diff --git a/programs/robocopy/robocopy.rc b/programs/robocopy/robocopy.rc new file mode 100644 index 00000000000..edae5b71d86 --- /dev/null +++ b/programs/robocopy/robocopy.rc @@ -0,0 +1,34 @@ +/* + * 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 + */ + +#include <windef.h> + +#pragma makedep po + +LANGUAGE LANG_NEUTRAL, SUBLANG_NEUTRAL + +#define WINE_FILEDESCRIPTION_STR "Wine Robocopy" +#define WINE_FILENAME_STR "robocopy.exe" +#define WINE_FILETYPE VFT_APP +#define WINE_FILEVERSION 5,1,10,1027 +#define WINE_FILEVERSION_STR "5.1.10.1027" + +#define WINE_PRODUCTVERSION 5,1,10,1027 +#define WINE_PRODUCTVERSION_STR "XP027" + +#include "wine/wine_common_ver.rc" -- 2.32.0
Basic files and required changes to configure(.ac) to create basic syntax tests and a scaffolding for the conformance tests to follow for the robocopy utility Signed-off-by: Florian Eder <others.meder(a)gmail.com> --- v6: Made execute_robocopy return boolean value and added a DWORD pointer as an argument to write the exit code into, just like the tests for reg Added syntax tests for /? and -? --- configure | 1 + configure.ac | 1 + programs/robocopy/tests/Makefile.in | 4 ++ programs/robocopy/tests/robocopy.c | 103 ++++++++++++++++++++++++++++ 4 files changed, 109 insertions(+) create mode 100644 programs/robocopy/tests/Makefile.in create mode 100644 programs/robocopy/tests/robocopy.c diff --git a/configure b/configure index 3a8dee81bbc..82fb0f13a43 100755 --- a/configure +++ b/configure @@ -21236,6 +21236,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 5bd54cfbba7..83c67911675 100644 --- a/configure.ac +++ b/configure.ac @@ -3960,6 +3960,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..f99a027baec --- /dev/null +++ b/programs/robocopy/tests/robocopy.c @@ -0,0 +1,103 @@ +/* + * 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> + +static BOOL execute_robocopy(const WCHAR *args, DWORD *exit_code) +{ + STARTUPINFOW startup_info; + PROCESS_INFORMATION process_info; + WCHAR cmd[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, ARRAY_SIZE(cmd), L"%s%s", L"robocopy.exe ", args); + + if (!CreateProcessW(NULL, cmd, 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; +} + +START_TEST(robocopy) +{ + DWORD exit_code; + WCHAR temp_path[MAX_PATH]; + + /* robocopy is only available from Vista onwards, abort test if not available */ + if (!execute_robocopy(L"", &exit_code)) return; + + ok(GetTempPathW(ARRAY_SIZE(temp_path), temp_path) != 0, "couldn't get temp folder path"); + + ok(SetCurrentDirectoryW(temp_path), "couldn't set CWD to temp folder \"%s\"", debugstr_w(temp_path)); + + winetest_push_context("syntax test 1"); + execute_robocopy(L"", &exit_code); + todo_wine ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + winetest_pop_context(); + + winetest_push_context("syntax test 2"); + execute_robocopy(L"invalid_folder", &exit_code); + todo_wine ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + winetest_pop_context(); + + winetest_push_context("syntax test 3"); + execute_robocopy(L"-flag invalid_folder", &exit_code); + todo_wine ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + winetest_pop_context(); + + winetest_push_context("syntax test 4"); + execute_robocopy(L"invalid_folder robocopy_destination", &exit_code); + todo_wine ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + winetest_pop_context(); + + winetest_push_context("syntax test 5"); + execute_robocopy(L"-?", &exit_code); + todo_wine ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + winetest_pop_context(); + + winetest_push_context("syntax test 6"); + execute_robocopy(L"invalid_folder -?", &exit_code); + todo_wine ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + winetest_pop_context(); + + winetest_push_context("syntax test 7"); + execute_robocopy(L"/?", &exit_code); + todo_wine ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + winetest_pop_context(); + + winetest_push_context("syntax test 8"); + execute_robocopy(L"invalid_folder /?", &exit_code); + todo_wine ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + winetest_pop_context(); +} -- 2.32.0
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
On 9/21/21 09:54, Florian Eder wrote:
diff --git a/programs/robocopy/tests/robocopy.c b/programs/robocopy/tests/robocopy.c new file mode 100644 index 00000000000..f99a027baec --- /dev/null +++ b/programs/robocopy/tests/robocopy.c @@ -0,0 +1,103 @@ +/* + * 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> + +static BOOL execute_robocopy(const WCHAR *args, DWORD *exit_code)
One alternative (suggested to me by Hugh) is to put robocopy.exe back into the args string, for clarity, and then call this function run_exe() or something similar for brevity. Ultimately I think it's up to personal taste, though.
+{ + STARTUPINFOW startup_info; + PROCESS_INFORMATION process_info; + WCHAR cmd[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, ARRAY_SIZE(cmd), L"%s%s", L"robocopy.exe ", args); + + if (!CreateProcessW(NULL, cmd, 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; +} + +START_TEST(robocopy) +{ + DWORD exit_code; + WCHAR temp_path[MAX_PATH]; + + /* robocopy is only available from Vista onwards, abort test if not available */ + if (!execute_robocopy(L"", &exit_code)) return; + + ok(GetTempPathW(ARRAY_SIZE(temp_path), temp_path) != 0, "couldn't get temp folder path"); + + ok(SetCurrentDirectoryW(temp_path), "couldn't set CWD to temp folder \"%s\"", debugstr_w(temp_path)); + + winetest_push_context("syntax test 1");
These calls to winetest_push_context() don't really make sense in this patch. The only ok() calls they encompass are already unambiguous due to their line number. That said, you might consider folding all of these calls together into a single array
+ execute_robocopy(L"", &exit_code); + todo_wine ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + winetest_pop_context(); + + winetest_push_context("syntax test 2"); + execute_robocopy(L"invalid_folder", &exit_code); + todo_wine ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + winetest_pop_context(); + + winetest_push_context("syntax test 3"); + execute_robocopy(L"-flag invalid_folder", &exit_code); + todo_wine ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + winetest_pop_context(); + + winetest_push_context("syntax test 4"); + execute_robocopy(L"invalid_folder robocopy_destination", &exit_code); + todo_wine ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + winetest_pop_context(); + + winetest_push_context("syntax test 5"); + execute_robocopy(L"-?", &exit_code); + todo_wine ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + winetest_pop_context(); + + winetest_push_context("syntax test 6"); + execute_robocopy(L"invalid_folder -?", &exit_code); + todo_wine ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + winetest_pop_context(); + + winetest_push_context("syntax test 7"); + execute_robocopy(L"/?", &exit_code); + todo_wine ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + winetest_pop_context(); + + winetest_push_context("syntax test 8"); + execute_robocopy(L"invalid_folder /?", &exit_code); + todo_wine ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + winetest_pop_context(); +}
Parses (relative / absolute) path arguments as source, destination and files to include Signed-off-by: Florian Eder <others.meder(a)gmail.com> --- v6: Identical to patch in v5 --- programs/robocopy/Makefile.in | 1 + programs/robocopy/main.c | 61 +++++++++++++++++++++++++++++++++++ programs/robocopy/robocopy.h | 33 +++++++++++++++++++ 3 files changed, 95 insertions(+) create mode 100644 programs/robocopy/robocopy.h diff --git a/programs/robocopy/Makefile.in b/programs/robocopy/Makefile.in index 5a48be3725b..0f4f5c76119 100644 --- a/programs/robocopy/Makefile.in +++ b/programs/robocopy/Makefile.in @@ -1,4 +1,5 @@ MODULE = robocopy.exe +IMPORTS = kernelbase EXTRADLLFLAGS = -mconsole -municode -mno-cygwin diff --git a/programs/robocopy/main.c b/programs/robocopy/main.c index bbda15f573a..e1800fbd74e 100644 --- a/programs/robocopy/main.c +++ b/programs/robocopy/main.c @@ -21,9 +21,70 @@ WINE_DEFAULT_DEBUG_CHANNEL(robocopy); #define WIN32_LEAN_AND_MEAN #include <windows.h> +#include <stdlib.h> +#include <pathcch.h> +#include "robocopy.h" + +struct robocopy_options options; + +static WCHAR *get_absolute_path(const WCHAR *path) +{ + DWORD size; + WCHAR *absolute_path; + + /* allocate absolute path + potential backslash + null WCHAR */ + 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 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] == L'/') && (wcschr(argv[i] + 1, L'/') == NULL)) + WINE_FIXME("encountered an unknown robocopy flag: %s\n", debugstr_w(argv[i])); + else + { + /* + *(Probably) not a flag, we can parse it as the source / the destination / a filename + * Priority: Source > Destination > (more than one) File + */ + 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[]) { + parse_arguments(argc, argv); + WINE_FIXME("robocopy stub"); return 1; } diff --git a/programs/robocopy/robocopy.h b/programs/robocopy/robocopy.h new file mode 100644 index 00000000000..f62eb92c663 --- /dev/null +++ b/programs/robocopy/robocopy.h @@ -0,0 +1,33 @@ +/* + * 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> + +struct path_array +{ + UINT size; + WCHAR *array[1]; +}; + +struct robocopy_options +{ + WCHAR *destination; + WCHAR *source; + struct path_array *files; +}; -- 2.32.0
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
On 9/21/21 09:54, Florian Eder wrote:
Parses (relative / absolute) path arguments as source, destination and files to include
Signed-off-by: Florian Eder <others.meder(a)gmail.com> --- v6: Identical to patch in v5 --- programs/robocopy/Makefile.in | 1 + programs/robocopy/main.c | 61 +++++++++++++++++++++++++++++++++++ programs/robocopy/robocopy.h | 33 +++++++++++++++++++ 3 files changed, 95 insertions(+) create mode 100644 programs/robocopy/robocopy.h
Although splitting is nice, This patch doesn't exactly do anything, which makes it hard to judge. (For one thing, why are we transforming everything to an absolute path? I guess 4/6 is the reason, although I kind of wonder if 4/6 should even exist.) I would suggest trying to reorder patches as follows: (1) stub (2) syntax tests (3) tests for basic functionality (4) argument parsing (5) basic functionality (6) print console output? And then it might even be reasonable to merge patches 4 and 5.
diff --git a/programs/robocopy/Makefile.in b/programs/robocopy/Makefile.in index 5a48be3725b..0f4f5c76119 100644 --- a/programs/robocopy/Makefile.in +++ b/programs/robocopy/Makefile.in @@ -1,4 +1,5 @@ MODULE = robocopy.exe +IMPORTS = kernelbase
EXTRADLLFLAGS = -mconsole -municode -mno-cygwin
diff --git a/programs/robocopy/main.c b/programs/robocopy/main.c index bbda15f573a..e1800fbd74e 100644 --- a/programs/robocopy/main.c +++ b/programs/robocopy/main.c @@ -21,9 +21,70 @@ WINE_DEFAULT_DEBUG_CHANNEL(robocopy);
#define WIN32_LEAN_AND_MEAN #include <windows.h> +#include <stdlib.h> +#include <pathcch.h> +#include "robocopy.h" + +struct robocopy_options options; + +static WCHAR *get_absolute_path(const WCHAR *path) +{ + DWORD size; + WCHAR *absolute_path; + + /* allocate absolute path + potential backslash + null WCHAR */
This is the sort of comment that is described not only by the function name but by reading its code.
+ 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 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 + */
Should we have tests for these? It gets a bit awkward because Wine interprets such paths (rightly) as Unix paths, but they could still be useful to demonstrate the Windows behaviour.
+ if ((argv[i][0] == L'/') && (wcschr(argv[i] + 1, L'/') == NULL))
Nitpick, but the L prefix is unnecessary. There's a few other such cases in these patches. I also find "!wcschr" more idiomatic than "== NULL", although others seem to disagree.
+ WINE_FIXME("encountered an unknown robocopy flag: %s\n", debugstr_w(argv[i])); + else + { + /* + *(Probably) not a flag, we can parse it as the source / the destination / a filename + * Priority: Source > Destination > (more than one) File + */
This is also kind of a redundant comment; the code below tells us as much.
+ 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[]) { + parse_arguments(argc, argv); + WINE_FIXME("robocopy stub"); return 1; } diff --git a/programs/robocopy/robocopy.h b/programs/robocopy/robocopy.h new file mode 100644 index 00000000000..f62eb92c663 --- /dev/null +++ b/programs/robocopy/robocopy.h @@ -0,0 +1,33 @@ +/* + * 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> + +struct path_array +{ + UINT size; + WCHAR *array[1]; +}; + +struct robocopy_options +{ + WCHAR *destination; + WCHAR *source; + struct path_array *files; +};
Do we gain a lot by having a separate header file?
Adds output method and prints a header and the source / destination / files to include back to the user Signed-off-by: Florian Eder <others.meder(a)gmail.com> --- v6: Identical to patch in v5 --- programs/robocopy/main.c | 63 +++++++++++++++++++++++++++++++++++ programs/robocopy/robocopy.h | 6 ++++ programs/robocopy/robocopy.rc | 11 ++++++ 3 files changed, 80 insertions(+) diff --git a/programs/robocopy/main.c b/programs/robocopy/main.c index e1800fbd74e..6781fc17504 100644 --- a/programs/robocopy/main.c +++ b/programs/robocopy/main.c @@ -27,6 +27,50 @@ WINE_DEFAULT_DEBUG_CHANNEL(robocopy); 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, (LPWSTR)&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 WCHAR *get_absolute_path(const WCHAR *path) { DWORD size; @@ -81,10 +125,29 @@ static void parse_arguments(int argc, WCHAR *argv[]) } } +static void print_header(void) +{ + UINT i; + + output_message(format_string(STRING_HEADER)); + + if (!options.source) output_message(format_string(STRING_SOURCE), L"-"); + else output_message(format_string(STRING_SOURCE), options.source); + + if (!options.destination) output_message(format_string(STRING_DESTINATION), L"-"); + else output_message(format_string(STRING_DESTINATION), options.destination); + + output_message(format_string(STRING_FILES)); + for (i = 0; i < options.files->size; i++) + output_message(L" %1\n", options.files->array[i]); +} + int __cdecl wmain(int argc, WCHAR *argv[]) { parse_arguments(argc, argv); + print_header(); + WINE_FIXME("robocopy stub"); return 1; } diff --git a/programs/robocopy/robocopy.h b/programs/robocopy/robocopy.h index f62eb92c663..f5c2ac56fcf 100644 --- a/programs/robocopy/robocopy.h +++ b/programs/robocopy/robocopy.h @@ -31,3 +31,9 @@ struct robocopy_options WCHAR *source; struct path_array *files; }; + +/* Resource strings */ +#define STRING_HEADER 1000 +#define STRING_SOURCE 1003 +#define STRING_DESTINATION 1004 +#define STRING_FILES 1005 diff --git a/programs/robocopy/robocopy.rc b/programs/robocopy/robocopy.rc index edae5b71d86..044bab77774 100644 --- a/programs/robocopy/robocopy.rc +++ b/programs/robocopy/robocopy.rc @@ -17,9 +17,20 @@ */ #include <windef.h> +#include "robocopy.h" #pragma makedep po +LANGUAGE LANG_ENGLISH, SUBLANG_DEFAULT + +STRINGTABLE +{ + STRING_HEADER, "Robocopy for Wine\n\n" + STRING_SOURCE, " Source: %1\n" + STRING_DESTINATION, " Destination: %1\n\n" + STRING_FILES, " Files:\n" +} + LANGUAGE LANG_NEUTRAL, SUBLANG_NEUTRAL #define WINE_FILEDESCRIPTION_STR "Wine Robocopy" -- 2.32.0
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
On 9/21/21 09:54, Florian Eder wrote:
Adds output method and prints a header and the source / destination / files to include back to the user
Signed-off-by: Florian Eder <others.meder(a)gmail.com> --- v6: Identical to patch in v5 --- programs/robocopy/main.c | 63 +++++++++++++++++++++++++++++++++++ programs/robocopy/robocopy.h | 6 ++++ programs/robocopy/robocopy.rc | 11 ++++++ 3 files changed, 80 insertions(+)
I know Microsoft's robocopy does this, but do we need to? I mean, it doesn't tell me anything I can't find just from reading the command line. And then on top of that it's more strings to translate, and more code, and so on. It does help validate 3/6, but ideally the implementation and tests should do that.
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(a)gmail.com> --- v6: Remove wine_todo from the additional syntax tests, as these tests pass now --- programs/robocopy/Makefile.in | 2 +- programs/robocopy/main.c | 177 ++++++++++++++++++++++++++++- programs/robocopy/robocopy.h | 17 +++ programs/robocopy/robocopy.rc | 6 + programs/robocopy/tests/robocopy.c | 16 +-- 5 files changed, 207 insertions(+), 11 deletions(-) diff --git a/programs/robocopy/Makefile.in b/programs/robocopy/Makefile.in index 0f4f5c76119..1c640599b4b 100644 --- a/programs/robocopy/Makefile.in +++ b/programs/robocopy/Makefile.in @@ -1,5 +1,5 @@ MODULE = robocopy.exe -IMPORTS = kernelbase +IMPORTS = kernelbase shlwapi EXTRADLLFLAGS = -mconsole -municode -mno-cygwin diff --git a/programs/robocopy/main.c b/programs/robocopy/main.c index 6781fc17504..05546715718 100644 --- a/programs/robocopy/main.c +++ b/programs/robocopy/main.c @@ -23,6 +23,8 @@ WINE_DEFAULT_DEBUG_CHANNEL(robocopy); #include <windows.h> #include <stdlib.h> #include <pathcch.h> +#include <shlwapi.h> +#include <wine/list.h> #include "robocopy.h" struct robocopy_options options; @@ -71,6 +73,21 @@ static void output_message(const WCHAR *format_string, ...) LocalFree(string); } +static void output_error(UINT format_string_id, HRESULT error_code, WCHAR* path) +{ + WCHAR *error_string, error_code_long[64], error_code_short[64]; + + FormatMessageW(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS | FORMAT_MESSAGE_ALLOCATE_BUFFER, + NULL, error_code, 0, (LPWSTR)&error_string, 0, NULL); + + swprintf(error_code_long, ARRAY_SIZE(error_code_long), L"0x%08x", error_code); + swprintf(error_code_short, ARRAY_SIZE(error_code_short), L"%u", error_code); + + output_message(format_string(format_string_id), L"", error_code_short, error_code_long, path, error_string); + + LocalFree(error_string); +} + static WCHAR *get_absolute_path(const WCHAR *path) { DWORD size; @@ -125,6 +142,146 @@ static void parse_arguments(int argc, WCHAR *argv[]) } } +static BOOL path_in_array(WCHAR *name, 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(WCHAR *path) +{ + WCHAR *pointer, *current_folder; + current_folder = calloc(wcslen(path) + 1, sizeof(WCHAR)); + /* ignore the "\\?\" prefix, so that those backslashes are not matched */ + pointer = wcschr(path, L'\\'); + while (pointer != NULL) + { + if (!lstrcpynW(current_folder, path, pointer - path + 2)) return FALSE; + /* try to create the folder, ignoring any failure due to ERROR_ALREADY_EXISTS */ + 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; +} + +static void get_file_paths_in_folder(WCHAR *directory_path, struct list *paths) +{ + HANDLE temp_handle; + struct path *new_path, *current_path; + WIN32_FIND_DATAW entry_data; + WCHAR *parent_absolute_path, *current_relative_path, *current_absolute_path, *current_search_path; + + list_init(paths); + + /* initialize list with a empty relative path */ + new_path = calloc(1, sizeof(struct path)); + new_path->name = calloc(2, sizeof(WCHAR)); + list_add_tail(paths, &new_path->entry); + + LIST_FOR_EACH_ENTRY(current_path, paths, struct path, entry) + { + /* append relative path to the (prefix) directory path */ + PathAllocCombine(directory_path, current_path->name, + PATHCCH_ALLOW_LONG_PATHS | PATHCCH_FORCE_ENABLE_LONG_NAME_PROCESS, + &parent_absolute_path); + + /* append * to recieve every file / subdirectory in this directory */ + PathAllocCombine(parent_absolute_path, L"*", + PATHCCH_ALLOW_LONG_PATHS | PATHCCH_FORCE_ENABLE_LONG_NAME_PROCESS, + ¤t_search_path); + /* walk through all files / directories in this directory */ + temp_handle = FindFirstFileExW(current_search_path, FindExInfoStandard, &entry_data, FindExSearchNameMatch, NULL, 0); + if (temp_handle != INVALID_HANDLE_VALUE) + { + do + { + /* Ignore . and .. entries */ + if (!wcscmp(L".", entry_data.cFileName) || !wcscmp(L"..", entry_data.cFileName)) continue; + + PathAllocCombine(current_path->name, entry_data.cFileName, + PATHCCH_ALLOW_LONG_PATHS | PATHCCH_FORCE_ENABLE_LONG_NAME_PROCESS, + ¤t_relative_path); + PathAllocCombine(directory_path, current_relative_path, + PATHCCH_ALLOW_LONG_PATHS | PATHCCH_FORCE_ENABLE_LONG_NAME_PROCESS, + ¤t_absolute_path); + + /* If this entry is a matching file or empty directory, add it to the list of results */ + if ((!PathIsDirectoryW(current_absolute_path) && path_in_array(entry_data.cFileName, options.files)) || + (PathIsDirectoryW(current_absolute_path))) + { + new_path = calloc(1, sizeof(struct path)); + new_path->name = wcsdup(current_relative_path); + list_add_tail(paths, &new_path->entry); + } + } + while (FindNextFileW(temp_handle, &entry_data) != 0); + } + } +} + +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)) + { + output_error(STRING_ERROR_READ_DIRECTORY, ERROR_FILE_NOT_FOUND, 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) + { + PathAllocCombine(options.source, current_path->name, + PATHCCH_ALLOW_LONG_PATHS | PATHCCH_FORCE_ENABLE_LONG_NAME_PROCESS, + ¤t_absolute_path); + + /* append the relative source path to the destination to get the target path */ + PathAllocCombine(options.destination, current_path->name, + PATHCCH_ALLOW_LONG_PATHS | PATHCCH_FORCE_ENABLE_LONG_NAME_PROCESS, + &target_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); + 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; +} + static void print_header(void) { UINT i; @@ -146,8 +303,24 @@ int __cdecl wmain(int argc, WCHAR *argv[]) { 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++; + } + print_header(); - WINE_FIXME("robocopy stub"); - return 1; + /* Break if Source or Destination not set */ + if (!options.destination || !options.source) + { + output_message(format_string(STRING_MISSING_DESTINATION_OR_SOURCE)); + return ROBOCOPY_ERROR_NO_FILES_COPIED; + } + + if (!perform_copy()) + return ROBOCOPY_ERROR_NO_FILES_COPIED; + + return ROBOCOPY_NO_ERROR_FILES_COPIED; } diff --git a/programs/robocopy/robocopy.h b/programs/robocopy/robocopy.h index f5c2ac56fcf..86be775da01 100644 --- a/programs/robocopy/robocopy.h +++ b/programs/robocopy/robocopy.h @@ -18,6 +18,13 @@ #define WIN32_LEAN_AND_MEAN #include <windows.h> +#include <wine/list.h> + +struct path +{ + struct list entry; + WCHAR *name; +}; struct path_array { @@ -32,8 +39,18 @@ struct robocopy_options struct path_array *files; }; +/* Exit codes */ +#define ROBOCOPY_NO_ERROR_FILES_COPIED 1 +#define ROBOCOPY_ERROR_NO_FILES_COPIED 16 + /* Resource strings */ #define STRING_HEADER 1000 #define STRING_SOURCE 1003 #define STRING_DESTINATION 1004 #define STRING_FILES 1005 +#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 044bab77774..1ce605f432d 100644 --- a/programs/robocopy/robocopy.rc +++ b/programs/robocopy/robocopy.rc @@ -29,6 +29,12 @@ STRINGTABLE STRING_SOURCE, " Source: %1\n" STRING_DESTINATION, " Destination: %1\n\n" STRING_FILES, " Files:\n" + STRING_MISSING_DESTINATION_OR_SOURCE, "No destination or source specified, can't copy anything\n" + STRING_ERROR_READ_DIRECTORY, "[%1] Error %2 (%3) occurred reading directory \"%4\":\n%5\n" + STRING_ERROR_WRITE_DIRECTORY, "[%1] Error %2 (%3) occurred writing directory \"%4\":\n%5\n" + STRING_ERROR_WRITE_FILE, "[%1] Error %2 (%3) occurred writing file \"%4\":\n%5\n" + STRING_CREATE_DIRECTORY, " Created Dir: %1\n" + STRING_CREATE_FILE, " Copied File: %1\n" } LANGUAGE LANG_NEUTRAL, SUBLANG_NEUTRAL diff --git a/programs/robocopy/tests/robocopy.c b/programs/robocopy/tests/robocopy.c index f99a027baec..d5d3dee473a 100644 --- a/programs/robocopy/tests/robocopy.c +++ b/programs/robocopy/tests/robocopy.c @@ -63,41 +63,41 @@ START_TEST(robocopy) winetest_push_context("syntax test 1"); execute_robocopy(L"", &exit_code); - todo_wine ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + ok(exit_code == 16, "unexpected exit code %d\n", exit_code); winetest_pop_context(); winetest_push_context("syntax test 2"); execute_robocopy(L"invalid_folder", &exit_code); - todo_wine ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + ok(exit_code == 16, "unexpected exit code %d\n", exit_code); winetest_pop_context(); winetest_push_context("syntax test 3"); execute_robocopy(L"-flag invalid_folder", &exit_code); - todo_wine ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + ok(exit_code == 16, "unexpected exit code %d\n", exit_code); winetest_pop_context(); winetest_push_context("syntax test 4"); execute_robocopy(L"invalid_folder robocopy_destination", &exit_code); - todo_wine ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + ok(exit_code == 16, "unexpected exit code %d\n", exit_code); winetest_pop_context(); winetest_push_context("syntax test 5"); execute_robocopy(L"-?", &exit_code); - todo_wine ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + ok(exit_code == 16, "unexpected exit code %d\n", exit_code); winetest_pop_context(); winetest_push_context("syntax test 6"); execute_robocopy(L"invalid_folder -?", &exit_code); - todo_wine ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + ok(exit_code == 16, "unexpected exit code %d\n", exit_code); winetest_pop_context(); winetest_push_context("syntax test 7"); execute_robocopy(L"/?", &exit_code); - todo_wine ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + ok(exit_code == 16, "unexpected exit code %d\n", exit_code); winetest_pop_context(); winetest_push_context("syntax test 8"); execute_robocopy(L"invalid_folder /?", &exit_code); - todo_wine ok(exit_code == 16, "unexpected exit code %d\n", exit_code); + ok(exit_code == 16, "unexpected exit code %d\n", exit_code); winetest_pop_context(); } -- 2.32.0
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
On 9/21/21 09:54, Florian Eder wrote:
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(a)gmail.com> --- v6: Remove wine_todo from the additional syntax tests, as these tests pass now --- programs/robocopy/Makefile.in | 2 +- programs/robocopy/main.c | 177 ++++++++++++++++++++++++++++- programs/robocopy/robocopy.h | 17 +++ programs/robocopy/robocopy.rc | 6 + programs/robocopy/tests/robocopy.c | 16 +-- 5 files changed, 207 insertions(+), 11 deletions(-)
The biggest problem I see with this patch is that it implements recursive copy, but the program by default isn't recursive, as the tests from 6/6 show. It's a bit awkward to implement behaviour controlled by a flag without implementing the flag, especially when recursiveness takes *more* work. I'd recommend starting with non-recursive tests, and implementing non-recursive behaviour, and then adding recursive tests and recursive behaviour in a separate patch.
diff --git a/programs/robocopy/Makefile.in b/programs/robocopy/Makefile.in index 0f4f5c76119..1c640599b4b 100644 --- a/programs/robocopy/Makefile.in +++ b/programs/robocopy/Makefile.in @@ -1,5 +1,5 @@ MODULE = robocopy.exe -IMPORTS = kernelbase +IMPORTS = kernelbase shlwapi
EXTRADLLFLAGS = -mconsole -municode -mno-cygwin
diff --git a/programs/robocopy/main.c b/programs/robocopy/main.c index 6781fc17504..05546715718 100644 --- a/programs/robocopy/main.c +++ b/programs/robocopy/main.c @@ -23,6 +23,8 @@ WINE_DEFAULT_DEBUG_CHANNEL(robocopy); #include <windows.h> #include <stdlib.h> #include <pathcch.h> +#include <shlwapi.h> +#include <wine/list.h> #include "robocopy.h"
struct robocopy_options options; @@ -71,6 +73,21 @@ static void output_message(const WCHAR *format_string, ...) LocalFree(string); }
+static void output_error(UINT format_string_id, HRESULT error_code, WCHAR* path)
Inconsistent asterisk here.
+{ + WCHAR *error_string, error_code_long[64], error_code_short[64]; + + FormatMessageW(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS | FORMAT_MESSAGE_ALLOCATE_BUFFER, + NULL, error_code, 0, (LPWSTR)&error_string, 0, NULL);
Please try to avoid LP* typedefs in new code. This line continuation is also weirdly spaced; I'd expect to align with the previous line or be a constant 4 or 8 bytes.
+ + swprintf(error_code_long, ARRAY_SIZE(error_code_long), L"0x%08x", error_code); + swprintf(error_code_short, ARRAY_SIZE(error_code_short), L"%u", error_code);
Printing out the error number doesn't strike me as useful, and printing it out in two bases definitely seems excessive.
+ output_message(format_string(format_string_id), L"", error_code_short, error_code_long, path, error_string); + + LocalFree(error_string); +} + static WCHAR *get_absolute_path(const WCHAR *path) { DWORD size; @@ -125,6 +142,146 @@ static void parse_arguments(int argc, WCHAR *argv[]) } }
+static BOOL path_in_array(WCHAR *name, 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(WCHAR *path) +{ + WCHAR *pointer, *current_folder; + current_folder = calloc(wcslen(path) + 1, sizeof(WCHAR)); + /* ignore the "\\?\" prefix, so that those backslashes are not matched */
I think this comment isn't relevant anymore?
+ pointer = wcschr(path, L'\\'); + while (pointer != NULL) + { + if (!lstrcpynW(current_folder, path, pointer - path + 2)) return FALSE; + /* try to create the folder, ignoring any failure due to ERROR_ALREADY_EXISTS */
The code below tells me as much ;-)
+ 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; +} + +static void get_file_paths_in_folder(WCHAR *directory_path, struct list *paths) +{ Maybe "root" instead of "directory_path" would make things a bit clearer?
+ HANDLE temp_handle; + struct path *new_path, *current_path; + WIN32_FIND_DATAW entry_data; + WCHAR *parent_absolute_path, *current_relative_path, *current_absolute_path, *current_search_path; + + list_init(paths); + + /* initialize list with a empty relative path */ + new_path = calloc(1, sizeof(struct path)); + new_path->name = calloc(2, sizeof(WCHAR)); + list_add_tail(paths, &new_path->entry);
This feels weird. Perhaps it would be better to move the FindFirst/FindNext loop into a helper, and then call that for "." before calling it for enumerated children?
+ + LIST_FOR_EACH_ENTRY(current_path, paths, struct path, entry) + { + /* append relative path to the (prefix) directory path */ + PathAllocCombine(directory_path, current_path->name, + PATHCCH_ALLOW_LONG_PATHS | PATHCCH_FORCE_ENABLE_LONG_NAME_PROCESS, + &parent_absolute_path); + + /* append * to recieve every file / subdirectory in this directory */ + PathAllocCombine(parent_absolute_path, L"*", + PATHCCH_ALLOW_LONG_PATHS | PATHCCH_FORCE_ENABLE_LONG_NAME_PROCESS, + ¤t_search_path);
This gets called frequently enough, and spans enough lines per call, that I'd even suggest adding a helper to shorten it.
+ /* walk through all files / directories in this directory */ + temp_handle = FindFirstFileExW(current_search_path, FindExInfoStandard, &entry_data, FindExSearchNameMatch, NULL, 0); + if (temp_handle != INVALID_HANDLE_VALUE) + { + do + { + /* Ignore . and .. entries */ + if (!wcscmp(L".", entry_data.cFileName) || !wcscmp(L"..", entry_data.cFileName)) continue; + + PathAllocCombine(current_path->name, entry_data.cFileName, + PATHCCH_ALLOW_LONG_PATHS | PATHCCH_FORCE_ENABLE_LONG_NAME_PROCESS, + ¤t_relative_path); + PathAllocCombine(directory_path, current_relative_path, + PATHCCH_ALLOW_LONG_PATHS | PATHCCH_FORCE_ENABLE_LONG_NAME_PROCESS, + ¤t_absolute_path); + + /* If this entry is a matching file or empty directory, add it to the list of results */
I feel like all of the comments in this function are redundant.
+ if ((!PathIsDirectoryW(current_absolute_path) && path_in_array(entry_data.cFileName, options.files)) || + (PathIsDirectoryW(current_absolute_path)))
This could be simplified to "PathIsDirectory || path_in_array(...)".
+ { + new_path = calloc(1, sizeof(struct path)); + new_path->name = wcsdup(current_relative_path); + list_add_tail(paths, &new_path->entry); + } + } + while (FindNextFileW(temp_handle, &entry_data) != 0);
FindNextFileW() returns BOOL; comparing it to 0 strikes me as a bit weird.
+ } + } +} + +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)) + { + output_error(STRING_ERROR_READ_DIRECTORY, ERROR_FILE_NOT_FOUND, 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) + { + PathAllocCombine(options.source, current_path->name, + PATHCCH_ALLOW_LONG_PATHS | PATHCCH_FORCE_ENABLE_LONG_NAME_PROCESS, + ¤t_absolute_path); + + /* append the relative source path to the destination to get the target path */ + PathAllocCombine(options.destination, current_path->name, + PATHCCH_ALLOW_LONG_PATHS | PATHCCH_FORCE_ENABLE_LONG_NAME_PROCESS, + &target_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); + 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); + }
These braces are redundant, which bothers me mostly because this block is inconsistent.
+ } + } + return TRUE; +} + static void print_header(void) { UINT i; @@ -146,8 +303,24 @@ int __cdecl wmain(int argc, WCHAR *argv[]) { 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++; + } + print_header();
- WINE_FIXME("robocopy stub"); - return 1; + /* Break if Source or Destination not set */ + if (!options.destination || !options.source) + { + output_message(format_string(STRING_MISSING_DESTINATION_OR_SOURCE)); + return ROBOCOPY_ERROR_NO_FILES_COPIED; + } + + if (!perform_copy()) + return ROBOCOPY_ERROR_NO_FILES_COPIED; + + return ROBOCOPY_NO_ERROR_FILES_COPIED;
I did a double take when reading these two lines. How about "SUCCESS" instead of "NO_ERROR"? Although it's not clear why "ROBOCOPY_SUCCESS" and "ROBOCOPY_ERROR" aren't enough by themselves; are there other error codes to worry about?
Basic conformance tests create a test source directory with files and 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(a)gmail.com> --- v6: exit_code is now set as a pointer / argument to execute_robocopy, instead of being its return value --- programs/robocopy/tests/robocopy.c | 188 ++++++++++++++++++++++++++++- 1 file changed, 187 insertions(+), 1 deletion(-) diff --git a/programs/robocopy/tests/robocopy.c b/programs/robocopy/tests/robocopy.c index d5d3dee473a..0eb975861bc 100644 --- a/programs/robocopy/tests/robocopy.c +++ b/programs/robocopy/tests/robocopy.c @@ -49,10 +49,153 @@ static BOOL execute_robocopy(const WCHAR *args, DWORD *exit_code) return TRUE; } +static void create_test_file(const WCHAR *relative_path, size_t size, LONGLONG fixed_filetime, long filetime_offset) +{ + HANDLE handle; + WCHAR path[1024]; + wcscpy(path, L"\\\\?\\"); + GetTempPathW(ARRAY_SIZE(path) - 4, &(path[4])); + wcscat(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()); + } + if (fixed_filetime != 0) + { + FILETIME time; + LARGE_INTEGER time_as_integer; + time_as_integer.QuadPart = fixed_filetime; + time.dwHighDateTime = time_as_integer.HighPart; + time.dwLowDateTime = time_as_integer.LowPart; + ok(SetFileTime(handle, &time, &time, &time), "filetime manipulation of %s failed (%d)\n", debugstr_w(path), GetLastError()); + } + if (filetime_offset != 0) + { + FILETIME time, modified_time, access_time; + LARGE_INTEGER time_as_integer; + GetFileTime(handle, &time, &modified_time, &access_time); + /* FILETIME is no union with LONGLONG / LONG64, casting could be unsafe */ + time_as_integer.HighPart = time.dwHighDateTime; + time_as_integer.LowPart = time.dwLowDateTime; + /* 1000 * 1000 * 60 * 60 * 24 = 86400000000ns per day */ + time_as_integer.QuadPart += 864000000000LL * filetime_offset; + time.dwHighDateTime = time_as_integer.HighPart; + time.dwLowDateTime = time_as_integer.LowPart; + ok(SetFileTime(handle, &time, &time, &time), "filetime manipulation of %s failed (%d)\n", debugstr_w(path), GetLastError()); + } + CloseHandle(handle); +} + +static void create_test_folder(const WCHAR *relative_path) +{ + WCHAR path[1024]; + wcscpy(path, L"\\\\?\\"); + GetTempPathW(ARRAY_SIZE(path) - 4, &(path[4])); + wcscat(path, relative_path); + CreateDirectoryW(path, NULL); +} + +static void create_test_source_folder(void) +{ + create_test_folder(L"robocopy_source"); + create_test_folder(L"robocopy_source\\folderA"); + create_test_folder(L"robocopy_source\\folderB"); + create_test_folder(L"robocopy_source\\folderC"); + create_test_folder(L"robocopy_source\\folderA\\folderD"); + create_test_folder(L"robocopy_source\\folderA\\folderE"); + create_test_file(L"robocopy_source\\fileA.a", 4000, 0, -10); + create_test_file(L"robocopy_source\\fileB.b", 8000, 0, -2); + create_test_file(L"robocopy_source\\folderA\\fileC.c", 60, 0, -2); + create_test_file(L"robocopy_source\\folderA\\fileD.d", 80, 0, 0); + create_test_file(L"robocopy_source\\folderA\\folderD\\fileE.e", 10000, 0, -10); + create_test_file(L"robocopy_source\\folderB\\fileF.f", 10000, 132223104000000000, 0); + create_test_file(L"robocopy_source\\folderB\\fileG.g", 200, 132223104000000000, 0); +} + +static void check_file_and_delete(const WCHAR *relative_path, BOOL should_exist) +{ + WCHAR path[1024]; + wcscpy(path, L"\\\\?\\"); + GetTempPathW(ARRAY_SIZE(path) - 4, &(path[4])); + wcscat(path, relative_path); + if (!DeleteFileW(path)) + { + if (GetLastError() == ERROR_FILE_NOT_FOUND) + ok(!should_exist, "file \"%s\" does not exist, but should exist\n", debugstr_w(relative_path)); + else if (GetLastError() == ERROR_PATH_NOT_FOUND) + ok(!should_exist, "file \"%s\" and the parent directory do not exist, but should exist\n", debugstr_w(relative_path)); + else + ok(FALSE, "file \"%s\" DeleteFileW returned error %d\n", debugstr_w(relative_path), GetLastError()); + } + else + { + ok(should_exist, "file \"%s\" should not exist, but does exist\n", debugstr_w(relative_path)); + } +} + +static void check_folder_and_delete(const WCHAR *relative_path, BOOL should_exist) +{ + WCHAR path[1024]; + wcscpy(path, L"\\\\?\\"); + GetTempPathW(ARRAY_SIZE(path) - 4, &(path[4])); + wcscat(path, relative_path); + if (!RemoveDirectoryW(path)) + { + if (GetLastError() == ERROR_FILE_NOT_FOUND) + ok(!should_exist, "directory \"%s\" does not exist, but should exist\n", debugstr_w(relative_path)); + else if (GetLastError() == ERROR_PATH_NOT_FOUND) + ok(!should_exist, "directory \"%s\" and the parent directory do not exist, but should exist\n", debugstr_w(relative_path)); + else if (GetLastError() == ERROR_DIR_NOT_EMPTY) + ok(FALSE, "directory \"%s\" is unexpectedly not empty\n", debugstr_w(relative_path)); + else + ok(FALSE, "directory \"%s\" DeleteFileW returned error %d\n", debugstr_w(relative_path), GetLastError()); + } + else + { + ok(should_exist, "directory \"%s\" should not exist, but does exist\n", debugstr_w(relative_path)); + } +} + +static void check_basic_copy_test(void) +{ + check_file_and_delete(L"robocopy_source\\fileA.a", TRUE); + check_file_and_delete(L"robocopy_source\\fileB.b", TRUE); + check_file_and_delete(L"robocopy_source\\folderA\\fileC.c", TRUE); + check_file_and_delete(L"robocopy_source\\folderA\\fileD.d", TRUE); + check_file_and_delete(L"robocopy_source\\folderA\\folderD\\fileE.e", TRUE); + check_file_and_delete(L"robocopy_source\\folderB\\fileF.f", TRUE); + check_file_and_delete(L"robocopy_source\\folderB\\fileG.g", TRUE); + check_folder_and_delete(L"robocopy_source\\folderA\\folderD", TRUE); + check_folder_and_delete(L"robocopy_source\\folderA\\folderE", TRUE); + check_folder_and_delete(L"robocopy_source\\folderA", TRUE); + check_folder_and_delete(L"robocopy_source\\folderB", TRUE); + check_folder_and_delete(L"robocopy_source\\folderC", TRUE); + check_folder_and_delete(L"robocopy_source", TRUE); + + check_file_and_delete(L"robocopy_destination\\fileA.a", TRUE); + check_file_and_delete(L"robocopy_destination\\fileB.b", TRUE); + todo_wine check_file_and_delete(L"robocopy_destination\\folderA\\fileC.c", FALSE); + todo_wine check_file_and_delete(L"robocopy_destination\\folderA\\fileD.d", FALSE); + todo_wine check_file_and_delete(L"robocopy_destination\\folderA\\folderD\\fileE.e", FALSE); + todo_wine check_file_and_delete(L"robocopy_destination\\folderB\\fileF.f", FALSE); + todo_wine check_file_and_delete(L"robocopy_destination\\folderB\\fileG.g", FALSE); + todo_wine check_folder_and_delete(L"robocopy_destination\\folderA\\folderD", FALSE); + check_folder_and_delete(L"robocopy_destination\\folderA\\folderE", FALSE); + todo_wine check_folder_and_delete(L"robocopy_destination\\folderA", FALSE); + todo_wine check_folder_and_delete(L"robocopy_destination\\folderB", FALSE); + check_folder_and_delete(L"robocopy_destination\\folderC", FALSE); + check_folder_and_delete(L"robocopy_destination", TRUE); +} + START_TEST(robocopy) { DWORD exit_code; - WCHAR temp_path[MAX_PATH]; + WCHAR temp_cmd[512], temp_path[MAX_PATH]; /* robocopy is only available from Vista onwards, abort test if not available */ if (!execute_robocopy(L"", &exit_code)) return; @@ -100,4 +243,47 @@ START_TEST(robocopy) execute_robocopy(L"invalid_folder /?", &exit_code); ok(exit_code == 16, "unexpected exit code %d\n", exit_code); winetest_pop_context(); + + winetest_push_context("basic copy test 1"); + create_test_source_folder(); + execute_robocopy(L"robocopy_source robocopy_destination /r:1 /w:0", &exit_code); + ok(exit_code == 1, "unexpected exit code %d\n", exit_code); + check_basic_copy_test(); + winetest_pop_context(); + + winetest_push_context("basic copy test 2"); + create_test_source_folder(); + execute_robocopy(L"./robocopy_source third_folder/../robocopy_destination /r:1 /w:0", &exit_code); + ok(exit_code == 1, "unexpected exit code %d\n", exit_code); + check_basic_copy_test(); + winetest_pop_context(); + + winetest_push_context("basic copy test 3"); + create_test_source_folder(); + create_test_folder(L"robocopy_destination"); + create_test_file(L"robocopy_destination\\fileA.a", 9000, 0, -10); + execute_robocopy(L"./robocopy_source robocopy_source/../robocopy_destination /r:1 /w:0", &exit_code); + ok(exit_code == 1, "unexpected exit code %d\n", exit_code); + check_basic_copy_test(); + winetest_pop_context(); + + winetest_push_context("basic copy test 4"); + create_test_source_folder(); + swprintf(temp_cmd, ARRAY_SIZE(temp_cmd), + L"%s\\robocopy_source %s\\robocopy_destination /r:1 /w:0", + temp_path, temp_path); + execute_robocopy(temp_cmd, &exit_code); + ok(exit_code == 1, "unexpected exit code %d\n", exit_code); + check_basic_copy_test(); + winetest_pop_context(); + + winetest_push_context("basic copy test 5"); + create_test_source_folder(); + swprintf(temp_cmd, ARRAY_SIZE(temp_cmd), + L"%s\\third_folder\\..\\robocopy_source %s\\third_folder\\..\\robocopy_destination /r:1 /w:0", + temp_path, temp_path); + execute_robocopy(temp_cmd, &exit_code); + ok(exit_code == 1, "unexpected exit code %d\n", exit_code); + check_basic_copy_test(); + winetest_pop_context(); } -- 2.32.0
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
On 9/21/21 09:54, Florian Eder wrote:
Basic conformance tests create a test source directory with files and 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(a)gmail.com> --- v6: exit_code is now set as a pointer / argument to execute_robocopy, instead of being its return value --- programs/robocopy/tests/robocopy.c | 188 ++++++++++++++++++++++++++++- 1 file changed, 187 insertions(+), 1 deletion(-)
diff --git a/programs/robocopy/tests/robocopy.c b/programs/robocopy/tests/robocopy.c index d5d3dee473a..0eb975861bc 100644 --- a/programs/robocopy/tests/robocopy.c +++ b/programs/robocopy/tests/robocopy.c @@ -49,10 +49,153 @@ static BOOL execute_robocopy(const WCHAR *args, DWORD *exit_code) return TRUE; }
+static void create_test_file(const WCHAR *relative_path, size_t size, LONGLONG fixed_filetime, long filetime_offset) +{ + HANDLE handle; + WCHAR path[1024]; + wcscpy(path, L"\\\\?\\"); + GetTempPathW(ARRAY_SIZE(path) - 4, &(path[4])); + wcscat(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()); + } + if (fixed_filetime != 0) + { + FILETIME time; + LARGE_INTEGER time_as_integer; + time_as_integer.QuadPart = fixed_filetime; + time.dwHighDateTime = time_as_integer.HighPart; + time.dwLowDateTime = time_as_integer.LowPart; + ok(SetFileTime(handle, &time, &time, &time), "filetime manipulation of %s failed (%d)\n", debugstr_w(path), GetLastError()); + } + if (filetime_offset != 0) + { + FILETIME time, modified_time, access_time; + LARGE_INTEGER time_as_integer; + GetFileTime(handle, &time, &modified_time, &access_time); + /* FILETIME is no union with LONGLONG / LONG64, casting could be unsafe */ + time_as_integer.HighPart = time.dwHighDateTime; + time_as_integer.LowPart = time.dwLowDateTime; + /* 1000 * 1000 * 60 * 60 * 24 = 86400000000ns per day */ + time_as_integer.QuadPart += 864000000000LL * filetime_offset; + time.dwHighDateTime = time_as_integer.HighPart; + time.dwLowDateTime = time_as_integer.LowPart; + ok(SetFileTime(handle, &time, &time, &time), "filetime manipulation of %s failed (%d)\n", debugstr_w(path), GetLastError()); + }
You don't actually test the file times yet; this probably deserves to be put off to a later patch.
+ CloseHandle(handle); +} + +static void create_test_folder(const WCHAR *relative_path) +{ + WCHAR path[1024]; + wcscpy(path, L"\\\\?\\"); + GetTempPathW(ARRAY_SIZE(path) - 4, &(path[4])); For tests I think it's reasonable to assume everything fits in MAX_PATH.
+ wcscat(path, relative_path); + CreateDirectoryW(path, NULL); +} + +static void create_test_source_folder(void) +{ + create_test_folder(L"robocopy_source"); + create_test_folder(L"robocopy_source\\folderA"); + create_test_folder(L"robocopy_source\\folderB"); + create_test_folder(L"robocopy_source\\folderC"); + create_test_folder(L"robocopy_source\\folderA\\folderD"); + create_test_folder(L"robocopy_source\\folderA\\folderE"); + create_test_file(L"robocopy_source\\fileA.a", 4000, 0, -10); + create_test_file(L"robocopy_source\\fileB.b", 8000, 0, -2); + create_test_file(L"robocopy_source\\folderA\\fileC.c", 60, 0, -2); + create_test_file(L"robocopy_source\\folderA\\fileD.d", 80, 0, 0); + create_test_file(L"robocopy_source\\folderA\\folderD\\fileE.e", 10000, 0, -10); + create_test_file(L"robocopy_source\\folderB\\fileF.f", 10000, 132223104000000000, 0); + create_test_file(L"robocopy_source\\folderB\\fileG.g", 200, 132223104000000000, 0); +} + +static void check_file_and_delete(const WCHAR *relative_path, BOOL should_exist) +{ + WCHAR path[1024]; + wcscpy(path, L"\\\\?\\"); + GetTempPathW(ARRAY_SIZE(path) - 4, &(path[4])); + wcscat(path, relative_path); + if (!DeleteFileW(path)) + { + if (GetLastError() == ERROR_FILE_NOT_FOUND) + ok(!should_exist, "file \"%s\" does not exist, but should exist\n", debugstr_w(relative_path)); + else if (GetLastError() == ERROR_PATH_NOT_FOUND) + ok(!should_exist, "file \"%s\" and the parent directory do not exist, but should exist\n", debugstr_w(relative_path)); + else + ok(FALSE, "file \"%s\" DeleteFileW returned error %d\n", debugstr_w(relative_path), GetLastError()); + } + else + { + ok(should_exist, "file \"%s\" should not exist, but does exist\n", debugstr_w(relative_path)); + }
This seems structured very awkwardly, and the fact that you have ok(FALSE) is good evidence of that. I'd suggest something like the following: ret = DeleteFileW(path); ok(ret == should_exist, ...); if (!should_exist) ok(GetLastError() == ERROR_FILE_NOT_FOUND || GetLastError() == ERROR_PATH_NOT_FOUND, ...); Similarly for check_folder_and_delete(). I might also get rid of the and_delete from the name.
+} + +static void check_folder_and_delete(const WCHAR *relative_path, BOOL should_exist) +{ + WCHAR path[1024]; + wcscpy(path, L"\\\\?\\"); + GetTempPathW(ARRAY_SIZE(path) - 4, &(path[4])); + wcscat(path, relative_path); + if (!RemoveDirectoryW(path)) + { + if (GetLastError() == ERROR_FILE_NOT_FOUND) + ok(!should_exist, "directory \"%s\" does not exist, but should exist\n", debugstr_w(relative_path)); + else if (GetLastError() == ERROR_PATH_NOT_FOUND) + ok(!should_exist, "directory \"%s\" and the parent directory do not exist, but should exist\n", debugstr_w(relative_path)); + else if (GetLastError() == ERROR_DIR_NOT_EMPTY) + ok(FALSE, "directory \"%s\" is unexpectedly not empty\n", debugstr_w(relative_path)); + else + ok(FALSE, "directory \"%s\" DeleteFileW returned error %d\n", debugstr_w(relative_path), GetLastError()); + } + else + { + ok(should_exist, "directory \"%s\" should not exist, but does exist\n", debugstr_w(relative_path)); + } +} + +static void check_basic_copy_test(void) +{ + check_file_and_delete(L"robocopy_source\\fileA.a", TRUE); + check_file_and_delete(L"robocopy_source\\fileB.b", TRUE); + check_file_and_delete(L"robocopy_source\\folderA\\fileC.c", TRUE); + check_file_and_delete(L"robocopy_source\\folderA\\fileD.d", TRUE); + check_file_and_delete(L"robocopy_source\\folderA\\folderD\\fileE.e", TRUE); + check_file_and_delete(L"robocopy_source\\folderB\\fileF.f", TRUE); + check_file_and_delete(L"robocopy_source\\folderB\\fileG.g", TRUE); + check_folder_and_delete(L"robocopy_source\\folderA\\folderD", TRUE); + check_folder_and_delete(L"robocopy_source\\folderA\\folderE", TRUE); + check_folder_and_delete(L"robocopy_source\\folderA", TRUE); + check_folder_and_delete(L"robocopy_source\\folderB", TRUE); + check_folder_and_delete(L"robocopy_source\\folderC", TRUE); + check_folder_and_delete(L"robocopy_source", TRUE); + + check_file_and_delete(L"robocopy_destination\\fileA.a", TRUE); + check_file_and_delete(L"robocopy_destination\\fileB.b", TRUE); + todo_wine check_file_and_delete(L"robocopy_destination\\folderA\\fileC.c", FALSE); + todo_wine check_file_and_delete(L"robocopy_destination\\folderA\\fileD.d", FALSE); + todo_wine check_file_and_delete(L"robocopy_destination\\folderA\\folderD\\fileE.e", FALSE); + todo_wine check_file_and_delete(L"robocopy_destination\\folderB\\fileF.f", FALSE); + todo_wine check_file_and_delete(L"robocopy_destination\\folderB\\fileG.g", FALSE); + todo_wine check_folder_and_delete(L"robocopy_destination\\folderA\\folderD", FALSE); + check_folder_and_delete(L"robocopy_destination\\folderA\\folderE", FALSE); + todo_wine check_folder_and_delete(L"robocopy_destination\\folderA", FALSE); + todo_wine check_folder_and_delete(L"robocopy_destination\\folderB", FALSE); + check_folder_and_delete(L"robocopy_destination\\folderC", FALSE); + check_folder_and_delete(L"robocopy_destination", TRUE); +} + START_TEST(robocopy) { DWORD exit_code; - WCHAR temp_path[MAX_PATH]; + WCHAR temp_cmd[512], temp_path[MAX_PATH];
/* robocopy is only available from Vista onwards, abort test if not available */ if (!execute_robocopy(L"", &exit_code)) return; @@ -100,4 +243,47 @@ START_TEST(robocopy) execute_robocopy(L"invalid_folder /?", &exit_code); ok(exit_code == 16, "unexpected exit code %d\n", exit_code); winetest_pop_context(); + + winetest_push_context("basic copy test 1"); + create_test_source_folder(); + execute_robocopy(L"robocopy_source robocopy_destination /r:1 /w:0", &exit_code);
You might consider shortening things by changing the working directory to a generated subdir of %temp% instead of %temp% itself, and then you don't have to worry about prepending "robocopy" to everything.
+ ok(exit_code == 1, "unexpected exit code %d\n", exit_code); + check_basic_copy_test(); + winetest_pop_context(); + + winetest_push_context("basic copy test 2"); + create_test_source_folder(); + execute_robocopy(L"./robocopy_source third_folder/../robocopy_destination /r:1 /w:0", &exit_code); + ok(exit_code == 1, "unexpected exit code %d\n", exit_code); + check_basic_copy_test(); + winetest_pop_context(); + + winetest_push_context("basic copy test 3"); + create_test_source_folder(); + create_test_folder(L"robocopy_destination"); + create_test_file(L"robocopy_destination\\fileA.a", 9000, 0, -10); + execute_robocopy(L"./robocopy_source robocopy_source/../robocopy_destination /r:1 /w:0", &exit_code); + ok(exit_code == 1, "unexpected exit code %d\n", exit_code); + check_basic_copy_test(); + winetest_pop_context(); + + winetest_push_context("basic copy test 4"); + create_test_source_folder(); + swprintf(temp_cmd, ARRAY_SIZE(temp_cmd), + L"%s\\robocopy_source %s\\robocopy_destination /r:1 /w:0", + temp_path, temp_path); + execute_robocopy(temp_cmd, &exit_code); + ok(exit_code == 1, "unexpected exit code %d\n", exit_code); + check_basic_copy_test(); + winetest_pop_context(); + + winetest_push_context("basic copy test 5"); + create_test_source_folder(); + swprintf(temp_cmd, ARRAY_SIZE(temp_cmd), + L"%s\\third_folder\\..\\robocopy_source %s\\third_folder\\..\\robocopy_destination /r:1 /w:0", + temp_path, temp_path); + execute_robocopy(temp_cmd, &exit_code); + ok(exit_code == 1, "unexpected exit code %d\n", exit_code); + check_basic_copy_test(); + winetest_pop_context(); }
There's a lot of behaviour covered in patch 5 that isn't covered here. For example: * There are no tests for path parameters (i.e. after the source and dest). When adding such tests, I would recommend making sure you test the following: - specifying the same path more than once; - specifying an absolute path; - specifying a path that doesn't exist in the source; - specifying both valid and invalid paths, in both orders, to see whether partial copies are done; - specifying wildcard paths [also, wildcard paths followed by a backslash, like a/*/b? I'm assuming this is why we need to use PathMatchSpec()...] * What happens if the source folder itself doesn't exist? * Testing what happens if the source and destination are the same seems helpful. I believe you have most of these tests, somewhere, but ideally they should all be before patch 5/6 in this series :-)
Hey all, I *really* don't wanna nag, but are there any issues with these patches that prevent them from being committed to the codebase? If that's the case, please feel absolutely free to tell me. :-) Sorry and kind regards, Florian Am Di., 21. Sept. 2021 um 16:55 Uhr schrieb Florian Eder < others.meder(a)gmail.com>:
Basic files and required changes to configure(.ac) to create a stub version of the robocopy utility
Signed-off-by: Florian Eder <others.meder(a)gmail.com> --- v6: Identical to patch in v5 --- configure | 2 +- configure.ac | 1 + programs/robocopy/Makefile.in | 8 ++++++++ programs/robocopy/main.c | 29 +++++++++++++++++++++++++++++ programs/robocopy/robocopy.rc | 34 ++++++++++++++++++++++++++++++++++ 5 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 programs/robocopy/Makefile.in create mode 100644 programs/robocopy/main.c create mode 100644 programs/robocopy/robocopy.rc
diff --git a/configure b/configure index db592f0868d..3a8dee81bbc 100755 --- a/configure +++ b/configure @@ -21235,6 +21235,7 @@ wine_fn_config_makefile programs/regedit/tests enable_tests 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/rpcss enable_rpcss wine_fn_config_makefile programs/rundll.exe16 enable_win16 wine_fn_config_makefile programs/rundll32 enable_rundll32 @@ -22768,4 +22769,3 @@ IFS="$ac_save_IFS" $as_echo " $as_me: Finished. Do '${ac_make}' to compile Wine. " >&6 - diff --git a/configure.ac b/configure.ac index 0bc46a1427b..5bd54cfbba7 100644 --- a/configure.ac +++ b/configure.ac @@ -3959,6 +3959,7 @@ WINE_CONFIG_MAKEFILE(programs/regedit/tests) WINE_CONFIG_MAKEFILE(programs/regini) WINE_CONFIG_MAKEFILE(programs/regsvcs) WINE_CONFIG_MAKEFILE(programs/regsvr32) +WINE_CONFIG_MAKEFILE(programs/robocopy) WINE_CONFIG_MAKEFILE(programs/rpcss) WINE_CONFIG_MAKEFILE(programs/rundll.exe16,enable_win16) WINE_CONFIG_MAKEFILE(programs/rundll32) diff --git a/programs/robocopy/Makefile.in b/programs/robocopy/Makefile.in new file mode 100644 index 00000000000..5a48be3725b --- /dev/null +++ b/programs/robocopy/Makefile.in @@ -0,0 +1,8 @@ +MODULE = robocopy.exe + +EXTRADLLFLAGS = -mconsole -municode -mno-cygwin + +C_SRCS = \ + main.c + +RC_SRCS = robocopy.rc diff --git a/programs/robocopy/main.c b/programs/robocopy/main.c new file mode 100644 index 00000000000..bbda15f573a --- /dev/null +++ b/programs/robocopy/main.c @@ -0,0 +1,29 @@ +/* + * 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 + */ + +#include "wine/debug.h" +WINE_DEFAULT_DEBUG_CHANNEL(robocopy); + +#define WIN32_LEAN_AND_MEAN +#include <windows.h> + +int __cdecl wmain(int argc, WCHAR *argv[]) +{ + WINE_FIXME("robocopy stub"); + return 1; +} diff --git a/programs/robocopy/robocopy.rc b/programs/robocopy/robocopy.rc new file mode 100644 index 00000000000..edae5b71d86 --- /dev/null +++ b/programs/robocopy/robocopy.rc @@ -0,0 +1,34 @@ +/* + * 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 + */ + +#include <windef.h> + +#pragma makedep po + +LANGUAGE LANG_NEUTRAL, SUBLANG_NEUTRAL + +#define WINE_FILEDESCRIPTION_STR "Wine Robocopy" +#define WINE_FILENAME_STR "robocopy.exe" +#define WINE_FILETYPE VFT_APP +#define WINE_FILEVERSION 5,1,10,1027 +#define WINE_FILEVERSION_STR "5.1.10.1027" + +#define WINE_PRODUCTVERSION 5,1,10,1027 +#define WINE_PRODUCTVERSION_STR "XP027" + +#include "wine/wine_common_ver.rc" -- 2.32.0
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=98768 Your paranoid android. === debiant2 (build log) === Task: Patch failed to apply === debiant2 (build log) === Task: Patch failed to apply
Sorry for the radio silence. Since these patches seem to be languishing, I'm going to give them a second, more careful review. On 9/21/21 09:54, Florian Eder wrote:
diff --git a/programs/robocopy/Makefile.in b/programs/robocopy/Makefile.in new file mode 100644 index 00000000000..5a48be3725b --- /dev/null +++ b/programs/robocopy/Makefile.in @@ -0,0 +1,8 @@ +MODULE = robocopy.exe + +EXTRADLLFLAGS = -mconsole -municode -mno-cygwin
This wasn't true when the patch was submitted, but we don't need -mno-cygwin anymore ;-)
+ +C_SRCS = \ + main.c + +RC_SRCS = robocopy.rc diff --git a/programs/robocopy/main.c b/programs/robocopy/main.c new file mode 100644 index 00000000000..bbda15f573a --- /dev/null +++ b/programs/robocopy/main.c @@ -0,0 +1,29 @@ +/* + * 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 + */ + +#include "wine/debug.h" +WINE_DEFAULT_DEBUG_CHANNEL(robocopy); + +#define WIN32_LEAN_AND_MEAN +#include <windows.h> + +int __cdecl wmain(int argc, WCHAR *argv[]) +{ + WINE_FIXME("robocopy stub");
This is missing a terminating newline.
+ return 1; +}
participants (3)
-
Florian Eder -
Marvin -
Zebediah Figura (she/her)