From dbcd970c27ba42ab09073211f858b1a89c7f8300 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 30 Sep 2019 02:55:31 -0700 Subject: push: do not pretend to return `int` from `die_push_simple()` This function is marked as `NORETURN`, and it indeed does not want to return anything. So let's not declare it with the return type `int`. This fixes the following warning when building with MSVC: C4646: function declared with 'noreturn' has non-void return type Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/builtin/push.c b/builtin/push.c index 021dd3b..d216270 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -143,8 +143,8 @@ static int push_url_of_remote(struct remote *remote, const char ***url_p) return remote->url_nr; } -static NORETURN int die_push_simple(struct branch *branch, - struct remote *remote) +static NORETURN void die_push_simple(struct branch *branch, + struct remote *remote) { /* * There's no point in using shorten_unambiguous_ref here, -- cgit v0.10.2-6-g49f6 From c097b95a260b5a870c26a61d032169b70769e31a Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 4 Oct 2019 08:09:26 -0700 Subject: msvc: avoid using minus operator on unsigned types MSVC complains about this with `-Wall`, which can be taken as a sign that this is indeed a real bug. The symptom is: C4146: unary minus operator applied to unsigned type, result still unsigned Let's avoid this warning in the minimal way, e.g. writing `-1 - ` instead of `- - 1`. Note that the change in the `estimate_cache_size()` function is needed because MSVC considers the "return type" of the `sizeof()` operator to be `size_t`, i.e. unsigned, and therefore it cannot be negated using the unary minus operator. Even worse, that arithmetic is doing extra work, in vain. We want to calculate the entry extra cache size as the difference between the size of the `cache_entry` structure minus the size of the `ondisk_cache_entry` structure, padded to the appropriate alignment boundary. To that end, we start by assigning that difference to the `per_entry` variable, and then abuse the `len` parameter of the `align_padding_size()` macro to take the negative size of the ondisk entry size. Essentially, we try to avoid passing the already calculated difference to that macro by passing the operands of that difference instead, when the macro expects operands of an addition: #define align_padding_size(size, len) \ ((size + (len) + 8) & ~7) - (size + len) Currently, we pass A and -B to that macro instead of passing A - B and 0, where A - B is already stored in the `per_entry` variable, ready to be used. This is neither necessary, nor intuitive. Let's fix this, and have code that is both easier to read and that also does not trigger MSVC's warning. While at it, we take care of reporting overflows (which are unlikely, but hey, defensive programming is good!). We _also_ take pains of casting the unsigned value to signed: otherwise, the signed operand (i.e. the `-1`) would be cast to unsigned before doing the arithmetic. Helped-by: Denton Liu Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/cache.h b/cache.h index 3167585..850e7b9 100644 --- a/cache.h +++ b/cache.h @@ -725,6 +725,19 @@ struct cache_entry *index_file_exists(struct index_state *istate, const char *na */ int index_name_pos(const struct index_state *, const char *name, int namelen); +/* + * Some functions return the negative complement of an insert position when a + * precise match was not found but a position was found where the entry would + * need to be inserted. This helper protects that logic from any integer + * underflow. + */ +static inline int index_pos_to_insert_pos(uintmax_t pos) +{ + if (pos > INT_MAX) + die("overflow: -1 - %"PRIuMAX, pos); + return -1 - (int)pos; +} + #define ADD_CACHE_OK_TO_ADD 1 /* Ok to add */ #define ADD_CACHE_OK_TO_REPLACE 2 /* Ok to replace file/directory */ #define ADD_CACHE_SKIP_DFCHECK 4 /* Ok to skip DF conflict checks */ diff --git a/read-cache.c b/read-cache.c index c701f7f..ec13953 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1276,7 +1276,7 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e */ if (istate->cache_nr > 0 && strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0) - pos = -istate->cache_nr - 1; + pos = index_pos_to_insert_pos(istate->cache_nr); else pos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce)); @@ -1894,7 +1894,7 @@ static size_t estimate_cache_size(size_t ondisk_size, unsigned int entries) /* * Account for potential alignment differences. */ - per_entry += align_padding_size(sizeof(struct cache_entry), -sizeof(struct ondisk_cache_entry)); + per_entry += align_padding_size(per_entry, 0); return ondisk_size + entries * per_entry; } diff --git a/sha1-lookup.c b/sha1-lookup.c index 796ab68..8590aac 100644 --- a/sha1-lookup.c +++ b/sha1-lookup.c @@ -70,7 +70,7 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr, if (miv < lov) return -1; if (hiv < miv) - return -1 - nr; + return index_pos_to_insert_pos(nr); if (lov != hiv) { /* * At this point miv could be equal @@ -97,7 +97,7 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr, lo = mi + 1; mi = lo + (hi - lo) / 2; } while (lo < hi); - return -lo-1; + return index_pos_to_insert_pos(lo); } int bsearch_hash(const unsigned char *sha1, const uint32_t *fanout_nbo, -- cgit v0.10.2-6-g49f6 From 41616ef6182271178c63e1caf0d21f12bea2d41e Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 4 Oct 2019 08:09:27 -0700 Subject: winansi: use FLEX_ARRAY to avoid compiler warning MSVC would complain thusly: C4200: nonstandard extension used: zero-sized array in struct/union Let's just use the `FLEX_ARRAY` constant that we introduced for exactly this type of scenario. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/compat/winansi.c b/compat/winansi.c index cacd82c..54fd701 100644 --- a/compat/winansi.c +++ b/compat/winansi.c @@ -546,7 +546,7 @@ static HANDLE swap_osfhnd(int fd, HANDLE new_handle) typedef struct _OBJECT_NAME_INFORMATION { UNICODE_STRING Name; - WCHAR NameBuffer[0]; + WCHAR NameBuffer[FLEX_ARRAY]; } OBJECT_NAME_INFORMATION, *POBJECT_NAME_INFORMATION; #define ObjectNameInformation 1 -- cgit v0.10.2-6-g49f6 From 5b8f9e2417d23a688004eaa402351a8c270845d0 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 4 Oct 2019 08:09:28 -0700 Subject: compat/win32/path-utils.h: add #include guards This adds the common guards that allow headers to be #include'd multiple times. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/compat/win32/path-utils.h b/compat/win32/path-utils.h index 0f70d43..8ed062a 100644 --- a/compat/win32/path-utils.h +++ b/compat/win32/path-utils.h @@ -1,3 +1,6 @@ +#ifndef WIN32_PATH_UTILS_H +#define WIN32_PATH_UTILS_H + #define has_dos_drive_prefix(path) \ (isalpha(*(path)) && (path)[1] == ':' ? 2 : 0) int win32_skip_dos_drive_prefix(char **path); @@ -18,3 +21,5 @@ static inline char *win32_find_last_dir_sep(const char *path) #define find_last_dir_sep win32_find_last_dir_sep int win32_offset_1st_component(const char *path); #define offset_1st_component win32_offset_1st_component + +#endif -- cgit v0.10.2-6-g49f6 From ed712ef8d5f7c11c0b23d06f37d9c3275dff8387 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 4 Oct 2019 08:09:29 -0700 Subject: msvc: ignore some libraries when linking To build with MSVC, we "translate" GCC options to MSVC options, and part of those options refer to the libraries to link into the final executable. Currently, this part looks somewhat like this on Windows: -lcurl -lnghttp2 -lidn2 -lssl -lcrypto -lssl -lcrypto -lgdi32 -lcrypt32 -lwldap32 -lz -lws2_32 -lexpat Some of those are direct dependencies (such as curl and ssl) and others are indirect (nghttp2 and idn2, for example, are dependencies of curl, but need to be linked in for reasons). We already handle the direct dependencies, e.g. `-liconv` is already handled as adding `libiconv.lib` to the list of libraries to link against. Let's just ignore the remaining `-l*` options so that MSVC does not have to warn us that it ignored e.g. the `/lnghttp2` option. We do that by extending the clause that already "eats" the `-R*` options to also eat the `-l*` options. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/compat/vcbuild/scripts/clink.pl b/compat/vcbuild/scripts/clink.pl index c7b021b..00fc339 100755 --- a/compat/vcbuild/scripts/clink.pl +++ b/compat/vcbuild/scripts/clink.pl @@ -68,7 +68,7 @@ while (@ARGV) { } elsif ("$arg" =~ /^-L/ && "$arg" ne "-LTCG") { $arg =~ s/^-L/-LIBPATH:/; push(@lflags, $arg); - } elsif ("$arg" =~ /^-R/) { + } elsif ("$arg" =~ /^-[Rl]/) { # eat } else { push(@args, $arg); -- cgit v0.10.2-6-g49f6 From e4347c943416da5aef56ee70c4488bc0725dfd2a Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 4 Oct 2019 08:09:30 -0700 Subject: msvc: handle DEVELOPER=1 We frequently build Git using the `DEVELOPER=1` make setting as a shortcut to enable all kinds of more stringent compiler warnings. Those compiler warnings are relatively specific to GCC, though, so let's try our best to translate them to the equivalent options to pass to MS Visual C++'s `cl.exe`. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/compat/vcbuild/scripts/clink.pl b/compat/vcbuild/scripts/clink.pl index 00fc339..ec95a3b 100755 --- a/compat/vcbuild/scripts/clink.pl +++ b/compat/vcbuild/scripts/clink.pl @@ -70,6 +70,52 @@ while (@ARGV) { push(@lflags, $arg); } elsif ("$arg" =~ /^-[Rl]/) { # eat + } elsif ("$arg" eq "-Werror") { + push(@cflags, "-WX"); + } elsif ("$arg" eq "-Wall") { + # cl.exe understands -Wall, but it is really overzealous + push(@cflags, "-W4"); + # disable the "signed/unsigned mismatch" warnings; our source code violates that + push(@cflags, "-wd4018"); + push(@cflags, "-wd4245"); + push(@cflags, "-wd4389"); + # disable the "unreferenced formal parameter" warning; our source code violates that + push(@cflags, "-wd4100"); + # disable the "conditional expression is constant" warning; our source code violates that + push(@cflags, "-wd4127"); + # disable the "const object should be initialized" warning; these warnings affect only objects that are `static` + push(@cflags, "-wd4132"); + # disable the "function/data pointer conversion in expression" warning; our source code violates that + push(@cflags, "-wd4152"); + # disable the "non-constant aggregate initializer" warning; our source code violates that + push(@cflags, "-wd4204"); + # disable the "cannot be initialized using address of automatic variable" warning; our source code violates that + push(@cflags, "-wd4221"); + # disable the "possible loss of data" warnings; our source code violates that + push(@cflags, "-wd4244"); + push(@cflags, "-wd4267"); + # disable the "array is too small to include a terminating null character" warning; we ab-use strings to initialize OIDs + push(@cflags, "-wd4295"); + # disable the "'<<': result of 32-bit shift implicitly converted to 64 bits" warning; our source code violates that + push(@cflags, "-wd4334"); + # disable the "declaration hides previous local declaration" warning; our source code violates that + push(@cflags, "-wd4456"); + # disable the "declaration hides function parameter" warning; our source code violates that + push(@cflags, "-wd4457"); + # disable the "declaration hides global declaration" warning; our source code violates that + push(@cflags, "-wd4459"); + # disable the "potentially uninitialized local variable '' used" warning; our source code violates that + push(@cflags, "-wd4701"); + # disable the "unreachable code" warning; our source code violates that + push(@cflags, "-wd4702"); + # disable the "potentially uninitialized local pointer variable used" warning; our source code violates that + push(@cflags, "-wd4703"); + # disable the "assignment within conditional expression" warning; our source code violates that + push(@cflags, "-wd4706"); + # disable the "'inet_ntoa': Use inet_ntop() or InetNtop() instead" warning; our source code violates that + push(@cflags, "-wd4996"); + } elsif ("$arg" =~ /^-W[a-z]/) { + # let's ignore those } else { push(@args, $arg); } -- cgit v0.10.2-6-g49f6 From 61d1d92aa4785090213e89566e01e342f60d9b92 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 4 Oct 2019 08:09:30 -0700 Subject: msvc: work around a bug in GetEnvironmentVariable() The return value of that function is 0 both for variables that are unset, as well as for variables whose values are empty. To discern those two cases, one has to call `GetLastError()`, whose return value is `ERROR_ENVVAR_NOT_FOUND` and `ERROR_SUCCESS`, respectively. Except that it is not actually set to `ERROR_SUCCESS` in the latter case, apparently, but the last error value seems to be simply unchanged. To work around this, let's just re-set the last error value just before inspecting the environment variable. This fixes a problem that triggers failures in t3301-notes.sh (where we try to override config settings by passing empty values for certain environment variables). This problem is hidden in the MINGW build by the fact that older MSVC runtimes (such as the one used by MINGW builds) have a `calloc()` that re-sets the last error value in case of success, while newer runtimes set the error value only if `NULL` is returned by that function. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/compat/mingw.c b/compat/mingw.c index 4891789..7d8cb81 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -1661,6 +1661,8 @@ char *mingw_getenv(const char *name) if (!w_key) die("Out of memory, (tried to allocate %u wchar_t's)", len_key); xutftowcs(w_key, name, len_key); + /* GetEnvironmentVariableW() only sets the last error upon failure */ + SetLastError(ERROR_SUCCESS); len_value = GetEnvironmentVariableW(w_key, w_value, ARRAY_SIZE(w_value)); if (!len_value && GetLastError() == ERROR_ENVVAR_NOT_FOUND) { free(w_key); -- cgit v0.10.2-6-g49f6 From 030a628b815f87d29def74bdf9d3635f7e4c0dac Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 4 Oct 2019 08:09:31 -0700 Subject: vcxproj: only copy `git-remote-http.exe` once it was built In b18ae14a8f6 (vcxproj: also link-or-copy builtins, 2019-07-29), we started to copy or hard-link the built-ins as a post-build step of the `git` project. At the same time, we tried to copy or hard-link `git-remote-http.exe`, but it is quite possible that it was not built at that time. Let's move that latter task into a post-install step of the `git-remote-http` project instead. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/config.mak.uname b/config.mak.uname index db7f06b..701aad6 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -703,20 +703,24 @@ vcxproj: perl contrib/buildsystems/generate -g Vcxproj git add -f git.sln {*,*/lib,t/helper/*}/*.vcxproj - # Generate the LinkOrCopyBuiltins.targets file + # Generate the LinkOrCopyBuiltins.targets and LinkOrCopyRemoteHttp.targets file (echo '' && \ echo ' ' && \ for name in $(BUILT_INS);\ do \ echo ' '; \ done && \ + echo ' ' && \ + echo '') >git/LinkOrCopyBuiltins.targets + (echo '' && \ + echo ' ' && \ for name in $(REMOTE_CURL_ALIASES); \ do \ echo ' '; \ done && \ echo ' ' && \ - echo '') >git/LinkOrCopyBuiltins.targets - git add -f git/LinkOrCopyBuiltins.targets + echo '') >git-remote-http/LinkOrCopyRemoteHttp.targets + git add -f git/LinkOrCopyBuiltins.targets git-remote-http/LinkOrCopyRemoteHttp.targets # Add command-list.h $(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 command-list.h diff --git a/contrib/buildsystems/Generators/Vcxproj.pm b/contrib/buildsystems/Generators/Vcxproj.pm index 576ccab..868b787 100644 --- a/contrib/buildsystems/Generators/Vcxproj.pm +++ b/contrib/buildsystems/Generators/Vcxproj.pm @@ -277,6 +277,9 @@ EOM if ($target eq 'git') { print F " \n"; } + if ($target eq 'git-remote-http') { + print F " \n"; + } print F << "EOM"; EOM -- cgit v0.10.2-6-g49f6 From 5d65ad17a979420fb02328540ec02a3b304aeeec Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 4 Oct 2019 08:09:32 -0700 Subject: vcxproj: include more generated files In the CI builds, we bundle all generated files into a so-called artifacts `.tar` file, so that the test phase can fan out into multiple parallel builds. This patch makes sure that all files are included in the `vcxproj` target which are needed for that artifacts `.tar` file. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/config.mak.uname b/config.mak.uname index 701aad6..cc8efd9 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -728,11 +728,10 @@ vcxproj: # Add scripts rm -f perl/perl.mak - $(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 \ - $(SCRIPT_LIB) $(SCRIPT_SH_GEN) $(SCRIPT_PERL_GEN) + $(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 $(SCRIPT_LIB) $(SCRIPTS) # Strip out the sane tool path, needed only for building sed -i '/^git_broken_path_fix ".*/d' git-sh-setup - git add -f $(SCRIPT_LIB) $(SCRIPT_SH_GEN) $(SCRIPT_PERL_GEN) + git add -f $(SCRIPT_LIB) $(SCRIPTS) # Add Perl module $(MAKE) $(LIB_PERL_GEN) @@ -762,6 +761,10 @@ vcxproj: $(MAKE) -C templates git add -f templates/boilerplates.made templates/blt/ + # Add the translated messages + make MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 $(MOFILES) + git add -f $(MOFILES) + # Add build options $(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 GIT-BUILD-OPTIONS git add -f GIT-BUILD-OPTIONS -- cgit v0.10.2-6-g49f6 From be5d88e11280eeeeae5c35711f4a8053b9f862ba Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 4 Oct 2019 08:09:33 -0700 Subject: test-tool run-command: learn to run (parts of) the testsuite Git for Windows jumps through hoops to provide a development environment that allows to build Git and to run its test suite. To that end, an entire MSYS2 system, including GNU make and GCC is offered as "the Git for Windows SDK". It does come at a price: an initial download of said SDK weighs in with several hundreds of megabytes, and the unpacked SDK occupies ~2GB of disk space. A much more native development environment on Windows is Visual Studio. To help contributors use that environment, we already have a Makefile target `vcxproj` that generates a commit with project files (and other generated files), and Git for Windows' `vs/master` branch is continuously re-generated using that target. The idea is to allow building Git in Visual Studio, and to run individual tests using a Portable Git. The one missing thing is a way to run the entire test suite: neither `make` nor `prove` are required to run Git, therefore Git for Windows does not support those commands in the Portable Git. To help with that, add a simple test helper that exercises the `run_processes_parallel()` function to allow for running test scripts in parallel (which is really necessary, especially on Windows, as Git's test suite takes such a long time to run). This will also come in handy for the upcoming change to our Azure Pipeline: we will use this helper in a Portable Git to test the Visual Studio build of Git. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c index 2cc93bb..ead6dc6 100644 --- a/t/helper/test-run-command.c +++ b/t/helper/test-run-command.c @@ -10,9 +10,14 @@ #include "test-tool.h" #include "git-compat-util.h" +#include "cache.h" #include "run-command.h" #include "argv-array.h" #include "strbuf.h" +#include "parse-options.h" +#include "string-list.h" +#include "thread-utils.h" +#include "wildmatch.h" #include #include @@ -50,11 +55,159 @@ static int task_finished(int result, return 1; } +struct testsuite { + struct string_list tests, failed; + int next; + int quiet, immediate, verbose, verbose_log, trace, write_junit_xml; +}; +#define TESTSUITE_INIT \ + { STRING_LIST_INIT_DUP, STRING_LIST_INIT_DUP, -1, 0, 0, 0, 0, 0, 0 } + +static int next_test(struct child_process *cp, struct strbuf *err, void *cb, + void **task_cb) +{ + struct testsuite *suite = cb; + const char *test; + if (suite->next >= suite->tests.nr) + return 0; + + test = suite->tests.items[suite->next++].string; + argv_array_pushl(&cp->args, "sh", test, NULL); + if (suite->quiet) + argv_array_push(&cp->args, "--quiet"); + if (suite->immediate) + argv_array_push(&cp->args, "-i"); + if (suite->verbose) + argv_array_push(&cp->args, "-v"); + if (suite->verbose_log) + argv_array_push(&cp->args, "-V"); + if (suite->trace) + argv_array_push(&cp->args, "-x"); + if (suite->write_junit_xml) + argv_array_push(&cp->args, "--write-junit-xml"); + + strbuf_addf(err, "Output of '%s':\n", test); + *task_cb = (void *)test; + + return 1; +} + +static int test_finished(int result, struct strbuf *err, void *cb, + void *task_cb) +{ + struct testsuite *suite = cb; + const char *name = (const char *)task_cb; + + if (result) + string_list_append(&suite->failed, name); + + strbuf_addf(err, "%s: '%s'\n", result ? "FAIL" : "SUCCESS", name); + + return 0; +} + +static int test_failed(struct strbuf *out, void *cb, void *task_cb) +{ + struct testsuite *suite = cb; + const char *name = (const char *)task_cb; + + string_list_append(&suite->failed, name); + strbuf_addf(out, "FAILED TO START: '%s'\n", name); + + return 0; +} + +static const char * const testsuite_usage[] = { + "test-run-command testsuite [] [...]", + NULL +}; + +static int testsuite(int argc, const char **argv) +{ + struct testsuite suite = TESTSUITE_INIT; + int max_jobs = 1, i, ret; + DIR *dir; + struct dirent *d; + struct option options[] = { + OPT_BOOL('i', "immediate", &suite.immediate, + "stop at first failed test case(s)"), + OPT_INTEGER('j', "jobs", &max_jobs, "run jobs in parallel"), + OPT_BOOL('q', "quiet", &suite.quiet, "be terse"), + OPT_BOOL('v', "verbose", &suite.verbose, "be verbose"), + OPT_BOOL('V', "verbose-log", &suite.verbose_log, + "be verbose, redirected to a file"), + OPT_BOOL('x', "trace", &suite.trace, "trace shell commands"), + OPT_BOOL(0, "write-junit-xml", &suite.write_junit_xml, + "write JUnit-style XML files"), + OPT_END() + }; + + memset(&suite, 0, sizeof(suite)); + suite.tests.strdup_strings = suite.failed.strdup_strings = 1; + + argc = parse_options(argc, argv, NULL, options, + testsuite_usage, PARSE_OPT_STOP_AT_NON_OPTION); + + if (max_jobs <= 0) + max_jobs = online_cpus(); + + dir = opendir("."); + if (!dir) + die("Could not open the current directory"); + while ((d = readdir(dir))) { + const char *p = d->d_name; + + if (*p != 't' || !isdigit(p[1]) || !isdigit(p[2]) || + !isdigit(p[3]) || !isdigit(p[4]) || p[5] != '-' || + !ends_with(p, ".sh")) + continue; + + /* No pattern: match all */ + if (!argc) { + string_list_append(&suite.tests, p); + continue; + } + + for (i = 0; i < argc; i++) + if (!wildmatch(argv[i], p, 0)) { + string_list_append(&suite.tests, p); + break; + } + } + closedir(dir); + + if (!suite.tests.nr) + die("No tests match!"); + if (max_jobs > suite.tests.nr) + max_jobs = suite.tests.nr; + + fprintf(stderr, "Running %d tests (%d at a time)\n", + suite.tests.nr, max_jobs); + + ret = run_processes_parallel(max_jobs, next_test, test_failed, + test_finished, &suite); + + if (suite.failed.nr > 0) { + ret = 1; + fprintf(stderr, "%d tests failed:\n\n", suite.failed.nr); + for (i = 0; i < suite.failed.nr; i++) + fprintf(stderr, "\t%s\n", suite.failed.items[i].string); + } + + string_list_clear(&suite.tests, 0); + string_list_clear(&suite.failed, 0); + + return !!ret; +} + int cmd__run_command(int argc, const char **argv) { struct child_process proc = CHILD_PROCESS_INIT; int jobs; + if (argc > 1 && !strcmp(argv[1], "testsuite")) + exit(testsuite(argc - 1, argv + 1)); + if (argc < 3) return 1; while (!strcmp(argv[1], "env")) { -- cgit v0.10.2-6-g49f6 From ab7d854abaada90febf5b2e0839dd662cb0c8d10 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 4 Oct 2019 08:09:34 -0700 Subject: tests: let --immediate and --write-junit-xml play well together When the `--immediate` option is in effect, any test failure will immediately exit the test script. Together with `--write-junit-xml`, we will want the JUnit-style `.xml` file to be finalized (and not leave the XML incomplete). Let's make it so. This comes in particularly handy when trying to debug via Azure Pipelines, where the JUnit-style XML is consumed to present the test results in an informative and helpful way. While at it, also handle the `error()` code path. The only remaining code path that sets `GIT_EXIT_OK` happens whenever the trash directory could not be set up, i.e. long before the JUnit XML was written, therefore we should _not_ try to finalize that XML in that case. It is tempting to change the `immediate` code path to just hand off to `error`, simplifying the code in the process. That would, however, result in a change of behavior (an additional error message) in the test suite, which is outside of the purview of the current patch series: its goal is to allow building Git with Visual Studio and testing it with a portable version of Git for Windows. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/t/test-lib.sh b/t/test-lib.sh index d1ba337..86b5e61 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -567,6 +567,7 @@ export TERM error () { say_color error "error: $*" + finalize_junit_xml GIT_EXIT_OK=t exit 1 } @@ -695,7 +696,7 @@ test_failure_ () { say_color error "not ok $test_count - $1" shift printf '%s\n' "$*" | sed -e 's/^/# /' - test "$immediate" = "" || { GIT_EXIT_OK=t; exit 1; } + test "$immediate" = "" || { finalize_junit_xml; GIT_EXIT_OK=t; exit 1; } } test_known_broken_ok_ () { @@ -1063,6 +1064,25 @@ write_junit_xml_testcase () { junit_have_testcase=t } +finalize_junit_xml () { + if test -n "$write_junit_xml" && test -n "$junit_xml_path" + then + test -n "$junit_have_testcase" || { + junit_start=$(test-tool date getnanos) + write_junit_xml_testcase "all tests skipped" + } + + # adjust the overall time + junit_time=$(test-tool date getnanos $junit_suite_start) + sed "s/]*/& time=\"$junit_time\"/" \ + <"$junit_xml_path" >"$junit_xml_path.new" + mv "$junit_xml_path.new" "$junit_xml_path" + + write_junit_xml " " "" + write_junit_xml= + fi +} + test_atexit_cleanup=: test_atexit_handler () { # In a succeeding test script 'test_atexit_handler' is invoked @@ -1085,21 +1105,7 @@ test_done () { # removed, so the commands can access pidfiles and socket files. test_atexit_handler - if test -n "$write_junit_xml" && test -n "$junit_xml_path" - then - test -n "$junit_have_testcase" || { - junit_start=$(test-tool date getnanos) - write_junit_xml_testcase "all tests skipped" - } - - # adjust the overall time - junit_time=$(test-tool date getnanos $junit_suite_start) - sed "s/]*/& time=\"$junit_time\"/" \ - <"$junit_xml_path" >"$junit_xml_path.new" - mv "$junit_xml_path.new" "$junit_xml_path" - - write_junit_xml " " "" - fi + finalize_junit_xml if test -z "$HARNESS_ACTIVE" then -- cgit v0.10.2-6-g49f6 From b35304bf958812a99dcbda366670ddc7bf9e086d Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 4 Oct 2019 08:09:35 -0700 Subject: ci: really use shallow clones on Azure Pipelines This was a left-over from the previous YAML schema, and it no longer works. The problem was noticed while editing `azure-pipelines.yml` in VS Code with the very helpful "Azure Pipelines" extension (syntax highlighting and intellisense for `azure-pipelines.yml`...). Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/azure-pipelines.yml b/azure-pipelines.yml index c329b72..55ee23a 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -1,6 +1,5 @@ -resources: -- repo: self - fetchDepth: 1 +variables: + Agent.Source.Git.ShallowFetchDepth: 1 jobs: - job: windows_build -- cgit v0.10.2-6-g49f6 From 46689317ac009ef4ae91235354b6df7bf6d11d17 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 4 Oct 2019 08:09:36 -0700 Subject: ci: also build and test with MS Visual Studio on Azure Pipelines ... because we can, now. Technically, we actually build using `MSBuild`, which is however pretty close to building interactively in Visual Studio. As there is no convenient way to run Git's test suite in Visual Studio, we unpack a Portable Git to run it, using the just-added test helper to allow running test scripts in parallel. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/Makefile b/Makefile index 3716dad..3f6dcec 100644 --- a/Makefile +++ b/Makefile @@ -3025,6 +3025,10 @@ rpm:: @false .PHONY: rpm +ifneq ($(INCLUDE_DLLS_IN_ARTIFACTS),) +OTHER_PROGRAMS += $(shell echo *.dll t/helper/*.dll) +endif + artifacts-tar:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) \ GIT-BUILD-OPTIONS $(TEST_PROGRAMS) $(test_bindir_programs) \ $(MOFILES) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 55ee23a..875e63c 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -130,6 +130,165 @@ jobs: PathtoPublish: t/failed-test-artifacts ArtifactName: failed-test-artifacts +- job: vs_build + displayName: Visual Studio Build + condition: succeeded() + pool: Hosted VS2017 + timeoutInMinutes: 240 + steps: + - powershell: | + if ("$GITFILESHAREPWD" -ne "" -and "$GITFILESHAREPWD" -ne "`$`(gitfileshare.pwd)") { + net use s: \\gitfileshare.file.core.windows.net\test-cache "$GITFILESHAREPWD" /user:AZURE\gitfileshare /persistent:no + cmd /c mklink /d "$(Build.SourcesDirectory)\test-cache" S:\ + } + displayName: 'Mount test-cache' + env: + GITFILESHAREPWD: $(gitfileshare.pwd) + - powershell: | + $urlbase = "https://dev.azure.com/git-for-windows/git/_apis/build/builds" + $id = ((Invoke-WebRequest -UseBasicParsing "${urlbase}?definitions=22&statusFilter=completed&resultFilter=succeeded&`$top=1").content | ConvertFrom-JSON).value[0].id + $downloadUrl = ((Invoke-WebRequest -UseBasicParsing "${urlbase}/$id/artifacts").content | ConvertFrom-JSON).value[1].resource.downloadUrl + (New-Object Net.WebClient).DownloadFile($downloadUrl,"git-sdk-64-minimal.zip") + Expand-Archive git-sdk-64-minimal.zip -DestinationPath . -Force + Remove-Item git-sdk-64-minimal.zip + + # Let Git ignore the SDK and the test-cache + "/git-sdk-64-minimal/`n/test-cache/`n" | Out-File -NoNewLine -Encoding ascii -Append "$(Build.SourcesDirectory)\.git\info\exclude" + displayName: 'Download git-sdk-64-minimal' + - powershell: | + & git-sdk-64-minimal\usr\bin\bash.exe -lc @" + make vcxproj + "@ + if (!$?) { exit(1) } + displayName: Generate Visual Studio Solution + env: + HOME: $(Build.SourcesDirectory) + MSYSTEM: MINGW64 + DEVELOPER: 1 + NO_PERL: 1 + GIT_CONFIG_PARAMETERS: "'user.name=CI' 'user.email=ci@git'" + - powershell: | + $urlbase = "https://dev.azure.com/git/git/_apis/build/builds" + $id = ((Invoke-WebRequest -UseBasicParsing "${urlbase}?definitions=9&statusFilter=completed&resultFilter=succeeded&`$top=1").content | ConvertFrom-JSON).value[0].id + $downloadUrl = ((Invoke-WebRequest -UseBasicParsing "${urlbase}/$id/artifacts").content | ConvertFrom-JSON).value[0].resource.downloadUrl + (New-Object Net.WebClient).DownloadFile($downloadUrl, "compat.zip") + Expand-Archive compat.zip -DestinationPath . -Force + Remove-Item compat.zip + displayName: 'Download vcpkg artifacts' + - task: MSBuild@1 + inputs: + solution: git.sln + platform: x64 + configuration: Release + maximumCpuCount: 4 + - powershell: | + & compat\vcbuild\vcpkg_copy_dlls.bat release + if (!$?) { exit(1) } + & git-sdk-64-minimal\usr\bin\bash.exe -lc @" + mkdir -p artifacts && + eval \"`$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts | grep ^tar)\" + "@ + if (!$?) { exit(1) } + displayName: Bundle artifact tar + env: + HOME: $(Build.SourcesDirectory) + MSYSTEM: MINGW64 + DEVELOPER: 1 + NO_PERL: 1 + MSVC: 1 + VCPKG_ROOT: $(Build.SourcesDirectory)\compat\vcbuild\vcpkg + - powershell: | + $tag = (Invoke-WebRequest -UseBasicParsing "https://gitforwindows.org/latest-tag.txt").content + $version = (Invoke-WebRequest -UseBasicParsing "https://gitforwindows.org/latest-version.txt").content + $url = "https://github.com/git-for-windows/git/releases/download/${tag}/PortableGit-${version}-64-bit.7z.exe" + (New-Object Net.WebClient).DownloadFile($url,"PortableGit.exe") + & .\PortableGit.exe -y -oartifacts\PortableGit + # Wait until it is unpacked + while (-not @(Remove-Item -ErrorAction SilentlyContinue PortableGit.exe; $?)) { sleep 1 } + displayName: Download & extract portable Git + - task: PublishPipelineArtifact@0 + displayName: 'Publish Pipeline Artifact: MSVC test artifacts' + inputs: + artifactName: 'vs-artifacts' + targetPath: '$(Build.SourcesDirectory)\artifacts' + - powershell: | + if ("$GITFILESHAREPWD" -ne "" -and "$GITFILESHAREPWD" -ne "`$`(gitfileshare.pwd)") { + cmd /c rmdir "$(Build.SourcesDirectory)\test-cache" + } + displayName: 'Unmount test-cache' + condition: true + env: + GITFILESHAREPWD: $(gitfileshare.pwd) + +- job: vs_test + displayName: Visual Studio Test + dependsOn: vs_build + condition: succeeded() + pool: Hosted + timeoutInMinutes: 240 + strategy: + parallel: 10 + steps: + - powershell: | + if ("$GITFILESHAREPWD" -ne "" -and "$GITFILESHAREPWD" -ne "`$`(gitfileshare.pwd)") { + net use s: \\gitfileshare.file.core.windows.net\test-cache "$GITFILESHAREPWD" /user:AZURE\gitfileshare /persistent:no + cmd /c mklink /d "$(Build.SourcesDirectory)\test-cache" S:\ + } + displayName: 'Mount test-cache' + env: + GITFILESHAREPWD: $(gitfileshare.pwd) + - task: DownloadPipelineArtifact@0 + displayName: 'Download Pipeline Artifact: VS test artifacts' + inputs: + artifactName: 'vs-artifacts' + targetPath: '$(Build.SourcesDirectory)' + - powershell: | + & PortableGit\git-cmd.exe --command=usr\bin\bash.exe -lc @" + test -f artifacts.tar.gz || { + echo No test artifacts found\; skipping >&2 + exit 0 + } + tar xf artifacts.tar.gz || exit 1 + + # Let Git ignore the SDK and the test-cache + printf '%s\n' /PortableGit/ /test-cache/ >>.git/info/exclude + + cd t && + PATH=\"`$PWD/helper:`$PATH\" && + test-tool.exe run-command testsuite -V -x --write-junit-xml \ + `$(test-tool.exe path-utils slice-tests \ + `$SYSTEM_JOBPOSITIONINPHASE `$SYSTEM_TOTALJOBSINPHASE t[0-9]*.sh) + "@ + if (!$?) { exit(1) } + displayName: 'Test (parallel)' + env: + HOME: $(Build.SourcesDirectory) + MSYSTEM: MINGW64 + NO_SVN_TESTS: 1 + GIT_TEST_SKIP_REBASE_P: 1 + - powershell: | + if ("$GITFILESHAREPWD" -ne "" -and "$GITFILESHAREPWD" -ne "`$`(gitfileshare.pwd)") { + cmd /c rmdir "$(Build.SourcesDirectory)\test-cache" + } + displayName: 'Unmount test-cache' + condition: true + env: + GITFILESHAREPWD: $(gitfileshare.pwd) + - task: PublishTestResults@2 + displayName: 'Publish Test Results **/TEST-*.xml' + inputs: + mergeTestResults: true + testRunTitle: 'vs' + platform: Windows + publishRunAttachments: false + condition: succeededOrFailed() + - task: PublishBuildArtifacts@1 + displayName: 'Publish trash directories of failed tests' + condition: failed() + inputs: + PathtoPublish: t/failed-test-artifacts + ArtifactName: failed-vs-test-artifacts + - job: linux_clang displayName: linux-clang condition: succeeded() -- cgit v0.10.2-6-g49f6