{"thread":{"id":"51924","subject":"[PATCH 00/13] ci: include a Visual Studio build & test in our Azure Pipeline","startedAt":"2019-09-26T08:30:13Z","lastAt":"2019-10-11T22:07:01Z","messageCount":72,"participants":["Johannes Schindelin via GitGitGadget","Denton Liu","Johannes Schindelin","Junio C Hamano","Johannes Sixt","Thomas Gummerer","Alban Gruin","Philip Oakley"],"isPatch":true,"patchVersion":1,"patchTotal":13},"messages":[{"id":"382958","messageId":"pull.288.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":null,"subject":"[PATCH 00/13] ci: include a Visual Studio build & test in our Azure Pipeline","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-26T08:30:08Z","receivedAt":"2019-09-26T08:30:13Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Git's Continuous Integration (CI) includes an Azure Pipeline that builds Git\non Linux, macOS and Windows, in the former two cases even in multiple\nconfigurations (using GCC vs clang, 32-bit vs 64-bit, etc). On Windows, we\nonly build using GCC, using (a subset of) Git for Windows' SDK.\n\nRecently, a patch series made it into Git that re-instates the ability to\ngenerate project files for use with Visual Studio. The idea there being:\ncontributors can check out a special branch that has those generated files\nin one generated commit on top of e.g. Git for Windows' master, allowing the\ncontributors to build Git in Visual Studio, without the need for downloading\nGit for Windows' SDK (which weighs quite a bit: ~600MB download, ~2GB disk\nfootprint). The tests can then be run from a regular Git for Windows Bash.\n\nThis patch series adds that axis to Git's Azure Pipeline: the project files\nare generated, MSBuild (which is kind of the command-line equivalent of\nVisual Studio's \"Build\" operation) is used to build Git, and then a\nparallelized test job runs the test suite in a Portable Git.\n\nThese patches are based on js/visual-studio.\n\nJohannes Schindelin (13):\n  push: do not pretend to return `int` from `die_push_simple()`\n  msvc: avoid using minus operator on unsigned types\n  winansi: use FLEX_ARRAY to avoid compiler warning\n  compat/win32/path-utils.h: add #include guards\n  msvc: ignore some libraries when linking\n  msvc: handle DEVELOPER=1\n  msvc: work around a bug in GetEnvironmentVariable()\n  vcxproj: only copy `git-remote-http.exe` once it was built\n  vcxproj: include more generated files\n  test-tool run-command: learn to run (parts of) the testsuite\n  tests: let --immediate and --write-junit-xml play well together\n  ci: really use shallow clones on Azure Pipelines\n  ci: also build and test with MS Visual Studio on Azure Pipelines\n\n Makefile                                   |   4 +\n azure-pipelines.yml                        | 164 ++++++++++++++++++++-\n builtin/push.c                             |   4 +-\n compat/mingw.c                             |   2 +\n compat/vcbuild/scripts/clink.pl            |  48 +++++-\n compat/win32/path-utils.h                  |   5 +\n compat/winansi.c                           |   2 +-\n config.mak.uname                           |  19 ++-\n contrib/buildsystems/Generators/Vcxproj.pm |   3 +\n read-cache.c                               |   4 +-\n sha1-lookup.c                              |   2 +-\n t/helper/test-run-command.c                | 153 +++++++++++++++++++\n t/test-lib.sh                              |  37 +++--\n 13 files changed, 415 insertions(+), 32 deletions(-)\n\n\nbase-commit: aac6ff7b5beeea9bca66ecda6eec60fc1dd447ec\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-288%2Fdscho%2Fazure-pipelines-msvc-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-288/dscho/azure-pipelines-msvc-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/288\n-- \ngitgitgadget\n"},{"id":"382959","messageId":"4d0b38125a13d85963be5e524becf48271893e2b.1569486607.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.git.gitgitgadget@gmail.com","subject":"[PATCH 01/13] push: do not pretend to return `int` from `die_push_simple()`","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-26T08:30:09Z","receivedAt":"2019-09-26T08:30:13Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThis function is marked as `NORETURN`, and it indeed does not want to\nreturn anything. So let's not declare it with the return type `int`.\nThis fixes the following warning when building with MSVC:\n\n\tC4646: function declared with 'noreturn' has non-void return type\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n builtin/push.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 021dd3b1e4..d216270d5f 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -143,8 +143,8 @@ static int push_url_of_remote(struct remote *remote, const char ***url_p)\n \treturn remote->url_nr;\n }\n \n-static NORETURN int die_push_simple(struct branch *branch,\n-\t\t\t\t    struct remote *remote)\n+static NORETURN void die_push_simple(struct branch *branch,\n+\t\t\t\t     struct remote *remote)\n {\n \t/*\n \t * There's no point in using shorten_unambiguous_ref here,\n-- \ngitgitgadget\n\n"},{"id":"382960","messageId":"2abe1e1fb0bf3025489c2e543b9a9c648a164827.1569486607.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.git.gitgitgadget@gmail.com","subject":"[PATCH 02/13] msvc: avoid using minus operator on unsigned types","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-26T08:30:10Z","receivedAt":"2019-09-26T08:30:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nMSVC complains about this with `-Wall`, which can be taken as a sign\nthat this is indeed a real bug. The symptom is:\n\n\tC4146: unary minus operator applied to unsigned type, result\n\tstill unsigned\n\nLet's avoid this warning in the minimal way, e.g. writing `-1 -\n<unsigned value>` instead of `-<unsigned value> - 1`.\n\nNote that the change in the `estimate_cache_size()` function is\nneeded because MSVC considers the \"return type\" of the `sizeof()`\noperator to be `size_t`, i.e. unsigned, and therefore it cannot be\nnegated using the unary minus operator.\n\nEven worse, that arithmetic is doing extra work, in vain. We want to\ncalculate the entry extra cache size as the difference between the\nsize of the `cache_entry` structure minus the size of the\n`ondisk_cache_entry` structure, padded to the appropriate alignment\nboundary.\n\nTo that end, we start by assigning that difference to the `per_entry`\nvariable, and then abuse the `len` parameter of the\n`align_padding_size()` macro to take the negative size of the ondisk\nentry size. Essentially, we try to avoid passing the already calculated\ndifference to that macro by passing the operands of that difference\ninstead, when the macro expects operands of an addition:\n\n\t#define align_padding_size(size, len) \\\n\t\t((size + (len) + 8) & ~7) - (size + len)\n\nCurrently, we pass A and -B to that macro instead of passing A - B and\n0, where A - B is already stored in the `per_entry` variable, ready to\nbe used.\n\nThis is neither necessary, nor intuitive. Let's fix this, and have code\nthat is both easier to read and that also does not trigger MSVC's\nwarning.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n read-cache.c  | 4 ++--\n sha1-lookup.c | 2 +-\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex c701f7f8b8..11f3357216 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1276,7 +1276,7 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e\n \t */\n \tif (istate->cache_nr > 0 &&\n \t\tstrcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)\n-\t\tpos = -istate->cache_nr - 1;\n+\t\tpos = -1 - istate->cache_nr;\n \telse\n \t\tpos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce));\n \n@@ -1894,7 +1894,7 @@ static size_t estimate_cache_size(size_t ondisk_size, unsigned int entries)\n \t/*\n \t * Account for potential alignment differences.\n \t */\n-\tper_entry += align_padding_size(sizeof(struct cache_entry), -sizeof(struct ondisk_cache_entry));\n+\tper_entry += align_padding_size(per_entry, 0);\n \treturn ondisk_size + entries * per_entry;\n }\n \ndiff --git a/sha1-lookup.c b/sha1-lookup.c\nindex 796ab68da8..c819687730 100644\n--- a/sha1-lookup.c\n+++ b/sha1-lookup.c\n@@ -97,7 +97,7 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,\n \t\t\tlo = mi + 1;\n \t\tmi = lo + (hi - lo) / 2;\n \t} while (lo < hi);\n-\treturn -lo-1;\n+\treturn -1 - lo;\n }\n \n int bsearch_hash(const unsigned char *sha1, const uint32_t *fanout_nbo,\n-- \ngitgitgadget\n\n"},{"id":"382961","messageId":"dbd9022ad540561f4401d43c3a6b7870f025ac27.1569486607.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.git.gitgitgadget@gmail.com","subject":"[PATCH 03/13] winansi: use FLEX_ARRAY to avoid compiler warning","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-26T08:30:12Z","receivedAt":"2019-09-26T08:30:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nMSVC would complain thusly:\n\n    C4200: nonstandard extension used: zero-sized array in struct/union\n\nLet's just use the `FLEX_ARRAY` constant that we introduced for exactly\nthis type of scenario.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n compat/winansi.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/compat/winansi.c b/compat/winansi.c\nindex cacd82c833..54fd701cbf 100644\n--- a/compat/winansi.c\n+++ b/compat/winansi.c\n@@ -546,7 +546,7 @@ static HANDLE swap_osfhnd(int fd, HANDLE new_handle)\n typedef struct _OBJECT_NAME_INFORMATION\n {\n \tUNICODE_STRING Name;\n-\tWCHAR NameBuffer[0];\n+\tWCHAR NameBuffer[FLEX_ARRAY];\n } OBJECT_NAME_INFORMATION, *POBJECT_NAME_INFORMATION;\n \n #define ObjectNameInformation 1\n-- \ngitgitgadget\n\n"},{"id":"382962","messageId":"6417d6f68953f0d7530e6c61bdd09348adade088.1569486607.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.git.gitgitgadget@gmail.com","subject":"[PATCH 04/13] compat/win32/path-utils.h: add #include guards","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-26T08:30:13Z","receivedAt":"2019-09-26T08:30:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThis adds the common guards that allow headers to be #include'd multiple\ntimes.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n compat/win32/path-utils.h | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/compat/win32/path-utils.h b/compat/win32/path-utils.h\nindex 0f70d43920..8ed062a6b7 100644\n--- a/compat/win32/path-utils.h\n+++ b/compat/win32/path-utils.h\n@@ -1,3 +1,6 @@\n+#ifndef WIN32_PATH_UTILS_H\n+#define WIN32_PATH_UTILS_H\n+\n #define has_dos_drive_prefix(path) \\\n \t(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)\n int win32_skip_dos_drive_prefix(char **path);\n@@ -18,3 +21,5 @@ static inline char *win32_find_last_dir_sep(const char *path)\n #define find_last_dir_sep win32_find_last_dir_sep\n int win32_offset_1st_component(const char *path);\n #define offset_1st_component win32_offset_1st_component\n+\n+#endif\n-- \ngitgitgadget\n\n"},{"id":"382963","messageId":"d1dee56fccba12e092a3737f9a7765c54c15c6c2.1569486607.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.git.gitgitgadget@gmail.com","subject":"[PATCH 05/13] msvc: ignore some libraries when linking","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-26T08:30:14Z","receivedAt":"2019-09-26T08:30:18Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nTo build with MSVC, we \"translate\" GCC options to MSVC options, and part\nof those options refer to the libraries to link into the final\nexecutable. Currently, this part looks somewhat like this on Windows:\n\n\t-lcurl -lnghttp2 -lidn2 -lssl -lcrypto -lssl -lcrypto -lgdi32\n\t-lcrypt32 -lwldap32 -lz -lws2_32 -lexpat\n\nSome of those are direct dependencies (such as curl and ssl) and others\nare indirect (nghttp2 and idn2, for example, are dependencies of curl,\nbut need to be linked in for reasons).\n\nWe already handle the direct dependencies, e.g. `-liconv` is already\nhandled as adding `libiconv.lib` to the list of libraries to link\nagainst.\n\nLet's just ignore the remaining `-l*` options so that MSVC does not have\nto warn us that it ignored e.g. the `/lnghttp2` option. We do that by\nextending the clause that already \"eats\" the `-R*` options to also eat\nthe `-l*` options.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n compat/vcbuild/scripts/clink.pl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/compat/vcbuild/scripts/clink.pl b/compat/vcbuild/scripts/clink.pl\nindex c7b021bfac..00fc339cba 100755\n--- a/compat/vcbuild/scripts/clink.pl\n+++ b/compat/vcbuild/scripts/clink.pl\n@@ -68,7 +68,7 @@\n \t} elsif (\"$arg\" =~ /^-L/ && \"$arg\" ne \"-LTCG\") {\n \t\t$arg =~ s/^-L/-LIBPATH:/;\n \t\tpush(@lflags, $arg);\n-\t} elsif (\"$arg\" =~ /^-R/) {\n+\t} elsif (\"$arg\" =~ /^-[Rl]/) {\n \t\t# eat\n \t} else {\n \t\tpush(@args, $arg);\n-- \ngitgitgadget\n\n"},{"id":"382964","messageId":"c978c7659909e21057cdbcaba7c1c8ea444867b9.1569486607.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.git.gitgitgadget@gmail.com","subject":"[PATCH 06/13] msvc: handle DEVELOPER=1","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-26T08:30:15Z","receivedAt":"2019-09-26T08:30:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWe frequently build Git using the `DEVELOPER=1` make setting as a\nshortcut to enable all kinds of more stringent compiler warnings.\n\nThose compiler warnings are relatively specific to GCC, though, so let's\ntry our best to translate them to the equivalent options to pass to MS\nVisual C++'s `cl.exe`.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n compat/vcbuild/scripts/clink.pl | 46 +++++++++++++++++++++++++++++++++\n 1 file changed, 46 insertions(+)\n\ndiff --git a/compat/vcbuild/scripts/clink.pl b/compat/vcbuild/scripts/clink.pl\nindex 00fc339cba..ec95a3b2d0 100755\n--- a/compat/vcbuild/scripts/clink.pl\n+++ b/compat/vcbuild/scripts/clink.pl\n@@ -70,6 +70,52 @@\n \t\tpush(@lflags, $arg);\n \t} elsif (\"$arg\" =~ /^-[Rl]/) {\n \t\t# eat\n+\t} elsif (\"$arg\" eq \"-Werror\") {\n+\t\tpush(@cflags, \"-WX\");\n+\t} elsif (\"$arg\" eq \"-Wall\") {\n+\t\t# cl.exe understands -Wall, but it is really overzealous\n+\t\tpush(@cflags, \"-W4\");\n+\t\t# disable the \"signed/unsigned mismatch\" warnings; our source code violates that\n+\t\tpush(@cflags, \"-wd4018\");\n+\t\tpush(@cflags, \"-wd4245\");\n+\t\tpush(@cflags, \"-wd4389\");\n+\t\t# disable the \"unreferenced formal parameter\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4100\");\n+\t\t# disable the \"conditional expression is constant\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4127\");\n+\t\t# disable the \"const object should be initialized\" warning; these warnings affect only objects that are `static`\n+\t\tpush(@cflags, \"-wd4132\");\n+\t\t# disable the \"function/data pointer conversion in expression\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4152\");\n+\t\t# disable the \"non-constant aggregate initializer\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4204\");\n+\t\t# disable the \"cannot be initialized using address of automatic variable\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4221\");\n+\t\t# disable the \"possible loss of data\" warnings; our source code violates that\n+\t\tpush(@cflags, \"-wd4244\");\n+\t\tpush(@cflags, \"-wd4267\");\n+\t\t# disable the \"array is too small to include a terminating null character\" warning; we ab-use strings to initialize OIDs\n+\t\tpush(@cflags, \"-wd4295\");\n+\t\t# disable the \"'<<': result of 32-bit shift implicitly converted to 64 bits\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4334\");\n+\t\t# disable the \"declaration hides previous local declaration\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4456\");\n+\t\t# disable the \"declaration hides function parameter\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4457\");\n+\t\t# disable the \"declaration hides global declaration\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4459\");\n+\t\t# disable the \"potentially uninitialized local variable '<name>' used\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4701\");\n+\t\t# disable the \"unreachable code\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4702\");\n+\t\t# disable the \"potentially uninitialized local pointer variable used\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4703\");\n+\t\t# disable the \"assignment within conditional expression\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4706\");\n+\t\t# disable the \"'inet_ntoa': Use inet_ntop() or InetNtop() instead\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4996\");\n+\t} elsif (\"$arg\" =~ /^-W[a-z]/) {\n+\t\t# let's ignore those\n \t} else {\n \t\tpush(@args, $arg);\n \t}\n-- \ngitgitgadget\n\n"},{"id":"382965","messageId":"0590514e43a32a96de87f144439936b66d0723e6.1569486607.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.git.gitgitgadget@gmail.com","subject":"[PATCH 07/13] msvc: work around a bug in GetEnvironmentVariable()","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-26T08:30:17Z","receivedAt":"2019-09-26T08:30:22Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThe return value of that function is 0 both for variables that are\nunset, as well as for variables whose values are empty. To discern those\ntwo cases, one has to call `GetLastError()`, whose return value is\n`ERROR_ENVVAR_NOT_FOUND` and `ERROR_SUCCESS`, respectively.\n\nExcept that it is not actually set to `ERROR_SUCCESS` in the latter\ncase, apparently, but the last error value seems to be simply unchanged.\n\nTo work around this, let's just re-set the last error value just before\ninspecting the environment variable.\n\nThis fixes a problem that triggers failures in t3301-notes.sh (where we\ntry to override config settings by passing empty values for certain\nenvironment variables).\n\nThis problem is hidden in the MINGW build by the fact that older\nMSVC runtimes (such as the one used by MINGW builds) have a `calloc()`\nthat re-sets the last error value in case of success, while newer\nruntimes set the error value only if `NULL` is returned by that\nfunction.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n compat/mingw.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 4891789771..7d8cb814ba 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -1661,6 +1661,8 @@ char *mingw_getenv(const char *name)\n \tif (!w_key)\n \t\tdie(\"Out of memory, (tried to allocate %u wchar_t's)\", len_key);\n \txutftowcs(w_key, name, len_key);\n+\t/* GetEnvironmentVariableW() only sets the last error upon failure */\n+\tSetLastError(ERROR_SUCCESS);\n \tlen_value = GetEnvironmentVariableW(w_key, w_value, ARRAY_SIZE(w_value));\n \tif (!len_value && GetLastError() == ERROR_ENVVAR_NOT_FOUND) {\n \t\tfree(w_key);\n-- \ngitgitgadget\n\n"},{"id":"382966","messageId":"f7d5d1a1bce8dd783914b4b5406abdae61440517.1569486607.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.git.gitgitgadget@gmail.com","subject":"[PATCH 08/13] vcxproj: only copy `git-remote-http.exe` once it was built","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-26T08:30:18Z","receivedAt":"2019-09-26T08:30:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nIn b18ae14a8f6 (vcxproj: also link-or-copy builtins, 2019-07-29), we\nstarted to copy or hard-link the built-ins as a post-build step of the\n`git` project.\n\nAt the same time, we tried to copy or hard-link `git-remote-http.exe`,\nbut it is quite possible that it was not built at that time.\n\nLet's move that latter task into a post-install step of the\n`git-remote-http` project instead.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n config.mak.uname                           | 10 +++++++---\n contrib/buildsystems/Generators/Vcxproj.pm |  3 +++\n 2 files changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git a/config.mak.uname b/config.mak.uname\nindex db7f06b95f..701aad62b1 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -703,20 +703,24 @@ vcxproj:\n \tperl contrib/buildsystems/generate -g Vcxproj\n \tgit add -f git.sln {*,*/lib,t/helper/*}/*.vcxproj\n \n-\t# Generate the LinkOrCopyBuiltins.targets file\n+\t# Generate the LinkOrCopyBuiltins.targets and LinkOrCopyRemoteHttp.targets file\n \t(echo '<Project xmlns=\"http://schemas.microsoft.com/developer/msbuild/2003\">' && \\\n \t echo '  <Target Name=\"CopyBuiltins_AfterBuild\" AfterTargets=\"AfterBuild\">' && \\\n \t for name in $(BUILT_INS);\\\n \t do \\\n \t   echo '    <Copy SourceFiles=\"$$(OutDir)\\git.exe\" DestinationFiles=\"$$(OutDir)\\'\"$$name\"'\" SkipUnchangedFiles=\"true\" UseHardlinksIfPossible=\"true\" />'; \\\n \t done && \\\n+\t echo '  </Target>' && \\\n+\t echo '</Project>') >git/LinkOrCopyBuiltins.targets\n+\t(echo '<Project xmlns=\"http://schemas.microsoft.com/developer/msbuild/2003\">' && \\\n+\t echo '  <Target Name=\"CopyBuiltins_AfterBuild\" AfterTargets=\"AfterBuild\">' && \\\n \t for name in $(REMOTE_CURL_ALIASES); \\\n \t do \\\n \t   echo '    <Copy SourceFiles=\"$$(OutDir)\\'\"$(REMOTE_CURL_PRIMARY)\"'\" DestinationFiles=\"$$(OutDir)\\'\"$$name\"'\" SkipUnchangedFiles=\"true\" UseHardlinksIfPossible=\"true\" />'; \\\n \t done && \\\n \t echo '  </Target>' && \\\n-\t echo '</Project>') >git/LinkOrCopyBuiltins.targets\n-\tgit add -f git/LinkOrCopyBuiltins.targets\n+\t echo '</Project>') >git-remote-http/LinkOrCopyRemoteHttp.targets\n+\tgit add -f git/LinkOrCopyBuiltins.targets git-remote-http/LinkOrCopyRemoteHttp.targets\n \n \t# Add command-list.h\n \t$(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 command-list.h\ndiff --git a/contrib/buildsystems/Generators/Vcxproj.pm b/contrib/buildsystems/Generators/Vcxproj.pm\nindex 576ccabe1d..868b787855 100644\n--- a/contrib/buildsystems/Generators/Vcxproj.pm\n+++ b/contrib/buildsystems/Generators/Vcxproj.pm\n@@ -277,6 +277,9 @@ sub createProject {\n     if ($target eq 'git') {\n       print F \"  <Import Project=\\\"LinkOrCopyBuiltins.targets\\\" />\\n\";\n     }\n+    if ($target eq 'git-remote-http') {\n+      print F \"  <Import Project=\\\"LinkOrCopyRemoteHttp.targets\\\" />\\n\";\n+    }\n     print F << \"EOM\";\n </Project>\n EOM\n-- \ngitgitgadget\n\n"},{"id":"382967","messageId":"ad9ab10ce0ec0615570f5d685f5765b27093dae7.1569486607.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.git.gitgitgadget@gmail.com","subject":"[PATCH 10/13] test-tool run-command: learn to run (parts of) the testsuite","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-26T08:30:21Z","receivedAt":"2019-09-26T08:30:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nGit for Windows jumps through hoops to provide a development environment\nthat allows to build Git and to run its test suite. To that end, an\nentire MSYS2 system, including GNU make and GCC is offered as \"the Git\nfor Windows SDK\". It does come at a price: an initial download of said\nSDK weighs in with several hundreds of megabytes, and the unpacked SDK\noccupies ~2GB of disk space.\n\nA much more native development environment on Windows is Visual Studio.\nTo help contributors use that environment, we already have a Makefile\ntarget `vcxproj` that generates a commit with project files (and other\ngenerated files), and Git for Windows' `vs/master` branch is\ncontinuously re-generated using that target.\n\nThe idea is to allow building Git in Visual Studio, and to run\nindividual tests using a Portable Git.\n\nThe one missing thing is a way to run the entire test suite: neither\n`make` nor `prove` are required to run Git, therefore Git for Windows\ndoes not support those commands in the Portable Git.\n\nTo help with that, add a simple test helper that exercises the\n`run_processes_parallel()` function to allow for running test scripts in\nparallel (which is really necessary, especially on Windows, as Git's\ntest suite takes such a long time to run).\n\nThis will also come in handy for the upcoming change to our Azure\nPipeline: we will use this helper in a Portable Git to test the Visual\nStudio build of Git.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n t/helper/test-run-command.c | 153 ++++++++++++++++++++++++++++++++++++\n 1 file changed, 153 insertions(+)\n\ndiff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c\nindex 2cc93bb69c..ead6dc611a 100644\n--- a/t/helper/test-run-command.c\n+++ b/t/helper/test-run-command.c\n@@ -10,9 +10,14 @@\n \n #include \"test-tool.h\"\n #include \"git-compat-util.h\"\n+#include \"cache.h\"\n #include \"run-command.h\"\n #include \"argv-array.h\"\n #include \"strbuf.h\"\n+#include \"parse-options.h\"\n+#include \"string-list.h\"\n+#include \"thread-utils.h\"\n+#include \"wildmatch.h\"\n #include <string.h>\n #include <errno.h>\n \n@@ -50,11 +55,159 @@ static int task_finished(int result,\n \treturn 1;\n }\n \n+struct testsuite {\n+\tstruct string_list tests, failed;\n+\tint next;\n+\tint quiet, immediate, verbose, verbose_log, trace, write_junit_xml;\n+};\n+#define TESTSUITE_INIT \\\n+\t{ STRING_LIST_INIT_DUP, STRING_LIST_INIT_DUP, -1, 0, 0, 0, 0, 0, 0 }\n+\n+static int next_test(struct child_process *cp, struct strbuf *err, void *cb,\n+\t\t     void **task_cb)\n+{\n+\tstruct testsuite *suite = cb;\n+\tconst char *test;\n+\tif (suite->next >= suite->tests.nr)\n+\t\treturn 0;\n+\n+\ttest = suite->tests.items[suite->next++].string;\n+\targv_array_pushl(&cp->args, \"sh\", test, NULL);\n+\tif (suite->quiet)\n+\t\targv_array_push(&cp->args, \"--quiet\");\n+\tif (suite->immediate)\n+\t\targv_array_push(&cp->args, \"-i\");\n+\tif (suite->verbose)\n+\t\targv_array_push(&cp->args, \"-v\");\n+\tif (suite->verbose_log)\n+\t\targv_array_push(&cp->args, \"-V\");\n+\tif (suite->trace)\n+\t\targv_array_push(&cp->args, \"-x\");\n+\tif (suite->write_junit_xml)\n+\t\targv_array_push(&cp->args, \"--write-junit-xml\");\n+\n+\tstrbuf_addf(err, \"Output of '%s':\\n\", test);\n+\t*task_cb = (void *)test;\n+\n+\treturn 1;\n+}\n+\n+static int test_finished(int result, struct strbuf *err, void *cb,\n+\t\t\t void *task_cb)\n+{\n+\tstruct testsuite *suite = cb;\n+\tconst char *name = (const char *)task_cb;\n+\n+\tif (result)\n+\t\tstring_list_append(&suite->failed, name);\n+\n+\tstrbuf_addf(err, \"%s: '%s'\\n\", result ? \"FAIL\" : \"SUCCESS\", name);\n+\n+\treturn 0;\n+}\n+\n+static int test_failed(struct strbuf *out, void *cb, void *task_cb)\n+{\n+\tstruct testsuite *suite = cb;\n+\tconst char *name = (const char *)task_cb;\n+\n+\tstring_list_append(&suite->failed, name);\n+\tstrbuf_addf(out, \"FAILED TO START: '%s'\\n\", name);\n+\n+\treturn 0;\n+}\n+\n+static const char * const testsuite_usage[] = {\n+\t\"test-run-command testsuite [<options>] [<pattern>...]\",\n+\tNULL\n+};\n+\n+static int testsuite(int argc, const char **argv)\n+{\n+\tstruct testsuite suite = TESTSUITE_INIT;\n+\tint max_jobs = 1, i, ret;\n+\tDIR *dir;\n+\tstruct dirent *d;\n+\tstruct option options[] = {\n+\t\tOPT_BOOL('i', \"immediate\", &suite.immediate,\n+\t\t\t \"stop at first failed test case(s)\"),\n+\t\tOPT_INTEGER('j', \"jobs\", &max_jobs, \"run <N> jobs in parallel\"),\n+\t\tOPT_BOOL('q', \"quiet\", &suite.quiet, \"be terse\"),\n+\t\tOPT_BOOL('v', \"verbose\", &suite.verbose, \"be verbose\"),\n+\t\tOPT_BOOL('V', \"verbose-log\", &suite.verbose_log,\n+\t\t\t \"be verbose, redirected to a file\"),\n+\t\tOPT_BOOL('x', \"trace\", &suite.trace, \"trace shell commands\"),\n+\t\tOPT_BOOL(0, \"write-junit-xml\", &suite.write_junit_xml,\n+\t\t\t \"write JUnit-style XML files\"),\n+\t\tOPT_END()\n+\t};\n+\n+\tmemset(&suite, 0, sizeof(suite));\n+\tsuite.tests.strdup_strings = suite.failed.strdup_strings = 1;\n+\n+\targc = parse_options(argc, argv, NULL, options,\n+\t\t\ttestsuite_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n+\n+\tif (max_jobs <= 0)\n+\t\tmax_jobs = online_cpus();\n+\n+\tdir = opendir(\".\");\n+\tif (!dir)\n+\t\tdie(\"Could not open the current directory\");\n+\twhile ((d = readdir(dir))) {\n+\t\tconst char *p = d->d_name;\n+\n+\t\tif (*p != 't' || !isdigit(p[1]) || !isdigit(p[2]) ||\n+\t\t    !isdigit(p[3]) || !isdigit(p[4]) || p[5] != '-' ||\n+\t\t    !ends_with(p, \".sh\"))\n+\t\t\tcontinue;\n+\n+\t\t/* No pattern: match all */\n+\t\tif (!argc) {\n+\t\t\tstring_list_append(&suite.tests, p);\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tfor (i = 0; i < argc; i++)\n+\t\t\tif (!wildmatch(argv[i], p, 0)) {\n+\t\t\t\tstring_list_append(&suite.tests, p);\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t}\n+\tclosedir(dir);\n+\n+\tif (!suite.tests.nr)\n+\t\tdie(\"No tests match!\");\n+\tif (max_jobs > suite.tests.nr)\n+\t\tmax_jobs = suite.tests.nr;\n+\n+\tfprintf(stderr, \"Running %d tests (%d at a time)\\n\",\n+\t\tsuite.tests.nr, max_jobs);\n+\n+\tret = run_processes_parallel(max_jobs, next_test, test_failed,\n+\t\t\t\t     test_finished, &suite);\n+\n+\tif (suite.failed.nr > 0) {\n+\t\tret = 1;\n+\t\tfprintf(stderr, \"%d tests failed:\\n\\n\", suite.failed.nr);\n+\t\tfor (i = 0; i < suite.failed.nr; i++)\n+\t\t\tfprintf(stderr, \"\\t%s\\n\", suite.failed.items[i].string);\n+\t}\n+\n+\tstring_list_clear(&suite.tests, 0);\n+\tstring_list_clear(&suite.failed, 0);\n+\n+\treturn !!ret;\n+}\n+\n int cmd__run_command(int argc, const char **argv)\n {\n \tstruct child_process proc = CHILD_PROCESS_INIT;\n \tint jobs;\n \n+\tif (argc > 1 && !strcmp(argv[1], \"testsuite\"))\n+\t\texit(testsuite(argc - 1, argv + 1));\n+\n \tif (argc < 3)\n \t\treturn 1;\n \twhile (!strcmp(argv[1], \"env\")) {\n-- \ngitgitgadget\n\n"},{"id":"382968","messageId":"6adfc63e98b481c25dc45ed96caeaeb20a70f9fa.1569486607.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.git.gitgitgadget@gmail.com","subject":"[PATCH 09/13] vcxproj: include more generated files","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-26T08:30:20Z","receivedAt":"2019-09-26T08:30:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nIn the CI builds, we bundle all generated files into a so-called\nartifacts `.tar` file, so that the test phase can fan out into multiple\nparallel builds.\n\nThis patch makes sure that all files are included in the `vcxproj`\ntarget which are needed for that artifacts `.tar` file.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n config.mak.uname | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/config.mak.uname b/config.mak.uname\nindex 701aad62b1..cc8efd95b1 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -728,11 +728,10 @@ vcxproj:\n \n \t# Add scripts\n \trm -f perl/perl.mak\n-\t$(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 \\\n-\t\t$(SCRIPT_LIB) $(SCRIPT_SH_GEN) $(SCRIPT_PERL_GEN)\n+\t$(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 $(SCRIPT_LIB) $(SCRIPTS)\n \t# Strip out the sane tool path, needed only for building\n \tsed -i '/^git_broken_path_fix \".*/d' git-sh-setup\n-\tgit add -f $(SCRIPT_LIB) $(SCRIPT_SH_GEN) $(SCRIPT_PERL_GEN)\n+\tgit add -f $(SCRIPT_LIB) $(SCRIPTS)\n \n \t# Add Perl module\n \t$(MAKE) $(LIB_PERL_GEN)\n@@ -762,6 +761,10 @@ vcxproj:\n \t$(MAKE) -C templates\n \tgit add -f templates/boilerplates.made templates/blt/\n \n+\t# Add the translated messages\n+\tmake MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 $(MOFILES)\n+\tgit add -f $(MOFILES)\n+\n \t# Add build options\n \t$(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 GIT-BUILD-OPTIONS\n \tgit add -f GIT-BUILD-OPTIONS\n-- \ngitgitgadget\n\n"},{"id":"382969","messageId":"99724f6a1e45b497e15037bbac1cb5f70a3bb236.1569486607.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.git.gitgitgadget@gmail.com","subject":"[PATCH 11/13] tests: let --immediate and --write-junit-xml play well together","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-26T08:30:22Z","receivedAt":"2019-09-26T08:30:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWhen the `--immediate` option is in effect, any test failure will\nimmediately exit the test script. Together with `--write-junit-xml`, we\nwill want the JUnit-style `.xml` file to be finalized (and not leave the\nXML incomplete). Let's make it so.\n\nThis comes in particularly handy when trying to debug via Azure\nPipelines, where the JUnit-style XML is consumed to present the test\nresults in an informative and helpful way.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n t/test-lib.sh | 37 +++++++++++++++++++++----------------\n 1 file changed, 21 insertions(+), 16 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex d1ba33745a..f21c781e68 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -695,7 +695,7 @@ test_failure_ () {\n \tsay_color error \"not ok $test_count - $1\"\n \tshift\n \tprintf '%s\\n' \"$*\" | sed -e 's/^/#\t/'\n-\ttest \"$immediate\" = \"\" || { GIT_EXIT_OK=t; exit 1; }\n+\ttest \"$immediate\" = \"\" || { finalize_junit_xml; GIT_EXIT_OK=t; exit 1; }\n }\n \n test_known_broken_ok_ () {\n@@ -1063,6 +1063,25 @@ write_junit_xml_testcase () {\n \tjunit_have_testcase=t\n }\n \n+finalize_junit_xml () {\n+\tif test -n \"$write_junit_xml\" && test -n \"$junit_xml_path\"\n+\tthen\n+\t\ttest -n \"$junit_have_testcase\" || {\n+\t\t\tjunit_start=$(test-tool date getnanos)\n+\t\t\twrite_junit_xml_testcase \"all tests skipped\"\n+\t\t}\n+\n+\t\t# adjust the overall time\n+\t\tjunit_time=$(test-tool date getnanos $junit_suite_start)\n+\t\tsed \"s/<testsuite [^>]*/& time=\\\"$junit_time\\\"/\" \\\n+\t\t\t<\"$junit_xml_path\" >\"$junit_xml_path.new\"\n+\t\tmv \"$junit_xml_path.new\" \"$junit_xml_path\"\n+\n+\t\twrite_junit_xml \"  </testsuite>\" \"</testsuites>\"\n+\t\twrite_junit_xml=\n+\tfi\n+}\n+\n test_atexit_cleanup=:\n test_atexit_handler () {\n \t# In a succeeding test script 'test_atexit_handler' is invoked\n@@ -1085,21 +1104,7 @@ test_done () {\n \t# removed, so the commands can access pidfiles and socket files.\n \ttest_atexit_handler\n \n-\tif test -n \"$write_junit_xml\" && test -n \"$junit_xml_path\"\n-\tthen\n-\t\ttest -n \"$junit_have_testcase\" || {\n-\t\t\tjunit_start=$(test-tool date getnanos)\n-\t\t\twrite_junit_xml_testcase \"all tests skipped\"\n-\t\t}\n-\n-\t\t# adjust the overall time\n-\t\tjunit_time=$(test-tool date getnanos $junit_suite_start)\n-\t\tsed \"s/<testsuite [^>]*/& time=\\\"$junit_time\\\"/\" \\\n-\t\t\t<\"$junit_xml_path\" >\"$junit_xml_path.new\"\n-\t\tmv \"$junit_xml_path.new\" \"$junit_xml_path\"\n-\n-\t\twrite_junit_xml \"  </testsuite>\" \"</testsuites>\"\n-\tfi\n+\tfinalize_junit_xml\n \n \tif test -z \"$HARNESS_ACTIVE\"\n \tthen\n-- \ngitgitgadget\n\n"},{"id":"382970","messageId":"29dceaae5ece947e7748330c3fdee4f9667f0fab.1569486607.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.git.gitgitgadget@gmail.com","subject":"[PATCH 12/13] ci: really use shallow clones on Azure Pipelines","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-26T08:30:23Z","receivedAt":"2019-09-26T08:30:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThis was a left-over from the previous YAML schema, and it no longer\nworks. The problem was noticed while editing `azure-pipelines.yml` in VS\nCode with the very helpful \"Azure Pipelines\" extension (syntax\nhighlighting and intellisense for `azure-pipelines.yml`...).\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n azure-pipelines.yml | 5 ++---\n 1 file changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/azure-pipelines.yml b/azure-pipelines.yml\nindex c329b7218b..55ee23ad0f 100644\n--- a/azure-pipelines.yml\n+++ b/azure-pipelines.yml\n@@ -1,6 +1,5 @@\n-resources:\n-- repo: self\n-  fetchDepth: 1\n+variables:\n+  Agent.Source.Git.ShallowFetchDepth: 1\n \n jobs:\n - job: windows_build\n-- \ngitgitgadget\n\n"},{"id":"382971","messageId":"ece296922be29dc0520c747d391a5cf3982973bc.1569486607.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.git.gitgitgadget@gmail.com","subject":"[PATCH 13/13] ci: also build and test with MS Visual Studio on Azure Pipelines","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-26T08:30:24Z","receivedAt":"2019-09-26T08:30:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\n... because we can, now. Technically, we actually build using `MSBuild`,\nwhich is however pretty close to building interactively in Visual\nStudio.\n\nAs there is no convenient way to run Git's test suite in Visual Studio,\nwe unpack a Portable Git to run it, using the just-added test helper to\nallow running test scripts in parallel.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n Makefile            |   4 ++\n azure-pipelines.yml | 159 ++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 163 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex 3716dadc08..3f6dcec54b 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3025,6 +3025,10 @@ rpm::\n \t@false\n .PHONY: rpm\n \n+ifneq ($(INCLUDE_DLLS_IN_ARTIFACTS),)\n+OTHER_PROGRAMS += $(shell echo *.dll t/helper/*.dll)\n+endif\n+\n artifacts-tar:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) \\\n \t\tGIT-BUILD-OPTIONS $(TEST_PROGRAMS) $(test_bindir_programs) \\\n \t\t$(MOFILES)\ndiff --git a/azure-pipelines.yml b/azure-pipelines.yml\nindex 55ee23ad0f..875e63cac1 100644\n--- a/azure-pipelines.yml\n+++ b/azure-pipelines.yml\n@@ -130,6 +130,165 @@ jobs:\n       PathtoPublish: t/failed-test-artifacts\n       ArtifactName: failed-test-artifacts\n \n+- job: vs_build\n+  displayName: Visual Studio Build\n+  condition: succeeded()\n+  pool: Hosted VS2017\n+  timeoutInMinutes: 240\n+  steps:\n+  - powershell: |\n+      if (\"$GITFILESHAREPWD\" -ne \"\" -and \"$GITFILESHAREPWD\" -ne \"`$`(gitfileshare.pwd)\") {\n+        net use s: \\\\gitfileshare.file.core.windows.net\\test-cache \"$GITFILESHAREPWD\" /user:AZURE\\gitfileshare /persistent:no\n+        cmd /c mklink /d \"$(Build.SourcesDirectory)\\test-cache\" S:\\\n+      }\n+    displayName: 'Mount test-cache'\n+    env:\n+      GITFILESHAREPWD: $(gitfileshare.pwd)\n+  - powershell: |\n+      $urlbase = \"https://dev.azure.com/git-for-windows/git/_apis/build/builds\"\n+      $id = ((Invoke-WebRequest -UseBasicParsing \"${urlbase}?definitions=22&statusFilter=completed&resultFilter=succeeded&`$top=1\").content | ConvertFrom-JSON).value[0].id\n+      $downloadUrl = ((Invoke-WebRequest -UseBasicParsing \"${urlbase}/$id/artifacts\").content | ConvertFrom-JSON).value[1].resource.downloadUrl\n+      (New-Object Net.WebClient).DownloadFile($downloadUrl,\"git-sdk-64-minimal.zip\")\n+      Expand-Archive git-sdk-64-minimal.zip -DestinationPath . -Force\n+      Remove-Item git-sdk-64-minimal.zip\n+\n+      # Let Git ignore the SDK and the test-cache\n+      \"/git-sdk-64-minimal/`n/test-cache/`n\" | Out-File -NoNewLine -Encoding ascii -Append \"$(Build.SourcesDirectory)\\.git\\info\\exclude\"\n+    displayName: 'Download git-sdk-64-minimal'\n+  - powershell: |\n+      & git-sdk-64-minimal\\usr\\bin\\bash.exe -lc @\"\n+        make vcxproj\n+      \"@\n+      if (!$?) { exit(1) }\n+    displayName: Generate Visual Studio Solution\n+    env:\n+      HOME: $(Build.SourcesDirectory)\n+      MSYSTEM: MINGW64\n+      DEVELOPER: 1\n+      NO_PERL: 1\n+      GIT_CONFIG_PARAMETERS: \"'user.name=CI' 'user.email=ci@git'\"\n+  - powershell: |\n+      $urlbase = \"https://dev.azure.com/git/git/_apis/build/builds\"\n+      $id = ((Invoke-WebRequest -UseBasicParsing \"${urlbase}?definitions=9&statusFilter=completed&resultFilter=succeeded&`$top=1\").content | ConvertFrom-JSON).value[0].id\n+      $downloadUrl = ((Invoke-WebRequest -UseBasicParsing \"${urlbase}/$id/artifacts\").content | ConvertFrom-JSON).value[0].resource.downloadUrl\n+      (New-Object Net.WebClient).DownloadFile($downloadUrl, \"compat.zip\")\n+      Expand-Archive compat.zip -DestinationPath . -Force\n+      Remove-Item compat.zip\n+    displayName: 'Download vcpkg artifacts'\n+  - task: MSBuild@1\n+    inputs:\n+      solution: git.sln\n+      platform: x64\n+      configuration: Release\n+      maximumCpuCount: 4\n+  - powershell: |\n+      & compat\\vcbuild\\vcpkg_copy_dlls.bat release\n+      if (!$?) { exit(1) }\n+      & git-sdk-64-minimal\\usr\\bin\\bash.exe -lc @\"\n+        mkdir -p artifacts &&\n+        eval \\\"`$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts | grep ^tar)\\\"\n+      \"@\n+      if (!$?) { exit(1) }\n+    displayName: Bundle artifact tar\n+    env:\n+      HOME: $(Build.SourcesDirectory)\n+      MSYSTEM: MINGW64\n+      DEVELOPER: 1\n+      NO_PERL: 1\n+      MSVC: 1\n+      VCPKG_ROOT: $(Build.SourcesDirectory)\\compat\\vcbuild\\vcpkg\n+  - powershell: |\n+      $tag = (Invoke-WebRequest -UseBasicParsing \"https://gitforwindows.org/latest-tag.txt\").content\n+      $version = (Invoke-WebRequest -UseBasicParsing \"https://gitforwindows.org/latest-version.txt\").content\n+      $url = \"https://github.com/git-for-windows/git/releases/download/${tag}/PortableGit-${version}-64-bit.7z.exe\"\n+      (New-Object Net.WebClient).DownloadFile($url,\"PortableGit.exe\")\n+      & .\\PortableGit.exe -y -oartifacts\\PortableGit\n+      # Wait until it is unpacked\n+      while (-not @(Remove-Item -ErrorAction SilentlyContinue PortableGit.exe; $?)) { sleep 1 }\n+    displayName: Download & extract portable Git\n+  - task: PublishPipelineArtifact@0\n+    displayName: 'Publish Pipeline Artifact: MSVC test artifacts'\n+    inputs:\n+      artifactName: 'vs-artifacts'\n+      targetPath: '$(Build.SourcesDirectory)\\artifacts'\n+  - powershell: |\n+      if (\"$GITFILESHAREPWD\" -ne \"\" -and \"$GITFILESHAREPWD\" -ne \"`$`(gitfileshare.pwd)\") {\n+        cmd /c rmdir \"$(Build.SourcesDirectory)\\test-cache\"\n+      }\n+    displayName: 'Unmount test-cache'\n+    condition: true\n+    env:\n+      GITFILESHAREPWD: $(gitfileshare.pwd)\n+\n+- job: vs_test\n+  displayName: Visual Studio Test\n+  dependsOn: vs_build\n+  condition: succeeded()\n+  pool: Hosted\n+  timeoutInMinutes: 240\n+  strategy:\n+    parallel: 10\n+  steps:\n+  - powershell: |\n+      if (\"$GITFILESHAREPWD\" -ne \"\" -and \"$GITFILESHAREPWD\" -ne \"`$`(gitfileshare.pwd)\") {\n+        net use s: \\\\gitfileshare.file.core.windows.net\\test-cache \"$GITFILESHAREPWD\" /user:AZURE\\gitfileshare /persistent:no\n+        cmd /c mklink /d \"$(Build.SourcesDirectory)\\test-cache\" S:\\\n+      }\n+    displayName: 'Mount test-cache'\n+    env:\n+      GITFILESHAREPWD: $(gitfileshare.pwd)\n+  - task: DownloadPipelineArtifact@0\n+    displayName: 'Download Pipeline Artifact: VS test artifacts'\n+    inputs:\n+      artifactName: 'vs-artifacts'\n+      targetPath: '$(Build.SourcesDirectory)'\n+  - powershell: |\n+      & PortableGit\\git-cmd.exe --command=usr\\bin\\bash.exe -lc @\"\n+        test -f artifacts.tar.gz || {\n+          echo No test artifacts found\\; skipping >&2\n+          exit 0\n+        }\n+        tar xf artifacts.tar.gz || exit 1\n+\n+        # Let Git ignore the SDK and the test-cache\n+        printf '%s\\n' /PortableGit/ /test-cache/ >>.git/info/exclude\n+\n+        cd t &&\n+        PATH=\\\"`$PWD/helper:`$PATH\\\" &&\n+        test-tool.exe run-command testsuite -V -x --write-junit-xml \\\n+                `$(test-tool.exe path-utils slice-tests \\\n+                        `$SYSTEM_JOBPOSITIONINPHASE `$SYSTEM_TOTALJOBSINPHASE t[0-9]*.sh)\n+      \"@\n+      if (!$?) { exit(1) }\n+    displayName: 'Test (parallel)'\n+    env:\n+      HOME: $(Build.SourcesDirectory)\n+      MSYSTEM: MINGW64\n+      NO_SVN_TESTS: 1\n+      GIT_TEST_SKIP_REBASE_P: 1\n+  - powershell: |\n+      if (\"$GITFILESHAREPWD\" -ne \"\" -and \"$GITFILESHAREPWD\" -ne \"`$`(gitfileshare.pwd)\") {\n+        cmd /c rmdir \"$(Build.SourcesDirectory)\\test-cache\"\n+      }\n+    displayName: 'Unmount test-cache'\n+    condition: true\n+    env:\n+      GITFILESHAREPWD: $(gitfileshare.pwd)\n+  - task: PublishTestResults@2\n+    displayName: 'Publish Test Results **/TEST-*.xml'\n+    inputs:\n+      mergeTestResults: true\n+      testRunTitle: 'vs'\n+      platform: Windows\n+      publishRunAttachments: false\n+    condition: succeededOrFailed()\n+  - task: PublishBuildArtifacts@1\n+    displayName: 'Publish trash directories of failed tests'\n+    condition: failed()\n+    inputs:\n+      PathtoPublish: t/failed-test-artifacts\n+      ArtifactName: failed-vs-test-artifacts\n+\n - job: linux_clang\n   displayName: linux-clang\n   condition: succeeded()\n-- \ngitgitgadget\n"},{"id":"382998","messageId":"20190926172022.GA41037@generichostname","threadId":"51924","inReplyTo":"2abe1e1fb0bf3025489c2e543b9a9c648a164827.1569486607.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 02/13] msvc: avoid using minus operator on unsigned types","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-09-26T17:20:22Z","receivedAt":"2019-09-26T17:20:28Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Hi Dscho,\n\nOn Thu, Sep 26, 2019 at 01:30:10AM -0700, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n> \n> MSVC complains about this with `-Wall`, which can be taken as a sign\n> that this is indeed a real bug. The symptom is:\n> \n> \tC4146: unary minus operator applied to unsigned type, result\n> \tstill unsigned\n> \n> Let's avoid this warning in the minimal way, e.g. writing `-1 -\n> <unsigned value>` instead of `-<unsigned value> - 1`.\n\n[...]\n\n> ---\n>  read-cache.c  | 4 ++--\n>  sha1-lookup.c | 2 +-\n>  2 files changed, 3 insertions(+), 3 deletions(-)\n> \n> diff --git a/read-cache.c b/read-cache.c\n> index c701f7f8b8..11f3357216 100644\n> --- a/read-cache.c\n> +++ b/read-cache.c\n> @@ -1276,7 +1276,7 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e\n>  \t */\n>  \tif (istate->cache_nr > 0 &&\n>  \t\tstrcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)\n> -\t\tpos = -istate->cache_nr - 1;\n> +\t\tpos = -1 - istate->cache_nr;\n\nI've been thinking about this and I'm still not certain that this 100%\ncorrect from a language-lawyer perspective.\n\nIf we do `-1 - istate->cache_nr`, then the unsignedness of\nistate->cache_nr takes over and the whole expression is a very large\nunsigned number.\n\nThen, when we assign to `int pos`, we are converting an unsigned number\nwhich is out of the range of the signed number. According to a\nStackOverflow post citing the C99 standard[1]:\n\n\tOtherwise, the new type is signed and the value cannot be\n\trepresented in it; either the result is implementation-defined\n\tor an implementation-defined signal is raised.\n\nI'm sure that most platforms that we support will handle it sanely but\ncould we write this as\n\n\tpos = -1 - (int) istate->cache_nr;\n\nto be doubly sure that no funny business will happen?\n\n>  \telse\n>  \t\tpos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce));\n>  \n> @@ -1894,7 +1894,7 @@ static size_t estimate_cache_size(size_t ondisk_size, unsigned int entries)\n>  \t/*\n>  \t * Account for potential alignment differences.\n>  \t */\n> -\tper_entry += align_padding_size(sizeof(struct cache_entry), -sizeof(struct ondisk_cache_entry));\n> +\tper_entry += align_padding_size(per_entry, 0);\n>  \treturn ondisk_size + entries * per_entry;\n>  }\n>  \n> diff --git a/sha1-lookup.c b/sha1-lookup.c\n> index 796ab68da8..c819687730 100644\n> --- a/sha1-lookup.c\n> +++ b/sha1-lookup.c\n> @@ -97,7 +97,7 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,\n>  \t\t\tlo = mi + 1;\n>  \t\tmi = lo + (hi - lo) / 2;\n>  \t} while (lo < hi);\n> -\treturn -lo-1;\n> +\treturn -1 - lo;\n\nSame thing here.\n\n[1]: https://stackoverflow.com/questions/50605/signed-to-unsigned-conversion-in-c-is-it-always-safe\n\n>  }\n>  \n>  int bsearch_hash(const unsigned char *sha1, const uint32_t *fanout_nbo,\n> -- \n> gitgitgadget\n> \n"},{"id":"383027","messageId":"nycvar.QRO.7.76.6.1909262236550.15067@tvgsbejvaqbjf.bet","threadId":"51924","inReplyTo":"20190926172022.GA41037@generichostname","subject":"Re: [PATCH 02/13] msvc: avoid using minus operator on unsigned types","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-09-26T21:01:32Z","receivedAt":"2019-09-26T21:01:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Denton,\n\nOn Thu, 26 Sep 2019, Denton Liu wrote:\n\n> Hi Dscho,\n>\n> On Thu, Sep 26, 2019 at 01:30:10AM -0700, Johannes Schindelin via GitGitGadget wrote:\n> > From: Johannes Schindelin <johannes.schindelin@gmx.de>\n> >\n> > MSVC complains about this with `-Wall`, which can be taken as a sign\n> > that this is indeed a real bug. The symptom is:\n> >\n> > \tC4146: unary minus operator applied to unsigned type, result\n> > \tstill unsigned\n> >\n> > Let's avoid this warning in the minimal way, e.g. writing `-1 -\n> > <unsigned value>` instead of `-<unsigned value> - 1`.\n>\n> [...]\n>\n> > ---\n> >  read-cache.c  | 4 ++--\n> >  sha1-lookup.c | 2 +-\n> >  2 files changed, 3 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/read-cache.c b/read-cache.c\n> > index c701f7f8b8..11f3357216 100644\n> > --- a/read-cache.c\n> > +++ b/read-cache.c\n> > @@ -1276,7 +1276,7 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e\n> >  \t */\n> >  \tif (istate->cache_nr > 0 &&\n> >  \t\tstrcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)\n> > -\t\tpos = -istate->cache_nr - 1;\n> > +\t\tpos = -1 - istate->cache_nr;\n>\n> I've been thinking about this and I'm still not certain that this 100%\n> correct from a language-lawyer perspective.\n>\n> If we do `-1 - istate->cache_nr`, then the unsignedness of\n> istate->cache_nr takes over and the whole expression is a very large\n> unsigned number.\n>\n> Then, when we assign to `int pos`, we are converting an unsigned number\n> which is out of the range of the signed number. According to a\n> StackOverflow post citing the C99 standard[1]:\n>\n> \tOtherwise, the new type is signed and the value cannot be\n> \trepresented in it; either the result is implementation-defined\n> \tor an implementation-defined signal is raised.\n>\n> I'm sure that most platforms that we support will handle it sanely but\n> could we write this as\n>\n> \tpos = -1 - (int) istate->cache_nr;\n>\n> to be doubly sure that no funny business will happen?\n\nI guess we should use `signed_add_overflows()` to make extra certain\nthat it does what we want it to do, kind of like `st_add()`. Or just do\nthe check explicitly, like so:\n\n\tif (istate->cache_nr > INT_MAX)\n\t\tdie(\"overflow: -1 - %u\", istate->cache_nr);\n\tpos = -1 - istate->cache_nr;\n}\n>\n> >  \telse\n> >  \t\tpos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce));\n> >\n> > @@ -1894,7 +1894,7 @@ static size_t estimate_cache_size(size_t ondisk_size, unsigned int entries)\n> >  \t/*\n> >  \t * Account for potential alignment differences.\n> >  \t */\n> > -\tper_entry += align_padding_size(sizeof(struct cache_entry), -sizeof(struct ondisk_cache_entry));\n> > +\tper_entry += align_padding_size(per_entry, 0);\n> >  \treturn ondisk_size + entries * per_entry;\n> >  }\n> >\n> > diff --git a/sha1-lookup.c b/sha1-lookup.c\n> > index 796ab68da8..c819687730 100644\n> > --- a/sha1-lookup.c\n> > +++ b/sha1-lookup.c\n> > @@ -97,7 +97,7 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,\n> >  \t\t\tlo = mi + 1;\n> >  \t\tmi = lo + (hi - lo) / 2;\n> >  \t} while (lo < hi);\n> > -\treturn -lo-1;\n> > +\treturn -1 - lo;\n>\n> Same thing here.\n\nThis is even more critical, as `lo` has the type `size_t`:\n\n\tif (lo > INT_MAX)\n\t\tdie(\"overflow: -1 - %\"PRIuMAX, (uintmax_t)lo);\n\treturn -1 - lo;\n>\nWhat do you think?\nDscho\n\n> [1]: https://stackoverflow.com/questions/50605/signed-to-unsigned-conversion-in-c-is-it-always-safe\n>\n> >  }\n> >\n> >  int bsearch_hash(const unsigned char *sha1, const uint32_t *fanout_nbo,\n> > --\n> > gitgitgadget\n> >\n>\n"},{"id":"383044","messageId":"20190926235753.GA94990@generichostname","threadId":"51924","inReplyTo":"nycvar.QRO.7.76.6.1909262236550.15067@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 02/13] msvc: avoid using minus operator on unsigned types","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-09-26T23:57:53Z","receivedAt":"2019-09-26T23:57:59Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Hi Dscho,\n\nOn Thu, Sep 26, 2019 at 11:01:32PM +0200, Johannes Schindelin wrote:\n> Hi Denton,\n> \n> On Thu, 26 Sep 2019, Denton Liu wrote:\n> \n> > Hi Dscho,\n> >\n> > On Thu, Sep 26, 2019 at 01:30:10AM -0700, Johannes Schindelin via GitGitGadget wrote:\n> > > From: Johannes Schindelin <johannes.schindelin@gmx.de>\n> > >\n> > > MSVC complains about this with `-Wall`, which can be taken as a sign\n> > > that this is indeed a real bug. The symptom is:\n> > >\n> > > \tC4146: unary minus operator applied to unsigned type, result\n> > > \tstill unsigned\n> > >\n> > > Let's avoid this warning in the minimal way, e.g. writing `-1 -\n> > > <unsigned value>` instead of `-<unsigned value> - 1`.\n> >\n> > [...]\n> >\n> > > ---\n> > >  read-cache.c  | 4 ++--\n> > >  sha1-lookup.c | 2 +-\n> > >  2 files changed, 3 insertions(+), 3 deletions(-)\n> > >\n> > > diff --git a/read-cache.c b/read-cache.c\n> > > index c701f7f8b8..11f3357216 100644\n> > > --- a/read-cache.c\n> > > +++ b/read-cache.c\n> > > @@ -1276,7 +1276,7 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e\n> > >  \t */\n> > >  \tif (istate->cache_nr > 0 &&\n> > >  \t\tstrcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)\n> > > -\t\tpos = -istate->cache_nr - 1;\n> > > +\t\tpos = -1 - istate->cache_nr;\n> >\n> > I've been thinking about this and I'm still not certain that this 100%\n> > correct from a language-lawyer perspective.\n> >\n> > If we do `-1 - istate->cache_nr`, then the unsignedness of\n> > istate->cache_nr takes over and the whole expression is a very large\n> > unsigned number.\n> >\n> > Then, when we assign to `int pos`, we are converting an unsigned number\n> > which is out of the range of the signed number. According to a\n> > StackOverflow post citing the C99 standard[1]:\n> >\n> > \tOtherwise, the new type is signed and the value cannot be\n> > \trepresented in it; either the result is implementation-defined\n> > \tor an implementation-defined signal is raised.\n> >\n> > I'm sure that most platforms that we support will handle it sanely but\n> > could we write this as\n> >\n> > \tpos = -1 - (int) istate->cache_nr;\n> >\n> > to be doubly sure that no funny business will happen?\n> \n> I guess we should use `signed_add_overflows()` to make extra certain\n> that it does what we want it to do, kind of like `st_add()`. Or just do\n> the check explicitly, like so:\n> \n> \tif (istate->cache_nr > INT_MAX)\n> \t\tdie(\"overflow: -1 - %u\", istate->cache_nr);\n> \tpos = -1 - istate->cache_nr;\n\nCould we change this to \n\n \tpos = -1 - (int) istate->cache_nr;\n\nso that we alleviate the problem I was talking about above?\n\nOther than that, it looks good. Well, it might break on one's complement\nsystems but I don't think we support UNIVACs anyway. ;)\n\n> }\n> >\n> > >  \telse\n> > >  \t\tpos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce));\n> > >\n> > > @@ -1894,7 +1894,7 @@ static size_t estimate_cache_size(size_t ondisk_size, unsigned int entries)\n> > >  \t/*\n> > >  \t * Account for potential alignment differences.\n> > >  \t */\n> > > -\tper_entry += align_padding_size(sizeof(struct cache_entry), -sizeof(struct ondisk_cache_entry));\n> > > +\tper_entry += align_padding_size(per_entry, 0);\n> > >  \treturn ondisk_size + entries * per_entry;\n> > >  }\n> > >\n> > > diff --git a/sha1-lookup.c b/sha1-lookup.c\n> > > index 796ab68da8..c819687730 100644\n> > > --- a/sha1-lookup.c\n> > > +++ b/sha1-lookup.c\n> > > @@ -97,7 +97,7 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,\n> > >  \t\t\tlo = mi + 1;\n> > >  \t\tmi = lo + (hi - lo) / 2;\n> > >  \t} while (lo < hi);\n> > > -\treturn -lo-1;\n> > > +\treturn -1 - lo;\n> >\n> > Same thing here.\n> \n> This is even more critical, as `lo` has the type `size_t`:\n> \n> \tif (lo > INT_MAX)\n> \t\tdie(\"overflow: -1 - %\"PRIuMAX, (uintmax_t)lo);\n> \treturn -1 - lo;\n\nAlso, could we change this to\n\n \treturn -1 - (int) lo;\n\nThanks,\n\nDenton\n\n> >\n> What do you think?\n> Dscho\n> \n> > [1]: https://stackoverflow.com/questions/50605/signed-to-unsigned-conversion-in-c-is-it-always-safe\n> >\n> > >  }\n> > >\n> > >  int bsearch_hash(const unsigned char *sha1, const uint32_t *fanout_nbo,\n> > > --\n> > > gitgitgadget\n> > >\n> >\n"},{"id":"383088","messageId":"xmqq7e5sjc1p.fsf@gitster-ct.c.googlers.com","threadId":"51924","inReplyTo":"99724f6a1e45b497e15037bbac1cb5f70a3bb236.1569486607.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 11/13] tests: let --immediate and --write-junit-xml play well together","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-09-28T22:22:58Z","receivedAt":"2019-09-28T22:23:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index d1ba33745a..f21c781e68 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -695,7 +695,7 @@ test_failure_ () {\n>  \tsay_color error \"not ok $test_count - $1\"\n>  \tshift\n>  \tprintf '%s\\n' \"$*\" | sed -e 's/^/#\t/'\n> -\ttest \"$immediate\" = \"\" || { GIT_EXIT_OK=t; exit 1; }\n> +\ttest \"$immediate\" = \"\" || { finalize_junit_xml; GIT_EXIT_OK=t; exit 1; }\n>  }\n\nThere are three places that do GIT_EXIT_OK=t in the test framework,\nand the above covers one of them.  The original in test_done is\nanother, and that place is made to call the \"finalize\" thing (it\nused to have the same finalization code inlined).\n\nThe remaining one appears in\n\n        error () {\n                say_color error \"error: $*\"\n                GIT_EXIT_OK=t\n                exit 1\n        }\n\nI wonder if we should cover this case, too.  One caller of \"error\" I\nknow is BUG that says \"bug in the test script\", which means that\nafter successfully passing 30 tests, when the 31st test has 5 params\nto test_expect_success by mistake, without finailzation we will lose\nthe result for the first 30.\n\nAnd if we call \"finalize\" from the \"error\" helper, perhaps it makes\neven more sense to update the above manual exit in test_failure_ to\ndo something like\n\n\tif test -n \"$immediate\"\n\tthen\n\t\terror \"immediate exit after the first error\"\n\tfi\n\nto delegate the finalization.\n\n> @@ -1085,21 +1104,7 @@ test_done () {\n>  \t# removed, so the commands can access pidfiles and socket files.\n>  \ttest_atexit_handler\n>  \n> -\tif test -n \"$write_junit_xml\" && test -n \"$junit_xml_path\"\n> -\tthen\n> -\t\ttest -n \"$junit_have_testcase\" || {\n> -\t\t\tjunit_start=$(test-tool date getnanos)\n> -\t\t\twrite_junit_xml_testcase \"all tests skipped\"\n> -\t\t}\n> -\n> -\t\t# adjust the overall time\n> -\t\tjunit_time=$(test-tool date getnanos $junit_suite_start)\n> -\t\tsed \"s/<testsuite [^>]*/& time=\\\"$junit_time\\\"/\" \\\n> -\t\t\t<\"$junit_xml_path\" >\"$junit_xml_path.new\"\n> -\t\tmv \"$junit_xml_path.new\" \"$junit_xml_path\"\n> -\n> -\t\twrite_junit_xml \"  </testsuite>\" \"</testsuites>\"\n> -\tfi\n> +\tfinalize_junit_xml\n>  \n>  \tif test -z \"$HARNESS_ACTIVE\"\n>  \tthen\n"},{"id":"383135","messageId":"nycvar.QRO.7.76.6.1909301149070.46@tvgsbejvaqbjf.bet","threadId":"51924","inReplyTo":"20190926235753.GA94990@generichostname","subject":"Re: [PATCH 02/13] msvc: avoid using minus operator on unsigned types","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-09-30T09:50:00Z","receivedAt":"2019-09-30T09:50:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Denton,\n\nOn Thu, 26 Sep 2019, Denton Liu wrote:\n\n> On Thu, Sep 26, 2019 at 11:01:32PM +0200, Johannes Schindelin wrote:\n> >\n> > On Thu, 26 Sep 2019, Denton Liu wrote:\n> >\n> > > Hi Dscho,\n> > >\n> > > On Thu, Sep 26, 2019 at 01:30:10AM -0700, Johannes Schindelin via GitGitGadget wrote:\n> > > > From: Johannes Schindelin <johannes.schindelin@gmx.de>\n> > > >\n> > > > MSVC complains about this with `-Wall`, which can be taken as a sign\n> > > > that this is indeed a real bug. The symptom is:\n> > > >\n> > > > \tC4146: unary minus operator applied to unsigned type, result\n> > > > \tstill unsigned\n> > > >\n> > > > Let's avoid this warning in the minimal way, e.g. writing `-1 -\n> > > > <unsigned value>` instead of `-<unsigned value> - 1`.\n> > >\n> > > [...]\n> > >\n> > > > ---\n> > > >  read-cache.c  | 4 ++--\n> > > >  sha1-lookup.c | 2 +-\n> > > >  2 files changed, 3 insertions(+), 3 deletions(-)\n> > > >\n> > > > diff --git a/read-cache.c b/read-cache.c\n> > > > index c701f7f8b8..11f3357216 100644\n> > > > --- a/read-cache.c\n> > > > +++ b/read-cache.c\n> > > > @@ -1276,7 +1276,7 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e\n> > > >  \t */\n> > > >  \tif (istate->cache_nr > 0 &&\n> > > >  \t\tstrcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)\n> > > > -\t\tpos = -istate->cache_nr - 1;\n> > > > +\t\tpos = -1 - istate->cache_nr;\n> > >\n> > > I've been thinking about this and I'm still not certain that this 100%\n> > > correct from a language-lawyer perspective.\n> > >\n> > > If we do `-1 - istate->cache_nr`, then the unsignedness of\n> > > istate->cache_nr takes over and the whole expression is a very large\n> > > unsigned number.\n> > >\n> > > Then, when we assign to `int pos`, we are converting an unsigned number\n> > > which is out of the range of the signed number. According to a\n> > > StackOverflow post citing the C99 standard[1]:\n> > >\n> > > \tOtherwise, the new type is signed and the value cannot be\n> > > \trepresented in it; either the result is implementation-defined\n> > > \tor an implementation-defined signal is raised.\n> > >\n> > > I'm sure that most platforms that we support will handle it sanely but\n> > > could we write this as\n> > >\n> > > \tpos = -1 - (int) istate->cache_nr;\n> > >\n> > > to be doubly sure that no funny business will happen?\n> >\n> > I guess we should use `signed_add_overflows()` to make extra certain\n> > that it does what we want it to do, kind of like `st_add()`. Or just do\n> > the check explicitly, like so:\n> >\n> > \tif (istate->cache_nr > INT_MAX)\n> > \t\tdie(\"overflow: -1 - %u\", istate->cache_nr);\n> > \tpos = -1 - istate->cache_nr;\n>\n> Could we change this to\n>\n>  \tpos = -1 - (int) istate->cache_nr;\n>\n> so that we alleviate the problem I was talking about above?\n\nThank you, I had missed that.\n\n> Other than that, it looks good. Well, it might break on one's complement\n> systems but I don't think we support UNIVACs anyway. ;)\n\nLet's hope that nobody tries to run Git on such a system, indeed.\n\nThanks,\nDscho\n\n>\n> > }\n> > >\n> > > >  \telse\n> > > >  \t\tpos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce));\n> > > >\n> > > > @@ -1894,7 +1894,7 @@ static size_t estimate_cache_size(size_t ondisk_size, unsigned int entries)\n> > > >  \t/*\n> > > >  \t * Account for potential alignment differences.\n> > > >  \t */\n> > > > -\tper_entry += align_padding_size(sizeof(struct cache_entry), -sizeof(struct ondisk_cache_entry));\n> > > > +\tper_entry += align_padding_size(per_entry, 0);\n> > > >  \treturn ondisk_size + entries * per_entry;\n> > > >  }\n> > > >\n> > > > diff --git a/sha1-lookup.c b/sha1-lookup.c\n> > > > index 796ab68da8..c819687730 100644\n> > > > --- a/sha1-lookup.c\n> > > > +++ b/sha1-lookup.c\n> > > > @@ -97,7 +97,7 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,\n> > > >  \t\t\tlo = mi + 1;\n> > > >  \t\tmi = lo + (hi - lo) / 2;\n> > > >  \t} while (lo < hi);\n> > > > -\treturn -lo-1;\n> > > > +\treturn -1 - lo;\n> > >\n> > > Same thing here.\n> >\n> > This is even more critical, as `lo` has the type `size_t`:\n> >\n> > \tif (lo > INT_MAX)\n> > \t\tdie(\"overflow: -1 - %\"PRIuMAX, (uintmax_t)lo);\n> > \treturn -1 - lo;\n>\n> Also, could we change this to\n>\n>  \treturn -1 - (int) lo;\n>\n> Thanks,\n>\n> Denton\n>\n> > >\n> > What do you think?\n> > Dscho\n> >\n> > > [1]: https://stackoverflow.com/questions/50605/signed-to-unsigned-conversion-in-c-is-it-always-safe\n> > >\n> > > >  }\n> > > >\n> > > >  int bsearch_hash(const unsigned char *sha1, const uint32_t *fanout_nbo,\n> > > > --\n> > > > gitgitgadget\n> > > >\n> > >\n>\n"},{"id":"383136","messageId":"nycvar.QRO.7.76.6.1909301151050.46@tvgsbejvaqbjf.bet","threadId":"51924","inReplyTo":"xmqq7e5sjc1p.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 11/13] tests: let --immediate and --write-junit-xml play well together","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-09-30T09:52:36Z","receivedAt":"2019-09-30T09:52:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Sun, 29 Sep 2019, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\n> writes:\n>\n> > diff --git a/t/test-lib.sh b/t/test-lib.sh\n> > index d1ba33745a..f21c781e68 100644\n> > --- a/t/test-lib.sh\n> > +++ b/t/test-lib.sh\n> > @@ -695,7 +695,7 @@ test_failure_ () {\n> >  \tsay_color error \"not ok $test_count - $1\"\n> >  \tshift\n> >  \tprintf '%s\\n' \"$*\" | sed -e 's/^/#\t/'\n> > -\ttest \"$immediate\" = \"\" || { GIT_EXIT_OK=t; exit 1; }\n> > +\ttest \"$immediate\" = \"\" || { finalize_junit_xml; GIT_EXIT_OK=t; exit 1; }\n> >  }\n>\n> There are three places that do GIT_EXIT_OK=t in the test framework,\n> and the above covers one of them.  The original in test_done is\n> another, and that place is made to call the \"finalize\" thing (it\n> used to have the same finalization code inlined).\n>\n> The remaining one appears in\n>\n>         error () {\n>                 say_color error \"error: $*\"\n>                 GIT_EXIT_OK=t\n>                 exit 1\n>         }\n>\n> I wonder if we should cover this case, too.  One caller of \"error\" I\n> know is BUG that says \"bug in the test script\", which means that\n> after successfully passing 30 tests, when the 31st test has 5 params\n> to test_expect_success by mistake, without finailzation we will lose\n> the result for the first 30.\n\nGood point.\n\n> And if we call \"finalize\" from the \"error\" helper, perhaps it makes\n> even more sense to update the above manual exit in test_failure_ to\n> do something like\n>\n> \tif test -n \"$immediate\"\n> \tthen\n> \t\terror \"immediate exit after the first error\"\n> \tfi\n>\n> to delegate the finalization.\n\nThis adds an additional message. I am not sure how many scripts/CI\nintegrations are there that rely on the current behavior, so I would\nlike to exclude this change from this here patch series: it is about\nincluding a Visual Studio build in our Azure Pipeline, nothing more,\nnothing less...\n\nCiao,\nDscho\n\n>\n> > @@ -1085,21 +1104,7 @@ test_done () {\n> >  \t# removed, so the commands can access pidfiles and socket files.\n> >  \ttest_atexit_handler\n> >\n> > -\tif test -n \"$write_junit_xml\" && test -n \"$junit_xml_path\"\n> > -\tthen\n> > -\t\ttest -n \"$junit_have_testcase\" || {\n> > -\t\t\tjunit_start=$(test-tool date getnanos)\n> > -\t\t\twrite_junit_xml_testcase \"all tests skipped\"\n> > -\t\t}\n> > -\n> > -\t\t# adjust the overall time\n> > -\t\tjunit_time=$(test-tool date getnanos $junit_suite_start)\n> > -\t\tsed \"s/<testsuite [^>]*/& time=\\\"$junit_time\\\"/\" \\\n> > -\t\t\t<\"$junit_xml_path\" >\"$junit_xml_path.new\"\n> > -\t\tmv \"$junit_xml_path.new\" \"$junit_xml_path\"\n> > -\n> > -\t\twrite_junit_xml \"  </testsuite>\" \"</testsuites>\"\n> > -\tfi\n> > +\tfinalize_junit_xml\n> >\n> >  \tif test -z \"$HARNESS_ACTIVE\"\n> >  \tthen\n>\n"},{"id":"383137","messageId":"4d0b38125a13d85963be5e524becf48271893e2b.1569837329.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 01/13] push: do not pretend to return `int` from `die_push_simple()`","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-30T09:55:31Z","receivedAt":"2019-09-30T09:55:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThis function is marked as `NORETURN`, and it indeed does not want to\nreturn anything. So let's not declare it with the return type `int`.\nThis fixes the following warning when building with MSVC:\n\n\tC4646: function declared with 'noreturn' has non-void return type\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n builtin/push.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 021dd3b1e4..d216270d5f 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -143,8 +143,8 @@ static int push_url_of_remote(struct remote *remote, const char ***url_p)\n \treturn remote->url_nr;\n }\n \n-static NORETURN int die_push_simple(struct branch *branch,\n-\t\t\t\t    struct remote *remote)\n+static NORETURN void die_push_simple(struct branch *branch,\n+\t\t\t\t     struct remote *remote)\n {\n \t/*\n \t * There's no point in using shorten_unambiguous_ref here,\n-- \ngitgitgadget\n\n"},{"id":"383138","messageId":"8512a3e96db80aa1474d679d544679abf811188c.1569837329.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 03/13] winansi: use FLEX_ARRAY to avoid compiler warning","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-30T09:55:32Z","receivedAt":"2019-09-30T09:55:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nMSVC would complain thusly:\n\n    C4200: nonstandard extension used: zero-sized array in struct/union\n\nLet's just use the `FLEX_ARRAY` constant that we introduced for exactly\nthis type of scenario.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n compat/winansi.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/compat/winansi.c b/compat/winansi.c\nindex cacd82c833..54fd701cbf 100644\n--- a/compat/winansi.c\n+++ b/compat/winansi.c\n@@ -546,7 +546,7 @@ static HANDLE swap_osfhnd(int fd, HANDLE new_handle)\n typedef struct _OBJECT_NAME_INFORMATION\n {\n \tUNICODE_STRING Name;\n-\tWCHAR NameBuffer[0];\n+\tWCHAR NameBuffer[FLEX_ARRAY];\n } OBJECT_NAME_INFORMATION, *POBJECT_NAME_INFORMATION;\n \n #define ObjectNameInformation 1\n-- \ngitgitgadget\n\n"},{"id":"383139","messageId":"8800320590e4d7218a80f80abca23a7f44b8747d.1569837329.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 02/13] msvc: avoid using minus operator on unsigned types","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-30T09:55:31Z","receivedAt":"2019-09-30T09:55:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nMSVC complains about this with `-Wall`, which can be taken as a sign\nthat this is indeed a real bug. The symptom is:\n\n\tC4146: unary minus operator applied to unsigned type, result\n\tstill unsigned\n\nLet's avoid this warning in the minimal way, e.g. writing `-1 -\n<unsigned value>` instead of `-<unsigned value> - 1`.\n\nNote that the change in the `estimate_cache_size()` function is\nneeded because MSVC considers the \"return type\" of the `sizeof()`\noperator to be `size_t`, i.e. unsigned, and therefore it cannot be\nnegated using the unary minus operator.\n\nEven worse, that arithmetic is doing extra work, in vain. We want to\ncalculate the entry extra cache size as the difference between the\nsize of the `cache_entry` structure minus the size of the\n`ondisk_cache_entry` structure, padded to the appropriate alignment\nboundary.\n\nTo that end, we start by assigning that difference to the `per_entry`\nvariable, and then abuse the `len` parameter of the\n`align_padding_size()` macro to take the negative size of the ondisk\nentry size. Essentially, we try to avoid passing the already calculated\ndifference to that macro by passing the operands of that difference\ninstead, when the macro expects operands of an addition:\n\n\t#define align_padding_size(size, len) \\\n\t\t((size + (len) + 8) & ~7) - (size + len)\n\nCurrently, we pass A and -B to that macro instead of passing A - B and\n0, where A - B is already stored in the `per_entry` variable, ready to\nbe used.\n\nThis is neither necessary, nor intuitive. Let's fix this, and have code\nthat is both easier to read and that also does not trigger MSVC's\nwarning.\n\nWhile at it, we take care of reporting overflows (which are unlikely,\nbut hey, defensive programming is good!).\n\nWe _also_ take pains of casting the unsigned value to signed: otherwise,\nthe signed operand (i.e. the `-1`) would be cast to unsigned before\ndoing the arithmetic.\n\nHelped-by: Denton Liu <liu.denton@gmail.com>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n read-cache.c  |  9 ++++++---\n sha1-lookup.c | 12 +++++++++---\n 2 files changed, 15 insertions(+), 6 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex c701f7f8b8..97745c2a31 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1275,8 +1275,11 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e\n \t * we can avoid searching for it.\n \t */\n \tif (istate->cache_nr > 0 &&\n-\t\tstrcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)\n-\t\tpos = -istate->cache_nr - 1;\n+\t\tstrcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0) {\n+\t\tif (istate->cache_nr > INT_MAX)\n+\t\t\tdie(\"overflow: -1 - %u\", istate->cache_nr);\n+\t\tpos = -1 - (int)istate->cache_nr;\n+\t}\n \telse\n \t\tpos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce));\n \n@@ -1894,7 +1897,7 @@ static size_t estimate_cache_size(size_t ondisk_size, unsigned int entries)\n \t/*\n \t * Account for potential alignment differences.\n \t */\n-\tper_entry += align_padding_size(sizeof(struct cache_entry), -sizeof(struct ondisk_cache_entry));\n+\tper_entry += align_padding_size(per_entry, 0);\n \treturn ondisk_size + entries * per_entry;\n }\n \ndiff --git a/sha1-lookup.c b/sha1-lookup.c\nindex 796ab68da8..bb786b5420 100644\n--- a/sha1-lookup.c\n+++ b/sha1-lookup.c\n@@ -69,8 +69,12 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,\n \t\t\tmiv = take2(sha1 + ofs);\n \t\t\tif (miv < lov)\n \t\t\t\treturn -1;\n-\t\t\tif (hiv < miv)\n-\t\t\t\treturn -1 - nr;\n+\t\t\tif (hiv < miv) {\n+\t\t\t\tif (nr > INT_MAX)\n+\t\t\t\t\tdie(\"overflow: -1 - %\"PRIuMAX,\n+\t\t\t\t\t    (uintmax_t)nr);\n+\t\t\t\treturn -1 - (int)nr;\n+\t\t\t}\n \t\t\tif (lov != hiv) {\n \t\t\t\t/*\n \t\t\t\t * At this point miv could be equal\n@@ -97,7 +101,9 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,\n \t\t\tlo = mi + 1;\n \t\tmi = lo + (hi - lo) / 2;\n \t} while (lo < hi);\n-\treturn -lo-1;\n+\tif (nr > INT_MAX)\n+\t\tdie(\"overflow: -1 - %\"PRIuMAX, (uintmax_t)lo);\n+\treturn -1 - (int)lo;\n }\n \n int bsearch_hash(const unsigned char *sha1, const uint32_t *fanout_nbo,\n-- \ngitgitgadget\n\n"},{"id":"383140","messageId":"5add01f8ffeec28f7d91a0e63c1ce28f4f0d831c.1569837329.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 05/13] msvc: ignore some libraries when linking","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-30T09:55:33Z","receivedAt":"2019-09-30T09:55:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nTo build with MSVC, we \"translate\" GCC options to MSVC options, and part\nof those options refer to the libraries to link into the final\nexecutable. Currently, this part looks somewhat like this on Windows:\n\n\t-lcurl -lnghttp2 -lidn2 -lssl -lcrypto -lssl -lcrypto -lgdi32\n\t-lcrypt32 -lwldap32 -lz -lws2_32 -lexpat\n\nSome of those are direct dependencies (such as curl and ssl) and others\nare indirect (nghttp2 and idn2, for example, are dependencies of curl,\nbut need to be linked in for reasons).\n\nWe already handle the direct dependencies, e.g. `-liconv` is already\nhandled as adding `libiconv.lib` to the list of libraries to link\nagainst.\n\nLet's just ignore the remaining `-l*` options so that MSVC does not have\nto warn us that it ignored e.g. the `/lnghttp2` option. We do that by\nextending the clause that already \"eats\" the `-R*` options to also eat\nthe `-l*` options.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n compat/vcbuild/scripts/clink.pl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/compat/vcbuild/scripts/clink.pl b/compat/vcbuild/scripts/clink.pl\nindex c7b021bfac..00fc339cba 100755\n--- a/compat/vcbuild/scripts/clink.pl\n+++ b/compat/vcbuild/scripts/clink.pl\n@@ -68,7 +68,7 @@\n \t} elsif (\"$arg\" =~ /^-L/ && \"$arg\" ne \"-LTCG\") {\n \t\t$arg =~ s/^-L/-LIBPATH:/;\n \t\tpush(@lflags, $arg);\n-\t} elsif (\"$arg\" =~ /^-R/) {\n+\t} elsif (\"$arg\" =~ /^-[Rl]/) {\n \t\t# eat\n \t} else {\n \t\tpush(@args, $arg);\n-- \ngitgitgadget\n\n"},{"id":"383141","messageId":"5c880f923ec9d5aa72b3b2dc6d54f427823b8389.1569837329.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 06/13] msvc: handle DEVELOPER=1","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-30T09:55:34Z","receivedAt":"2019-09-30T09:55:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWe frequently build Git using the `DEVELOPER=1` make setting as a\nshortcut to enable all kinds of more stringent compiler warnings.\n\nThose compiler warnings are relatively specific to GCC, though, so let's\ntry our best to translate them to the equivalent options to pass to MS\nVisual C++'s `cl.exe`.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n compat/vcbuild/scripts/clink.pl | 46 +++++++++++++++++++++++++++++++++\n 1 file changed, 46 insertions(+)\n\ndiff --git a/compat/vcbuild/scripts/clink.pl b/compat/vcbuild/scripts/clink.pl\nindex 00fc339cba..ec95a3b2d0 100755\n--- a/compat/vcbuild/scripts/clink.pl\n+++ b/compat/vcbuild/scripts/clink.pl\n@@ -70,6 +70,52 @@\n \t\tpush(@lflags, $arg);\n \t} elsif (\"$arg\" =~ /^-[Rl]/) {\n \t\t# eat\n+\t} elsif (\"$arg\" eq \"-Werror\") {\n+\t\tpush(@cflags, \"-WX\");\n+\t} elsif (\"$arg\" eq \"-Wall\") {\n+\t\t# cl.exe understands -Wall, but it is really overzealous\n+\t\tpush(@cflags, \"-W4\");\n+\t\t# disable the \"signed/unsigned mismatch\" warnings; our source code violates that\n+\t\tpush(@cflags, \"-wd4018\");\n+\t\tpush(@cflags, \"-wd4245\");\n+\t\tpush(@cflags, \"-wd4389\");\n+\t\t# disable the \"unreferenced formal parameter\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4100\");\n+\t\t# disable the \"conditional expression is constant\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4127\");\n+\t\t# disable the \"const object should be initialized\" warning; these warnings affect only objects that are `static`\n+\t\tpush(@cflags, \"-wd4132\");\n+\t\t# disable the \"function/data pointer conversion in expression\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4152\");\n+\t\t# disable the \"non-constant aggregate initializer\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4204\");\n+\t\t# disable the \"cannot be initialized using address of automatic variable\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4221\");\n+\t\t# disable the \"possible loss of data\" warnings; our source code violates that\n+\t\tpush(@cflags, \"-wd4244\");\n+\t\tpush(@cflags, \"-wd4267\");\n+\t\t# disable the \"array is too small to include a terminating null character\" warning; we ab-use strings to initialize OIDs\n+\t\tpush(@cflags, \"-wd4295\");\n+\t\t# disable the \"'<<': result of 32-bit shift implicitly converted to 64 bits\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4334\");\n+\t\t# disable the \"declaration hides previous local declaration\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4456\");\n+\t\t# disable the \"declaration hides function parameter\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4457\");\n+\t\t# disable the \"declaration hides global declaration\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4459\");\n+\t\t# disable the \"potentially uninitialized local variable '<name>' used\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4701\");\n+\t\t# disable the \"unreachable code\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4702\");\n+\t\t# disable the \"potentially uninitialized local pointer variable used\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4703\");\n+\t\t# disable the \"assignment within conditional expression\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4706\");\n+\t\t# disable the \"'inet_ntoa': Use inet_ntop() or InetNtop() instead\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4996\");\n+\t} elsif (\"$arg\" =~ /^-W[a-z]/) {\n+\t\t# let's ignore those\n \t} else {\n \t\tpush(@args, $arg);\n \t}\n-- \ngitgitgadget\n\n"},{"id":"383142","messageId":"0345b08f542b1ba950882be2932e1643890bf8d2.1569837329.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 04/13] compat/win32/path-utils.h: add #include guards","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-30T09:55:33Z","receivedAt":"2019-09-30T09:55:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThis adds the common guards that allow headers to be #include'd multiple\ntimes.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n compat/win32/path-utils.h | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/compat/win32/path-utils.h b/compat/win32/path-utils.h\nindex 0f70d43920..8ed062a6b7 100644\n--- a/compat/win32/path-utils.h\n+++ b/compat/win32/path-utils.h\n@@ -1,3 +1,6 @@\n+#ifndef WIN32_PATH_UTILS_H\n+#define WIN32_PATH_UTILS_H\n+\n #define has_dos_drive_prefix(path) \\\n \t(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)\n int win32_skip_dos_drive_prefix(char **path);\n@@ -18,3 +21,5 @@ static inline char *win32_find_last_dir_sep(const char *path)\n #define find_last_dir_sep win32_find_last_dir_sep\n int win32_offset_1st_component(const char *path);\n #define offset_1st_component win32_offset_1st_component\n+\n+#endif\n-- \ngitgitgadget\n\n"},{"id":"383143","messageId":"pull.288.v2.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.git.gitgitgadget@gmail.com","subject":"[PATCH v2 00/13] ci: include a Visual Studio build & test in our Azure Pipeline","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-30T09:55:30Z","receivedAt":"2019-09-30T09:55:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Git's Continuous Integration (CI) includes an Azure Pipeline that builds Git\non Linux, macOS and Windows, in the former two cases even in multiple\nconfigurations (using GCC vs clang, 32-bit vs 64-bit, etc). On Windows, we\nonly build using GCC, using (a subset of) Git for Windows' SDK.\n\nRecently, a patch series made it into Git that re-instates the ability to\ngenerate project files for use with Visual Studio. The idea there being:\ncontributors can check out a special branch that has those generated files\nin one generated commit on top of e.g. Git for Windows' master, allowing the\ncontributors to build Git in Visual Studio, without the need for downloading\nGit for Windows' SDK (which weighs quite a bit: ~600MB download, ~2GB disk\nfootprint). The tests can then be run from a regular Git for Windows Bash.\n\nThis patch series adds that axis to Git's Azure Pipeline: the project files\nare generated, MSBuild (which is kind of the command-line equivalent of\nVisual Studio's \"Build\" operation) is used to build Git, and then a\nparallelized test job runs the test suite in a Portable Git.\n\nThese patches are based on js/visual-studio.\n\nChanges since v1:\n\n * \"While at it\", we now also check for overflows when doing that -1 -\n   <unsigned> arithmetic.\n * The JUnit-style XML is finalized also in case that the script aborts e.g.\n   due to an incorrect number of arguments in a test_expect_success call.\n\nJohannes Schindelin (13):\n  push: do not pretend to return `int` from `die_push_simple()`\n  msvc: avoid using minus operator on unsigned types\n  winansi: use FLEX_ARRAY to avoid compiler warning\n  compat/win32/path-utils.h: add #include guards\n  msvc: ignore some libraries when linking\n  msvc: handle DEVELOPER=1\n  msvc: work around a bug in GetEnvironmentVariable()\n  vcxproj: only copy `git-remote-http.exe` once it was built\n  vcxproj: include more generated files\n  test-tool run-command: learn to run (parts of) the testsuite\n  tests: let --immediate and --write-junit-xml play well together\n  ci: really use shallow clones on Azure Pipelines\n  ci: also build and test with MS Visual Studio on Azure Pipelines\n\n Makefile                                   |   4 +\n azure-pipelines.yml                        | 164 ++++++++++++++++++++-\n builtin/push.c                             |   4 +-\n compat/mingw.c                             |   2 +\n compat/vcbuild/scripts/clink.pl            |  48 +++++-\n compat/win32/path-utils.h                  |   5 +\n compat/winansi.c                           |   2 +-\n config.mak.uname                           |  19 ++-\n contrib/buildsystems/Generators/Vcxproj.pm |   3 +\n read-cache.c                               |   9 +-\n sha1-lookup.c                              |  12 +-\n t/helper/test-run-command.c                | 153 +++++++++++++++++++\n t/test-lib.sh                              |  38 +++--\n 13 files changed, 428 insertions(+), 35 deletions(-)\n\n\nbase-commit: aac6ff7b5beeea9bca66ecda6eec60fc1dd447ec\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-288%2Fdscho%2Fazure-pipelines-msvc-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-288/dscho/azure-pipelines-msvc-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/288\n\nRange-diff vs v1:\n\n  1:  4d0b38125a =  1:  4d0b38125a push: do not pretend to return `int` from `die_push_simple()`\n  2:  2abe1e1fb0 !  2:  8800320590 msvc: avoid using minus operator on unsigned types\n     @@ -40,17 +40,30 @@\n          that is both easier to read and that also does not trigger MSVC's\n          warning.\n      \n     +    While at it, we take care of reporting overflows (which are unlikely,\n     +    but hey, defensive programming is good!).\n     +\n     +    We _also_ take pains of casting the unsigned value to signed: otherwise,\n     +    the signed operand (i.e. the `-1`) would be cast to unsigned before\n     +    doing the arithmetic.\n     +\n     +    Helped-by: Denton Liu <liu.denton@gmail.com>\n          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n      \n       diff --git a/read-cache.c b/read-cache.c\n       --- a/read-cache.c\n       +++ b/read-cache.c\n      @@\n     + \t * we can avoid searching for it.\n       \t */\n       \tif (istate->cache_nr > 0 &&\n     - \t\tstrcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)\n     +-\t\tstrcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)\n      -\t\tpos = -istate->cache_nr - 1;\n     -+\t\tpos = -1 - istate->cache_nr;\n     ++\t\tstrcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0) {\n     ++\t\tif (istate->cache_nr > INT_MAX)\n     ++\t\t\tdie(\"overflow: -1 - %u\", istate->cache_nr);\n     ++\t\tpos = -1 - (int)istate->cache_nr;\n     ++\t}\n       \telse\n       \t\tpos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce));\n       \n     @@ -67,12 +80,29 @@\n       diff --git a/sha1-lookup.c b/sha1-lookup.c\n       --- a/sha1-lookup.c\n       +++ b/sha1-lookup.c\n     +@@\n     + \t\t\tmiv = take2(sha1 + ofs);\n     + \t\t\tif (miv < lov)\n     + \t\t\t\treturn -1;\n     +-\t\t\tif (hiv < miv)\n     +-\t\t\t\treturn -1 - nr;\n     ++\t\t\tif (hiv < miv) {\n     ++\t\t\t\tif (nr > INT_MAX)\n     ++\t\t\t\t\tdie(\"overflow: -1 - %\"PRIuMAX,\n     ++\t\t\t\t\t    (uintmax_t)nr);\n     ++\t\t\t\treturn -1 - (int)nr;\n     ++\t\t\t}\n     + \t\t\tif (lov != hiv) {\n     + \t\t\t\t/*\n     + \t\t\t\t * At this point miv could be equal\n      @@\n       \t\t\tlo = mi + 1;\n       \t\tmi = lo + (hi - lo) / 2;\n       \t} while (lo < hi);\n      -\treturn -lo-1;\n     -+\treturn -1 - lo;\n     ++\tif (nr > INT_MAX)\n     ++\t\tdie(\"overflow: -1 - %\"PRIuMAX, (uintmax_t)lo);\n     ++\treturn -1 - (int)lo;\n       }\n       \n       int bsearch_hash(const unsigned char *sha1, const uint32_t *fanout_nbo,\n  3:  dbd9022ad5 =  3:  8512a3e96d winansi: use FLEX_ARRAY to avoid compiler warning\n  4:  6417d6f689 =  4:  0345b08f54 compat/win32/path-utils.h: add #include guards\n  5:  d1dee56fcc =  5:  5add01f8ff msvc: ignore some libraries when linking\n  6:  c978c76599 =  6:  5c880f923e msvc: handle DEVELOPER=1\n  7:  0590514e43 =  7:  1f2245a228 msvc: work around a bug in GetEnvironmentVariable()\n  8:  f7d5d1a1bc =  8:  582b299634 vcxproj: only copy `git-remote-http.exe` once it was built\n  9:  6adfc63e98 =  9:  38ccf999e7 vcxproj: include more generated files\n 10:  ad9ab10ce0 = 10:  24b1c7bff3 test-tool run-command: learn to run (parts of) the testsuite\n 11:  99724f6a1e ! 11:  7be13d19e1 tests: let --immediate and --write-junit-xml play well together\n     @@ -11,12 +11,34 @@\n          Pipelines, where the JUnit-style XML is consumed to present the test\n          results in an informative and helpful way.\n      \n     +    While at it, also handle the `error()` code path.\n     +\n     +    The only remaining code path that sets `GIT_EXIT_OK` happens whenever\n     +    the trash directory could not be set up, i.e. long before the JUnit XML\n     +    was written, therefore we should _not_ try to finalize that XML in that\n     +    case.\n     +\n     +    It is tempting to change the `immediate` code path to just hand off to\n     +    `error`, simplifying the code in the process. That would, however,\n     +    result in a change of behavior (an additional error message) in the test\n     +    suite, which is outside of the purview of the current patch series: its\n     +    goal is to allow building Git with Visual Studio and testing it with a\n     +    portable version of Git for Windows.\n     +\n          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n      \n       diff --git a/t/test-lib.sh b/t/test-lib.sh\n       --- a/t/test-lib.sh\n       +++ b/t/test-lib.sh\n      @@\n     + \n     + error () {\n     + \tsay_color error \"error: $*\"\n     ++\tfinalize_junit_xml\n     + \tGIT_EXIT_OK=t\n     + \texit 1\n     + }\n     +@@\n       \tsay_color error \"not ok $test_count - $1\"\n       \tshift\n       \tprintf '%s\\n' \"$*\" | sed -e 's/^/#\t/'\n 12:  29dceaae5e = 12:  bde1e8ef65 ci: really use shallow clones on Azure Pipelines\n 13:  ece296922b = 13:  7af1c01a08 ci: also build and test with MS Visual Studio on Azure Pipelines\n\n-- \ngitgitgadget\n"},{"id":"383144","messageId":"1f2245a228e1e1a9b6f0bb70f8f6987af2140182.1569837329.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 07/13] msvc: work around a bug in GetEnvironmentVariable()","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-30T09:55:35Z","receivedAt":"2019-09-30T09:55:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThe return value of that function is 0 both for variables that are\nunset, as well as for variables whose values are empty. To discern those\ntwo cases, one has to call `GetLastError()`, whose return value is\n`ERROR_ENVVAR_NOT_FOUND` and `ERROR_SUCCESS`, respectively.\n\nExcept that it is not actually set to `ERROR_SUCCESS` in the latter\ncase, apparently, but the last error value seems to be simply unchanged.\n\nTo work around this, let's just re-set the last error value just before\ninspecting the environment variable.\n\nThis fixes a problem that triggers failures in t3301-notes.sh (where we\ntry to override config settings by passing empty values for certain\nenvironment variables).\n\nThis problem is hidden in the MINGW build by the fact that older\nMSVC runtimes (such as the one used by MINGW builds) have a `calloc()`\nthat re-sets the last error value in case of success, while newer\nruntimes set the error value only if `NULL` is returned by that\nfunction.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n compat/mingw.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 4891789771..7d8cb814ba 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -1661,6 +1661,8 @@ char *mingw_getenv(const char *name)\n \tif (!w_key)\n \t\tdie(\"Out of memory, (tried to allocate %u wchar_t's)\", len_key);\n \txutftowcs(w_key, name, len_key);\n+\t/* GetEnvironmentVariableW() only sets the last error upon failure */\n+\tSetLastError(ERROR_SUCCESS);\n \tlen_value = GetEnvironmentVariableW(w_key, w_value, ARRAY_SIZE(w_value));\n \tif (!len_value && GetLastError() == ERROR_ENVVAR_NOT_FOUND) {\n \t\tfree(w_key);\n-- \ngitgitgadget\n\n"},{"id":"383145","messageId":"7be13d19e1000adc0d5f02e67a799b1b87524322.1569837330.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 11/13] tests: let --immediate and --write-junit-xml play well together","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-30T09:55:38Z","receivedAt":"2019-09-30T09:55:45Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWhen the `--immediate` option is in effect, any test failure will\nimmediately exit the test script. Together with `--write-junit-xml`, we\nwill want the JUnit-style `.xml` file to be finalized (and not leave the\nXML incomplete). Let's make it so.\n\nThis comes in particularly handy when trying to debug via Azure\nPipelines, where the JUnit-style XML is consumed to present the test\nresults in an informative and helpful way.\n\nWhile at it, also handle the `error()` code path.\n\nThe only remaining code path that sets `GIT_EXIT_OK` happens whenever\nthe trash directory could not be set up, i.e. long before the JUnit XML\nwas written, therefore we should _not_ try to finalize that XML in that\ncase.\n\nIt is tempting to change the `immediate` code path to just hand off to\n`error`, simplifying the code in the process. That would, however,\nresult in a change of behavior (an additional error message) in the test\nsuite, which is outside of the purview of the current patch series: its\ngoal is to allow building Git with Visual Studio and testing it with a\nportable version of Git for Windows.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n t/test-lib.sh | 38 ++++++++++++++++++++++----------------\n 1 file changed, 22 insertions(+), 16 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex d1ba33745a..86b5e6133b 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -567,6 +567,7 @@ export TERM\n \n error () {\n \tsay_color error \"error: $*\"\n+\tfinalize_junit_xml\n \tGIT_EXIT_OK=t\n \texit 1\n }\n@@ -695,7 +696,7 @@ test_failure_ () {\n \tsay_color error \"not ok $test_count - $1\"\n \tshift\n \tprintf '%s\\n' \"$*\" | sed -e 's/^/#\t/'\n-\ttest \"$immediate\" = \"\" || { GIT_EXIT_OK=t; exit 1; }\n+\ttest \"$immediate\" = \"\" || { finalize_junit_xml; GIT_EXIT_OK=t; exit 1; }\n }\n \n test_known_broken_ok_ () {\n@@ -1063,6 +1064,25 @@ write_junit_xml_testcase () {\n \tjunit_have_testcase=t\n }\n \n+finalize_junit_xml () {\n+\tif test -n \"$write_junit_xml\" && test -n \"$junit_xml_path\"\n+\tthen\n+\t\ttest -n \"$junit_have_testcase\" || {\n+\t\t\tjunit_start=$(test-tool date getnanos)\n+\t\t\twrite_junit_xml_testcase \"all tests skipped\"\n+\t\t}\n+\n+\t\t# adjust the overall time\n+\t\tjunit_time=$(test-tool date getnanos $junit_suite_start)\n+\t\tsed \"s/<testsuite [^>]*/& time=\\\"$junit_time\\\"/\" \\\n+\t\t\t<\"$junit_xml_path\" >\"$junit_xml_path.new\"\n+\t\tmv \"$junit_xml_path.new\" \"$junit_xml_path\"\n+\n+\t\twrite_junit_xml \"  </testsuite>\" \"</testsuites>\"\n+\t\twrite_junit_xml=\n+\tfi\n+}\n+\n test_atexit_cleanup=:\n test_atexit_handler () {\n \t# In a succeeding test script 'test_atexit_handler' is invoked\n@@ -1085,21 +1105,7 @@ test_done () {\n \t# removed, so the commands can access pidfiles and socket files.\n \ttest_atexit_handler\n \n-\tif test -n \"$write_junit_xml\" && test -n \"$junit_xml_path\"\n-\tthen\n-\t\ttest -n \"$junit_have_testcase\" || {\n-\t\t\tjunit_start=$(test-tool date getnanos)\n-\t\t\twrite_junit_xml_testcase \"all tests skipped\"\n-\t\t}\n-\n-\t\t# adjust the overall time\n-\t\tjunit_time=$(test-tool date getnanos $junit_suite_start)\n-\t\tsed \"s/<testsuite [^>]*/& time=\\\"$junit_time\\\"/\" \\\n-\t\t\t<\"$junit_xml_path\" >\"$junit_xml_path.new\"\n-\t\tmv \"$junit_xml_path.new\" \"$junit_xml_path\"\n-\n-\t\twrite_junit_xml \"  </testsuite>\" \"</testsuites>\"\n-\tfi\n+\tfinalize_junit_xml\n \n \tif test -z \"$HARNESS_ACTIVE\"\n \tthen\n-- \ngitgitgadget\n\n"},{"id":"383146","messageId":"7af1c01a0889502f5938992c4cba748d202fbbd8.1569837330.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 13/13] ci: also build and test with MS Visual Studio on Azure Pipelines","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-30T09:55:39Z","receivedAt":"2019-09-30T09:55:45Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\n... because we can, now. Technically, we actually build using `MSBuild`,\nwhich is however pretty close to building interactively in Visual\nStudio.\n\nAs there is no convenient way to run Git's test suite in Visual Studio,\nwe unpack a Portable Git to run it, using the just-added test helper to\nallow running test scripts in parallel.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n Makefile            |   4 ++\n azure-pipelines.yml | 159 ++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 163 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex 3716dadc08..3f6dcec54b 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3025,6 +3025,10 @@ rpm::\n \t@false\n .PHONY: rpm\n \n+ifneq ($(INCLUDE_DLLS_IN_ARTIFACTS),)\n+OTHER_PROGRAMS += $(shell echo *.dll t/helper/*.dll)\n+endif\n+\n artifacts-tar:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) \\\n \t\tGIT-BUILD-OPTIONS $(TEST_PROGRAMS) $(test_bindir_programs) \\\n \t\t$(MOFILES)\ndiff --git a/azure-pipelines.yml b/azure-pipelines.yml\nindex 55ee23ad0f..875e63cac1 100644\n--- a/azure-pipelines.yml\n+++ b/azure-pipelines.yml\n@@ -130,6 +130,165 @@ jobs:\n       PathtoPublish: t/failed-test-artifacts\n       ArtifactName: failed-test-artifacts\n \n+- job: vs_build\n+  displayName: Visual Studio Build\n+  condition: succeeded()\n+  pool: Hosted VS2017\n+  timeoutInMinutes: 240\n+  steps:\n+  - powershell: |\n+      if (\"$GITFILESHAREPWD\" -ne \"\" -and \"$GITFILESHAREPWD\" -ne \"`$`(gitfileshare.pwd)\") {\n+        net use s: \\\\gitfileshare.file.core.windows.net\\test-cache \"$GITFILESHAREPWD\" /user:AZURE\\gitfileshare /persistent:no\n+        cmd /c mklink /d \"$(Build.SourcesDirectory)\\test-cache\" S:\\\n+      }\n+    displayName: 'Mount test-cache'\n+    env:\n+      GITFILESHAREPWD: $(gitfileshare.pwd)\n+  - powershell: |\n+      $urlbase = \"https://dev.azure.com/git-for-windows/git/_apis/build/builds\"\n+      $id = ((Invoke-WebRequest -UseBasicParsing \"${urlbase}?definitions=22&statusFilter=completed&resultFilter=succeeded&`$top=1\").content | ConvertFrom-JSON).value[0].id\n+      $downloadUrl = ((Invoke-WebRequest -UseBasicParsing \"${urlbase}/$id/artifacts\").content | ConvertFrom-JSON).value[1].resource.downloadUrl\n+      (New-Object Net.WebClient).DownloadFile($downloadUrl,\"git-sdk-64-minimal.zip\")\n+      Expand-Archive git-sdk-64-minimal.zip -DestinationPath . -Force\n+      Remove-Item git-sdk-64-minimal.zip\n+\n+      # Let Git ignore the SDK and the test-cache\n+      \"/git-sdk-64-minimal/`n/test-cache/`n\" | Out-File -NoNewLine -Encoding ascii -Append \"$(Build.SourcesDirectory)\\.git\\info\\exclude\"\n+    displayName: 'Download git-sdk-64-minimal'\n+  - powershell: |\n+      & git-sdk-64-minimal\\usr\\bin\\bash.exe -lc @\"\n+        make vcxproj\n+      \"@\n+      if (!$?) { exit(1) }\n+    displayName: Generate Visual Studio Solution\n+    env:\n+      HOME: $(Build.SourcesDirectory)\n+      MSYSTEM: MINGW64\n+      DEVELOPER: 1\n+      NO_PERL: 1\n+      GIT_CONFIG_PARAMETERS: \"'user.name=CI' 'user.email=ci@git'\"\n+  - powershell: |\n+      $urlbase = \"https://dev.azure.com/git/git/_apis/build/builds\"\n+      $id = ((Invoke-WebRequest -UseBasicParsing \"${urlbase}?definitions=9&statusFilter=completed&resultFilter=succeeded&`$top=1\").content | ConvertFrom-JSON).value[0].id\n+      $downloadUrl = ((Invoke-WebRequest -UseBasicParsing \"${urlbase}/$id/artifacts\").content | ConvertFrom-JSON).value[0].resource.downloadUrl\n+      (New-Object Net.WebClient).DownloadFile($downloadUrl, \"compat.zip\")\n+      Expand-Archive compat.zip -DestinationPath . -Force\n+      Remove-Item compat.zip\n+    displayName: 'Download vcpkg artifacts'\n+  - task: MSBuild@1\n+    inputs:\n+      solution: git.sln\n+      platform: x64\n+      configuration: Release\n+      maximumCpuCount: 4\n+  - powershell: |\n+      & compat\\vcbuild\\vcpkg_copy_dlls.bat release\n+      if (!$?) { exit(1) }\n+      & git-sdk-64-minimal\\usr\\bin\\bash.exe -lc @\"\n+        mkdir -p artifacts &&\n+        eval \\\"`$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts | grep ^tar)\\\"\n+      \"@\n+      if (!$?) { exit(1) }\n+    displayName: Bundle artifact tar\n+    env:\n+      HOME: $(Build.SourcesDirectory)\n+      MSYSTEM: MINGW64\n+      DEVELOPER: 1\n+      NO_PERL: 1\n+      MSVC: 1\n+      VCPKG_ROOT: $(Build.SourcesDirectory)\\compat\\vcbuild\\vcpkg\n+  - powershell: |\n+      $tag = (Invoke-WebRequest -UseBasicParsing \"https://gitforwindows.org/latest-tag.txt\").content\n+      $version = (Invoke-WebRequest -UseBasicParsing \"https://gitforwindows.org/latest-version.txt\").content\n+      $url = \"https://github.com/git-for-windows/git/releases/download/${tag}/PortableGit-${version}-64-bit.7z.exe\"\n+      (New-Object Net.WebClient).DownloadFile($url,\"PortableGit.exe\")\n+      & .\\PortableGit.exe -y -oartifacts\\PortableGit\n+      # Wait until it is unpacked\n+      while (-not @(Remove-Item -ErrorAction SilentlyContinue PortableGit.exe; $?)) { sleep 1 }\n+    displayName: Download & extract portable Git\n+  - task: PublishPipelineArtifact@0\n+    displayName: 'Publish Pipeline Artifact: MSVC test artifacts'\n+    inputs:\n+      artifactName: 'vs-artifacts'\n+      targetPath: '$(Build.SourcesDirectory)\\artifacts'\n+  - powershell: |\n+      if (\"$GITFILESHAREPWD\" -ne \"\" -and \"$GITFILESHAREPWD\" -ne \"`$`(gitfileshare.pwd)\") {\n+        cmd /c rmdir \"$(Build.SourcesDirectory)\\test-cache\"\n+      }\n+    displayName: 'Unmount test-cache'\n+    condition: true\n+    env:\n+      GITFILESHAREPWD: $(gitfileshare.pwd)\n+\n+- job: vs_test\n+  displayName: Visual Studio Test\n+  dependsOn: vs_build\n+  condition: succeeded()\n+  pool: Hosted\n+  timeoutInMinutes: 240\n+  strategy:\n+    parallel: 10\n+  steps:\n+  - powershell: |\n+      if (\"$GITFILESHAREPWD\" -ne \"\" -and \"$GITFILESHAREPWD\" -ne \"`$`(gitfileshare.pwd)\") {\n+        net use s: \\\\gitfileshare.file.core.windows.net\\test-cache \"$GITFILESHAREPWD\" /user:AZURE\\gitfileshare /persistent:no\n+        cmd /c mklink /d \"$(Build.SourcesDirectory)\\test-cache\" S:\\\n+      }\n+    displayName: 'Mount test-cache'\n+    env:\n+      GITFILESHAREPWD: $(gitfileshare.pwd)\n+  - task: DownloadPipelineArtifact@0\n+    displayName: 'Download Pipeline Artifact: VS test artifacts'\n+    inputs:\n+      artifactName: 'vs-artifacts'\n+      targetPath: '$(Build.SourcesDirectory)'\n+  - powershell: |\n+      & PortableGit\\git-cmd.exe --command=usr\\bin\\bash.exe -lc @\"\n+        test -f artifacts.tar.gz || {\n+          echo No test artifacts found\\; skipping >&2\n+          exit 0\n+        }\n+        tar xf artifacts.tar.gz || exit 1\n+\n+        # Let Git ignore the SDK and the test-cache\n+        printf '%s\\n' /PortableGit/ /test-cache/ >>.git/info/exclude\n+\n+        cd t &&\n+        PATH=\\\"`$PWD/helper:`$PATH\\\" &&\n+        test-tool.exe run-command testsuite -V -x --write-junit-xml \\\n+                `$(test-tool.exe path-utils slice-tests \\\n+                        `$SYSTEM_JOBPOSITIONINPHASE `$SYSTEM_TOTALJOBSINPHASE t[0-9]*.sh)\n+      \"@\n+      if (!$?) { exit(1) }\n+    displayName: 'Test (parallel)'\n+    env:\n+      HOME: $(Build.SourcesDirectory)\n+      MSYSTEM: MINGW64\n+      NO_SVN_TESTS: 1\n+      GIT_TEST_SKIP_REBASE_P: 1\n+  - powershell: |\n+      if (\"$GITFILESHAREPWD\" -ne \"\" -and \"$GITFILESHAREPWD\" -ne \"`$`(gitfileshare.pwd)\") {\n+        cmd /c rmdir \"$(Build.SourcesDirectory)\\test-cache\"\n+      }\n+    displayName: 'Unmount test-cache'\n+    condition: true\n+    env:\n+      GITFILESHAREPWD: $(gitfileshare.pwd)\n+  - task: PublishTestResults@2\n+    displayName: 'Publish Test Results **/TEST-*.xml'\n+    inputs:\n+      mergeTestResults: true\n+      testRunTitle: 'vs'\n+      platform: Windows\n+      publishRunAttachments: false\n+    condition: succeededOrFailed()\n+  - task: PublishBuildArtifacts@1\n+    displayName: 'Publish trash directories of failed tests'\n+    condition: failed()\n+    inputs:\n+      PathtoPublish: t/failed-test-artifacts\n+      ArtifactName: failed-vs-test-artifacts\n+\n - job: linux_clang\n   displayName: linux-clang\n   condition: succeeded()\n-- \ngitgitgadget\n"},{"id":"383147","messageId":"24b1c7bff35187f8e34012294db3a6670bcddb2a.1569837330.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 10/13] test-tool run-command: learn to run (parts of) the testsuite","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-30T09:55:37Z","receivedAt":"2019-09-30T09:55:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nGit for Windows jumps through hoops to provide a development environment\nthat allows to build Git and to run its test suite. To that end, an\nentire MSYS2 system, including GNU make and GCC is offered as \"the Git\nfor Windows SDK\". It does come at a price: an initial download of said\nSDK weighs in with several hundreds of megabytes, and the unpacked SDK\noccupies ~2GB of disk space.\n\nA much more native development environment on Windows is Visual Studio.\nTo help contributors use that environment, we already have a Makefile\ntarget `vcxproj` that generates a commit with project files (and other\ngenerated files), and Git for Windows' `vs/master` branch is\ncontinuously re-generated using that target.\n\nThe idea is to allow building Git in Visual Studio, and to run\nindividual tests using a Portable Git.\n\nThe one missing thing is a way to run the entire test suite: neither\n`make` nor `prove` are required to run Git, therefore Git for Windows\ndoes not support those commands in the Portable Git.\n\nTo help with that, add a simple test helper that exercises the\n`run_processes_parallel()` function to allow for running test scripts in\nparallel (which is really necessary, especially on Windows, as Git's\ntest suite takes such a long time to run).\n\nThis will also come in handy for the upcoming change to our Azure\nPipeline: we will use this helper in a Portable Git to test the Visual\nStudio build of Git.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n t/helper/test-run-command.c | 153 ++++++++++++++++++++++++++++++++++++\n 1 file changed, 153 insertions(+)\n\ndiff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c\nindex 2cc93bb69c..ead6dc611a 100644\n--- a/t/helper/test-run-command.c\n+++ b/t/helper/test-run-command.c\n@@ -10,9 +10,14 @@\n \n #include \"test-tool.h\"\n #include \"git-compat-util.h\"\n+#include \"cache.h\"\n #include \"run-command.h\"\n #include \"argv-array.h\"\n #include \"strbuf.h\"\n+#include \"parse-options.h\"\n+#include \"string-list.h\"\n+#include \"thread-utils.h\"\n+#include \"wildmatch.h\"\n #include <string.h>\n #include <errno.h>\n \n@@ -50,11 +55,159 @@ static int task_finished(int result,\n \treturn 1;\n }\n \n+struct testsuite {\n+\tstruct string_list tests, failed;\n+\tint next;\n+\tint quiet, immediate, verbose, verbose_log, trace, write_junit_xml;\n+};\n+#define TESTSUITE_INIT \\\n+\t{ STRING_LIST_INIT_DUP, STRING_LIST_INIT_DUP, -1, 0, 0, 0, 0, 0, 0 }\n+\n+static int next_test(struct child_process *cp, struct strbuf *err, void *cb,\n+\t\t     void **task_cb)\n+{\n+\tstruct testsuite *suite = cb;\n+\tconst char *test;\n+\tif (suite->next >= suite->tests.nr)\n+\t\treturn 0;\n+\n+\ttest = suite->tests.items[suite->next++].string;\n+\targv_array_pushl(&cp->args, \"sh\", test, NULL);\n+\tif (suite->quiet)\n+\t\targv_array_push(&cp->args, \"--quiet\");\n+\tif (suite->immediate)\n+\t\targv_array_push(&cp->args, \"-i\");\n+\tif (suite->verbose)\n+\t\targv_array_push(&cp->args, \"-v\");\n+\tif (suite->verbose_log)\n+\t\targv_array_push(&cp->args, \"-V\");\n+\tif (suite->trace)\n+\t\targv_array_push(&cp->args, \"-x\");\n+\tif (suite->write_junit_xml)\n+\t\targv_array_push(&cp->args, \"--write-junit-xml\");\n+\n+\tstrbuf_addf(err, \"Output of '%s':\\n\", test);\n+\t*task_cb = (void *)test;\n+\n+\treturn 1;\n+}\n+\n+static int test_finished(int result, struct strbuf *err, void *cb,\n+\t\t\t void *task_cb)\n+{\n+\tstruct testsuite *suite = cb;\n+\tconst char *name = (const char *)task_cb;\n+\n+\tif (result)\n+\t\tstring_list_append(&suite->failed, name);\n+\n+\tstrbuf_addf(err, \"%s: '%s'\\n\", result ? \"FAIL\" : \"SUCCESS\", name);\n+\n+\treturn 0;\n+}\n+\n+static int test_failed(struct strbuf *out, void *cb, void *task_cb)\n+{\n+\tstruct testsuite *suite = cb;\n+\tconst char *name = (const char *)task_cb;\n+\n+\tstring_list_append(&suite->failed, name);\n+\tstrbuf_addf(out, \"FAILED TO START: '%s'\\n\", name);\n+\n+\treturn 0;\n+}\n+\n+static const char * const testsuite_usage[] = {\n+\t\"test-run-command testsuite [<options>] [<pattern>...]\",\n+\tNULL\n+};\n+\n+static int testsuite(int argc, const char **argv)\n+{\n+\tstruct testsuite suite = TESTSUITE_INIT;\n+\tint max_jobs = 1, i, ret;\n+\tDIR *dir;\n+\tstruct dirent *d;\n+\tstruct option options[] = {\n+\t\tOPT_BOOL('i', \"immediate\", &suite.immediate,\n+\t\t\t \"stop at first failed test case(s)\"),\n+\t\tOPT_INTEGER('j', \"jobs\", &max_jobs, \"run <N> jobs in parallel\"),\n+\t\tOPT_BOOL('q', \"quiet\", &suite.quiet, \"be terse\"),\n+\t\tOPT_BOOL('v', \"verbose\", &suite.verbose, \"be verbose\"),\n+\t\tOPT_BOOL('V', \"verbose-log\", &suite.verbose_log,\n+\t\t\t \"be verbose, redirected to a file\"),\n+\t\tOPT_BOOL('x', \"trace\", &suite.trace, \"trace shell commands\"),\n+\t\tOPT_BOOL(0, \"write-junit-xml\", &suite.write_junit_xml,\n+\t\t\t \"write JUnit-style XML files\"),\n+\t\tOPT_END()\n+\t};\n+\n+\tmemset(&suite, 0, sizeof(suite));\n+\tsuite.tests.strdup_strings = suite.failed.strdup_strings = 1;\n+\n+\targc = parse_options(argc, argv, NULL, options,\n+\t\t\ttestsuite_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n+\n+\tif (max_jobs <= 0)\n+\t\tmax_jobs = online_cpus();\n+\n+\tdir = opendir(\".\");\n+\tif (!dir)\n+\t\tdie(\"Could not open the current directory\");\n+\twhile ((d = readdir(dir))) {\n+\t\tconst char *p = d->d_name;\n+\n+\t\tif (*p != 't' || !isdigit(p[1]) || !isdigit(p[2]) ||\n+\t\t    !isdigit(p[3]) || !isdigit(p[4]) || p[5] != '-' ||\n+\t\t    !ends_with(p, \".sh\"))\n+\t\t\tcontinue;\n+\n+\t\t/* No pattern: match all */\n+\t\tif (!argc) {\n+\t\t\tstring_list_append(&suite.tests, p);\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tfor (i = 0; i < argc; i++)\n+\t\t\tif (!wildmatch(argv[i], p, 0)) {\n+\t\t\t\tstring_list_append(&suite.tests, p);\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t}\n+\tclosedir(dir);\n+\n+\tif (!suite.tests.nr)\n+\t\tdie(\"No tests match!\");\n+\tif (max_jobs > suite.tests.nr)\n+\t\tmax_jobs = suite.tests.nr;\n+\n+\tfprintf(stderr, \"Running %d tests (%d at a time)\\n\",\n+\t\tsuite.tests.nr, max_jobs);\n+\n+\tret = run_processes_parallel(max_jobs, next_test, test_failed,\n+\t\t\t\t     test_finished, &suite);\n+\n+\tif (suite.failed.nr > 0) {\n+\t\tret = 1;\n+\t\tfprintf(stderr, \"%d tests failed:\\n\\n\", suite.failed.nr);\n+\t\tfor (i = 0; i < suite.failed.nr; i++)\n+\t\t\tfprintf(stderr, \"\\t%s\\n\", suite.failed.items[i].string);\n+\t}\n+\n+\tstring_list_clear(&suite.tests, 0);\n+\tstring_list_clear(&suite.failed, 0);\n+\n+\treturn !!ret;\n+}\n+\n int cmd__run_command(int argc, const char **argv)\n {\n \tstruct child_process proc = CHILD_PROCESS_INIT;\n \tint jobs;\n \n+\tif (argc > 1 && !strcmp(argv[1], \"testsuite\"))\n+\t\texit(testsuite(argc - 1, argv + 1));\n+\n \tif (argc < 3)\n \t\treturn 1;\n \twhile (!strcmp(argv[1], \"env\")) {\n-- \ngitgitgadget\n\n"},{"id":"383149","messageId":"582b299634f2ffe1cf81aea3ef1a4625ccd6e893.1569837329.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 08/13] vcxproj: only copy `git-remote-http.exe` once it was built","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-30T09:55:36Z","receivedAt":"2019-09-30T09:55:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nIn b18ae14a8f6 (vcxproj: also link-or-copy builtins, 2019-07-29), we\nstarted to copy or hard-link the built-ins as a post-build step of the\n`git` project.\n\nAt the same time, we tried to copy or hard-link `git-remote-http.exe`,\nbut it is quite possible that it was not built at that time.\n\nLet's move that latter task into a post-install step of the\n`git-remote-http` project instead.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n config.mak.uname                           | 10 +++++++---\n contrib/buildsystems/Generators/Vcxproj.pm |  3 +++\n 2 files changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git a/config.mak.uname b/config.mak.uname\nindex db7f06b95f..701aad62b1 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -703,20 +703,24 @@ vcxproj:\n \tperl contrib/buildsystems/generate -g Vcxproj\n \tgit add -f git.sln {*,*/lib,t/helper/*}/*.vcxproj\n \n-\t# Generate the LinkOrCopyBuiltins.targets file\n+\t# Generate the LinkOrCopyBuiltins.targets and LinkOrCopyRemoteHttp.targets file\n \t(echo '<Project xmlns=\"http://schemas.microsoft.com/developer/msbuild/2003\">' && \\\n \t echo '  <Target Name=\"CopyBuiltins_AfterBuild\" AfterTargets=\"AfterBuild\">' && \\\n \t for name in $(BUILT_INS);\\\n \t do \\\n \t   echo '    <Copy SourceFiles=\"$$(OutDir)\\git.exe\" DestinationFiles=\"$$(OutDir)\\'\"$$name\"'\" SkipUnchangedFiles=\"true\" UseHardlinksIfPossible=\"true\" />'; \\\n \t done && \\\n+\t echo '  </Target>' && \\\n+\t echo '</Project>') >git/LinkOrCopyBuiltins.targets\n+\t(echo '<Project xmlns=\"http://schemas.microsoft.com/developer/msbuild/2003\">' && \\\n+\t echo '  <Target Name=\"CopyBuiltins_AfterBuild\" AfterTargets=\"AfterBuild\">' && \\\n \t for name in $(REMOTE_CURL_ALIASES); \\\n \t do \\\n \t   echo '    <Copy SourceFiles=\"$$(OutDir)\\'\"$(REMOTE_CURL_PRIMARY)\"'\" DestinationFiles=\"$$(OutDir)\\'\"$$name\"'\" SkipUnchangedFiles=\"true\" UseHardlinksIfPossible=\"true\" />'; \\\n \t done && \\\n \t echo '  </Target>' && \\\n-\t echo '</Project>') >git/LinkOrCopyBuiltins.targets\n-\tgit add -f git/LinkOrCopyBuiltins.targets\n+\t echo '</Project>') >git-remote-http/LinkOrCopyRemoteHttp.targets\n+\tgit add -f git/LinkOrCopyBuiltins.targets git-remote-http/LinkOrCopyRemoteHttp.targets\n \n \t# Add command-list.h\n \t$(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 command-list.h\ndiff --git a/contrib/buildsystems/Generators/Vcxproj.pm b/contrib/buildsystems/Generators/Vcxproj.pm\nindex 576ccabe1d..868b787855 100644\n--- a/contrib/buildsystems/Generators/Vcxproj.pm\n+++ b/contrib/buildsystems/Generators/Vcxproj.pm\n@@ -277,6 +277,9 @@ sub createProject {\n     if ($target eq 'git') {\n       print F \"  <Import Project=\\\"LinkOrCopyBuiltins.targets\\\" />\\n\";\n     }\n+    if ($target eq 'git-remote-http') {\n+      print F \"  <Import Project=\\\"LinkOrCopyRemoteHttp.targets\\\" />\\n\";\n+    }\n     print F << \"EOM\";\n </Project>\n EOM\n-- \ngitgitgadget\n\n"},{"id":"383148","messageId":"38ccf999e786a4abfda1f88301c224b75baade6c.1569837330.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 09/13] vcxproj: include more generated files","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-30T09:55:36Z","receivedAt":"2019-09-30T09:55:49Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nIn the CI builds, we bundle all generated files into a so-called\nartifacts `.tar` file, so that the test phase can fan out into multiple\nparallel builds.\n\nThis patch makes sure that all files are included in the `vcxproj`\ntarget which are needed for that artifacts `.tar` file.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n config.mak.uname | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/config.mak.uname b/config.mak.uname\nindex 701aad62b1..cc8efd95b1 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -728,11 +728,10 @@ vcxproj:\n \n \t# Add scripts\n \trm -f perl/perl.mak\n-\t$(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 \\\n-\t\t$(SCRIPT_LIB) $(SCRIPT_SH_GEN) $(SCRIPT_PERL_GEN)\n+\t$(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 $(SCRIPT_LIB) $(SCRIPTS)\n \t# Strip out the sane tool path, needed only for building\n \tsed -i '/^git_broken_path_fix \".*/d' git-sh-setup\n-\tgit add -f $(SCRIPT_LIB) $(SCRIPT_SH_GEN) $(SCRIPT_PERL_GEN)\n+\tgit add -f $(SCRIPT_LIB) $(SCRIPTS)\n \n \t# Add Perl module\n \t$(MAKE) $(LIB_PERL_GEN)\n@@ -762,6 +761,10 @@ vcxproj:\n \t$(MAKE) -C templates\n \tgit add -f templates/boilerplates.made templates/blt/\n \n+\t# Add the translated messages\n+\tmake MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 $(MOFILES)\n+\tgit add -f $(MOFILES)\n+\n \t# Add build options\n \t$(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 GIT-BUILD-OPTIONS\n \tgit add -f GIT-BUILD-OPTIONS\n-- \ngitgitgadget\n\n"},{"id":"383150","messageId":"bde1e8ef658c3978a85b58c3f2c4ffbd49a562f0.1569837330.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 12/13] ci: really use shallow clones on Azure Pipelines","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-30T09:55:39Z","receivedAt":"2019-09-30T09:55:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThis was a left-over from the previous YAML schema, and it no longer\nworks. The problem was noticed while editing `azure-pipelines.yml` in VS\nCode with the very helpful \"Azure Pipelines\" extension (syntax\nhighlighting and intellisense for `azure-pipelines.yml`...).\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n azure-pipelines.yml | 5 ++---\n 1 file changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/azure-pipelines.yml b/azure-pipelines.yml\nindex c329b7218b..55ee23ad0f 100644\n--- a/azure-pipelines.yml\n+++ b/azure-pipelines.yml\n@@ -1,6 +1,5 @@\n-resources:\n-- repo: self\n-  fetchDepth: 1\n+variables:\n+  Agent.Source.Git.ShallowFetchDepth: 1\n \n jobs:\n - job: windows_build\n-- \ngitgitgadget\n\n"},{"id":"383373","messageId":"xmqq36g9bgl6.fsf@gitster-ct.c.googlers.com","threadId":"51924","inReplyTo":"4d0b38125a13d85963be5e524becf48271893e2b.1569837329.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 01/13] push: do not pretend to return `int` from `die_push_simple()`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-03T22:37:57Z","receivedAt":"2019-10-03T22:38:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n>\n> This function is marked as `NORETURN`, and it indeed does not want to\n> return anything. So let's not declare it with the return type `int`.\n> This fixes the following warning when building with MSVC:\n>\n> \tC4646: function declared with 'noreturn' has non-void return type\n>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n>  builtin/push.c | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/push.c b/builtin/push.c\n> index 021dd3b1e4..d216270d5f 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -143,8 +143,8 @@ static int push_url_of_remote(struct remote *remote, const char ***url_p)\n>  \treturn remote->url_nr;\n>  }\n>  \n> -static NORETURN int die_push_simple(struct branch *branch,\n> -\t\t\t\t    struct remote *remote)\n> +static NORETURN void die_push_simple(struct branch *branch,\n> +\t\t\t\t     struct remote *remote)\n\nHaha ;-)  \"I won't return and I'll return int\" sounds like an\noxymoron.\n\n>  {\n>  \t/*\n>  \t * There's no point in using shorten_unambiguous_ref here,\n"},{"id":"383374","messageId":"xmqqy2y1a1qk.fsf@gitster-ct.c.googlers.com","threadId":"51924","inReplyTo":"8800320590e4d7218a80f80abca23a7f44b8747d.1569837329.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 02/13] msvc: avoid using minus operator on unsigned types","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-03T22:44:03Z","receivedAt":"2019-10-03T22:44:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> While at it, we take care of reporting overflows (which are unlikely,\n> but hey, defensive programming is good!).\n>\n> We _also_ take pains of casting the unsigned value to signed: otherwise,\n> the signed operand (i.e. the `-1`) would be cast to unsigned before\n> doing the arithmetic.\n\nThese three look good and too similar to each other, which makes me\nwonder if we want to allow them simply write\n\n\treturn insert_pos_as_negative_offset(nr);\n\nwith something like\n\n\tstatic int insert_pos_as_negative_offset(uintmax_t nr)\n\t{\n\t\tif (INT_MAX < nr)\n\t\t\tdie(\"overflow: -1 - %\"PRIuMAX, nr);\n\t\treturn -1 - (int)nr;\n\t}\n\nto avoid repetition.\n\n> Helped-by: Denton Liu <liu.denton@gmail.com>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n>  read-cache.c  |  9 ++++++---\n>  sha1-lookup.c | 12 +++++++++---\n>  2 files changed, 15 insertions(+), 6 deletions(-)\n>\n> diff --git a/read-cache.c b/read-cache.c\n> index c701f7f8b8..97745c2a31 100644\n> --- a/read-cache.c\n> +++ b/read-cache.c\n> @@ -1275,8 +1275,11 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e\n>  \t * we can avoid searching for it.\n>  \t */\n>  \tif (istate->cache_nr > 0 &&\n> -\t\tstrcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)\n> -\t\tpos = -istate->cache_nr - 1;\n> +\t\tstrcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0) {\n> +\t\tif (istate->cache_nr > INT_MAX)\n> +\t\t\tdie(\"overflow: -1 - %u\", istate->cache_nr);\n> +\t\tpos = -1 - (int)istate->cache_nr;\n> +\t}\n>  \telse\n>  \t\tpos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce));\n>  \n> @@ -1894,7 +1897,7 @@ static size_t estimate_cache_size(size_t ondisk_size, unsigned int entries)\n>  \t/*\n>  \t * Account for potential alignment differences.\n>  \t */\n> -\tper_entry += align_padding_size(sizeof(struct cache_entry), -sizeof(struct ondisk_cache_entry));\n> +\tper_entry += align_padding_size(per_entry, 0);\n>  \treturn ondisk_size + entries * per_entry;\n>  }\n>  \n> diff --git a/sha1-lookup.c b/sha1-lookup.c\n> index 796ab68da8..bb786b5420 100644\n> --- a/sha1-lookup.c\n> +++ b/sha1-lookup.c\n> @@ -69,8 +69,12 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,\n>  \t\t\tmiv = take2(sha1 + ofs);\n>  \t\t\tif (miv < lov)\n>  \t\t\t\treturn -1;\n> -\t\t\tif (hiv < miv)\n> -\t\t\t\treturn -1 - nr;\n> +\t\t\tif (hiv < miv) {\n> +\t\t\t\tif (nr > INT_MAX)\n> +\t\t\t\t\tdie(\"overflow: -1 - %\"PRIuMAX,\n> +\t\t\t\t\t    (uintmax_t)nr);\n> +\t\t\t\treturn -1 - (int)nr;\n> +\t\t\t}\n>  \t\t\tif (lov != hiv) {\n>  \t\t\t\t/*\n>  \t\t\t\t * At this point miv could be equal\n> @@ -97,7 +101,9 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,\n>  \t\t\tlo = mi + 1;\n>  \t\tmi = lo + (hi - lo) / 2;\n>  \t} while (lo < hi);\n> -\treturn -lo-1;\n> +\tif (nr > INT_MAX)\n> +\t\tdie(\"overflow: -1 - %\"PRIuMAX, (uintmax_t)lo);\n> +\treturn -1 - (int)lo;\n>  }\n>  \n>  int bsearch_hash(const unsigned char *sha1, const uint32_t *fanout_nbo,\n"},{"id":"383407","messageId":"nycvar.QRO.7.76.6.1910041136150.46@tvgsbejvaqbjf.bet","threadId":"51924","inReplyTo":"xmqq36g9bgl6.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 01/13] push: do not pretend to return `int` from `die_push_simple()`","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-04T09:36:52Z","receivedAt":"2019-10-04T09:37:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Fri, 4 Oct 2019, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\n> writes:\n>\n> > From: Johannes Schindelin <johannes.schindelin@gmx.de>\n> >\n> > This function is marked as `NORETURN`, and it indeed does not want to\n> > return anything. So let's not declare it with the return type `int`.\n> > This fixes the following warning when building with MSVC:\n> >\n> > \tC4646: function declared with 'noreturn' has non-void return type\n> >\n> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> > ---\n> >  builtin/push.c | 4 ++--\n> >  1 file changed, 2 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/builtin/push.c b/builtin/push.c\n> > index 021dd3b1e4..d216270d5f 100644\n> > --- a/builtin/push.c\n> > +++ b/builtin/push.c\n> > @@ -143,8 +143,8 @@ static int push_url_of_remote(struct remote *remote, const char ***url_p)\n> >  \treturn remote->url_nr;\n> >  }\n> >\n> > -static NORETURN int die_push_simple(struct branch *branch,\n> > -\t\t\t\t    struct remote *remote)\n> > +static NORETURN void die_push_simple(struct branch *branch,\n> > +\t\t\t\t     struct remote *remote)\n>\n> Haha ;-)  \"I won't return and I'll return int\" sounds like an\n> oxymoron.\n\nFunny, right? ;-)\n\nI was a bit concerned that GCC did not catch it, nor Clang, but hey,\nthat's the benefit of compiling with more than just one compiler.\n\nCiao,\nDscho\n\n>\n> >  {\n> >  \t/*\n> >  \t * There's no point in using shorten_unambiguous_ref here,\n>\n"},{"id":"383410","messageId":"nycvar.QRO.7.76.6.1910041154120.46@tvgsbejvaqbjf.bet","threadId":"51924","inReplyTo":"xmqqy2y1a1qk.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 02/13] msvc: avoid using minus operator on unsigned types","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-04T09:55:27Z","receivedAt":"2019-10-04T09:55:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Fri, 4 Oct 2019, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\n> writes:\n>\n> > While at it, we take care of reporting overflows (which are unlikely,\n> > but hey, defensive programming is good!).\n> >\n> > We _also_ take pains of casting the unsigned value to signed: otherwise,\n> > the signed operand (i.e. the `-1`) would be cast to unsigned before\n> > doing the arithmetic.\n>\n> These three look good and too similar to each other, which makes me\n> wonder if we want to allow them simply write\n>\n> \treturn insert_pos_as_negative_offset(nr);\n>\n> with something like\n>\n> \tstatic int insert_pos_as_negative_offset(uintmax_t nr)\n> \t{\n> \t\tif (INT_MAX < nr)\n> \t\t\tdie(\"overflow: -1 - %\"PRIuMAX, nr);\n> \t\treturn -1 - (int)nr;\n> \t}\n>\n> to avoid repetition.\n\nI tried not to do that because there are two different data types in\nplay: `unsigned int` and `size_t`. But I guess by making this an\n`inline` function, compilers can optimize for the common case and avoid\ncasting _twice_.\n\nWill be fixed in v2,\nDscho\n\n>\n> > Helped-by: Denton Liu <liu.denton@gmail.com>\n> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> > ---\n> >  read-cache.c  |  9 ++++++---\n> >  sha1-lookup.c | 12 +++++++++---\n> >  2 files changed, 15 insertions(+), 6 deletions(-)\n> >\n> > diff --git a/read-cache.c b/read-cache.c\n> > index c701f7f8b8..97745c2a31 100644\n> > --- a/read-cache.c\n> > +++ b/read-cache.c\n> > @@ -1275,8 +1275,11 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e\n> >  \t * we can avoid searching for it.\n> >  \t */\n> >  \tif (istate->cache_nr > 0 &&\n> > -\t\tstrcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)\n> > -\t\tpos = -istate->cache_nr - 1;\n> > +\t\tstrcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0) {\n> > +\t\tif (istate->cache_nr > INT_MAX)\n> > +\t\t\tdie(\"overflow: -1 - %u\", istate->cache_nr);\n> > +\t\tpos = -1 - (int)istate->cache_nr;\n> > +\t}\n> >  \telse\n> >  \t\tpos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce));\n> >\n> > @@ -1894,7 +1897,7 @@ static size_t estimate_cache_size(size_t ondisk_size, unsigned int entries)\n> >  \t/*\n> >  \t * Account for potential alignment differences.\n> >  \t */\n> > -\tper_entry += align_padding_size(sizeof(struct cache_entry), -sizeof(struct ondisk_cache_entry));\n> > +\tper_entry += align_padding_size(per_entry, 0);\n> >  \treturn ondisk_size + entries * per_entry;\n> >  }\n> >\n> > diff --git a/sha1-lookup.c b/sha1-lookup.c\n> > index 796ab68da8..bb786b5420 100644\n> > --- a/sha1-lookup.c\n> > +++ b/sha1-lookup.c\n> > @@ -69,8 +69,12 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,\n> >  \t\t\tmiv = take2(sha1 + ofs);\n> >  \t\t\tif (miv < lov)\n> >  \t\t\t\treturn -1;\n> > -\t\t\tif (hiv < miv)\n> > -\t\t\t\treturn -1 - nr;\n> > +\t\t\tif (hiv < miv) {\n> > +\t\t\t\tif (nr > INT_MAX)\n> > +\t\t\t\t\tdie(\"overflow: -1 - %\"PRIuMAX,\n> > +\t\t\t\t\t    (uintmax_t)nr);\n> > +\t\t\t\treturn -1 - (int)nr;\n> > +\t\t\t}\n> >  \t\t\tif (lov != hiv) {\n> >  \t\t\t\t/*\n> >  \t\t\t\t * At this point miv could be equal\n> > @@ -97,7 +101,9 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,\n> >  \t\t\tlo = mi + 1;\n> >  \t\tmi = lo + (hi - lo) / 2;\n> >  \t} while (lo < hi);\n> > -\treturn -lo-1;\n> > +\tif (nr > INT_MAX)\n> > +\t\tdie(\"overflow: -1 - %\"PRIuMAX, (uintmax_t)lo);\n> > +\treturn -1 - (int)lo;\n> >  }\n> >\n> >  int bsearch_hash(const unsigned char *sha1, const uint32_t *fanout_nbo,\n>\n"},{"id":"383421","messageId":"pull.288.v3.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-04T15:09:23Z","receivedAt":"2019-10-04T15:09:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Git's Continuous Integration (CI) includes an Azure Pipeline that builds Git\non Linux, macOS and Windows, in the former two cases even in multiple\nconfigurations (using GCC vs clang, 32-bit vs 64-bit, etc). On Windows, we\nonly build using GCC, using (a subset of) Git for Windows' SDK.\n\nRecently, a patch series made it into Git that re-instates the ability to\ngenerate project files for use with Visual Studio. The idea there being:\ncontributors can check out a special branch that has those generated files\nin one generated commit on top of e.g. Git for Windows' master, allowing the\ncontributors to build Git in Visual Studio, without the need for downloading\nGit for Windows' SDK (which weighs quite a bit: ~600MB download, ~2GB disk\nfootprint). The tests can then be run from a regular Git for Windows Bash.\n\nThis patch series adds that axis to Git's Azure Pipeline: the project files\nare generated, MSBuild (which is kind of the command-line equivalent of\nVisual Studio's \"Build\" operation) is used to build Git, and then a\nparallelized test job runs the test suite in a Portable Git.\n\nThese patches are based on js/visual-studio.\n\nChanges since v2:\n\n * The overflow check introduced in v1 was consolidated into a single\n   helper.\n\nChanges since v1:\n\n * \"While at it\", we now also check for overflows when doing that -1 -\n   <unsigned> arithmetic.\n * The JUnit-style XML is finalized also in case that the script aborts e.g.\n   due to an incorrect number of arguments in a test_expect_success call.\n\nJohannes Schindelin (13):\n  push: do not pretend to return `int` from `die_push_simple()`\n  msvc: avoid using minus operator on unsigned types\n  winansi: use FLEX_ARRAY to avoid compiler warning\n  compat/win32/path-utils.h: add #include guards\n  msvc: ignore some libraries when linking\n  msvc: handle DEVELOPER=1\n  msvc: work around a bug in GetEnvironmentVariable()\n  vcxproj: only copy `git-remote-http.exe` once it was built\n  vcxproj: include more generated files\n  test-tool run-command: learn to run (parts of) the testsuite\n  tests: let --immediate and --write-junit-xml play well together\n  ci: really use shallow clones on Azure Pipelines\n  ci: also build and test with MS Visual Studio on Azure Pipelines\n\n Makefile                                   |   4 +\n azure-pipelines.yml                        | 164 ++++++++++++++++++++-\n builtin/push.c                             |   4 +-\n cache.h                                    |  13 ++\n compat/mingw.c                             |   2 +\n compat/vcbuild/scripts/clink.pl            |  48 +++++-\n compat/win32/path-utils.h                  |   5 +\n compat/winansi.c                           |   2 +-\n config.mak.uname                           |  19 ++-\n contrib/buildsystems/Generators/Vcxproj.pm |   3 +\n read-cache.c                               |   4 +-\n sha1-lookup.c                              |   4 +-\n t/helper/test-run-command.c                | 153 +++++++++++++++++++\n t/test-lib.sh                              |  38 +++--\n 14 files changed, 430 insertions(+), 33 deletions(-)\n\n\nbase-commit: aac6ff7b5beeea9bca66ecda6eec60fc1dd447ec\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-288%2Fdscho%2Fazure-pipelines-msvc-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-288/dscho/azure-pipelines-msvc-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/288\n\nRange-diff vs v2:\n\n  1:  4d0b38125a =  1:  4d0b38125a push: do not pretend to return `int` from `die_push_simple()`\n  2:  8800320590 <  -:  ---------- msvc: avoid using minus operator on unsigned types\n  -:  ---------- >  2:  7fe2a85506 msvc: avoid using minus operator on unsigned types\n  3:  8512a3e96d =  3:  e632a4eef4 winansi: use FLEX_ARRAY to avoid compiler warning\n  4:  0345b08f54 =  4:  cd96d7ff70 compat/win32/path-utils.h: add #include guards\n  5:  5add01f8ff =  5:  bf09257f65 msvc: ignore some libraries when linking\n  6:  5c880f923e =  6:  39c707464c msvc: handle DEVELOPER=1\n  7:  1f2245a228 =  7:  91b09cfdd8 msvc: work around a bug in GetEnvironmentVariable()\n  8:  582b299634 =  8:  cca891450d vcxproj: only copy `git-remote-http.exe` once it was built\n  9:  38ccf999e7 =  9:  e6e60b3c2b vcxproj: include more generated files\n 10:  24b1c7bff3 = 10:  d2c4973431 test-tool run-command: learn to run (parts of) the testsuite\n 11:  7be13d19e1 = 11:  0644a2f8da tests: let --immediate and --write-junit-xml play well together\n 12:  bde1e8ef65 = 12:  4495c392fd ci: really use shallow clones on Azure Pipelines\n 13:  7af1c01a08 = 13:  b1ff8eff4d ci: also build and test with MS Visual Studio on Azure Pipelines\n\n-- \ngitgitgadget\n"},{"id":"383422","messageId":"4d0b38125a13d85963be5e524becf48271893e2b.1570201763.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v3 01/13] push: do not pretend to return `int` from `die_push_simple()`","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-04T15:09:25Z","receivedAt":"2019-10-04T15:09:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThis function is marked as `NORETURN`, and it indeed does not want to\nreturn anything. So let's not declare it with the return type `int`.\nThis fixes the following warning when building with MSVC:\n\n\tC4646: function declared with 'noreturn' has non-void return type\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n builtin/push.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 021dd3b1e4..d216270d5f 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -143,8 +143,8 @@ static int push_url_of_remote(struct remote *remote, const char ***url_p)\n \treturn remote->url_nr;\n }\n \n-static NORETURN int die_push_simple(struct branch *branch,\n-\t\t\t\t    struct remote *remote)\n+static NORETURN void die_push_simple(struct branch *branch,\n+\t\t\t\t     struct remote *remote)\n {\n \t/*\n \t * There's no point in using shorten_unambiguous_ref here,\n-- \ngitgitgadget\n\n"},{"id":"383423","messageId":"e632a4eef46720d43fd014644ff24d3dac61ebe9.1570201763.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v3 03/13] winansi: use FLEX_ARRAY to avoid compiler warning","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-04T15:09:27Z","receivedAt":"2019-10-04T15:09:31Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nMSVC would complain thusly:\n\n    C4200: nonstandard extension used: zero-sized array in struct/union\n\nLet's just use the `FLEX_ARRAY` constant that we introduced for exactly\nthis type of scenario.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n compat/winansi.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/compat/winansi.c b/compat/winansi.c\nindex cacd82c833..54fd701cbf 100644\n--- a/compat/winansi.c\n+++ b/compat/winansi.c\n@@ -546,7 +546,7 @@ static HANDLE swap_osfhnd(int fd, HANDLE new_handle)\n typedef struct _OBJECT_NAME_INFORMATION\n {\n \tUNICODE_STRING Name;\n-\tWCHAR NameBuffer[0];\n+\tWCHAR NameBuffer[FLEX_ARRAY];\n } OBJECT_NAME_INFORMATION, *POBJECT_NAME_INFORMATION;\n \n #define ObjectNameInformation 1\n-- \ngitgitgadget\n\n"},{"id":"383424","messageId":"bf09257f65e8acf9a5fed7d27378e187e7328f91.1570201763.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v3 05/13] msvc: ignore some libraries when linking","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-04T15:09:29Z","receivedAt":"2019-10-04T15:09:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nTo build with MSVC, we \"translate\" GCC options to MSVC options, and part\nof those options refer to the libraries to link into the final\nexecutable. Currently, this part looks somewhat like this on Windows:\n\n\t-lcurl -lnghttp2 -lidn2 -lssl -lcrypto -lssl -lcrypto -lgdi32\n\t-lcrypt32 -lwldap32 -lz -lws2_32 -lexpat\n\nSome of those are direct dependencies (such as curl and ssl) and others\nare indirect (nghttp2 and idn2, for example, are dependencies of curl,\nbut need to be linked in for reasons).\n\nWe already handle the direct dependencies, e.g. `-liconv` is already\nhandled as adding `libiconv.lib` to the list of libraries to link\nagainst.\n\nLet's just ignore the remaining `-l*` options so that MSVC does not have\nto warn us that it ignored e.g. the `/lnghttp2` option. We do that by\nextending the clause that already \"eats\" the `-R*` options to also eat\nthe `-l*` options.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n compat/vcbuild/scripts/clink.pl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/compat/vcbuild/scripts/clink.pl b/compat/vcbuild/scripts/clink.pl\nindex c7b021bfac..00fc339cba 100755\n--- a/compat/vcbuild/scripts/clink.pl\n+++ b/compat/vcbuild/scripts/clink.pl\n@@ -68,7 +68,7 @@\n \t} elsif (\"$arg\" =~ /^-L/ && \"$arg\" ne \"-LTCG\") {\n \t\t$arg =~ s/^-L/-LIBPATH:/;\n \t\tpush(@lflags, $arg);\n-\t} elsif (\"$arg\" =~ /^-R/) {\n+\t} elsif (\"$arg\" =~ /^-[Rl]/) {\n \t\t# eat\n \t} else {\n \t\tpush(@args, $arg);\n-- \ngitgitgadget\n\n"},{"id":"383425","messageId":"7fe2a85506d2489dc17a05bca5f95303892351a9.1570201763.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v3 02/13] msvc: avoid using minus operator on unsigned types","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-04T15:09:26Z","receivedAt":"2019-10-04T15:09:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nMSVC complains about this with `-Wall`, which can be taken as a sign\nthat this is indeed a real bug. The symptom is:\n\n\tC4146: unary minus operator applied to unsigned type, result\n\tstill unsigned\n\nLet's avoid this warning in the minimal way, e.g. writing `-1 -\n<unsigned value>` instead of `-<unsigned value> - 1`.\n\nNote that the change in the `estimate_cache_size()` function is\nneeded because MSVC considers the \"return type\" of the `sizeof()`\noperator to be `size_t`, i.e. unsigned, and therefore it cannot be\nnegated using the unary minus operator.\n\nEven worse, that arithmetic is doing extra work, in vain. We want to\ncalculate the entry extra cache size as the difference between the\nsize of the `cache_entry` structure minus the size of the\n`ondisk_cache_entry` structure, padded to the appropriate alignment\nboundary.\n\nTo that end, we start by assigning that difference to the `per_entry`\nvariable, and then abuse the `len` parameter of the\n`align_padding_size()` macro to take the negative size of the ondisk\nentry size. Essentially, we try to avoid passing the already calculated\ndifference to that macro by passing the operands of that difference\ninstead, when the macro expects operands of an addition:\n\n\t#define align_padding_size(size, len) \\\n\t\t((size + (len) + 8) & ~7) - (size + len)\n\nCurrently, we pass A and -B to that macro instead of passing A - B and\n0, where A - B is already stored in the `per_entry` variable, ready to\nbe used.\n\nThis is neither necessary, nor intuitive. Let's fix this, and have code\nthat is both easier to read and that also does not trigger MSVC's\nwarning.\n\nWhile at it, we take care of reporting overflows (which are unlikely,\nbut hey, defensive programming is good!).\n\nWe _also_ take pains of casting the unsigned value to signed: otherwise,\nthe signed operand (i.e. the `-1`) would be cast to unsigned before\ndoing the arithmetic.\n\nHelped-by: Denton Liu <liu.denton@gmail.com>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n cache.h       | 13 +++++++++++++\n read-cache.c  |  4 ++--\n sha1-lookup.c |  4 ++--\n 3 files changed, 17 insertions(+), 4 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 3167585cab..850e7b945a 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -725,6 +725,19 @@ struct cache_entry *index_file_exists(struct index_state *istate, const char *na\n  */\n int index_name_pos(const struct index_state *, const char *name, int namelen);\n \n+/*\n+ * Some functions return the negative complement of an insert position when a\n+ * precise match was not found but a position was found where the entry would\n+ * need to be inserted. This helper protects that logic from any integer\n+ * underflow.\n+ */\n+static inline int index_pos_to_insert_pos(uintmax_t pos)\n+{\n+\tif (pos > INT_MAX)\n+\t\tdie(\"overflow: -1 - %\"PRIuMAX, pos);\n+\treturn -1 - (int)pos;\n+}\n+\n #define ADD_CACHE_OK_TO_ADD 1\t\t/* Ok to add */\n #define ADD_CACHE_OK_TO_REPLACE 2\t/* Ok to replace file/directory */\n #define ADD_CACHE_SKIP_DFCHECK 4\t/* Ok to skip DF conflict checks */\ndiff --git a/read-cache.c b/read-cache.c\nindex c701f7f8b8..ec13953a21 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1276,7 +1276,7 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e\n \t */\n \tif (istate->cache_nr > 0 &&\n \t\tstrcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)\n-\t\tpos = -istate->cache_nr - 1;\n+\t\tpos = index_pos_to_insert_pos(istate->cache_nr);\n \telse\n \t\tpos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce));\n \n@@ -1894,7 +1894,7 @@ static size_t estimate_cache_size(size_t ondisk_size, unsigned int entries)\n \t/*\n \t * Account for potential alignment differences.\n \t */\n-\tper_entry += align_padding_size(sizeof(struct cache_entry), -sizeof(struct ondisk_cache_entry));\n+\tper_entry += align_padding_size(per_entry, 0);\n \treturn ondisk_size + entries * per_entry;\n }\n \ndiff --git a/sha1-lookup.c b/sha1-lookup.c\nindex 796ab68da8..8590aac254 100644\n--- a/sha1-lookup.c\n+++ b/sha1-lookup.c\n@@ -70,7 +70,7 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,\n \t\t\tif (miv < lov)\n \t\t\t\treturn -1;\n \t\t\tif (hiv < miv)\n-\t\t\t\treturn -1 - nr;\n+\t\t\t\treturn index_pos_to_insert_pos(nr);\n \t\t\tif (lov != hiv) {\n \t\t\t\t/*\n \t\t\t\t * At this point miv could be equal\n@@ -97,7 +97,7 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,\n \t\t\tlo = mi + 1;\n \t\tmi = lo + (hi - lo) / 2;\n \t} while (lo < hi);\n-\treturn -lo-1;\n+\treturn index_pos_to_insert_pos(lo);\n }\n \n int bsearch_hash(const unsigned char *sha1, const uint32_t *fanout_nbo,\n-- \ngitgitgadget\n\n"},{"id":"383426","messageId":"cca891450df4eddaa1cb73e4aaa70c444f2531d2.1570201763.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v3 08/13] vcxproj: only copy `git-remote-http.exe` once it was built","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-04T15:09:31Z","receivedAt":"2019-10-04T15:09:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nIn b18ae14a8f6 (vcxproj: also link-or-copy builtins, 2019-07-29), we\nstarted to copy or hard-link the built-ins as a post-build step of the\n`git` project.\n\nAt the same time, we tried to copy or hard-link `git-remote-http.exe`,\nbut it is quite possible that it was not built at that time.\n\nLet's move that latter task into a post-install step of the\n`git-remote-http` project instead.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n config.mak.uname                           | 10 +++++++---\n contrib/buildsystems/Generators/Vcxproj.pm |  3 +++\n 2 files changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git a/config.mak.uname b/config.mak.uname\nindex db7f06b95f..701aad62b1 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -703,20 +703,24 @@ vcxproj:\n \tperl contrib/buildsystems/generate -g Vcxproj\n \tgit add -f git.sln {*,*/lib,t/helper/*}/*.vcxproj\n \n-\t# Generate the LinkOrCopyBuiltins.targets file\n+\t# Generate the LinkOrCopyBuiltins.targets and LinkOrCopyRemoteHttp.targets file\n \t(echo '<Project xmlns=\"http://schemas.microsoft.com/developer/msbuild/2003\">' && \\\n \t echo '  <Target Name=\"CopyBuiltins_AfterBuild\" AfterTargets=\"AfterBuild\">' && \\\n \t for name in $(BUILT_INS);\\\n \t do \\\n \t   echo '    <Copy SourceFiles=\"$$(OutDir)\\git.exe\" DestinationFiles=\"$$(OutDir)\\'\"$$name\"'\" SkipUnchangedFiles=\"true\" UseHardlinksIfPossible=\"true\" />'; \\\n \t done && \\\n+\t echo '  </Target>' && \\\n+\t echo '</Project>') >git/LinkOrCopyBuiltins.targets\n+\t(echo '<Project xmlns=\"http://schemas.microsoft.com/developer/msbuild/2003\">' && \\\n+\t echo '  <Target Name=\"CopyBuiltins_AfterBuild\" AfterTargets=\"AfterBuild\">' && \\\n \t for name in $(REMOTE_CURL_ALIASES); \\\n \t do \\\n \t   echo '    <Copy SourceFiles=\"$$(OutDir)\\'\"$(REMOTE_CURL_PRIMARY)\"'\" DestinationFiles=\"$$(OutDir)\\'\"$$name\"'\" SkipUnchangedFiles=\"true\" UseHardlinksIfPossible=\"true\" />'; \\\n \t done && \\\n \t echo '  </Target>' && \\\n-\t echo '</Project>') >git/LinkOrCopyBuiltins.targets\n-\tgit add -f git/LinkOrCopyBuiltins.targets\n+\t echo '</Project>') >git-remote-http/LinkOrCopyRemoteHttp.targets\n+\tgit add -f git/LinkOrCopyBuiltins.targets git-remote-http/LinkOrCopyRemoteHttp.targets\n \n \t# Add command-list.h\n \t$(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 command-list.h\ndiff --git a/contrib/buildsystems/Generators/Vcxproj.pm b/contrib/buildsystems/Generators/Vcxproj.pm\nindex 576ccabe1d..868b787855 100644\n--- a/contrib/buildsystems/Generators/Vcxproj.pm\n+++ b/contrib/buildsystems/Generators/Vcxproj.pm\n@@ -277,6 +277,9 @@ sub createProject {\n     if ($target eq 'git') {\n       print F \"  <Import Project=\\\"LinkOrCopyBuiltins.targets\\\" />\\n\";\n     }\n+    if ($target eq 'git-remote-http') {\n+      print F \"  <Import Project=\\\"LinkOrCopyRemoteHttp.targets\\\" />\\n\";\n+    }\n     print F << \"EOM\";\n </Project>\n EOM\n-- \ngitgitgadget\n\n"},{"id":"383427","messageId":"91b09cfdd8738f27009c86760d1e6a24a46017eb.1570201763.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v3 07/13] msvc: work around a bug in GetEnvironmentVariable()","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-04T15:09:30Z","receivedAt":"2019-10-04T15:09:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThe return value of that function is 0 both for variables that are\nunset, as well as for variables whose values are empty. To discern those\ntwo cases, one has to call `GetLastError()`, whose return value is\n`ERROR_ENVVAR_NOT_FOUND` and `ERROR_SUCCESS`, respectively.\n\nExcept that it is not actually set to `ERROR_SUCCESS` in the latter\ncase, apparently, but the last error value seems to be simply unchanged.\n\nTo work around this, let's just re-set the last error value just before\ninspecting the environment variable.\n\nThis fixes a problem that triggers failures in t3301-notes.sh (where we\ntry to override config settings by passing empty values for certain\nenvironment variables).\n\nThis problem is hidden in the MINGW build by the fact that older\nMSVC runtimes (such as the one used by MINGW builds) have a `calloc()`\nthat re-sets the last error value in case of success, while newer\nruntimes set the error value only if `NULL` is returned by that\nfunction.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n compat/mingw.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 4891789771..7d8cb814ba 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -1661,6 +1661,8 @@ char *mingw_getenv(const char *name)\n \tif (!w_key)\n \t\tdie(\"Out of memory, (tried to allocate %u wchar_t's)\", len_key);\n \txutftowcs(w_key, name, len_key);\n+\t/* GetEnvironmentVariableW() only sets the last error upon failure */\n+\tSetLastError(ERROR_SUCCESS);\n \tlen_value = GetEnvironmentVariableW(w_key, w_value, ARRAY_SIZE(w_value));\n \tif (!len_value && GetLastError() == ERROR_ENVVAR_NOT_FOUND) {\n \t\tfree(w_key);\n-- \ngitgitgadget\n\n"},{"id":"383428","messageId":"e6e60b3c2b878d90b96c227c10f432375051144e.1570201763.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v3 09/13] vcxproj: include more generated files","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-04T15:09:32Z","receivedAt":"2019-10-04T15:09:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nIn the CI builds, we bundle all generated files into a so-called\nartifacts `.tar` file, so that the test phase can fan out into multiple\nparallel builds.\n\nThis patch makes sure that all files are included in the `vcxproj`\ntarget which are needed for that artifacts `.tar` file.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n config.mak.uname | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/config.mak.uname b/config.mak.uname\nindex 701aad62b1..cc8efd95b1 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -728,11 +728,10 @@ vcxproj:\n \n \t# Add scripts\n \trm -f perl/perl.mak\n-\t$(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 \\\n-\t\t$(SCRIPT_LIB) $(SCRIPT_SH_GEN) $(SCRIPT_PERL_GEN)\n+\t$(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 $(SCRIPT_LIB) $(SCRIPTS)\n \t# Strip out the sane tool path, needed only for building\n \tsed -i '/^git_broken_path_fix \".*/d' git-sh-setup\n-\tgit add -f $(SCRIPT_LIB) $(SCRIPT_SH_GEN) $(SCRIPT_PERL_GEN)\n+\tgit add -f $(SCRIPT_LIB) $(SCRIPTS)\n \n \t# Add Perl module\n \t$(MAKE) $(LIB_PERL_GEN)\n@@ -762,6 +761,10 @@ vcxproj:\n \t$(MAKE) -C templates\n \tgit add -f templates/boilerplates.made templates/blt/\n \n+\t# Add the translated messages\n+\tmake MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 $(MOFILES)\n+\tgit add -f $(MOFILES)\n+\n \t# Add build options\n \t$(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 GIT-BUILD-OPTIONS\n \tgit add -f GIT-BUILD-OPTIONS\n-- \ngitgitgadget\n\n"},{"id":"383429","messageId":"cd96d7ff70d129543ff6b482b98f0827929525d1.1570201763.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v3 04/13] compat/win32/path-utils.h: add #include guards","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-04T15:09:28Z","receivedAt":"2019-10-04T15:09:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThis adds the common guards that allow headers to be #include'd multiple\ntimes.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n compat/win32/path-utils.h | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/compat/win32/path-utils.h b/compat/win32/path-utils.h\nindex 0f70d43920..8ed062a6b7 100644\n--- a/compat/win32/path-utils.h\n+++ b/compat/win32/path-utils.h\n@@ -1,3 +1,6 @@\n+#ifndef WIN32_PATH_UTILS_H\n+#define WIN32_PATH_UTILS_H\n+\n #define has_dos_drive_prefix(path) \\\n \t(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)\n int win32_skip_dos_drive_prefix(char **path);\n@@ -18,3 +21,5 @@ static inline char *win32_find_last_dir_sep(const char *path)\n #define find_last_dir_sep win32_find_last_dir_sep\n int win32_offset_1st_component(const char *path);\n #define offset_1st_component win32_offset_1st_component\n+\n+#endif\n-- \ngitgitgadget\n\n"},{"id":"383430","messageId":"4495c392fd47db0b66c93b6d3d512ec606038471.1570201763.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v3 12/13] ci: really use shallow clones on Azure Pipelines","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-04T15:09:35Z","receivedAt":"2019-10-04T15:09:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThis was a left-over from the previous YAML schema, and it no longer\nworks. The problem was noticed while editing `azure-pipelines.yml` in VS\nCode with the very helpful \"Azure Pipelines\" extension (syntax\nhighlighting and intellisense for `azure-pipelines.yml`...).\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n azure-pipelines.yml | 5 ++---\n 1 file changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/azure-pipelines.yml b/azure-pipelines.yml\nindex c329b7218b..55ee23ad0f 100644\n--- a/azure-pipelines.yml\n+++ b/azure-pipelines.yml\n@@ -1,6 +1,5 @@\n-resources:\n-- repo: self\n-  fetchDepth: 1\n+variables:\n+  Agent.Source.Git.ShallowFetchDepth: 1\n \n jobs:\n - job: windows_build\n-- \ngitgitgadget\n\n"},{"id":"383431","messageId":"39c707464cf46f199157db0c37bf6dec1da7c3af.1570201763.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v3 06/13] msvc: handle DEVELOPER=1","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-04T15:09:30Z","receivedAt":"2019-10-04T15:09:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWe frequently build Git using the `DEVELOPER=1` make setting as a\nshortcut to enable all kinds of more stringent compiler warnings.\n\nThose compiler warnings are relatively specific to GCC, though, so let's\ntry our best to translate them to the equivalent options to pass to MS\nVisual C++'s `cl.exe`.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n compat/vcbuild/scripts/clink.pl | 46 +++++++++++++++++++++++++++++++++\n 1 file changed, 46 insertions(+)\n\ndiff --git a/compat/vcbuild/scripts/clink.pl b/compat/vcbuild/scripts/clink.pl\nindex 00fc339cba..ec95a3b2d0 100755\n--- a/compat/vcbuild/scripts/clink.pl\n+++ b/compat/vcbuild/scripts/clink.pl\n@@ -70,6 +70,52 @@\n \t\tpush(@lflags, $arg);\n \t} elsif (\"$arg\" =~ /^-[Rl]/) {\n \t\t# eat\n+\t} elsif (\"$arg\" eq \"-Werror\") {\n+\t\tpush(@cflags, \"-WX\");\n+\t} elsif (\"$arg\" eq \"-Wall\") {\n+\t\t# cl.exe understands -Wall, but it is really overzealous\n+\t\tpush(@cflags, \"-W4\");\n+\t\t# disable the \"signed/unsigned mismatch\" warnings; our source code violates that\n+\t\tpush(@cflags, \"-wd4018\");\n+\t\tpush(@cflags, \"-wd4245\");\n+\t\tpush(@cflags, \"-wd4389\");\n+\t\t# disable the \"unreferenced formal parameter\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4100\");\n+\t\t# disable the \"conditional expression is constant\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4127\");\n+\t\t# disable the \"const object should be initialized\" warning; these warnings affect only objects that are `static`\n+\t\tpush(@cflags, \"-wd4132\");\n+\t\t# disable the \"function/data pointer conversion in expression\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4152\");\n+\t\t# disable the \"non-constant aggregate initializer\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4204\");\n+\t\t# disable the \"cannot be initialized using address of automatic variable\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4221\");\n+\t\t# disable the \"possible loss of data\" warnings; our source code violates that\n+\t\tpush(@cflags, \"-wd4244\");\n+\t\tpush(@cflags, \"-wd4267\");\n+\t\t# disable the \"array is too small to include a terminating null character\" warning; we ab-use strings to initialize OIDs\n+\t\tpush(@cflags, \"-wd4295\");\n+\t\t# disable the \"'<<': result of 32-bit shift implicitly converted to 64 bits\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4334\");\n+\t\t# disable the \"declaration hides previous local declaration\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4456\");\n+\t\t# disable the \"declaration hides function parameter\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4457\");\n+\t\t# disable the \"declaration hides global declaration\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4459\");\n+\t\t# disable the \"potentially uninitialized local variable '<name>' used\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4701\");\n+\t\t# disable the \"unreachable code\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4702\");\n+\t\t# disable the \"potentially uninitialized local pointer variable used\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4703\");\n+\t\t# disable the \"assignment within conditional expression\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4706\");\n+\t\t# disable the \"'inet_ntoa': Use inet_ntop() or InetNtop() instead\" warning; our source code violates that\n+\t\tpush(@cflags, \"-wd4996\");\n+\t} elsif (\"$arg\" =~ /^-W[a-z]/) {\n+\t\t# let's ignore those\n \t} else {\n \t\tpush(@args, $arg);\n \t}\n-- \ngitgitgadget\n\n"},{"id":"383432","messageId":"0644a2f8da8d04d505d3c70224ca806b52523daf.1570201763.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v3 11/13] tests: let --immediate and --write-junit-xml play well together","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-04T15:09:34Z","receivedAt":"2019-10-04T15:09:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWhen the `--immediate` option is in effect, any test failure will\nimmediately exit the test script. Together with `--write-junit-xml`, we\nwill want the JUnit-style `.xml` file to be finalized (and not leave the\nXML incomplete). Let's make it so.\n\nThis comes in particularly handy when trying to debug via Azure\nPipelines, where the JUnit-style XML is consumed to present the test\nresults in an informative and helpful way.\n\nWhile at it, also handle the `error()` code path.\n\nThe only remaining code path that sets `GIT_EXIT_OK` happens whenever\nthe trash directory could not be set up, i.e. long before the JUnit XML\nwas written, therefore we should _not_ try to finalize that XML in that\ncase.\n\nIt is tempting to change the `immediate` code path to just hand off to\n`error`, simplifying the code in the process. That would, however,\nresult in a change of behavior (an additional error message) in the test\nsuite, which is outside of the purview of the current patch series: its\ngoal is to allow building Git with Visual Studio and testing it with a\nportable version of Git for Windows.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n t/test-lib.sh | 38 ++++++++++++++++++++++----------------\n 1 file changed, 22 insertions(+), 16 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex d1ba33745a..86b5e6133b 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -567,6 +567,7 @@ export TERM\n \n error () {\n \tsay_color error \"error: $*\"\n+\tfinalize_junit_xml\n \tGIT_EXIT_OK=t\n \texit 1\n }\n@@ -695,7 +696,7 @@ test_failure_ () {\n \tsay_color error \"not ok $test_count - $1\"\n \tshift\n \tprintf '%s\\n' \"$*\" | sed -e 's/^/#\t/'\n-\ttest \"$immediate\" = \"\" || { GIT_EXIT_OK=t; exit 1; }\n+\ttest \"$immediate\" = \"\" || { finalize_junit_xml; GIT_EXIT_OK=t; exit 1; }\n }\n \n test_known_broken_ok_ () {\n@@ -1063,6 +1064,25 @@ write_junit_xml_testcase () {\n \tjunit_have_testcase=t\n }\n \n+finalize_junit_xml () {\n+\tif test -n \"$write_junit_xml\" && test -n \"$junit_xml_path\"\n+\tthen\n+\t\ttest -n \"$junit_have_testcase\" || {\n+\t\t\tjunit_start=$(test-tool date getnanos)\n+\t\t\twrite_junit_xml_testcase \"all tests skipped\"\n+\t\t}\n+\n+\t\t# adjust the overall time\n+\t\tjunit_time=$(test-tool date getnanos $junit_suite_start)\n+\t\tsed \"s/<testsuite [^>]*/& time=\\\"$junit_time\\\"/\" \\\n+\t\t\t<\"$junit_xml_path\" >\"$junit_xml_path.new\"\n+\t\tmv \"$junit_xml_path.new\" \"$junit_xml_path\"\n+\n+\t\twrite_junit_xml \"  </testsuite>\" \"</testsuites>\"\n+\t\twrite_junit_xml=\n+\tfi\n+}\n+\n test_atexit_cleanup=:\n test_atexit_handler () {\n \t# In a succeeding test script 'test_atexit_handler' is invoked\n@@ -1085,21 +1105,7 @@ test_done () {\n \t# removed, so the commands can access pidfiles and socket files.\n \ttest_atexit_handler\n \n-\tif test -n \"$write_junit_xml\" && test -n \"$junit_xml_path\"\n-\tthen\n-\t\ttest -n \"$junit_have_testcase\" || {\n-\t\t\tjunit_start=$(test-tool date getnanos)\n-\t\t\twrite_junit_xml_testcase \"all tests skipped\"\n-\t\t}\n-\n-\t\t# adjust the overall time\n-\t\tjunit_time=$(test-tool date getnanos $junit_suite_start)\n-\t\tsed \"s/<testsuite [^>]*/& time=\\\"$junit_time\\\"/\" \\\n-\t\t\t<\"$junit_xml_path\" >\"$junit_xml_path.new\"\n-\t\tmv \"$junit_xml_path.new\" \"$junit_xml_path\"\n-\n-\t\twrite_junit_xml \"  </testsuite>\" \"</testsuites>\"\n-\tfi\n+\tfinalize_junit_xml\n \n \tif test -z \"$HARNESS_ACTIVE\"\n \tthen\n-- \ngitgitgadget\n\n"},{"id":"383433","messageId":"d2c4973431e36b7b7b9f05ae30d96541d5fc1938.1570201763.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v3 10/13] test-tool run-command: learn to run (parts of) the testsuite","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-04T15:09:33Z","receivedAt":"2019-10-04T15:09:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nGit for Windows jumps through hoops to provide a development environment\nthat allows to build Git and to run its test suite. To that end, an\nentire MSYS2 system, including GNU make and GCC is offered as \"the Git\nfor Windows SDK\". It does come at a price: an initial download of said\nSDK weighs in with several hundreds of megabytes, and the unpacked SDK\noccupies ~2GB of disk space.\n\nA much more native development environment on Windows is Visual Studio.\nTo help contributors use that environment, we already have a Makefile\ntarget `vcxproj` that generates a commit with project files (and other\ngenerated files), and Git for Windows' `vs/master` branch is\ncontinuously re-generated using that target.\n\nThe idea is to allow building Git in Visual Studio, and to run\nindividual tests using a Portable Git.\n\nThe one missing thing is a way to run the entire test suite: neither\n`make` nor `prove` are required to run Git, therefore Git for Windows\ndoes not support those commands in the Portable Git.\n\nTo help with that, add a simple test helper that exercises the\n`run_processes_parallel()` function to allow for running test scripts in\nparallel (which is really necessary, especially on Windows, as Git's\ntest suite takes such a long time to run).\n\nThis will also come in handy for the upcoming change to our Azure\nPipeline: we will use this helper in a Portable Git to test the Visual\nStudio build of Git.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n t/helper/test-run-command.c | 153 ++++++++++++++++++++++++++++++++++++\n 1 file changed, 153 insertions(+)\n\ndiff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c\nindex 2cc93bb69c..ead6dc611a 100644\n--- a/t/helper/test-run-command.c\n+++ b/t/helper/test-run-command.c\n@@ -10,9 +10,14 @@\n \n #include \"test-tool.h\"\n #include \"git-compat-util.h\"\n+#include \"cache.h\"\n #include \"run-command.h\"\n #include \"argv-array.h\"\n #include \"strbuf.h\"\n+#include \"parse-options.h\"\n+#include \"string-list.h\"\n+#include \"thread-utils.h\"\n+#include \"wildmatch.h\"\n #include <string.h>\n #include <errno.h>\n \n@@ -50,11 +55,159 @@ static int task_finished(int result,\n \treturn 1;\n }\n \n+struct testsuite {\n+\tstruct string_list tests, failed;\n+\tint next;\n+\tint quiet, immediate, verbose, verbose_log, trace, write_junit_xml;\n+};\n+#define TESTSUITE_INIT \\\n+\t{ STRING_LIST_INIT_DUP, STRING_LIST_INIT_DUP, -1, 0, 0, 0, 0, 0, 0 }\n+\n+static int next_test(struct child_process *cp, struct strbuf *err, void *cb,\n+\t\t     void **task_cb)\n+{\n+\tstruct testsuite *suite = cb;\n+\tconst char *test;\n+\tif (suite->next >= suite->tests.nr)\n+\t\treturn 0;\n+\n+\ttest = suite->tests.items[suite->next++].string;\n+\targv_array_pushl(&cp->args, \"sh\", test, NULL);\n+\tif (suite->quiet)\n+\t\targv_array_push(&cp->args, \"--quiet\");\n+\tif (suite->immediate)\n+\t\targv_array_push(&cp->args, \"-i\");\n+\tif (suite->verbose)\n+\t\targv_array_push(&cp->args, \"-v\");\n+\tif (suite->verbose_log)\n+\t\targv_array_push(&cp->args, \"-V\");\n+\tif (suite->trace)\n+\t\targv_array_push(&cp->args, \"-x\");\n+\tif (suite->write_junit_xml)\n+\t\targv_array_push(&cp->args, \"--write-junit-xml\");\n+\n+\tstrbuf_addf(err, \"Output of '%s':\\n\", test);\n+\t*task_cb = (void *)test;\n+\n+\treturn 1;\n+}\n+\n+static int test_finished(int result, struct strbuf *err, void *cb,\n+\t\t\t void *task_cb)\n+{\n+\tstruct testsuite *suite = cb;\n+\tconst char *name = (const char *)task_cb;\n+\n+\tif (result)\n+\t\tstring_list_append(&suite->failed, name);\n+\n+\tstrbuf_addf(err, \"%s: '%s'\\n\", result ? \"FAIL\" : \"SUCCESS\", name);\n+\n+\treturn 0;\n+}\n+\n+static int test_failed(struct strbuf *out, void *cb, void *task_cb)\n+{\n+\tstruct testsuite *suite = cb;\n+\tconst char *name = (const char *)task_cb;\n+\n+\tstring_list_append(&suite->failed, name);\n+\tstrbuf_addf(out, \"FAILED TO START: '%s'\\n\", name);\n+\n+\treturn 0;\n+}\n+\n+static const char * const testsuite_usage[] = {\n+\t\"test-run-command testsuite [<options>] [<pattern>...]\",\n+\tNULL\n+};\n+\n+static int testsuite(int argc, const char **argv)\n+{\n+\tstruct testsuite suite = TESTSUITE_INIT;\n+\tint max_jobs = 1, i, ret;\n+\tDIR *dir;\n+\tstruct dirent *d;\n+\tstruct option options[] = {\n+\t\tOPT_BOOL('i', \"immediate\", &suite.immediate,\n+\t\t\t \"stop at first failed test case(s)\"),\n+\t\tOPT_INTEGER('j', \"jobs\", &max_jobs, \"run <N> jobs in parallel\"),\n+\t\tOPT_BOOL('q', \"quiet\", &suite.quiet, \"be terse\"),\n+\t\tOPT_BOOL('v', \"verbose\", &suite.verbose, \"be verbose\"),\n+\t\tOPT_BOOL('V', \"verbose-log\", &suite.verbose_log,\n+\t\t\t \"be verbose, redirected to a file\"),\n+\t\tOPT_BOOL('x', \"trace\", &suite.trace, \"trace shell commands\"),\n+\t\tOPT_BOOL(0, \"write-junit-xml\", &suite.write_junit_xml,\n+\t\t\t \"write JUnit-style XML files\"),\n+\t\tOPT_END()\n+\t};\n+\n+\tmemset(&suite, 0, sizeof(suite));\n+\tsuite.tests.strdup_strings = suite.failed.strdup_strings = 1;\n+\n+\targc = parse_options(argc, argv, NULL, options,\n+\t\t\ttestsuite_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n+\n+\tif (max_jobs <= 0)\n+\t\tmax_jobs = online_cpus();\n+\n+\tdir = opendir(\".\");\n+\tif (!dir)\n+\t\tdie(\"Could not open the current directory\");\n+\twhile ((d = readdir(dir))) {\n+\t\tconst char *p = d->d_name;\n+\n+\t\tif (*p != 't' || !isdigit(p[1]) || !isdigit(p[2]) ||\n+\t\t    !isdigit(p[3]) || !isdigit(p[4]) || p[5] != '-' ||\n+\t\t    !ends_with(p, \".sh\"))\n+\t\t\tcontinue;\n+\n+\t\t/* No pattern: match all */\n+\t\tif (!argc) {\n+\t\t\tstring_list_append(&suite.tests, p);\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tfor (i = 0; i < argc; i++)\n+\t\t\tif (!wildmatch(argv[i], p, 0)) {\n+\t\t\t\tstring_list_append(&suite.tests, p);\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t}\n+\tclosedir(dir);\n+\n+\tif (!suite.tests.nr)\n+\t\tdie(\"No tests match!\");\n+\tif (max_jobs > suite.tests.nr)\n+\t\tmax_jobs = suite.tests.nr;\n+\n+\tfprintf(stderr, \"Running %d tests (%d at a time)\\n\",\n+\t\tsuite.tests.nr, max_jobs);\n+\n+\tret = run_processes_parallel(max_jobs, next_test, test_failed,\n+\t\t\t\t     test_finished, &suite);\n+\n+\tif (suite.failed.nr > 0) {\n+\t\tret = 1;\n+\t\tfprintf(stderr, \"%d tests failed:\\n\\n\", suite.failed.nr);\n+\t\tfor (i = 0; i < suite.failed.nr; i++)\n+\t\t\tfprintf(stderr, \"\\t%s\\n\", suite.failed.items[i].string);\n+\t}\n+\n+\tstring_list_clear(&suite.tests, 0);\n+\tstring_list_clear(&suite.failed, 0);\n+\n+\treturn !!ret;\n+}\n+\n int cmd__run_command(int argc, const char **argv)\n {\n \tstruct child_process proc = CHILD_PROCESS_INIT;\n \tint jobs;\n \n+\tif (argc > 1 && !strcmp(argv[1], \"testsuite\"))\n+\t\texit(testsuite(argc - 1, argv + 1));\n+\n \tif (argc < 3)\n \t\treturn 1;\n \twhile (!strcmp(argv[1], \"env\")) {\n-- \ngitgitgadget\n\n"},{"id":"383434","messageId":"b1ff8eff4d209d2ab44c76da6146bb7ad49d9650.1570201763.git.gitgitgadget@gmail.com","threadId":"51924","inReplyTo":"pull.288.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v3 13/13] ci: also build and test with MS Visual Studio on Azure Pipelines","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-04T15:09:36Z","receivedAt":"2019-10-04T15:09:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\n... because we can, now. Technically, we actually build using `MSBuild`,\nwhich is however pretty close to building interactively in Visual\nStudio.\n\nAs there is no convenient way to run Git's test suite in Visual Studio,\nwe unpack a Portable Git to run it, using the just-added test helper to\nallow running test scripts in parallel.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n Makefile            |   4 ++\n azure-pipelines.yml | 159 ++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 163 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex 3716dadc08..3f6dcec54b 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3025,6 +3025,10 @@ rpm::\n \t@false\n .PHONY: rpm\n \n+ifneq ($(INCLUDE_DLLS_IN_ARTIFACTS),)\n+OTHER_PROGRAMS += $(shell echo *.dll t/helper/*.dll)\n+endif\n+\n artifacts-tar:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) \\\n \t\tGIT-BUILD-OPTIONS $(TEST_PROGRAMS) $(test_bindir_programs) \\\n \t\t$(MOFILES)\ndiff --git a/azure-pipelines.yml b/azure-pipelines.yml\nindex 55ee23ad0f..875e63cac1 100644\n--- a/azure-pipelines.yml\n+++ b/azure-pipelines.yml\n@@ -130,6 +130,165 @@ jobs:\n       PathtoPublish: t/failed-test-artifacts\n       ArtifactName: failed-test-artifacts\n \n+- job: vs_build\n+  displayName: Visual Studio Build\n+  condition: succeeded()\n+  pool: Hosted VS2017\n+  timeoutInMinutes: 240\n+  steps:\n+  - powershell: |\n+      if (\"$GITFILESHAREPWD\" -ne \"\" -and \"$GITFILESHAREPWD\" -ne \"`$`(gitfileshare.pwd)\") {\n+        net use s: \\\\gitfileshare.file.core.windows.net\\test-cache \"$GITFILESHAREPWD\" /user:AZURE\\gitfileshare /persistent:no\n+        cmd /c mklink /d \"$(Build.SourcesDirectory)\\test-cache\" S:\\\n+      }\n+    displayName: 'Mount test-cache'\n+    env:\n+      GITFILESHAREPWD: $(gitfileshare.pwd)\n+  - powershell: |\n+      $urlbase = \"https://dev.azure.com/git-for-windows/git/_apis/build/builds\"\n+      $id = ((Invoke-WebRequest -UseBasicParsing \"${urlbase}?definitions=22&statusFilter=completed&resultFilter=succeeded&`$top=1\").content | ConvertFrom-JSON).value[0].id\n+      $downloadUrl = ((Invoke-WebRequest -UseBasicParsing \"${urlbase}/$id/artifacts\").content | ConvertFrom-JSON).value[1].resource.downloadUrl\n+      (New-Object Net.WebClient).DownloadFile($downloadUrl,\"git-sdk-64-minimal.zip\")\n+      Expand-Archive git-sdk-64-minimal.zip -DestinationPath . -Force\n+      Remove-Item git-sdk-64-minimal.zip\n+\n+      # Let Git ignore the SDK and the test-cache\n+      \"/git-sdk-64-minimal/`n/test-cache/`n\" | Out-File -NoNewLine -Encoding ascii -Append \"$(Build.SourcesDirectory)\\.git\\info\\exclude\"\n+    displayName: 'Download git-sdk-64-minimal'\n+  - powershell: |\n+      & git-sdk-64-minimal\\usr\\bin\\bash.exe -lc @\"\n+        make vcxproj\n+      \"@\n+      if (!$?) { exit(1) }\n+    displayName: Generate Visual Studio Solution\n+    env:\n+      HOME: $(Build.SourcesDirectory)\n+      MSYSTEM: MINGW64\n+      DEVELOPER: 1\n+      NO_PERL: 1\n+      GIT_CONFIG_PARAMETERS: \"'user.name=CI' 'user.email=ci@git'\"\n+  - powershell: |\n+      $urlbase = \"https://dev.azure.com/git/git/_apis/build/builds\"\n+      $id = ((Invoke-WebRequest -UseBasicParsing \"${urlbase}?definitions=9&statusFilter=completed&resultFilter=succeeded&`$top=1\").content | ConvertFrom-JSON).value[0].id\n+      $downloadUrl = ((Invoke-WebRequest -UseBasicParsing \"${urlbase}/$id/artifacts\").content | ConvertFrom-JSON).value[0].resource.downloadUrl\n+      (New-Object Net.WebClient).DownloadFile($downloadUrl, \"compat.zip\")\n+      Expand-Archive compat.zip -DestinationPath . -Force\n+      Remove-Item compat.zip\n+    displayName: 'Download vcpkg artifacts'\n+  - task: MSBuild@1\n+    inputs:\n+      solution: git.sln\n+      platform: x64\n+      configuration: Release\n+      maximumCpuCount: 4\n+  - powershell: |\n+      & compat\\vcbuild\\vcpkg_copy_dlls.bat release\n+      if (!$?) { exit(1) }\n+      & git-sdk-64-minimal\\usr\\bin\\bash.exe -lc @\"\n+        mkdir -p artifacts &&\n+        eval \\\"`$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts | grep ^tar)\\\"\n+      \"@\n+      if (!$?) { exit(1) }\n+    displayName: Bundle artifact tar\n+    env:\n+      HOME: $(Build.SourcesDirectory)\n+      MSYSTEM: MINGW64\n+      DEVELOPER: 1\n+      NO_PERL: 1\n+      MSVC: 1\n+      VCPKG_ROOT: $(Build.SourcesDirectory)\\compat\\vcbuild\\vcpkg\n+  - powershell: |\n+      $tag = (Invoke-WebRequest -UseBasicParsing \"https://gitforwindows.org/latest-tag.txt\").content\n+      $version = (Invoke-WebRequest -UseBasicParsing \"https://gitforwindows.org/latest-version.txt\").content\n+      $url = \"https://github.com/git-for-windows/git/releases/download/${tag}/PortableGit-${version}-64-bit.7z.exe\"\n+      (New-Object Net.WebClient).DownloadFile($url,\"PortableGit.exe\")\n+      & .\\PortableGit.exe -y -oartifacts\\PortableGit\n+      # Wait until it is unpacked\n+      while (-not @(Remove-Item -ErrorAction SilentlyContinue PortableGit.exe; $?)) { sleep 1 }\n+    displayName: Download & extract portable Git\n+  - task: PublishPipelineArtifact@0\n+    displayName: 'Publish Pipeline Artifact: MSVC test artifacts'\n+    inputs:\n+      artifactName: 'vs-artifacts'\n+      targetPath: '$(Build.SourcesDirectory)\\artifacts'\n+  - powershell: |\n+      if (\"$GITFILESHAREPWD\" -ne \"\" -and \"$GITFILESHAREPWD\" -ne \"`$`(gitfileshare.pwd)\") {\n+        cmd /c rmdir \"$(Build.SourcesDirectory)\\test-cache\"\n+      }\n+    displayName: 'Unmount test-cache'\n+    condition: true\n+    env:\n+      GITFILESHAREPWD: $(gitfileshare.pwd)\n+\n+- job: vs_test\n+  displayName: Visual Studio Test\n+  dependsOn: vs_build\n+  condition: succeeded()\n+  pool: Hosted\n+  timeoutInMinutes: 240\n+  strategy:\n+    parallel: 10\n+  steps:\n+  - powershell: |\n+      if (\"$GITFILESHAREPWD\" -ne \"\" -and \"$GITFILESHAREPWD\" -ne \"`$`(gitfileshare.pwd)\") {\n+        net use s: \\\\gitfileshare.file.core.windows.net\\test-cache \"$GITFILESHAREPWD\" /user:AZURE\\gitfileshare /persistent:no\n+        cmd /c mklink /d \"$(Build.SourcesDirectory)\\test-cache\" S:\\\n+      }\n+    displayName: 'Mount test-cache'\n+    env:\n+      GITFILESHAREPWD: $(gitfileshare.pwd)\n+  - task: DownloadPipelineArtifact@0\n+    displayName: 'Download Pipeline Artifact: VS test artifacts'\n+    inputs:\n+      artifactName: 'vs-artifacts'\n+      targetPath: '$(Build.SourcesDirectory)'\n+  - powershell: |\n+      & PortableGit\\git-cmd.exe --command=usr\\bin\\bash.exe -lc @\"\n+        test -f artifacts.tar.gz || {\n+          echo No test artifacts found\\; skipping >&2\n+          exit 0\n+        }\n+        tar xf artifacts.tar.gz || exit 1\n+\n+        # Let Git ignore the SDK and the test-cache\n+        printf '%s\\n' /PortableGit/ /test-cache/ >>.git/info/exclude\n+\n+        cd t &&\n+        PATH=\\\"`$PWD/helper:`$PATH\\\" &&\n+        test-tool.exe run-command testsuite -V -x --write-junit-xml \\\n+                `$(test-tool.exe path-utils slice-tests \\\n+                        `$SYSTEM_JOBPOSITIONINPHASE `$SYSTEM_TOTALJOBSINPHASE t[0-9]*.sh)\n+      \"@\n+      if (!$?) { exit(1) }\n+    displayName: 'Test (parallel)'\n+    env:\n+      HOME: $(Build.SourcesDirectory)\n+      MSYSTEM: MINGW64\n+      NO_SVN_TESTS: 1\n+      GIT_TEST_SKIP_REBASE_P: 1\n+  - powershell: |\n+      if (\"$GITFILESHAREPWD\" -ne \"\" -and \"$GITFILESHAREPWD\" -ne \"`$`(gitfileshare.pwd)\") {\n+        cmd /c rmdir \"$(Build.SourcesDirectory)\\test-cache\"\n+      }\n+    displayName: 'Unmount test-cache'\n+    condition: true\n+    env:\n+      GITFILESHAREPWD: $(gitfileshare.pwd)\n+  - task: PublishTestResults@2\n+    displayName: 'Publish Test Results **/TEST-*.xml'\n+    inputs:\n+      mergeTestResults: true\n+      testRunTitle: 'vs'\n+      platform: Windows\n+      publishRunAttachments: false\n+    condition: succeededOrFailed()\n+  - task: PublishBuildArtifacts@1\n+    displayName: 'Publish trash directories of failed tests'\n+    condition: failed()\n+    inputs:\n+      PathtoPublish: t/failed-test-artifacts\n+      ArtifactName: failed-vs-test-artifacts\n+\n - job: linux_clang\n   displayName: linux-clang\n   condition: succeeded()\n-- \ngitgitgadget\n"},{"id":"383437","messageId":"ae6a64f4-8f46-cbaf-5004-defc316c5157@kdbg.org","threadId":"51924","inReplyTo":"nycvar.QRO.7.76.6.1910041154120.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2 02/13] msvc: avoid using minus operator on unsigned types","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2019-10-04T17:09:43Z","receivedAt":"2019-10-04T17:09:49Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 04.10.19 um 11:55 schrieb Johannes Schindelin:\n> On Fri, 4 Oct 2019, Junio C Hamano wrote:\n>> These three look good and too similar to each other, which makes me\n>> wonder if we want to allow them simply write\n>>\n>> \treturn insert_pos_as_negative_offset(nr);\n>>\n>> with something like\n>>\n>> \tstatic int insert_pos_as_negative_offset(uintmax_t nr)\n>> \t{\n>> \t\tif (INT_MAX < nr)\n>> \t\t\tdie(\"overflow: -1 - %\"PRIuMAX, nr);\n>> \t\treturn -1 - (int)nr;\n>> \t}\n>>\n>> to avoid repetition.\n> \n> I tried not to do that because there are two different data types in\n> play: `unsigned int` and `size_t`. But I guess by making this an\n> `inline` function, compilers can optimize for the common case and avoid\n> casting _twice_.\n> \n> Will be fixed in v2,\n\nIMHO, if you don't accompany insert_pos_as_negative_offset() with a\ncorresponding extract_pos_and_found_condition() and use it everywhere,\nit is more obfuscating than necessary.\n\nThe *real* problem to solve is to ensure that the index/cache does not\ngrow so large that 32-bit indexes would be needed. Then the calculation\nthat you want to hide here cannot overflow.\n\n-- Hannes\n"},{"id":"383442","messageId":"nycvar.QRO.7.76.6.1910042319190.46@tvgsbejvaqbjf.bet","threadId":"51924","inReplyTo":"ae6a64f4-8f46-cbaf-5004-defc316c5157@kdbg.org","subject":"Re: [PATCH v2 02/13] msvc: avoid using minus operator on unsigned types","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-04T21:24:26Z","receivedAt":"2019-10-04T21:24:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Hannes,\n\nOn Fri, 4 Oct 2019, Johannes Sixt wrote:\n\n> Am 04.10.19 um 11:55 schrieb Johannes Schindelin:\n> > On Fri, 4 Oct 2019, Junio C Hamano wrote:\n> >> These three look good and too similar to each other, which makes me\n> >> wonder if we want to allow them simply write\n> >>\n> >> \treturn insert_pos_as_negative_offset(nr);\n> >>\n> >> with something like\n> >>\n> >> \tstatic int insert_pos_as_negative_offset(uintmax_t nr)\n> >> \t{\n> >> \t\tif (INT_MAX < nr)\n> >> \t\t\tdie(\"overflow: -1 - %\"PRIuMAX, nr);\n> >> \t\treturn -1 - (int)nr;\n> >> \t}\n> >>\n> >> to avoid repetition.\n> >\n> > I tried not to do that because there are two different data types in\n> > play: `unsigned int` and `size_t`. But I guess by making this an\n> > `inline` function, compilers can optimize for the common case and avoid\n> > casting _twice_.\n> >\n> > Will be fixed in v2,\n>\n> IMHO, if you don't accompany insert_pos_as_negative_offset() with a\n> corresponding extract_pos_and_found_condition() and use it everywhere,\n> it is more obfuscating than necessary.\n\nI do disagree here. No overflow checking needs to be performed for `-1 -\n<int-value>`. And that's what the opposite of this function really boils\ndown to.\n\n> The *real* problem to solve is to ensure that the index/cache does not\n> grow so large that 32-bit indexes would be needed. Then the calculation\n> that you want to hide here cannot overflow.\n\nWell, that may be the real problem of another patch series. This patch\nseries' problem is to add a job to our Azure Pipeline that builds Git\nwith Visual Studio, and it patches the code minimally so that it builds\neven in `DEVELOPER=1` mode, for good measure.\n\nSo I'd like not to dilute the purpose of this patch series.\n\nThanks,\nDscho\n"},{"id":"383503","messageId":"xmqqr23q68rg.fsf@gitster-ct.c.googlers.com","threadId":"51924","inReplyTo":"nycvar.QRO.7.76.6.1910042319190.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2 02/13] msvc: avoid using minus operator on unsigned types","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-06T00:02:43Z","receivedAt":"2019-10-06T00:02:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> IMHO, if you don't accompany insert_pos_as_negative_offset() with a\n>> corresponding extract_pos_and_found_condition() and use it everywhere,\n>> it is more obfuscating than necessary.\n>\n> I do disagree here. No overflow checking needs to be performed for `-1 -\n> <int-value>`. And that's what the opposite of this function really boils\n> down to.\n\nI do not think j6t is referring to the over/underflow issues at all.\n\nThe suggestion is that, because insert-pos-as-negative-offset\nabstracts away (in addition to the overflow checks) the fact that\n\"does not exist but here is the location it would be inserted\" is\nencoded in a certain way (i.e. not just the array index negated, but\nalso is offset by -1, because we wouldn't be able to say \"at the\nvery beginning at index 0\" without the -1 offset), the side that\nconsumes the encoded \"pos\" (i.e. \"we got a negative, so we know the\nelement does not exist, and the index into the array we would insert\na new element is computed this way\") should be abstracted away, as\nit must know that the extra negative offset used when encoding is\n\"-1\".\n\nI think that is a reasonable thing to consider; it is not necessary\nfor correctness, but contributes to the conceptual clarity (iow, it\ncan be left as a separate clean-up step done after the series is\ndone).\n"},{"id":"383505","messageId":"xmqqimp26808.fsf@gitster-ct.c.googlers.com","threadId":"51924","inReplyTo":"pull.288.v3.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-06T00:19:03Z","receivedAt":"2019-10-06T00:19:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> Changes since v2:\n>\n>  * The overflow check introduced in v1 was consolidated into a single\n>    helper.\n\nLooks good to me.\n\n> Range-diff vs v2:\n>\n>   1:  4d0b38125a =  1:  4d0b38125a push: do not pretend to return `int` from `die_push_simple()`\n>   2:  8800320590 <  -:  ---------- msvc: avoid using minus operator on unsigned types\n>   -:  ---------- >  2:  7fe2a85506 msvc: avoid using minus operator on unsigned types\n>   3:  8512a3e96d =  3:  e632a4eef4 winansi: use FLEX_ARRAY to avoid compiler warning\n\nThis is less useful than it could be.  \n\nWith a larger creation-factor (and we can afford using a larger one,\nsimply because the user of GGG _knows_ that the two series being\ncompared are closely related), what is output is entirely readable\n(attached at the end).\n\nOh, while I am suggesting possible improvements on GGG, can we\nplease tweak the sender date like git-send-email does so that two\nmessages in the same series do not share the same timestamp?  When\nmulti-patch series are displayed in MUA or public-inbox News feed\nout of order, it almost always is from GGG that gave the same\ntimestamp to adjacent messages in a series, and it prevents me from\napplying them in one go (or saving in one action to a mbox).\n\nWhat send-email does is, at the beginning for N patch series, to\ntake the current wallclock time and subtract N seconds from it, and\nthen give that timestamp to the first message it sends out, and\nafter that, it increments the timestamp by 1 seconds.\n\nNote that there is no need for any \"sleep\"---the timestamps are\ngiven by explicitly generating the \"Date: \" header.  The last time\nwe looked into this issue, I think the code was trying to do almost\nthe right thing but it was giving a malformatted timezone and forcing\nthe sending MTA to override it with the wallclock time or something.\n\nThanks.\n\n1:  9629f3c751 ! 1:  c097b95a26 msvc: avoid using minus operator on unsigned types\n    @@ Commit message\n         Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n         Signed-off-by: Junio C Hamano <gitster@pobox.com>\n     \n    + ## cache.h ##\n    +@@ cache.h: struct cache_entry *index_file_exists(struct index_state *istate, const char *na\n    +  */\n    + int index_name_pos(const struct index_state *, const char *name, int namelen);\n    + \n    ++/*\n    ++ * Some functions return the negative complement of an insert position when a\n    ++ * precise match was not found but a position was found where the entry would\n    ++ * need to be inserted. This helper protects that logic from any integer\n    ++ * underflow.\n    ++ */\n    ++static inline int index_pos_to_insert_pos(uintmax_t pos)\n    ++{\n    ++\tif (pos > INT_MAX)\n    ++\t\tdie(\"overflow: -1 - %\"PRIuMAX, pos);\n    ++\treturn -1 - (int)pos;\n    ++}\n    ++\n    + #define ADD_CACHE_OK_TO_ADD 1\t\t/* Ok to add */\n    + #define ADD_CACHE_OK_TO_REPLACE 2\t/* Ok to replace file/directory */\n    + #define ADD_CACHE_SKIP_DFCHECK 4\t/* Ok to skip DF conflict checks */\n    +\n      ## read-cache.c ##\n     @@ read-cache.c: static int add_index_entry_with_check(struct index_state *istate, struct cache_e\n    - \t * we can avoid searching for it.\n      \t */\n      \tif (istate->cache_nr > 0 &&\n    --\t\tstrcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)\n    + \t\tstrcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)\n     -\t\tpos = -istate->cache_nr - 1;\n    -+\t\tstrcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0) {\n    -+\t\tif (istate->cache_nr > INT_MAX)\n    -+\t\t\tdie(\"overflow: -1 - %u\", istate->cache_nr);\n    -+\t\tpos = -1 - (int)istate->cache_nr;\n    -+\t}\n    ++\t\tpos = index_pos_to_insert_pos(istate->cache_nr);\n      \telse\n      \t\tpos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce));\n      \n    @@ read-cache.c: static size_t estimate_cache_size(size_t ondisk_size, unsigned int\n     \n      ## sha1-lookup.c ##\n     @@ sha1-lookup.c: int sha1_pos(const unsigned char *sha1, void *table, size_t nr,\n    - \t\t\tmiv = take2(sha1 + ofs);\n      \t\t\tif (miv < lov)\n      \t\t\t\treturn -1;\n    --\t\t\tif (hiv < miv)\n    + \t\t\tif (hiv < miv)\n     -\t\t\t\treturn -1 - nr;\n    -+\t\t\tif (hiv < miv) {\n    -+\t\t\t\tif (nr > INT_MAX)\n    -+\t\t\t\t\tdie(\"overflow: -1 - %\"PRIuMAX,\n    -+\t\t\t\t\t    (uintmax_t)nr);\n    -+\t\t\t\treturn -1 - (int)nr;\n    -+\t\t\t}\n    ++\t\t\t\treturn index_pos_to_insert_pos(nr);\n      \t\t\tif (lov != hiv) {\n      \t\t\t\t/*\n      \t\t\t\t * At this point miv could be equal\n    @@ sha1-lookup.c: int sha1_pos(const unsigned char *sha1, void *table, size_t nr,\n      \t\tmi = lo + (hi - lo) / 2;\n      \t} while (lo < hi);\n     -\treturn -lo-1;\n    -+\tif (nr > INT_MAX)\n    -+\t\tdie(\"overflow: -1 - %\"PRIuMAX, (uintmax_t)lo);\n    -+\treturn -1 - (int)lo;\n    ++\treturn index_pos_to_insert_pos(lo);\n      }\n      \n      int bsearch_hash(const unsigned char *sha1, const uint32_t *fanout_nbo,\n\n"},{"id":"383516","messageId":"nycvar.QRO.7.76.6.1910061157320.46@tvgsbejvaqbjf.bet","threadId":"51924","inReplyTo":"xmqqimp26808.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-06T10:45:26Z","receivedAt":"2019-10-06T10:45:51Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Sun, 6 Oct 2019, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\n> writes:\n>\n> > Range-diff vs v2:\n> >\n> >   1:  4d0b38125a =  1:  4d0b38125a push: do not pretend to return `int` from `die_push_simple()`\n> >   2:  8800320590 <  -:  ---------- msvc: avoid using minus operator on unsigned types\n> >   -:  ---------- >  2:  7fe2a85506 msvc: avoid using minus operator on unsigned types\n> >   3:  8512a3e96d =  3:  e632a4eef4 winansi: use FLEX_ARRAY to avoid compiler warning\n>\n> This is less useful than it could be.\n>\n> With a larger creation-factor (and we can afford using a larger one,\n> simply because the user of GGG _knows_ that the two series being\n> compared are closely related), what is output is entirely readable\n> (attached at the end).\n\nI just implemented this here:\nhttps://github.com/gitgitgadget/gitgitgadget/pull/128 (it still needs to\nbe reviewed and merged before it takes effect).\n\n> Oh, while I am suggesting possible improvements on GGG, can we\n> please tweak the sender date like git-send-email does so that two\n> messages in the same series do not share the same timestamp?  When\n> multi-patch series are displayed in MUA or public-inbox News feed\n> out of order, it almost always is from GGG that gave the same\n> timestamp to adjacent messages in a series, and it prevents me from\n> applying them in one go (or saving in one action to a mbox).\n>\n> What send-email does is, at the beginning for N patch series, to\n> take the current wallclock time and subtract N seconds from it, and\n> then give that timestamp to the first message it sends out, and\n> after that, it increments the timestamp by 1 seconds.\n>\n> Note that there is no need for any \"sleep\"---the timestamps are\n> given by explicitly generating the \"Date: \" header.  The last time\n> we looked into this issue, I think the code was trying to do almost\n> the right thing but it was giving a malformatted timezone and forcing\n> the sending MTA to override it with the wallclock time or something.\n\nYou mentioned this before, and I implemented it. But GMail ignores the\n`Date:` header sent by GitGitGadget, and I don't know why. See e.g.\nhttps://public-inbox.org/git/4d0b38125a13d85963be5e524becf48271893e2b.1570201763.git.gitgitgadget@gmail.com/raw\n\n\t[...]\n\tDate:   Fri, 04 Oct 2019 08:09:25 -0700 (PDT)\n\t[...]\n\tX-Google-Original-Date: Fri, 04 Oct 2019 15:09:10 GMT\n\t[...]\n\nI am fairly certain that the latter is the actual `Date:` line sent to\nGMail, and GMail just decides that it will not respect it.\n\nOnce https://github.com/gitgitgadget/gitgitgadget/pull/125 makes it into\nGitGitGadget (adding the `/preview` command that allows to send patch\nseries to the PR owner as a test), it should be easier to start\ninvestigating further.\n\nUnless anybody here knows why GMail rejects the header. Maybe it is the\n`GMT`?\n\nCiao,\nDscho\n"},{"id":"383517","messageId":"ff6896de-ba12-6669-d6e0-980ed0fcd1e4@kdbg.org","threadId":"51924","inReplyTo":"xmqqr23q68rg.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 02/13] msvc: avoid using minus operator on unsigned types","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2019-10-06T10:53:26Z","receivedAt":"2019-10-06T10:53:32Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 06.10.19 um 02:02 schrieb Junio C Hamano:\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n>>> IMHO, if you don't accompany insert_pos_as_negative_offset() with a\n>>> corresponding extract_pos_and_found_condition() and use it everywhere,\n>>> it is more obfuscating than necessary.\n>>\n>> I do disagree here. No overflow checking needs to be performed for `-1 -\n>> <int-value>`. And that's what the opposite of this function really boils\n>> down to.\n> \n> I do not think j6t is referring to the over/underflow issues at all.\n> \n> The suggestion is that, because insert-pos-as-negative-offset\n> abstracts away [...] the fact that\n> \"does not exist but here is the location it would be inserted\" is\n> encoded in a certain way [...], the side that\n> consumes the encoded \"pos\" [...] should be abstracted away [...].\n\nThank you, that summarizes my thoughts very well.\n\n> I think that is a reasonable thing to consider; it is not necessary\n> for correctness, but contributes to the conceptual clarity (iow, it\n> can be left as a separate clean-up step done after the series is\n> done).\n\nThanks, but I disagree with the course of action: I think we should do\nboth sides at the same time. (And if we aren't ready to do the decoding\nside now, then we should delay the encoding side).\n\nConsider someone is looking at the call site without knowing the\ndetailed meaning of the return value (\"What the heck is this -1\nbusiness?\"), it is a matter to look at the function being called to know\nwhat it is. With another function that does the encoding, it is one\nadditional hop. That is my reason for saying \"more obfuscating than\nnecessary\".\n\nThat the helper function would reduce some small code duplication does\nnot outweigh the obfuscation in my book. That's just MHO, of course.\n\n-- Hannes\n"},{"id":"383526","messageId":"nycvar.QRO.7.76.6.1910062237440.46@tvgsbejvaqbjf.bet","threadId":"51924","inReplyTo":"nycvar.QRO.7.76.6.1910061157320.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-06T20:38:26Z","receivedAt":"2019-10-06T20:38:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Sun, 6 Oct 2019, Johannes Schindelin wrote:\n\n> On Sun, 6 Oct 2019, Junio C Hamano wrote:\n>\n> > \"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\n> > writes:\n> >\n> > > Range-diff vs v2:\n> > >\n> > >   1:  4d0b38125a =  1:  4d0b38125a push: do not pretend to return `int` from `die_push_simple()`\n> > >   2:  8800320590 <  -:  ---------- msvc: avoid using minus operator on unsigned types\n> > >   -:  ---------- >  2:  7fe2a85506 msvc: avoid using minus operator on unsigned types\n> > >   3:  8512a3e96d =  3:  e632a4eef4 winansi: use FLEX_ARRAY to avoid compiler warning\n> >\n> > This is less useful than it could be.\n> >\n> > With a larger creation-factor (and we can afford using a larger one,\n> > simply because the user of GGG _knows_ that the two series being\n> > compared are closely related), what is output is entirely readable\n> > (attached at the end).\n>\n> I just implemented this here:\n> https://github.com/gitgitgadget/gitgitgadget/pull/128 (it still needs to\n> be reviewed and merged before it takes effect).\n\nFWIW this is now merged.\n\nCiao,\nDscho\n"},{"id":"383551","messageId":"xmqq5zl15q1c.fsf@gitster-ct.c.googlers.com","threadId":"51924","inReplyTo":"nycvar.QRO.7.76.6.1910061157320.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-07T00:59:27Z","receivedAt":"2019-10-07T00:59:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> \tDate:   Fri, 04 Oct 2019 08:09:25 -0700 (PDT)\n> \t[...]\n> \tX-Google-Original-Date: Fri, 04 Oct 2019 15:09:10 GMT\n> \t[...]\n>\n> I am fairly certain that the latter is the actual `Date:` line sent to\n> GMail, and GMail just decides that it will not respect it.\n\nIf the submitting program said \"Fri, 04 Oct 2019 15:09:10 +0000\n(GMT)\" instead of \"Fri, 04 Oct 2019 15:09:10 GMT\", that would match\nthe format the MTA produced itself, I guess.  I am kind-of surprised\nif the problem is the use of the obs-zone format (RFC 2822 page 31),\nbut anything is possible with GMail X-<.\n\nHow does send-email write that date header?  Matching that would be\nprobably the most appropriate, if possible, given that GGG was\nwritten for send-email refugees, I guess ;-)\n\nHere is what its format_2822_time sub does, so +0000 without any\ntextual zone name, it is.\n\n\treturn sprintf(\"%s, %2d %s %d %02d:%02d:%02d %s%02d%02d\",\n\t\t       qw(Sun Mon Tue Wed Thu Fri Sat)[$localtm[6]],\n\t\t       $localtm[3],\n\t\t       qw(Jan Feb Mar Apr May Jun\n\t\t\t  Jul Aug Sep Oct Nov Dec)[$localtm[4]],\n\t\t       $localtm[5]+1900,\n\t\t       $localtm[2],\n\t\t       $localtm[1],\n\t\t       $localtm[0],\n\t\t       ($offset >= 0) ? '+' : '-',\n\t\t       abs($offhour),\n\t\t       $offmin,\n\t\t       );\n\n\n"},{"id":"383552","messageId":"xmqq1rvp5pc0.fsf@gitster-ct.c.googlers.com","threadId":"51924","inReplyTo":"nycvar.QRO.7.76.6.1910062237440.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-07T01:14:39Z","receivedAt":"2019-10-07T01:14:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> I just implemented this here:\n>> https://github.com/gitgitgadget/gitgitgadget/pull/128 (it still needs to\n>> be reviewed and merged before it takes effect).\n>\n> FWIW this is now merged.\n\nNice.\n\nI didn't quite understand this part, though.\n\n    The default creation factor is 60 (roughly speaking, it wants 60% of\n    the lines to match between two patches, otherwise it considers the\n    patches to be unrelated).\n\nWould the updated creation factor used which is 95 (roughly\nspeaking) want 95% of the lines to match between two patches?\n\nThat would make the matching logic even pickier and reject more\nparing, so I must be reading the statement wrong X-<.\n"},{"id":"383581","messageId":"20191007160816.GB74671@cat","threadId":"51924","inReplyTo":"xmqq5zl15q1c.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2019-10-07T16:08:16Z","receivedAt":"2019-10-07T16:08:21Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 10/07, Junio C Hamano wrote:\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > \tDate:   Fri, 04 Oct 2019 08:09:25 -0700 (PDT)\n> > \t[...]\n> > \tX-Google-Original-Date: Fri, 04 Oct 2019 15:09:10 GMT\n> > \t[...]\n> >\n> > I am fairly certain that the latter is the actual `Date:` line sent to\n> > GMail, and GMail just decides that it will not respect it.\n> \n> If the submitting program said \"Fri, 04 Oct 2019 15:09:10 +0000\n> (GMT)\" instead of \"Fri, 04 Oct 2019 15:09:10 GMT\", that would match\n> the format the MTA produced itself, I guess.  I am kind-of surprised\n> if the problem is the use of the obs-zone format (RFC 2822 page 31),\n> but anything is possible with GMail X-<.\n\nYeah, the obs-zone format did seem to be the problem.  I just dug up\nthe previous thread we had about this, where I confirmed that +0000 as\nthe timezone worked just fine in my setup through GMail [*1*].  Note\nsure if the (GMT) would cause any problems, but I'd agree with\navoiding it as you mention below to make sure GMail doesn't do\nanything funny with it.\n\n*1*: https://public-inbox.org/git/20190318214842.GA32487@hank.intra.tgummerer.com/\n\n> How does send-email write that date header?  Matching that would be\n> probably the most appropriate, if possible, given that GGG was\n> written for send-email refugees, I guess ;-)\n> \n> Here is what its format_2822_time sub does, so +0000 without any\n> textual zone name, it is.\n> \n> \treturn sprintf(\"%s, %2d %s %d %02d:%02d:%02d %s%02d%02d\",\n> \t\t       qw(Sun Mon Tue Wed Thu Fri Sat)[$localtm[6]],\n> \t\t       $localtm[3],\n> \t\t       qw(Jan Feb Mar Apr May Jun\n> \t\t\t  Jul Aug Sep Oct Nov Dec)[$localtm[4]],\n> \t\t       $localtm[5]+1900,\n> \t\t       $localtm[2],\n> \t\t       $localtm[1],\n> \t\t       $localtm[0],\n> \t\t       ($offset >= 0) ? '+' : '-',\n> \t\t       abs($offhour),\n> \t\t       $offmin,\n> \t\t       );\n> \n> \n"},{"id":"383604","messageId":"d1f53812-ebaf-4290-f867-6d34735d314d@gmail.com","threadId":"51924","inReplyTo":"e632a4eef46720d43fd014644ff24d3dac61ebe9.1570201763.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 03/13] winansi: use FLEX_ARRAY to avoid compiler warning","fromName":"Alban Gruin","fromEmail":"alban.gruin@gmail.com","sentAt":"2019-10-07T19:16:55Z","receivedAt":"2019-10-07T19:17:00Z","isPatch":true,"sender":{"key":"alban.gruin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6310153?v=4"},"body":"Hi Johannes,\n\nLe 04/10/2019 à 17:09, Johannes Schindelin via GitGitGadget a écrit :\n> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n> \n> MSVC would complain thusly:\n> \n>     C4200: nonstandard extension used: zero-sized array in struct/union\n> \n> Let's just use the `FLEX_ARRAY` constant that we introduced for exactly\n> this type of scenario.\n\nPerhaps this is a good candidate for a semantic patch?\n\nCheers,\nAlban\n\n"},{"id":"383629","messageId":"nycvar.QRO.7.76.6.1910072350300.46@tvgsbejvaqbjf.bet","threadId":"51924","inReplyTo":"xmqq1rvp5pc0.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-07T21:51:29Z","receivedAt":"2019-10-07T21:51:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 7 Oct 2019, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n>\n> >> I just implemented this here:\n> >> https://github.com/gitgitgadget/gitgitgadget/pull/128 (it still needs to\n> >> be reviewed and merged before it takes effect).\n> >\n> > FWIW this is now merged.\n>\n> Nice.\n>\n> I didn't quite understand this part, though.\n>\n>     The default creation factor is 60 (roughly speaking, it wants 60% of\n>     the lines to match between two patches, otherwise it considers the\n>     patches to be unrelated).\n>\n> Would the updated creation factor used which is 95 (roughly\n> speaking) want 95% of the lines to match between two patches?\n>\n> That would make the matching logic even pickier and reject more\n> paring, so I must be reading the statement wrong X-<.\n\nNo, I must have written the opposite of what I tried to say, is all.\nSorry about the confusion.\n\nCiao,\nDscho\n"},{"id":"383641","messageId":"xmqqzhicnfmr.fsf@gitster-ct.c.googlers.com","threadId":"51924","inReplyTo":"nycvar.QRO.7.76.6.1910072350300.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-08T02:19:08Z","receivedAt":"2019-10-08T02:19:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> I didn't quite understand this part, though.\n>>\n>>     The default creation factor is 60 (roughly speaking, it wants 60% of\n>>     the lines to match between two patches, otherwise it considers the\n>>     patches to be unrelated).\n>>\n>> Would the updated creation factor used which is 95 (roughly\n>> speaking) want 95% of the lines to match between two patches?\n>>\n>> That would make the matching logic even pickier and reject more\n>> paring, so I must be reading the statement wrong X-<.\n>\n> No, I must have written the opposite of what I tried to say, is all.\n\nSo, cfactor of 60 means at most 60% is allowed to differ and the\ntwo patches are still considered to be related, while 95 means only\n5% needs to be common?  That would make more sense to me.\n\nThanks.\n\n"},{"id":"383674","messageId":"nycvar.QRO.7.76.6.1910081404310.46@tvgsbejvaqbjf.bet","threadId":"51924","inReplyTo":"ff6896de-ba12-6669-d6e0-980ed0fcd1e4@kdbg.org","subject":"Re: [PATCH v2 02/13] msvc: avoid using minus operator on unsigned types","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-08T12:04:56Z","receivedAt":"2019-10-08T12:05:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Hannes,\n\nOn Sun, 6 Oct 2019, Johannes Sixt wrote:\n\n> Am 06.10.19 um 02:02 schrieb Junio C Hamano:\n> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> >\n> >>> IMHO, if you don't accompany insert_pos_as_negative_offset() with a\n> >>> corresponding extract_pos_and_found_condition() and use it everywhere,\n> >>> it is more obfuscating than necessary.\n> >>\n> >> I do disagree here. No overflow checking needs to be performed for `-1 -\n> >> <int-value>`. And that's what the opposite of this function really boils\n> >> down to.\n> >\n> > I do not think j6t is referring to the over/underflow issues at all.\n> >\n> > The suggestion is that, because insert-pos-as-negative-offset\n> > abstracts away [...] the fact that\n> > \"does not exist but here is the location it would be inserted\" is\n> > encoded in a certain way [...], the side that\n> > consumes the encoded \"pos\" [...] should be abstracted away [...].\n>\n> Thank you, that summarizes my thoughts very well.\n>\n> > I think that is a reasonable thing to consider; it is not necessary\n> > for correctness, but contributes to the conceptual clarity (iow, it\n> > can be left as a separate clean-up step done after the series is\n> > done).\n>\n> Thanks, but I disagree with the course of action: I think we should do\n> both sides at the same time. (And if we aren't ready to do the decoding\n> side now, then we should delay the encoding side).\n>\n> Consider someone is looking at the call site without knowing the\n> detailed meaning of the return value (\"What the heck is this -1\n> business?\"), it is a matter to look at the function being called to know\n> what it is. With another function that does the encoding, it is one\n> additional hop. That is my reason for saying \"more obfuscating than\n> necessary\".\n>\n> That the helper function would reduce some small code duplication does\n> not outweigh the obfuscation in my book. That's just MHO, of course.\n\nSo you got what you wished for:\nhttps://public-inbox.org/git/pull.378.git.gitgitgadget@gmail.com\n\nCare to review?\n\nThanks,\nDscho\n"},{"id":"383682","messageId":"nycvar.QRO.7.76.6.1910081423250.46@tvgsbejvaqbjf.bet","threadId":"51924","inReplyTo":"xmqqzhicnfmr.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-08T12:46:34Z","receivedAt":"2019-10-08T12:46:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Tue, 8 Oct 2019, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n>\n> >> I didn't quite understand this part, though.\n> >>\n> >>     The default creation factor is 60 (roughly speaking, it wants 60% of\n> >>     the lines to match between two patches, otherwise it considers the\n> >>     patches to be unrelated).\n> >>\n> >> Would the updated creation factor used which is 95 (roughly\n> >> speaking) want 95% of the lines to match between two patches?\n> >>\n> >> That would make the matching logic even pickier and reject more\n> >> paring, so I must be reading the statement wrong X-<.\n> >\n> > No, I must have written the opposite of what I tried to say, is all.\n>\n> So, cfactor of 60 means at most 60% is allowed to differ and the\n> two patches are still considered to be related, while 95 means only\n> 5% needs to be common?  That would make more sense to me.\n\nOkay, I not only wrote the opposite of what I wanted to say, I also\nmisremembered.\n\nWhen `range-diff` tries to determine matching pairs of patches, it\nbuilds an `(m+n)x(m+n)` cost matrix, where `m` is the number of patches\nin the first commit range and `n` is the number of patches in the second\none.\n\nWhy not `m x n`? Well, that's the obvious matrix, and that's what it\nstarts with, essentially assigning the number of lines of the diff\nbetween the diffs as \"cost\".\n\nBut then `git range-diff` extends the cost matrix to allow for _all_ of\nthe `m` patches to be considered deleted, and _all_ of the `n` patches\nto be added. As cost, it cannot use a \"diff of diffs\" because there is\nno second diff. So it uses the number of lines of the one diff it has,\nmultiplied by the creation factor interpreted as a percentage.\n\nThe naive creation factor would be 100%, which is (almost) as if we\nassumed an empty diff for the missing diff. But that would make the\nrange-diff too eager to dismiss rewrites, as experience obviously showed\n(not my experience, but Thomas Rast's, who came up with `tbdiff` after\nall): the diff of diffs includes a diff header, for example.\n\nThe interpretation I offered (although I inverted what I wanted to say)\nis similar in spirit to that metric (which is not actually a metric, I\nbelieve, because I expect it to violate the triangle inequality) is\nobviously inaccurate: the number of lines of the diff of diffs does not\nsay anything about the number of matching lines, quite to the contrary,\nit correlates somewhat to the number of non-matching lines.\n\nSo a better interpretation would have been:\n\n\tThe default creation factor is 60 (roughly speaking, it wants at\n\tmost 60% of the diffs' lines to differ, otherwise it considers\n\tthem not to be a match.\n\nThis is still inaccurate, but at least it gets the idea of the\nrange-diff across.\n\nOf course, I will never be able to amend the commit message in\nGitGitGadget anyway, as I have merged that PR already.\n\nCiao,\nDscho\n"},{"id":"383711","messageId":"584bdc31-f4d7-8bf5-3233-a2695b706e21@kdbg.org","threadId":"51924","inReplyTo":"nycvar.QRO.7.76.6.1910081404310.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2 02/13] msvc: avoid using minus operator on unsigned types","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2019-10-08T21:13:05Z","receivedAt":"2019-10-08T21:13:09Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 08.10.19 um 14:04 schrieb Johannes Schindelin:\n> So you got what you wished for:\n> https://public-inbox.org/git/pull.378.git.gitgitgadget@gmail.com\n\nAfter having seen the result I do not wish for it anymore. (Not that I\nhad \"wished\" for it in the first place...) It does not make the result\nany more readable than the original.\n\nI do wish you had rejected Junio's suggestion to introduce\nindex_pos_to_insert_pos(). It doesn't make the code a lot more readable,\neither, in my eyes.\n\n-- Hannes\n"},{"id":"383751","messageId":"9ccbdb9a-845f-a534-29b6-52cfe9eb3229@iee.email","threadId":"51924","inReplyTo":"nycvar.QRO.7.76.6.1910081423250.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2019-10-09T13:57:22Z","receivedAt":"2019-10-09T13:57:29Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Hi Dscho,\n\nOn 08/10/2019 13:46, Johannes Schindelin wrote:\n> Hi Junio,\n>\n> On Tue, 8 Oct 2019, Junio C Hamano wrote:\n>\n>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n>>\n>>>> I didn't quite understand this part, though.\n>>>>\n>>>>      The default creation factor is 60 (roughly speaking, it wants 60% of\n>>>>      the lines to match between two patches, otherwise it considers the\n>>>>      patches to be unrelated).\n>>>>\n>>>> Would the updated creation factor used which is 95 (roughly\n>>>> speaking) want 95% of the lines to match between two patches?\n>>>>\n>>>> That would make the matching logic even pickier and reject more\n>>>> paring, so I must be reading the statement wrong X-<.\n>>> No, I must have written the opposite of what I tried to say, is all.\n>> So, cfactor of 60 means at most 60% is allowed to differ and the\n>> two patches are still considered to be related, while 95 means only\n>> 5% needs to be common?  That would make more sense to me.\n> Okay, I not only wrote the opposite of what I wanted to say, I also\n> misremembered.\n>\n> When `range-diff` tries to determine matching pairs of patches, it\n> builds an `(m+n)x(m+n)` cost matrix, where `m` is the number of patches\n> in the first commit range and `n` is the number of patches in the second\n> one.\n>\n> Why not `m x n`? Well, that's the obvious matrix, and that's what it\n> starts with, essentially assigning the number of lines of the diff\n> between the diffs as \"cost\".\n>\n> But then `git range-diff` extends the cost matrix to allow for _all_ of\n> the `m` patches to be considered deleted, and _all_ of the `n` patches\n> to be added. As cost, it cannot use a \"diff of diffs\" because there is\n> no second diff. So it uses the number of lines of the one diff it has,\n> multiplied by the creation factor interpreted as a percentage.\n>\n> The naive creation factor would be 100%, which is (almost) as if we\n> assumed an empty diff for the missing diff. But that would make the\n> range-diff too eager to dismiss rewrites, as experience obviously showed\n> (not my experience, but Thomas Rast's, who came up with `tbdiff` after\n> all): the diff of diffs includes a diff header, for example.\n>\n> The interpretation I offered (although I inverted what I wanted to say)\n> is similar in spirit to that metric (which is not actually a metric, I\n> believe, because I expect it to violate the triangle inequality) is\n> obviously inaccurate: the number of lines of the diff of diffs does not\n> say anything about the number of matching lines, quite to the contrary,\n> it correlates somewhat to the number of non-matching lines.\n>\n> So a better interpretation would have been:\n>\n> \tThe default creation factor is 60 (roughly speaking, it wants at\n> \tmost 60% of the diffs' lines to differ, otherwise it considers\n> \tthem not to be a match.\n>\n> This is still inaccurate, but at least it gets the idea of the\n> range-diff across.\n>\n> Of course, I will never be able to amend the commit message in\n> GitGitGadget anyway, as I have merged that PR already.\n>\n> Ciao,\n> Dscho\nMedium term, is this something that could go in the algorithms section \nof the range-diff man page, especially if the upstream commit message is \nalready in place.\n\n#leftoverdocs ?\n\nPhilip\n"},{"id":"383783","messageId":"nycvar.QRO.7.76.6.1910101102280.46@tvgsbejvaqbjf.bet","threadId":"51924","inReplyTo":"9ccbdb9a-845f-a534-29b6-52cfe9eb3229@iee.email","subject":"Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-10T09:03:08Z","receivedAt":"2019-10-10T09:03:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Philip,\n\nOn Wed, 9 Oct 2019, Philip Oakley wrote:\n\n> On 08/10/2019 13:46, Johannes Schindelin wrote:\n> > Hi Junio,\n> >\n> > On Tue, 8 Oct 2019, Junio C Hamano wrote:\n> >\n> > > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> > >\n> > > > > I didn't quite understand this part, though.\n> > > > >\n> > > > >      The default creation factor is 60 (roughly speaking, it wants 60%\n> > > > >      of\n> > > > >      the lines to match between two patches, otherwise it considers\n> > > > >      the\n> > > > >      patches to be unrelated).\n> > > > >\n> > > > > Would the updated creation factor used which is 95 (roughly\n> > > > > speaking) want 95% of the lines to match between two patches?\n> > > > >\n> > > > > That would make the matching logic even pickier and reject more\n> > > > > paring, so I must be reading the statement wrong X-<.\n> > > > No, I must have written the opposite of what I tried to say, is all.\n> > > So, cfactor of 60 means at most 60% is allowed to differ and the\n> > > two patches are still considered to be related, while 95 means only\n> > > 5% needs to be common?  That would make more sense to me.\n> > Okay, I not only wrote the opposite of what I wanted to say, I also\n> > misremembered.\n> >\n> > When `range-diff` tries to determine matching pairs of patches, it\n> > builds an `(m+n)x(m+n)` cost matrix, where `m` is the number of patches\n> > in the first commit range and `n` is the number of patches in the second\n> > one.\n> >\n> > Why not `m x n`? Well, that's the obvious matrix, and that's what it\n> > starts with, essentially assigning the number of lines of the diff\n> > between the diffs as \"cost\".\n> >\n> > But then `git range-diff` extends the cost matrix to allow for _all_ of\n> > the `m` patches to be considered deleted, and _all_ of the `n` patches\n> > to be added. As cost, it cannot use a \"diff of diffs\" because there is\n> > no second diff. So it uses the number of lines of the one diff it has,\n> > multiplied by the creation factor interpreted as a percentage.\n> >\n> > The naive creation factor would be 100%, which is (almost) as if we\n> > assumed an empty diff for the missing diff. But that would make the\n> > range-diff too eager to dismiss rewrites, as experience obviously showed\n> > (not my experience, but Thomas Rast's, who came up with `tbdiff` after\n> > all): the diff of diffs includes a diff header, for example.\n> >\n> > The interpretation I offered (although I inverted what I wanted to say)\n> > is similar in spirit to that metric (which is not actually a metric, I\n> > believe, because I expect it to violate the triangle inequality) is\n> > obviously inaccurate: the number of lines of the diff of diffs does not\n> > say anything about the number of matching lines, quite to the contrary,\n> > it correlates somewhat to the number of non-matching lines.\n> >\n> > So a better interpretation would have been:\n> >\n> >  The default creation factor is 60 (roughly speaking, it wants at\n> >  most 60% of the diffs' lines to differ, otherwise it considers\n> >  them not to be a match.\n> >\n> > This is still inaccurate, but at least it gets the idea of the\n> > range-diff across.\n> >\n> > Of course, I will never be able to amend the commit message in\n> > GitGitGadget anyway, as I have merged that PR already.\n> >\n> > Ciao,\n> > Dscho\n> Medium term, is this something that could go in the algorithms section of the\n> range-diff man page, especially if the upstream commit message is already in\n> place.\n>\n> #leftoverdocs ?\n\nSure. How about giving it a try while our memory is still fresh? You\nwould help me immensely if you could take that task off of my plate...\n\nThanks,\nDscho\n"},{"id":"383786","messageId":"a41a083e-ed18-eaed-8305-b7cf82d92878@iee.email","threadId":"51924","inReplyTo":"nycvar.QRO.7.76.6.1910101102280.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2019-10-10T10:12:00Z","receivedAt":"2019-10-10T10:12:05Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Hi Dscho,\nOn 10/10/2019 10:03, Johannes Schindelin wrote:\n>>> So a better interpretation would have been:\n>>>\n>>>   The default creation factor is 60 (roughly speaking, it wants at\n>>>   most 60% of the diffs' lines to differ, otherwise it considers\n>>>   them not to be a match.\n>>>\n>>> This is still inaccurate, but at least it gets the idea of the\n>>> range-diff across.\n>>>\n>>> Of course, I will never be able to amend the commit message in\n>>> GitGitGadget anyway, as I have merged that PR already.\n>>>\n>>> Ciao,\n>>> Dscho\n>> Medium term, is this something that could go in the algorithms section of the\n>> range-diff man page, especially if the upstream commit message is already in\n>> place.\n>>\n>> #leftoverdocs ?\n> Sure. How about giving it a try while our memory is still fresh? You\n> would help me immensely if you could take that task off of my plate...\nMy sneak think was to introduce the new hash tag for useful explanations \nfrom the list that could be copied into the man/guide pages, especially \nfor the project pages.\n\nAt the moment I'm trying to get the >4Gb on Window' series sorted, which \nthough working has been a load of bear traps for testing and configs and \nVS compiling, etc.\nBut yes, it is on the back-list.\n-- \nPhilip\n"},{"id":"383938","messageId":"nycvar.QRO.7.76.6.1910112359560.46@tvgsbejvaqbjf.bet","threadId":"51924","inReplyTo":"20191007160816.GB74671@cat","subject":"Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-11T22:06:34Z","receivedAt":"2019-10-11T22:07:01Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Thomas & Junio,\n\nOn Mon, 7 Oct 2019, Thomas Gummerer wrote:\n\n> On 10/07, Junio C Hamano wrote:\n> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> >\n> > > \tDate:   Fri, 04 Oct 2019 08:09:25 -0700 (PDT)\n> > > \t[...]\n> > > \tX-Google-Original-Date: Fri, 04 Oct 2019 15:09:10 GMT\n> > > \t[...]\n> > >\n> > > I am fairly certain that the latter is the actual `Date:` line sent to\n> > > GMail, and GMail just decides that it will not respect it.\n> >\n> > If the submitting program said \"Fri, 04 Oct 2019 15:09:10 +0000\n> > (GMT)\" instead of \"Fri, 04 Oct 2019 15:09:10 GMT\", that would match\n> > the format the MTA produced itself, I guess.  I am kind-of surprised\n> > if the problem is the use of the obs-zone format (RFC 2822 page 31),\n> > but anything is possible with GMail X-<.\n>\n> Yeah, the obs-zone format did seem to be the problem.  I just dug up\n> the previous thread we had about this, where I confirmed that +0000 as\n> the timezone worked just fine in my setup through GMail [*1*].  Note\n> sure if the (GMT) would cause any problems, but I'd agree with\n> avoiding it as you mention below to make sure GMail doesn't do\n> anything funny with it.\n>\n> *1*: https://public-inbox.org/git/20190318214842.GA32487@hank.intra.tgummerer.com/\n\nUrg. Sorry for dropping the ball on that one.\n\nI changed the GitGitGadget code accordingly (here is the commit:\nhttps://github.com/gitgitgadget/gitgitgadget/commit/6117c61be5c2d13b46229ed12b2a9f63ef80f3c9).\n\nThanks,\nDscho\n\n>\n> > How does send-email write that date header?  Matching that would be\n> > probably the most appropriate, if possible, given that GGG was\n> > written for send-email refugees, I guess ;-)\n> >\n> > Here is what its format_2822_time sub does, so +0000 without any\n> > textual zone name, it is.\n> >\n> > \treturn sprintf(\"%s, %2d %s %d %02d:%02d:%02d %s%02d%02d\",\n> > \t\t       qw(Sun Mon Tue Wed Thu Fri Sat)[$localtm[6]],\n> > \t\t       $localtm[3],\n> > \t\t       qw(Jan Feb Mar Apr May Jun\n> > \t\t\t  Jul Aug Sep Oct Nov Dec)[$localtm[4]],\n> > \t\t       $localtm[5]+1900,\n> > \t\t       $localtm[2],\n> > \t\t       $localtm[1],\n> > \t\t       $localtm[0],\n> > \t\t       ($offset >= 0) ? '+' : '-',\n> > \t\t       abs($offhour),\n> > \t\t       $offmin,\n> > \t\t       );\n> >\n> >\n>\n"}]}