* [PATCH 00/13] ci: include a Visual Studio build & test in our Azure Pipeline
@ 2019-09-26 8:30 Johannes Schindelin via GitGitGadget
2019-09-26 8:30 ` [PATCH 01/13] push: do not pretend to return `int` from `die_push_simple()` Johannes Schindelin via GitGitGadget
` (13 more replies)
0 siblings, 14 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-26 8:30 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
Git's Continuous Integration (CI) includes an Azure Pipeline that builds Git
on Linux, macOS and Windows, in the former two cases even in multiple
configurations (using GCC vs clang, 32-bit vs 64-bit, etc). On Windows, we
only build using GCC, using (a subset of) Git for Windows' SDK.
Recently, a patch series made it into Git that re-instates the ability to
generate project files for use with Visual Studio. The idea there being:
contributors can check out a special branch that has those generated files
in one generated commit on top of e.g. Git for Windows' master, allowing the
contributors to build Git in Visual Studio, without the need for downloading
Git for Windows' SDK (which weighs quite a bit: ~600MB download, ~2GB disk
footprint). The tests can then be run from a regular Git for Windows Bash.
This patch series adds that axis to Git's Azure Pipeline: the project files
are generated, MSBuild (which is kind of the command-line equivalent of
Visual Studio's "Build" operation) is used to build Git, and then a
parallelized test job runs the test suite in a Portable Git.
These patches are based on js/visual-studio.
Johannes Schindelin (13):
push: do not pretend to return `int` from `die_push_simple()`
msvc: avoid using minus operator on unsigned types
winansi: use FLEX_ARRAY to avoid compiler warning
compat/win32/path-utils.h: add #include guards
msvc: ignore some libraries when linking
msvc: handle DEVELOPER=1
msvc: work around a bug in GetEnvironmentVariable()
vcxproj: only copy `git-remote-http.exe` once it was built
vcxproj: include more generated files
test-tool run-command: learn to run (parts of) the testsuite
tests: let --immediate and --write-junit-xml play well together
ci: really use shallow clones on Azure Pipelines
ci: also build and test with MS Visual Studio on Azure Pipelines
Makefile | 4 +
azure-pipelines.yml | 164 ++++++++++++++++++++-
builtin/push.c | 4 +-
compat/mingw.c | 2 +
compat/vcbuild/scripts/clink.pl | 48 +++++-
compat/win32/path-utils.h | 5 +
compat/winansi.c | 2 +-
config.mak.uname | 19 ++-
contrib/buildsystems/Generators/Vcxproj.pm | 3 +
read-cache.c | 4 +-
sha1-lookup.c | 2 +-
t/helper/test-run-command.c | 153 +++++++++++++++++++
t/test-lib.sh | 37 +++--
13 files changed, 415 insertions(+), 32 deletions(-)
base-commit: aac6ff7b5beeea9bca66ecda6eec60fc1dd447ec
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-288%2Fdscho%2Fazure-pipelines-msvc-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-288/dscho/azure-pipelines-msvc-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/288
--
gitgitgadget
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH 01/13] push: do not pretend to return `int` from `die_push_simple()`
2019-09-26 8:30 [PATCH 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
@ 2019-09-26 8:30 ` Johannes Schindelin via GitGitGadget
2019-09-26 8:30 ` [PATCH 02/13] msvc: avoid using minus operator on unsigned types Johannes Schindelin via GitGitGadget
` (12 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-26 8:30 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
This function is marked as `NORETURN`, and it indeed does not want to
return anything. So let's not declare it with the return type `int`.
This fixes the following warning when building with MSVC:
C4646: function declared with 'noreturn' has non-void return type
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/push.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/push.c b/builtin/push.c
index 021dd3b1e4..d216270d5f 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -143,8 +143,8 @@ static int push_url_of_remote(struct remote *remote, const char ***url_p)
return remote->url_nr;
}
-static NORETURN int die_push_simple(struct branch *branch,
- struct remote *remote)
+static NORETURN void die_push_simple(struct branch *branch,
+ struct remote *remote)
{
/*
* There's no point in using shorten_unambiguous_ref here,
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 02/13] msvc: avoid using minus operator on unsigned types
2019-09-26 8:30 [PATCH 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
2019-09-26 8:30 ` [PATCH 01/13] push: do not pretend to return `int` from `die_push_simple()` Johannes Schindelin via GitGitGadget
@ 2019-09-26 8:30 ` Johannes Schindelin via GitGitGadget
2019-09-26 17:20 ` Denton Liu
2019-09-26 8:30 ` [PATCH 03/13] winansi: use FLEX_ARRAY to avoid compiler warning Johannes Schindelin via GitGitGadget
` (11 subsequent siblings)
13 siblings, 1 reply; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-26 8:30 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
MSVC complains about this with `-Wall`, which can be taken as a sign
that this is indeed a real bug. The symptom is:
C4146: unary minus operator applied to unsigned type, result
still unsigned
Let's avoid this warning in the minimal way, e.g. writing `-1 -
<unsigned value>` instead of `-<unsigned value> - 1`.
Note that the change in the `estimate_cache_size()` function is
needed because MSVC considers the "return type" of the `sizeof()`
operator to be `size_t`, i.e. unsigned, and therefore it cannot be
negated using the unary minus operator.
Even worse, that arithmetic is doing extra work, in vain. We want to
calculate the entry extra cache size as the difference between the
size of the `cache_entry` structure minus the size of the
`ondisk_cache_entry` structure, padded to the appropriate alignment
boundary.
To that end, we start by assigning that difference to the `per_entry`
variable, and then abuse the `len` parameter of the
`align_padding_size()` macro to take the negative size of the ondisk
entry size. Essentially, we try to avoid passing the already calculated
difference to that macro by passing the operands of that difference
instead, when the macro expects operands of an addition:
#define align_padding_size(size, len) \
((size + (len) + 8) & ~7) - (size + len)
Currently, we pass A and -B to that macro instead of passing A - B and
0, where A - B is already stored in the `per_entry` variable, ready to
be used.
This is neither necessary, nor intuitive. Let's fix this, and have code
that is both easier to read and that also does not trigger MSVC's
warning.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
read-cache.c | 4 ++--
sha1-lookup.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index c701f7f8b8..11f3357216 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1276,7 +1276,7 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e
*/
if (istate->cache_nr > 0 &&
strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)
- pos = -istate->cache_nr - 1;
+ pos = -1 - istate->cache_nr;
else
pos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce));
@@ -1894,7 +1894,7 @@ static size_t estimate_cache_size(size_t ondisk_size, unsigned int entries)
/*
* Account for potential alignment differences.
*/
- per_entry += align_padding_size(sizeof(struct cache_entry), -sizeof(struct ondisk_cache_entry));
+ per_entry += align_padding_size(per_entry, 0);
return ondisk_size + entries * per_entry;
}
diff --git a/sha1-lookup.c b/sha1-lookup.c
index 796ab68da8..c819687730 100644
--- a/sha1-lookup.c
+++ b/sha1-lookup.c
@@ -97,7 +97,7 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,
lo = mi + 1;
mi = lo + (hi - lo) / 2;
} while (lo < hi);
- return -lo-1;
+ return -1 - lo;
}
int bsearch_hash(const unsigned char *sha1, const uint32_t *fanout_nbo,
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 03/13] winansi: use FLEX_ARRAY to avoid compiler warning
2019-09-26 8:30 [PATCH 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
2019-09-26 8:30 ` [PATCH 01/13] push: do not pretend to return `int` from `die_push_simple()` Johannes Schindelin via GitGitGadget
2019-09-26 8:30 ` [PATCH 02/13] msvc: avoid using minus operator on unsigned types Johannes Schindelin via GitGitGadget
@ 2019-09-26 8:30 ` Johannes Schindelin via GitGitGadget
2019-09-26 8:30 ` [PATCH 04/13] compat/win32/path-utils.h: add #include guards Johannes Schindelin via GitGitGadget
` (10 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-26 8:30 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
MSVC would complain thusly:
C4200: nonstandard extension used: zero-sized array in struct/union
Let's just use the `FLEX_ARRAY` constant that we introduced for exactly
this type of scenario.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
compat/winansi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compat/winansi.c b/compat/winansi.c
index cacd82c833..54fd701cbf 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -546,7 +546,7 @@ static HANDLE swap_osfhnd(int fd, HANDLE new_handle)
typedef struct _OBJECT_NAME_INFORMATION
{
UNICODE_STRING Name;
- WCHAR NameBuffer[0];
+ WCHAR NameBuffer[FLEX_ARRAY];
} OBJECT_NAME_INFORMATION, *POBJECT_NAME_INFORMATION;
#define ObjectNameInformation 1
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 04/13] compat/win32/path-utils.h: add #include guards
2019-09-26 8:30 [PATCH 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (2 preceding siblings ...)
2019-09-26 8:30 ` [PATCH 03/13] winansi: use FLEX_ARRAY to avoid compiler warning Johannes Schindelin via GitGitGadget
@ 2019-09-26 8:30 ` Johannes Schindelin via GitGitGadget
2019-09-26 8:30 ` [PATCH 05/13] msvc: ignore some libraries when linking Johannes Schindelin via GitGitGadget
` (9 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-26 8:30 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
This adds the common guards that allow headers to be #include'd multiple
times.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
compat/win32/path-utils.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/compat/win32/path-utils.h b/compat/win32/path-utils.h
index 0f70d43920..8ed062a6b7 100644
--- a/compat/win32/path-utils.h
+++ b/compat/win32/path-utils.h
@@ -1,3 +1,6 @@
+#ifndef WIN32_PATH_UTILS_H
+#define WIN32_PATH_UTILS_H
+
#define has_dos_drive_prefix(path) \
(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
int win32_skip_dos_drive_prefix(char **path);
@@ -18,3 +21,5 @@ static inline char *win32_find_last_dir_sep(const char *path)
#define find_last_dir_sep win32_find_last_dir_sep
int win32_offset_1st_component(const char *path);
#define offset_1st_component win32_offset_1st_component
+
+#endif
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 05/13] msvc: ignore some libraries when linking
2019-09-26 8:30 [PATCH 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (3 preceding siblings ...)
2019-09-26 8:30 ` [PATCH 04/13] compat/win32/path-utils.h: add #include guards Johannes Schindelin via GitGitGadget
@ 2019-09-26 8:30 ` Johannes Schindelin via GitGitGadget
2019-09-26 8:30 ` [PATCH 06/13] msvc: handle DEVELOPER=1 Johannes Schindelin via GitGitGadget
` (8 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-26 8:30 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
To build with MSVC, we "translate" GCC options to MSVC options, and part
of those options refer to the libraries to link into the final
executable. Currently, this part looks somewhat like this on Windows:
-lcurl -lnghttp2 -lidn2 -lssl -lcrypto -lssl -lcrypto -lgdi32
-lcrypt32 -lwldap32 -lz -lws2_32 -lexpat
Some of those are direct dependencies (such as curl and ssl) and others
are indirect (nghttp2 and idn2, for example, are dependencies of curl,
but need to be linked in for reasons).
We already handle the direct dependencies, e.g. `-liconv` is already
handled as adding `libiconv.lib` to the list of libraries to link
against.
Let's just ignore the remaining `-l*` options so that MSVC does not have
to warn us that it ignored e.g. the `/lnghttp2` option. We do that by
extending the clause that already "eats" the `-R*` options to also eat
the `-l*` options.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
compat/vcbuild/scripts/clink.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compat/vcbuild/scripts/clink.pl b/compat/vcbuild/scripts/clink.pl
index c7b021bfac..00fc339cba 100755
--- a/compat/vcbuild/scripts/clink.pl
+++ b/compat/vcbuild/scripts/clink.pl
@@ -68,7 +68,7 @@
} elsif ("$arg" =~ /^-L/ && "$arg" ne "-LTCG") {
$arg =~ s/^-L/-LIBPATH:/;
push(@lflags, $arg);
- } elsif ("$arg" =~ /^-R/) {
+ } elsif ("$arg" =~ /^-[Rl]/) {
# eat
} else {
push(@args, $arg);
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 06/13] msvc: handle DEVELOPER=1
2019-09-26 8:30 [PATCH 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (4 preceding siblings ...)
2019-09-26 8:30 ` [PATCH 05/13] msvc: ignore some libraries when linking Johannes Schindelin via GitGitGadget
@ 2019-09-26 8:30 ` Johannes Schindelin via GitGitGadget
2019-09-26 8:30 ` [PATCH 07/13] msvc: work around a bug in GetEnvironmentVariable() Johannes Schindelin via GitGitGadget
` (7 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-26 8:30 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
We frequently build Git using the `DEVELOPER=1` make setting as a
shortcut to enable all kinds of more stringent compiler warnings.
Those compiler warnings are relatively specific to GCC, though, so let's
try our best to translate them to the equivalent options to pass to MS
Visual C++'s `cl.exe`.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
compat/vcbuild/scripts/clink.pl | 46 +++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/compat/vcbuild/scripts/clink.pl b/compat/vcbuild/scripts/clink.pl
index 00fc339cba..ec95a3b2d0 100755
--- a/compat/vcbuild/scripts/clink.pl
+++ b/compat/vcbuild/scripts/clink.pl
@@ -70,6 +70,52 @@
push(@lflags, $arg);
} elsif ("$arg" =~ /^-[Rl]/) {
# eat
+ } elsif ("$arg" eq "-Werror") {
+ push(@cflags, "-WX");
+ } elsif ("$arg" eq "-Wall") {
+ # cl.exe understands -Wall, but it is really overzealous
+ push(@cflags, "-W4");
+ # disable the "signed/unsigned mismatch" warnings; our source code violates that
+ push(@cflags, "-wd4018");
+ push(@cflags, "-wd4245");
+ push(@cflags, "-wd4389");
+ # disable the "unreferenced formal parameter" warning; our source code violates that
+ push(@cflags, "-wd4100");
+ # disable the "conditional expression is constant" warning; our source code violates that
+ push(@cflags, "-wd4127");
+ # disable the "const object should be initialized" warning; these warnings affect only objects that are `static`
+ push(@cflags, "-wd4132");
+ # disable the "function/data pointer conversion in expression" warning; our source code violates that
+ push(@cflags, "-wd4152");
+ # disable the "non-constant aggregate initializer" warning; our source code violates that
+ push(@cflags, "-wd4204");
+ # disable the "cannot be initialized using address of automatic variable" warning; our source code violates that
+ push(@cflags, "-wd4221");
+ # disable the "possible loss of data" warnings; our source code violates that
+ push(@cflags, "-wd4244");
+ push(@cflags, "-wd4267");
+ # disable the "array is too small to include a terminating null character" warning; we ab-use strings to initialize OIDs
+ push(@cflags, "-wd4295");
+ # disable the "'<<': result of 32-bit shift implicitly converted to 64 bits" warning; our source code violates that
+ push(@cflags, "-wd4334");
+ # disable the "declaration hides previous local declaration" warning; our source code violates that
+ push(@cflags, "-wd4456");
+ # disable the "declaration hides function parameter" warning; our source code violates that
+ push(@cflags, "-wd4457");
+ # disable the "declaration hides global declaration" warning; our source code violates that
+ push(@cflags, "-wd4459");
+ # disable the "potentially uninitialized local variable '<name>' used" warning; our source code violates that
+ push(@cflags, "-wd4701");
+ # disable the "unreachable code" warning; our source code violates that
+ push(@cflags, "-wd4702");
+ # disable the "potentially uninitialized local pointer variable used" warning; our source code violates that
+ push(@cflags, "-wd4703");
+ # disable the "assignment within conditional expression" warning; our source code violates that
+ push(@cflags, "-wd4706");
+ # disable the "'inet_ntoa': Use inet_ntop() or InetNtop() instead" warning; our source code violates that
+ push(@cflags, "-wd4996");
+ } elsif ("$arg" =~ /^-W[a-z]/) {
+ # let's ignore those
} else {
push(@args, $arg);
}
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 07/13] msvc: work around a bug in GetEnvironmentVariable()
2019-09-26 8:30 [PATCH 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (5 preceding siblings ...)
2019-09-26 8:30 ` [PATCH 06/13] msvc: handle DEVELOPER=1 Johannes Schindelin via GitGitGadget
@ 2019-09-26 8:30 ` Johannes Schindelin via GitGitGadget
2019-09-26 8:30 ` [PATCH 08/13] vcxproj: only copy `git-remote-http.exe` once it was built Johannes Schindelin via GitGitGadget
` (6 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-26 8:30 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The return value of that function is 0 both for variables that are
unset, as well as for variables whose values are empty. To discern those
two cases, one has to call `GetLastError()`, whose return value is
`ERROR_ENVVAR_NOT_FOUND` and `ERROR_SUCCESS`, respectively.
Except that it is not actually set to `ERROR_SUCCESS` in the latter
case, apparently, but the last error value seems to be simply unchanged.
To work around this, let's just re-set the last error value just before
inspecting the environment variable.
This fixes a problem that triggers failures in t3301-notes.sh (where we
try to override config settings by passing empty values for certain
environment variables).
This problem is hidden in the MINGW build by the fact that older
MSVC runtimes (such as the one used by MINGW builds) have a `calloc()`
that re-sets the last error value in case of success, while newer
runtimes set the error value only if `NULL` is returned by that
function.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
compat/mingw.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/compat/mingw.c b/compat/mingw.c
index 4891789771..7d8cb814ba 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1661,6 +1661,8 @@ char *mingw_getenv(const char *name)
if (!w_key)
die("Out of memory, (tried to allocate %u wchar_t's)", len_key);
xutftowcs(w_key, name, len_key);
+ /* GetEnvironmentVariableW() only sets the last error upon failure */
+ SetLastError(ERROR_SUCCESS);
len_value = GetEnvironmentVariableW(w_key, w_value, ARRAY_SIZE(w_value));
if (!len_value && GetLastError() == ERROR_ENVVAR_NOT_FOUND) {
free(w_key);
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 08/13] vcxproj: only copy `git-remote-http.exe` once it was built
2019-09-26 8:30 [PATCH 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (6 preceding siblings ...)
2019-09-26 8:30 ` [PATCH 07/13] msvc: work around a bug in GetEnvironmentVariable() Johannes Schindelin via GitGitGadget
@ 2019-09-26 8:30 ` Johannes Schindelin via GitGitGadget
2019-09-26 8:30 ` [PATCH 09/13] vcxproj: include more generated files Johannes Schindelin via GitGitGadget
` (5 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-26 8:30 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
In b18ae14a8f6 (vcxproj: also link-or-copy builtins, 2019-07-29), we
started to copy or hard-link the built-ins as a post-build step of the
`git` project.
At the same time, we tried to copy or hard-link `git-remote-http.exe`,
but it is quite possible that it was not built at that time.
Let's move that latter task into a post-install step of the
`git-remote-http` project instead.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
config.mak.uname | 10 +++++++---
contrib/buildsystems/Generators/Vcxproj.pm | 3 +++
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/config.mak.uname b/config.mak.uname
index db7f06b95f..701aad62b1 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -703,20 +703,24 @@ vcxproj:
perl contrib/buildsystems/generate -g Vcxproj
git add -f git.sln {*,*/lib,t/helper/*}/*.vcxproj
- # Generate the LinkOrCopyBuiltins.targets file
+ # Generate the LinkOrCopyBuiltins.targets and LinkOrCopyRemoteHttp.targets file
(echo '<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">' && \
echo ' <Target Name="CopyBuiltins_AfterBuild" AfterTargets="AfterBuild">' && \
for name in $(BUILT_INS);\
do \
echo ' <Copy SourceFiles="$$(OutDir)\git.exe" DestinationFiles="$$(OutDir)\'"$$name"'" SkipUnchangedFiles="true" UseHardlinksIfPossible="true" />'; \
done && \
+ echo ' </Target>' && \
+ echo '</Project>') >git/LinkOrCopyBuiltins.targets
+ (echo '<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">' && \
+ echo ' <Target Name="CopyBuiltins_AfterBuild" AfterTargets="AfterBuild">' && \
for name in $(REMOTE_CURL_ALIASES); \
do \
echo ' <Copy SourceFiles="$$(OutDir)\'"$(REMOTE_CURL_PRIMARY)"'" DestinationFiles="$$(OutDir)\'"$$name"'" SkipUnchangedFiles="true" UseHardlinksIfPossible="true" />'; \
done && \
echo ' </Target>' && \
- echo '</Project>') >git/LinkOrCopyBuiltins.targets
- git add -f git/LinkOrCopyBuiltins.targets
+ echo '</Project>') >git-remote-http/LinkOrCopyRemoteHttp.targets
+ git add -f git/LinkOrCopyBuiltins.targets git-remote-http/LinkOrCopyRemoteHttp.targets
# Add command-list.h
$(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 command-list.h
diff --git a/contrib/buildsystems/Generators/Vcxproj.pm b/contrib/buildsystems/Generators/Vcxproj.pm
index 576ccabe1d..868b787855 100644
--- a/contrib/buildsystems/Generators/Vcxproj.pm
+++ b/contrib/buildsystems/Generators/Vcxproj.pm
@@ -277,6 +277,9 @@ sub createProject {
if ($target eq 'git') {
print F " <Import Project=\"LinkOrCopyBuiltins.targets\" />\n";
}
+ if ($target eq 'git-remote-http') {
+ print F " <Import Project=\"LinkOrCopyRemoteHttp.targets\" />\n";
+ }
print F << "EOM";
</Project>
EOM
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 09/13] vcxproj: include more generated files
2019-09-26 8:30 [PATCH 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (7 preceding siblings ...)
2019-09-26 8:30 ` [PATCH 08/13] vcxproj: only copy `git-remote-http.exe` once it was built Johannes Schindelin via GitGitGadget
@ 2019-09-26 8:30 ` Johannes Schindelin via GitGitGadget
2019-09-26 8:30 ` [PATCH 10/13] test-tool run-command: learn to run (parts of) the testsuite Johannes Schindelin via GitGitGadget
` (4 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-26 8:30 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
In the CI builds, we bundle all generated files into a so-called
artifacts `.tar` file, so that the test phase can fan out into multiple
parallel builds.
This patch makes sure that all files are included in the `vcxproj`
target which are needed for that artifacts `.tar` file.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
config.mak.uname | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/config.mak.uname b/config.mak.uname
index 701aad62b1..cc8efd95b1 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -728,11 +728,10 @@ vcxproj:
# Add scripts
rm -f perl/perl.mak
- $(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 \
- $(SCRIPT_LIB) $(SCRIPT_SH_GEN) $(SCRIPT_PERL_GEN)
+ $(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 $(SCRIPT_LIB) $(SCRIPTS)
# Strip out the sane tool path, needed only for building
sed -i '/^git_broken_path_fix ".*/d' git-sh-setup
- git add -f $(SCRIPT_LIB) $(SCRIPT_SH_GEN) $(SCRIPT_PERL_GEN)
+ git add -f $(SCRIPT_LIB) $(SCRIPTS)
# Add Perl module
$(MAKE) $(LIB_PERL_GEN)
@@ -762,6 +761,10 @@ vcxproj:
$(MAKE) -C templates
git add -f templates/boilerplates.made templates/blt/
+ # Add the translated messages
+ make MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 $(MOFILES)
+ git add -f $(MOFILES)
+
# Add build options
$(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 GIT-BUILD-OPTIONS
git add -f GIT-BUILD-OPTIONS
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 10/13] test-tool run-command: learn to run (parts of) the testsuite
2019-09-26 8:30 [PATCH 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (8 preceding siblings ...)
2019-09-26 8:30 ` [PATCH 09/13] vcxproj: include more generated files Johannes Schindelin via GitGitGadget
@ 2019-09-26 8:30 ` Johannes Schindelin via GitGitGadget
2019-09-26 8:30 ` [PATCH 11/13] tests: let --immediate and --write-junit-xml play well together Johannes Schindelin via GitGitGadget
` (3 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-26 8:30 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Git for Windows jumps through hoops to provide a development environment
that allows to build Git and to run its test suite. To that end, an
entire MSYS2 system, including GNU make and GCC is offered as "the Git
for Windows SDK". It does come at a price: an initial download of said
SDK weighs in with several hundreds of megabytes, and the unpacked SDK
occupies ~2GB of disk space.
A much more native development environment on Windows is Visual Studio.
To help contributors use that environment, we already have a Makefile
target `vcxproj` that generates a commit with project files (and other
generated files), and Git for Windows' `vs/master` branch is
continuously re-generated using that target.
The idea is to allow building Git in Visual Studio, and to run
individual tests using a Portable Git.
The one missing thing is a way to run the entire test suite: neither
`make` nor `prove` are required to run Git, therefore Git for Windows
does not support those commands in the Portable Git.
To help with that, add a simple test helper that exercises the
`run_processes_parallel()` function to allow for running test scripts in
parallel (which is really necessary, especially on Windows, as Git's
test suite takes such a long time to run).
This will also come in handy for the upcoming change to our Azure
Pipeline: we will use this helper in a Portable Git to test the Visual
Studio build of Git.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/helper/test-run-command.c | 153 ++++++++++++++++++++++++++++++++++++
1 file changed, 153 insertions(+)
diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c
index 2cc93bb69c..ead6dc611a 100644
--- a/t/helper/test-run-command.c
+++ b/t/helper/test-run-command.c
@@ -10,9 +10,14 @@
#include "test-tool.h"
#include "git-compat-util.h"
+#include "cache.h"
#include "run-command.h"
#include "argv-array.h"
#include "strbuf.h"
+#include "parse-options.h"
+#include "string-list.h"
+#include "thread-utils.h"
+#include "wildmatch.h"
#include <string.h>
#include <errno.h>
@@ -50,11 +55,159 @@ static int task_finished(int result,
return 1;
}
+struct testsuite {
+ struct string_list tests, failed;
+ int next;
+ int quiet, immediate, verbose, verbose_log, trace, write_junit_xml;
+};
+#define TESTSUITE_INIT \
+ { STRING_LIST_INIT_DUP, STRING_LIST_INIT_DUP, -1, 0, 0, 0, 0, 0, 0 }
+
+static int next_test(struct child_process *cp, struct strbuf *err, void *cb,
+ void **task_cb)
+{
+ struct testsuite *suite = cb;
+ const char *test;
+ if (suite->next >= suite->tests.nr)
+ return 0;
+
+ test = suite->tests.items[suite->next++].string;
+ argv_array_pushl(&cp->args, "sh", test, NULL);
+ if (suite->quiet)
+ argv_array_push(&cp->args, "--quiet");
+ if (suite->immediate)
+ argv_array_push(&cp->args, "-i");
+ if (suite->verbose)
+ argv_array_push(&cp->args, "-v");
+ if (suite->verbose_log)
+ argv_array_push(&cp->args, "-V");
+ if (suite->trace)
+ argv_array_push(&cp->args, "-x");
+ if (suite->write_junit_xml)
+ argv_array_push(&cp->args, "--write-junit-xml");
+
+ strbuf_addf(err, "Output of '%s':\n", test);
+ *task_cb = (void *)test;
+
+ return 1;
+}
+
+static int test_finished(int result, struct strbuf *err, void *cb,
+ void *task_cb)
+{
+ struct testsuite *suite = cb;
+ const char *name = (const char *)task_cb;
+
+ if (result)
+ string_list_append(&suite->failed, name);
+
+ strbuf_addf(err, "%s: '%s'\n", result ? "FAIL" : "SUCCESS", name);
+
+ return 0;
+}
+
+static int test_failed(struct strbuf *out, void *cb, void *task_cb)
+{
+ struct testsuite *suite = cb;
+ const char *name = (const char *)task_cb;
+
+ string_list_append(&suite->failed, name);
+ strbuf_addf(out, "FAILED TO START: '%s'\n", name);
+
+ return 0;
+}
+
+static const char * const testsuite_usage[] = {
+ "test-run-command testsuite [<options>] [<pattern>...]",
+ NULL
+};
+
+static int testsuite(int argc, const char **argv)
+{
+ struct testsuite suite = TESTSUITE_INIT;
+ int max_jobs = 1, i, ret;
+ DIR *dir;
+ struct dirent *d;
+ struct option options[] = {
+ OPT_BOOL('i', "immediate", &suite.immediate,
+ "stop at first failed test case(s)"),
+ OPT_INTEGER('j', "jobs", &max_jobs, "run <N> jobs in parallel"),
+ OPT_BOOL('q', "quiet", &suite.quiet, "be terse"),
+ OPT_BOOL('v', "verbose", &suite.verbose, "be verbose"),
+ OPT_BOOL('V', "verbose-log", &suite.verbose_log,
+ "be verbose, redirected to a file"),
+ OPT_BOOL('x', "trace", &suite.trace, "trace shell commands"),
+ OPT_BOOL(0, "write-junit-xml", &suite.write_junit_xml,
+ "write JUnit-style XML files"),
+ OPT_END()
+ };
+
+ memset(&suite, 0, sizeof(suite));
+ suite.tests.strdup_strings = suite.failed.strdup_strings = 1;
+
+ argc = parse_options(argc, argv, NULL, options,
+ testsuite_usage, PARSE_OPT_STOP_AT_NON_OPTION);
+
+ if (max_jobs <= 0)
+ max_jobs = online_cpus();
+
+ dir = opendir(".");
+ if (!dir)
+ die("Could not open the current directory");
+ while ((d = readdir(dir))) {
+ const char *p = d->d_name;
+
+ if (*p != 't' || !isdigit(p[1]) || !isdigit(p[2]) ||
+ !isdigit(p[3]) || !isdigit(p[4]) || p[5] != '-' ||
+ !ends_with(p, ".sh"))
+ continue;
+
+ /* No pattern: match all */
+ if (!argc) {
+ string_list_append(&suite.tests, p);
+ continue;
+ }
+
+ for (i = 0; i < argc; i++)
+ if (!wildmatch(argv[i], p, 0)) {
+ string_list_append(&suite.tests, p);
+ break;
+ }
+ }
+ closedir(dir);
+
+ if (!suite.tests.nr)
+ die("No tests match!");
+ if (max_jobs > suite.tests.nr)
+ max_jobs = suite.tests.nr;
+
+ fprintf(stderr, "Running %d tests (%d at a time)\n",
+ suite.tests.nr, max_jobs);
+
+ ret = run_processes_parallel(max_jobs, next_test, test_failed,
+ test_finished, &suite);
+
+ if (suite.failed.nr > 0) {
+ ret = 1;
+ fprintf(stderr, "%d tests failed:\n\n", suite.failed.nr);
+ for (i = 0; i < suite.failed.nr; i++)
+ fprintf(stderr, "\t%s\n", suite.failed.items[i].string);
+ }
+
+ string_list_clear(&suite.tests, 0);
+ string_list_clear(&suite.failed, 0);
+
+ return !!ret;
+}
+
int cmd__run_command(int argc, const char **argv)
{
struct child_process proc = CHILD_PROCESS_INIT;
int jobs;
+ if (argc > 1 && !strcmp(argv[1], "testsuite"))
+ exit(testsuite(argc - 1, argv + 1));
+
if (argc < 3)
return 1;
while (!strcmp(argv[1], "env")) {
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 11/13] tests: let --immediate and --write-junit-xml play well together
2019-09-26 8:30 [PATCH 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (9 preceding siblings ...)
2019-09-26 8:30 ` [PATCH 10/13] test-tool run-command: learn to run (parts of) the testsuite Johannes Schindelin via GitGitGadget
@ 2019-09-26 8:30 ` Johannes Schindelin via GitGitGadget
2019-09-28 22:22 ` Junio C Hamano
2019-09-26 8:30 ` [PATCH 12/13] ci: really use shallow clones on Azure Pipelines Johannes Schindelin via GitGitGadget
` (2 subsequent siblings)
13 siblings, 1 reply; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-26 8:30 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
When the `--immediate` option is in effect, any test failure will
immediately exit the test script. Together with `--write-junit-xml`, we
will want the JUnit-style `.xml` file to be finalized (and not leave the
XML incomplete). Let's make it so.
This comes in particularly handy when trying to debug via Azure
Pipelines, where the JUnit-style XML is consumed to present the test
results in an informative and helpful way.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/test-lib.sh | 37 +++++++++++++++++++++----------------
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index d1ba33745a..f21c781e68 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -695,7 +695,7 @@ test_failure_ () {
say_color error "not ok $test_count - $1"
shift
printf '%s\n' "$*" | sed -e 's/^/# /'
- test "$immediate" = "" || { GIT_EXIT_OK=t; exit 1; }
+ test "$immediate" = "" || { finalize_junit_xml; GIT_EXIT_OK=t; exit 1; }
}
test_known_broken_ok_ () {
@@ -1063,6 +1063,25 @@ write_junit_xml_testcase () {
junit_have_testcase=t
}
+finalize_junit_xml () {
+ if test -n "$write_junit_xml" && test -n "$junit_xml_path"
+ then
+ test -n "$junit_have_testcase" || {
+ junit_start=$(test-tool date getnanos)
+ write_junit_xml_testcase "all tests skipped"
+ }
+
+ # adjust the overall time
+ junit_time=$(test-tool date getnanos $junit_suite_start)
+ sed "s/<testsuite [^>]*/& time=\"$junit_time\"/" \
+ <"$junit_xml_path" >"$junit_xml_path.new"
+ mv "$junit_xml_path.new" "$junit_xml_path"
+
+ write_junit_xml " </testsuite>" "</testsuites>"
+ write_junit_xml=
+ fi
+}
+
test_atexit_cleanup=:
test_atexit_handler () {
# In a succeeding test script 'test_atexit_handler' is invoked
@@ -1085,21 +1104,7 @@ test_done () {
# removed, so the commands can access pidfiles and socket files.
test_atexit_handler
- if test -n "$write_junit_xml" && test -n "$junit_xml_path"
- then
- test -n "$junit_have_testcase" || {
- junit_start=$(test-tool date getnanos)
- write_junit_xml_testcase "all tests skipped"
- }
-
- # adjust the overall time
- junit_time=$(test-tool date getnanos $junit_suite_start)
- sed "s/<testsuite [^>]*/& time=\"$junit_time\"/" \
- <"$junit_xml_path" >"$junit_xml_path.new"
- mv "$junit_xml_path.new" "$junit_xml_path"
-
- write_junit_xml " </testsuite>" "</testsuites>"
- fi
+ finalize_junit_xml
if test -z "$HARNESS_ACTIVE"
then
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 12/13] ci: really use shallow clones on Azure Pipelines
2019-09-26 8:30 [PATCH 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (10 preceding siblings ...)
2019-09-26 8:30 ` [PATCH 11/13] tests: let --immediate and --write-junit-xml play well together Johannes Schindelin via GitGitGadget
@ 2019-09-26 8:30 ` Johannes Schindelin via GitGitGadget
2019-09-26 8:30 ` [PATCH 13/13] ci: also build and test with MS Visual Studio " Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-26 8:30 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
This was a left-over from the previous YAML schema, and it no longer
works. The problem was noticed while editing `azure-pipelines.yml` in VS
Code with the very helpful "Azure Pipelines" extension (syntax
highlighting and intellisense for `azure-pipelines.yml`...).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
azure-pipelines.yml | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/azure-pipelines.yml b/azure-pipelines.yml
index c329b7218b..55ee23ad0f 100644
--- a/azure-pipelines.yml
+++ b/azure-pipelines.yml
@@ -1,6 +1,5 @@
-resources:
-- repo: self
- fetchDepth: 1
+variables:
+ Agent.Source.Git.ShallowFetchDepth: 1
jobs:
- job: windows_build
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH 13/13] ci: also build and test with MS Visual Studio on Azure Pipelines
2019-09-26 8:30 [PATCH 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (11 preceding siblings ...)
2019-09-26 8:30 ` [PATCH 12/13] ci: really use shallow clones on Azure Pipelines Johannes Schindelin via GitGitGadget
@ 2019-09-26 8:30 ` Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-26 8:30 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
... because we can, now. Technically, we actually build using `MSBuild`,
which is however pretty close to building interactively in Visual
Studio.
As there is no convenient way to run Git's test suite in Visual Studio,
we unpack a Portable Git to run it, using the just-added test helper to
allow running test scripts in parallel.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Makefile | 4 ++
azure-pipelines.yml | 159 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 163 insertions(+)
diff --git a/Makefile b/Makefile
index 3716dadc08..3f6dcec54b 100644
--- a/Makefile
+++ b/Makefile
@@ -3025,6 +3025,10 @@ rpm::
@false
.PHONY: rpm
+ifneq ($(INCLUDE_DLLS_IN_ARTIFACTS),)
+OTHER_PROGRAMS += $(shell echo *.dll t/helper/*.dll)
+endif
+
artifacts-tar:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) \
GIT-BUILD-OPTIONS $(TEST_PROGRAMS) $(test_bindir_programs) \
$(MOFILES)
diff --git a/azure-pipelines.yml b/azure-pipelines.yml
index 55ee23ad0f..875e63cac1 100644
--- a/azure-pipelines.yml
+++ b/azure-pipelines.yml
@@ -130,6 +130,165 @@ jobs:
PathtoPublish: t/failed-test-artifacts
ArtifactName: failed-test-artifacts
+- job: vs_build
+ displayName: Visual Studio Build
+ condition: succeeded()
+ pool: Hosted VS2017
+ timeoutInMinutes: 240
+ steps:
+ - powershell: |
+ if ("$GITFILESHAREPWD" -ne "" -and "$GITFILESHAREPWD" -ne "`$`(gitfileshare.pwd)") {
+ net use s: \\gitfileshare.file.core.windows.net\test-cache "$GITFILESHAREPWD" /user:AZURE\gitfileshare /persistent:no
+ cmd /c mklink /d "$(Build.SourcesDirectory)\test-cache" S:\
+ }
+ displayName: 'Mount test-cache'
+ env:
+ GITFILESHAREPWD: $(gitfileshare.pwd)
+ - powershell: |
+ $urlbase = "https://dev.azure.com/git-for-windows/git/_apis/build/builds"
+ $id = ((Invoke-WebRequest -UseBasicParsing "${urlbase}?definitions=22&statusFilter=completed&resultFilter=succeeded&`$top=1").content | ConvertFrom-JSON).value[0].id
+ $downloadUrl = ((Invoke-WebRequest -UseBasicParsing "${urlbase}/$id/artifacts").content | ConvertFrom-JSON).value[1].resource.downloadUrl
+ (New-Object Net.WebClient).DownloadFile($downloadUrl,"git-sdk-64-minimal.zip")
+ Expand-Archive git-sdk-64-minimal.zip -DestinationPath . -Force
+ Remove-Item git-sdk-64-minimal.zip
+
+ # Let Git ignore the SDK and the test-cache
+ "/git-sdk-64-minimal/`n/test-cache/`n" | Out-File -NoNewLine -Encoding ascii -Append "$(Build.SourcesDirectory)\.git\info\exclude"
+ displayName: 'Download git-sdk-64-minimal'
+ - powershell: |
+ & git-sdk-64-minimal\usr\bin\bash.exe -lc @"
+ make vcxproj
+ "@
+ if (!$?) { exit(1) }
+ displayName: Generate Visual Studio Solution
+ env:
+ HOME: $(Build.SourcesDirectory)
+ MSYSTEM: MINGW64
+ DEVELOPER: 1
+ NO_PERL: 1
+ GIT_CONFIG_PARAMETERS: "'user.name=CI' 'user.email=ci@git'"
+ - powershell: |
+ $urlbase = "https://dev.azure.com/git/git/_apis/build/builds"
+ $id = ((Invoke-WebRequest -UseBasicParsing "${urlbase}?definitions=9&statusFilter=completed&resultFilter=succeeded&`$top=1").content | ConvertFrom-JSON).value[0].id
+ $downloadUrl = ((Invoke-WebRequest -UseBasicParsing "${urlbase}/$id/artifacts").content | ConvertFrom-JSON).value[0].resource.downloadUrl
+ (New-Object Net.WebClient).DownloadFile($downloadUrl, "compat.zip")
+ Expand-Archive compat.zip -DestinationPath . -Force
+ Remove-Item compat.zip
+ displayName: 'Download vcpkg artifacts'
+ - task: MSBuild@1
+ inputs:
+ solution: git.sln
+ platform: x64
+ configuration: Release
+ maximumCpuCount: 4
+ - powershell: |
+ & compat\vcbuild\vcpkg_copy_dlls.bat release
+ if (!$?) { exit(1) }
+ & git-sdk-64-minimal\usr\bin\bash.exe -lc @"
+ mkdir -p artifacts &&
+ eval \"`$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts | grep ^tar)\"
+ "@
+ if (!$?) { exit(1) }
+ displayName: Bundle artifact tar
+ env:
+ HOME: $(Build.SourcesDirectory)
+ MSYSTEM: MINGW64
+ DEVELOPER: 1
+ NO_PERL: 1
+ MSVC: 1
+ VCPKG_ROOT: $(Build.SourcesDirectory)\compat\vcbuild\vcpkg
+ - powershell: |
+ $tag = (Invoke-WebRequest -UseBasicParsing "https://gitforwindows.org/latest-tag.txt").content
+ $version = (Invoke-WebRequest -UseBasicParsing "https://gitforwindows.org/latest-version.txt").content
+ $url = "https://github.com/git-for-windows/git/releases/download/${tag}/PortableGit-${version}-64-bit.7z.exe"
+ (New-Object Net.WebClient).DownloadFile($url,"PortableGit.exe")
+ & .\PortableGit.exe -y -oartifacts\PortableGit
+ # Wait until it is unpacked
+ while (-not @(Remove-Item -ErrorAction SilentlyContinue PortableGit.exe; $?)) { sleep 1 }
+ displayName: Download & extract portable Git
+ - task: PublishPipelineArtifact@0
+ displayName: 'Publish Pipeline Artifact: MSVC test artifacts'
+ inputs:
+ artifactName: 'vs-artifacts'
+ targetPath: '$(Build.SourcesDirectory)\artifacts'
+ - powershell: |
+ if ("$GITFILESHAREPWD" -ne "" -and "$GITFILESHAREPWD" -ne "`$`(gitfileshare.pwd)") {
+ cmd /c rmdir "$(Build.SourcesDirectory)\test-cache"
+ }
+ displayName: 'Unmount test-cache'
+ condition: true
+ env:
+ GITFILESHAREPWD: $(gitfileshare.pwd)
+
+- job: vs_test
+ displayName: Visual Studio Test
+ dependsOn: vs_build
+ condition: succeeded()
+ pool: Hosted
+ timeoutInMinutes: 240
+ strategy:
+ parallel: 10
+ steps:
+ - powershell: |
+ if ("$GITFILESHAREPWD" -ne "" -and "$GITFILESHAREPWD" -ne "`$`(gitfileshare.pwd)") {
+ net use s: \\gitfileshare.file.core.windows.net\test-cache "$GITFILESHAREPWD" /user:AZURE\gitfileshare /persistent:no
+ cmd /c mklink /d "$(Build.SourcesDirectory)\test-cache" S:\
+ }
+ displayName: 'Mount test-cache'
+ env:
+ GITFILESHAREPWD: $(gitfileshare.pwd)
+ - task: DownloadPipelineArtifact@0
+ displayName: 'Download Pipeline Artifact: VS test artifacts'
+ inputs:
+ artifactName: 'vs-artifacts'
+ targetPath: '$(Build.SourcesDirectory)'
+ - powershell: |
+ & PortableGit\git-cmd.exe --command=usr\bin\bash.exe -lc @"
+ test -f artifacts.tar.gz || {
+ echo No test artifacts found\; skipping >&2
+ exit 0
+ }
+ tar xf artifacts.tar.gz || exit 1
+
+ # Let Git ignore the SDK and the test-cache
+ printf '%s\n' /PortableGit/ /test-cache/ >>.git/info/exclude
+
+ cd t &&
+ PATH=\"`$PWD/helper:`$PATH\" &&
+ test-tool.exe run-command testsuite -V -x --write-junit-xml \
+ `$(test-tool.exe path-utils slice-tests \
+ `$SYSTEM_JOBPOSITIONINPHASE `$SYSTEM_TOTALJOBSINPHASE t[0-9]*.sh)
+ "@
+ if (!$?) { exit(1) }
+ displayName: 'Test (parallel)'
+ env:
+ HOME: $(Build.SourcesDirectory)
+ MSYSTEM: MINGW64
+ NO_SVN_TESTS: 1
+ GIT_TEST_SKIP_REBASE_P: 1
+ - powershell: |
+ if ("$GITFILESHAREPWD" -ne "" -and "$GITFILESHAREPWD" -ne "`$`(gitfileshare.pwd)") {
+ cmd /c rmdir "$(Build.SourcesDirectory)\test-cache"
+ }
+ displayName: 'Unmount test-cache'
+ condition: true
+ env:
+ GITFILESHAREPWD: $(gitfileshare.pwd)
+ - task: PublishTestResults@2
+ displayName: 'Publish Test Results **/TEST-*.xml'
+ inputs:
+ mergeTestResults: true
+ testRunTitle: 'vs'
+ platform: Windows
+ publishRunAttachments: false
+ condition: succeededOrFailed()
+ - task: PublishBuildArtifacts@1
+ displayName: 'Publish trash directories of failed tests'
+ condition: failed()
+ inputs:
+ PathtoPublish: t/failed-test-artifacts
+ ArtifactName: failed-vs-test-artifacts
+
- job: linux_clang
displayName: linux-clang
condition: succeeded()
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* Re: [PATCH 02/13] msvc: avoid using minus operator on unsigned types
2019-09-26 8:30 ` [PATCH 02/13] msvc: avoid using minus operator on unsigned types Johannes Schindelin via GitGitGadget
@ 2019-09-26 17:20 ` Denton Liu
2019-09-26 21:01 ` Johannes Schindelin
0 siblings, 1 reply; 72+ messages in thread
From: Denton Liu @ 2019-09-26 17:20 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget
Cc: git, Junio C Hamano, Johannes Schindelin
Hi Dscho,
On Thu, Sep 26, 2019 at 01:30:10AM -0700, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> MSVC complains about this with `-Wall`, which can be taken as a sign
> that this is indeed a real bug. The symptom is:
>
> C4146: unary minus operator applied to unsigned type, result
> still unsigned
>
> Let's avoid this warning in the minimal way, e.g. writing `-1 -
> <unsigned value>` instead of `-<unsigned value> - 1`.
[...]
> ---
> read-cache.c | 4 ++--
> sha1-lookup.c | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/read-cache.c b/read-cache.c
> index c701f7f8b8..11f3357216 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -1276,7 +1276,7 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e
> */
> if (istate->cache_nr > 0 &&
> strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)
> - pos = -istate->cache_nr - 1;
> + pos = -1 - istate->cache_nr;
I've been thinking about this and I'm still not certain that this 100%
correct from a language-lawyer perspective.
If we do `-1 - istate->cache_nr`, then the unsignedness of
istate->cache_nr takes over and the whole expression is a very large
unsigned number.
Then, when we assign to `int pos`, we are converting an unsigned number
which is out of the range of the signed number. According to a
StackOverflow post citing the C99 standard[1]:
Otherwise, the new type is signed and the value cannot be
represented in it; either the result is implementation-defined
or an implementation-defined signal is raised.
I'm sure that most platforms that we support will handle it sanely but
could we write this as
pos = -1 - (int) istate->cache_nr;
to be doubly sure that no funny business will happen?
> else
> pos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce));
>
> @@ -1894,7 +1894,7 @@ static size_t estimate_cache_size(size_t ondisk_size, unsigned int entries)
> /*
> * Account for potential alignment differences.
> */
> - per_entry += align_padding_size(sizeof(struct cache_entry), -sizeof(struct ondisk_cache_entry));
> + per_entry += align_padding_size(per_entry, 0);
> return ondisk_size + entries * per_entry;
> }
>
> diff --git a/sha1-lookup.c b/sha1-lookup.c
> index 796ab68da8..c819687730 100644
> --- a/sha1-lookup.c
> +++ b/sha1-lookup.c
> @@ -97,7 +97,7 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,
> lo = mi + 1;
> mi = lo + (hi - lo) / 2;
> } while (lo < hi);
> - return -lo-1;
> + return -1 - lo;
Same thing here.
[1]: https://stackoverflow.com/questions/50605/signed-to-unsigned-conversion-in-c-is-it-always-safe
> }
>
> int bsearch_hash(const unsigned char *sha1, const uint32_t *fanout_nbo,
> --
> gitgitgadget
>
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 02/13] msvc: avoid using minus operator on unsigned types
2019-09-26 17:20 ` Denton Liu
@ 2019-09-26 21:01 ` Johannes Schindelin
2019-09-26 23:57 ` Denton Liu
0 siblings, 1 reply; 72+ messages in thread
From: Johannes Schindelin @ 2019-09-26 21:01 UTC (permalink / raw)
To: Denton Liu; +Cc: Johannes Schindelin via GitGitGadget, git, Junio C Hamano
Hi Denton,
On Thu, 26 Sep 2019, Denton Liu wrote:
> Hi Dscho,
>
> On Thu, Sep 26, 2019 at 01:30:10AM -0700, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > MSVC complains about this with `-Wall`, which can be taken as a sign
> > that this is indeed a real bug. The symptom is:
> >
> > C4146: unary minus operator applied to unsigned type, result
> > still unsigned
> >
> > Let's avoid this warning in the minimal way, e.g. writing `-1 -
> > <unsigned value>` instead of `-<unsigned value> - 1`.
>
> [...]
>
> > ---
> > read-cache.c | 4 ++--
> > sha1-lookup.c | 2 +-
> > 2 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/read-cache.c b/read-cache.c
> > index c701f7f8b8..11f3357216 100644
> > --- a/read-cache.c
> > +++ b/read-cache.c
> > @@ -1276,7 +1276,7 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e
> > */
> > if (istate->cache_nr > 0 &&
> > strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)
> > - pos = -istate->cache_nr - 1;
> > + pos = -1 - istate->cache_nr;
>
> I've been thinking about this and I'm still not certain that this 100%
> correct from a language-lawyer perspective.
>
> If we do `-1 - istate->cache_nr`, then the unsignedness of
> istate->cache_nr takes over and the whole expression is a very large
> unsigned number.
>
> Then, when we assign to `int pos`, we are converting an unsigned number
> which is out of the range of the signed number. According to a
> StackOverflow post citing the C99 standard[1]:
>
> Otherwise, the new type is signed and the value cannot be
> represented in it; either the result is implementation-defined
> or an implementation-defined signal is raised.
>
> I'm sure that most platforms that we support will handle it sanely but
> could we write this as
>
> pos = -1 - (int) istate->cache_nr;
>
> to be doubly sure that no funny business will happen?
I guess we should use `signed_add_overflows()` to make extra certain
that it does what we want it to do, kind of like `st_add()`. Or just do
the check explicitly, like so:
if (istate->cache_nr > INT_MAX)
die("overflow: -1 - %u", istate->cache_nr);
pos = -1 - istate->cache_nr;
}
>
> > else
> > pos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce));
> >
> > @@ -1894,7 +1894,7 @@ static size_t estimate_cache_size(size_t ondisk_size, unsigned int entries)
> > /*
> > * Account for potential alignment differences.
> > */
> > - per_entry += align_padding_size(sizeof(struct cache_entry), -sizeof(struct ondisk_cache_entry));
> > + per_entry += align_padding_size(per_entry, 0);
> > return ondisk_size + entries * per_entry;
> > }
> >
> > diff --git a/sha1-lookup.c b/sha1-lookup.c
> > index 796ab68da8..c819687730 100644
> > --- a/sha1-lookup.c
> > +++ b/sha1-lookup.c
> > @@ -97,7 +97,7 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,
> > lo = mi + 1;
> > mi = lo + (hi - lo) / 2;
> > } while (lo < hi);
> > - return -lo-1;
> > + return -1 - lo;
>
> Same thing here.
This is even more critical, as `lo` has the type `size_t`:
if (lo > INT_MAX)
die("overflow: -1 - %"PRIuMAX, (uintmax_t)lo);
return -1 - lo;
>
What do you think?
Dscho
> [1]: https://stackoverflow.com/questions/50605/signed-to-unsigned-conversion-in-c-is-it-always-safe
>
> > }
> >
> > int bsearch_hash(const unsigned char *sha1, const uint32_t *fanout_nbo,
> > --
> > gitgitgadget
> >
>
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 02/13] msvc: avoid using minus operator on unsigned types
2019-09-26 21:01 ` Johannes Schindelin
@ 2019-09-26 23:57 ` Denton Liu
2019-09-30 9:50 ` Johannes Schindelin
0 siblings, 1 reply; 72+ messages in thread
From: Denton Liu @ 2019-09-26 23:57 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Johannes Schindelin via GitGitGadget, git, Junio C Hamano
Hi Dscho,
On Thu, Sep 26, 2019 at 11:01:32PM +0200, Johannes Schindelin wrote:
> Hi Denton,
>
> On Thu, 26 Sep 2019, Denton Liu wrote:
>
> > Hi Dscho,
> >
> > On Thu, Sep 26, 2019 at 01:30:10AM -0700, Johannes Schindelin via GitGitGadget wrote:
> > > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> > >
> > > MSVC complains about this with `-Wall`, which can be taken as a sign
> > > that this is indeed a real bug. The symptom is:
> > >
> > > C4146: unary minus operator applied to unsigned type, result
> > > still unsigned
> > >
> > > Let's avoid this warning in the minimal way, e.g. writing `-1 -
> > > <unsigned value>` instead of `-<unsigned value> - 1`.
> >
> > [...]
> >
> > > ---
> > > read-cache.c | 4 ++--
> > > sha1-lookup.c | 2 +-
> > > 2 files changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/read-cache.c b/read-cache.c
> > > index c701f7f8b8..11f3357216 100644
> > > --- a/read-cache.c
> > > +++ b/read-cache.c
> > > @@ -1276,7 +1276,7 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e
> > > */
> > > if (istate->cache_nr > 0 &&
> > > strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)
> > > - pos = -istate->cache_nr - 1;
> > > + pos = -1 - istate->cache_nr;
> >
> > I've been thinking about this and I'm still not certain that this 100%
> > correct from a language-lawyer perspective.
> >
> > If we do `-1 - istate->cache_nr`, then the unsignedness of
> > istate->cache_nr takes over and the whole expression is a very large
> > unsigned number.
> >
> > Then, when we assign to `int pos`, we are converting an unsigned number
> > which is out of the range of the signed number. According to a
> > StackOverflow post citing the C99 standard[1]:
> >
> > Otherwise, the new type is signed and the value cannot be
> > represented in it; either the result is implementation-defined
> > or an implementation-defined signal is raised.
> >
> > I'm sure that most platforms that we support will handle it sanely but
> > could we write this as
> >
> > pos = -1 - (int) istate->cache_nr;
> >
> > to be doubly sure that no funny business will happen?
>
> I guess we should use `signed_add_overflows()` to make extra certain
> that it does what we want it to do, kind of like `st_add()`. Or just do
> the check explicitly, like so:
>
> if (istate->cache_nr > INT_MAX)
> die("overflow: -1 - %u", istate->cache_nr);
> pos = -1 - istate->cache_nr;
Could we change this to
pos = -1 - (int) istate->cache_nr;
so that we alleviate the problem I was talking about above?
Other than that, it looks good. Well, it might break on one's complement
systems but I don't think we support UNIVACs anyway. ;)
> }
> >
> > > else
> > > pos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce));
> > >
> > > @@ -1894,7 +1894,7 @@ static size_t estimate_cache_size(size_t ondisk_size, unsigned int entries)
> > > /*
> > > * Account for potential alignment differences.
> > > */
> > > - per_entry += align_padding_size(sizeof(struct cache_entry), -sizeof(struct ondisk_cache_entry));
> > > + per_entry += align_padding_size(per_entry, 0);
> > > return ondisk_size + entries * per_entry;
> > > }
> > >
> > > diff --git a/sha1-lookup.c b/sha1-lookup.c
> > > index 796ab68da8..c819687730 100644
> > > --- a/sha1-lookup.c
> > > +++ b/sha1-lookup.c
> > > @@ -97,7 +97,7 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,
> > > lo = mi + 1;
> > > mi = lo + (hi - lo) / 2;
> > > } while (lo < hi);
> > > - return -lo-1;
> > > + return -1 - lo;
> >
> > Same thing here.
>
> This is even more critical, as `lo` has the type `size_t`:
>
> if (lo > INT_MAX)
> die("overflow: -1 - %"PRIuMAX, (uintmax_t)lo);
> return -1 - lo;
Also, could we change this to
return -1 - (int) lo;
Thanks,
Denton
> >
> What do you think?
> Dscho
>
> > [1]: https://stackoverflow.com/questions/50605/signed-to-unsigned-conversion-in-c-is-it-always-safe
> >
> > > }
> > >
> > > int bsearch_hash(const unsigned char *sha1, const uint32_t *fanout_nbo,
> > > --
> > > gitgitgadget
> > >
> >
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 11/13] tests: let --immediate and --write-junit-xml play well together
2019-09-26 8:30 ` [PATCH 11/13] tests: let --immediate and --write-junit-xml play well together Johannes Schindelin via GitGitGadget
@ 2019-09-28 22:22 ` Junio C Hamano
2019-09-30 9:52 ` Johannes Schindelin
0 siblings, 1 reply; 72+ messages in thread
From: Junio C Hamano @ 2019-09-28 22:22 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index d1ba33745a..f21c781e68 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -695,7 +695,7 @@ test_failure_ () {
> say_color error "not ok $test_count - $1"
> shift
> printf '%s\n' "$*" | sed -e 's/^/# /'
> - test "$immediate" = "" || { GIT_EXIT_OK=t; exit 1; }
> + test "$immediate" = "" || { finalize_junit_xml; GIT_EXIT_OK=t; exit 1; }
> }
There are three places that do GIT_EXIT_OK=t in the test framework,
and the above covers one of them. The original in test_done is
another, and that place is made to call the "finalize" thing (it
used to have the same finalization code inlined).
The remaining one appears in
error () {
say_color error "error: $*"
GIT_EXIT_OK=t
exit 1
}
I wonder if we should cover this case, too. One caller of "error" I
know is BUG that says "bug in the test script", which means that
after successfully passing 30 tests, when the 31st test has 5 params
to test_expect_success by mistake, without finailzation we will lose
the result for the first 30.
And if we call "finalize" from the "error" helper, perhaps it makes
even more sense to update the above manual exit in test_failure_ to
do something like
if test -n "$immediate"
then
error "immediate exit after the first error"
fi
to delegate the finalization.
> @@ -1085,21 +1104,7 @@ test_done () {
> # removed, so the commands can access pidfiles and socket files.
> test_atexit_handler
>
> - if test -n "$write_junit_xml" && test -n "$junit_xml_path"
> - then
> - test -n "$junit_have_testcase" || {
> - junit_start=$(test-tool date getnanos)
> - write_junit_xml_testcase "all tests skipped"
> - }
> -
> - # adjust the overall time
> - junit_time=$(test-tool date getnanos $junit_suite_start)
> - sed "s/<testsuite [^>]*/& time=\"$junit_time\"/" \
> - <"$junit_xml_path" >"$junit_xml_path.new"
> - mv "$junit_xml_path.new" "$junit_xml_path"
> -
> - write_junit_xml " </testsuite>" "</testsuites>"
> - fi
> + finalize_junit_xml
>
> if test -z "$HARNESS_ACTIVE"
> then
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 02/13] msvc: avoid using minus operator on unsigned types
2019-09-26 23:57 ` Denton Liu
@ 2019-09-30 9:50 ` Johannes Schindelin
0 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin @ 2019-09-30 9:50 UTC (permalink / raw)
To: Denton Liu; +Cc: Johannes Schindelin via GitGitGadget, git, Junio C Hamano
Hi Denton,
On Thu, 26 Sep 2019, Denton Liu wrote:
> On Thu, Sep 26, 2019 at 11:01:32PM +0200, Johannes Schindelin wrote:
> >
> > On Thu, 26 Sep 2019, Denton Liu wrote:
> >
> > > Hi Dscho,
> > >
> > > On Thu, Sep 26, 2019 at 01:30:10AM -0700, Johannes Schindelin via GitGitGadget wrote:
> > > > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> > > >
> > > > MSVC complains about this with `-Wall`, which can be taken as a sign
> > > > that this is indeed a real bug. The symptom is:
> > > >
> > > > C4146: unary minus operator applied to unsigned type, result
> > > > still unsigned
> > > >
> > > > Let's avoid this warning in the minimal way, e.g. writing `-1 -
> > > > <unsigned value>` instead of `-<unsigned value> - 1`.
> > >
> > > [...]
> > >
> > > > ---
> > > > read-cache.c | 4 ++--
> > > > sha1-lookup.c | 2 +-
> > > > 2 files changed, 3 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/read-cache.c b/read-cache.c
> > > > index c701f7f8b8..11f3357216 100644
> > > > --- a/read-cache.c
> > > > +++ b/read-cache.c
> > > > @@ -1276,7 +1276,7 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e
> > > > */
> > > > if (istate->cache_nr > 0 &&
> > > > strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)
> > > > - pos = -istate->cache_nr - 1;
> > > > + pos = -1 - istate->cache_nr;
> > >
> > > I've been thinking about this and I'm still not certain that this 100%
> > > correct from a language-lawyer perspective.
> > >
> > > If we do `-1 - istate->cache_nr`, then the unsignedness of
> > > istate->cache_nr takes over and the whole expression is a very large
> > > unsigned number.
> > >
> > > Then, when we assign to `int pos`, we are converting an unsigned number
> > > which is out of the range of the signed number. According to a
> > > StackOverflow post citing the C99 standard[1]:
> > >
> > > Otherwise, the new type is signed and the value cannot be
> > > represented in it; either the result is implementation-defined
> > > or an implementation-defined signal is raised.
> > >
> > > I'm sure that most platforms that we support will handle it sanely but
> > > could we write this as
> > >
> > > pos = -1 - (int) istate->cache_nr;
> > >
> > > to be doubly sure that no funny business will happen?
> >
> > I guess we should use `signed_add_overflows()` to make extra certain
> > that it does what we want it to do, kind of like `st_add()`. Or just do
> > the check explicitly, like so:
> >
> > if (istate->cache_nr > INT_MAX)
> > die("overflow: -1 - %u", istate->cache_nr);
> > pos = -1 - istate->cache_nr;
>
> Could we change this to
>
> pos = -1 - (int) istate->cache_nr;
>
> so that we alleviate the problem I was talking about above?
Thank you, I had missed that.
> Other than that, it looks good. Well, it might break on one's complement
> systems but I don't think we support UNIVACs anyway. ;)
Let's hope that nobody tries to run Git on such a system, indeed.
Thanks,
Dscho
>
> > }
> > >
> > > > else
> > > > pos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce));
> > > >
> > > > @@ -1894,7 +1894,7 @@ static size_t estimate_cache_size(size_t ondisk_size, unsigned int entries)
> > > > /*
> > > > * Account for potential alignment differences.
> > > > */
> > > > - per_entry += align_padding_size(sizeof(struct cache_entry), -sizeof(struct ondisk_cache_entry));
> > > > + per_entry += align_padding_size(per_entry, 0);
> > > > return ondisk_size + entries * per_entry;
> > > > }
> > > >
> > > > diff --git a/sha1-lookup.c b/sha1-lookup.c
> > > > index 796ab68da8..c819687730 100644
> > > > --- a/sha1-lookup.c
> > > > +++ b/sha1-lookup.c
> > > > @@ -97,7 +97,7 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,
> > > > lo = mi + 1;
> > > > mi = lo + (hi - lo) / 2;
> > > > } while (lo < hi);
> > > > - return -lo-1;
> > > > + return -1 - lo;
> > >
> > > Same thing here.
> >
> > This is even more critical, as `lo` has the type `size_t`:
> >
> > if (lo > INT_MAX)
> > die("overflow: -1 - %"PRIuMAX, (uintmax_t)lo);
> > return -1 - lo;
>
> Also, could we change this to
>
> return -1 - (int) lo;
>
> Thanks,
>
> Denton
>
> > >
> > What do you think?
> > Dscho
> >
> > > [1]: https://stackoverflow.com/questions/50605/signed-to-unsigned-conversion-in-c-is-it-always-safe
> > >
> > > > }
> > > >
> > > > int bsearch_hash(const unsigned char *sha1, const uint32_t *fanout_nbo,
> > > > --
> > > > gitgitgadget
> > > >
> > >
>
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 11/13] tests: let --immediate and --write-junit-xml play well together
2019-09-28 22:22 ` Junio C Hamano
@ 2019-09-30 9:52 ` Johannes Schindelin
0 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin @ 2019-09-30 9:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin via GitGitGadget, git
Hi Junio,
On Sun, 29 Sep 2019, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > diff --git a/t/test-lib.sh b/t/test-lib.sh
> > index d1ba33745a..f21c781e68 100644
> > --- a/t/test-lib.sh
> > +++ b/t/test-lib.sh
> > @@ -695,7 +695,7 @@ test_failure_ () {
> > say_color error "not ok $test_count - $1"
> > shift
> > printf '%s\n' "$*" | sed -e 's/^/# /'
> > - test "$immediate" = "" || { GIT_EXIT_OK=t; exit 1; }
> > + test "$immediate" = "" || { finalize_junit_xml; GIT_EXIT_OK=t; exit 1; }
> > }
>
> There are three places that do GIT_EXIT_OK=t in the test framework,
> and the above covers one of them. The original in test_done is
> another, and that place is made to call the "finalize" thing (it
> used to have the same finalization code inlined).
>
> The remaining one appears in
>
> error () {
> say_color error "error: $*"
> GIT_EXIT_OK=t
> exit 1
> }
>
> I wonder if we should cover this case, too. One caller of "error" I
> know is BUG that says "bug in the test script", which means that
> after successfully passing 30 tests, when the 31st test has 5 params
> to test_expect_success by mistake, without finailzation we will lose
> the result for the first 30.
Good point.
> And if we call "finalize" from the "error" helper, perhaps it makes
> even more sense to update the above manual exit in test_failure_ to
> do something like
>
> if test -n "$immediate"
> then
> error "immediate exit after the first error"
> fi
>
> to delegate the finalization.
This adds an additional message. I am not sure how many scripts/CI
integrations are there that rely on the current behavior, so I would
like to exclude this change from this here patch series: it is about
including a Visual Studio build in our Azure Pipeline, nothing more,
nothing less...
Ciao,
Dscho
>
> > @@ -1085,21 +1104,7 @@ test_done () {
> > # removed, so the commands can access pidfiles and socket files.
> > test_atexit_handler
> >
> > - if test -n "$write_junit_xml" && test -n "$junit_xml_path"
> > - then
> > - test -n "$junit_have_testcase" || {
> > - junit_start=$(test-tool date getnanos)
> > - write_junit_xml_testcase "all tests skipped"
> > - }
> > -
> > - # adjust the overall time
> > - junit_time=$(test-tool date getnanos $junit_suite_start)
> > - sed "s/<testsuite [^>]*/& time=\"$junit_time\"/" \
> > - <"$junit_xml_path" >"$junit_xml_path.new"
> > - mv "$junit_xml_path.new" "$junit_xml_path"
> > -
> > - write_junit_xml " </testsuite>" "</testsuites>"
> > - fi
> > + finalize_junit_xml
> >
> > if test -z "$HARNESS_ACTIVE"
> > then
>
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH v2 00/13] ci: include a Visual Studio build & test in our Azure Pipeline
2019-09-26 8:30 [PATCH 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (12 preceding siblings ...)
2019-09-26 8:30 ` [PATCH 13/13] ci: also build and test with MS Visual Studio " Johannes Schindelin via GitGitGadget
@ 2019-09-30 9:55 ` Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 02/13] msvc: avoid using minus operator on unsigned types Johannes Schindelin via GitGitGadget
` (13 more replies)
13 siblings, 14 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-30 9:55 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano
Git's Continuous Integration (CI) includes an Azure Pipeline that builds Git
on Linux, macOS and Windows, in the former two cases even in multiple
configurations (using GCC vs clang, 32-bit vs 64-bit, etc). On Windows, we
only build using GCC, using (a subset of) Git for Windows' SDK.
Recently, a patch series made it into Git that re-instates the ability to
generate project files for use with Visual Studio. The idea there being:
contributors can check out a special branch that has those generated files
in one generated commit on top of e.g. Git for Windows' master, allowing the
contributors to build Git in Visual Studio, without the need for downloading
Git for Windows' SDK (which weighs quite a bit: ~600MB download, ~2GB disk
footprint). The tests can then be run from a regular Git for Windows Bash.
This patch series adds that axis to Git's Azure Pipeline: the project files
are generated, MSBuild (which is kind of the command-line equivalent of
Visual Studio's "Build" operation) is used to build Git, and then a
parallelized test job runs the test suite in a Portable Git.
These patches are based on js/visual-studio.
Changes since v1:
* "While at it", we now also check for overflows when doing that -1 -
<unsigned> arithmetic.
* The JUnit-style XML is finalized also in case that the script aborts e.g.
due to an incorrect number of arguments in a test_expect_success call.
Johannes Schindelin (13):
push: do not pretend to return `int` from `die_push_simple()`
msvc: avoid using minus operator on unsigned types
winansi: use FLEX_ARRAY to avoid compiler warning
compat/win32/path-utils.h: add #include guards
msvc: ignore some libraries when linking
msvc: handle DEVELOPER=1
msvc: work around a bug in GetEnvironmentVariable()
vcxproj: only copy `git-remote-http.exe` once it was built
vcxproj: include more generated files
test-tool run-command: learn to run (parts of) the testsuite
tests: let --immediate and --write-junit-xml play well together
ci: really use shallow clones on Azure Pipelines
ci: also build and test with MS Visual Studio on Azure Pipelines
Makefile | 4 +
azure-pipelines.yml | 164 ++++++++++++++++++++-
builtin/push.c | 4 +-
compat/mingw.c | 2 +
compat/vcbuild/scripts/clink.pl | 48 +++++-
compat/win32/path-utils.h | 5 +
compat/winansi.c | 2 +-
config.mak.uname | 19 ++-
contrib/buildsystems/Generators/Vcxproj.pm | 3 +
read-cache.c | 9 +-
sha1-lookup.c | 12 +-
t/helper/test-run-command.c | 153 +++++++++++++++++++
t/test-lib.sh | 38 +++--
13 files changed, 428 insertions(+), 35 deletions(-)
base-commit: aac6ff7b5beeea9bca66ecda6eec60fc1dd447ec
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-288%2Fdscho%2Fazure-pipelines-msvc-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-288/dscho/azure-pipelines-msvc-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/288
Range-diff vs v1:
1: 4d0b38125a = 1: 4d0b38125a push: do not pretend to return `int` from `die_push_simple()`
2: 2abe1e1fb0 ! 2: 8800320590 msvc: avoid using minus operator on unsigned types
@@ -40,17 +40,30 @@
that is both easier to read and that also does not trigger MSVC's
warning.
+ While at it, we take care of reporting overflows (which are unlikely,
+ but hey, defensive programming is good!).
+
+ We _also_ take pains of casting the unsigned value to signed: otherwise,
+ the signed operand (i.e. the `-1`) would be cast to unsigned before
+ doing the arithmetic.
+
+ Helped-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
diff --git a/read-cache.c b/read-cache.c
--- a/read-cache.c
+++ b/read-cache.c
@@
+ * we can avoid searching for it.
*/
if (istate->cache_nr > 0 &&
- strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)
+- strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)
- pos = -istate->cache_nr - 1;
-+ pos = -1 - istate->cache_nr;
++ strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0) {
++ if (istate->cache_nr > INT_MAX)
++ die("overflow: -1 - %u", istate->cache_nr);
++ pos = -1 - (int)istate->cache_nr;
++ }
else
pos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce));
@@ -67,12 +80,29 @@
diff --git a/sha1-lookup.c b/sha1-lookup.c
--- a/sha1-lookup.c
+++ b/sha1-lookup.c
+@@
+ miv = take2(sha1 + ofs);
+ if (miv < lov)
+ return -1;
+- if (hiv < miv)
+- return -1 - nr;
++ if (hiv < miv) {
++ if (nr > INT_MAX)
++ die("overflow: -1 - %"PRIuMAX,
++ (uintmax_t)nr);
++ return -1 - (int)nr;
++ }
+ if (lov != hiv) {
+ /*
+ * At this point miv could be equal
@@
lo = mi + 1;
mi = lo + (hi - lo) / 2;
} while (lo < hi);
- return -lo-1;
-+ return -1 - lo;
++ if (nr > INT_MAX)
++ die("overflow: -1 - %"PRIuMAX, (uintmax_t)lo);
++ return -1 - (int)lo;
}
int bsearch_hash(const unsigned char *sha1, const uint32_t *fanout_nbo,
3: dbd9022ad5 = 3: 8512a3e96d winansi: use FLEX_ARRAY to avoid compiler warning
4: 6417d6f689 = 4: 0345b08f54 compat/win32/path-utils.h: add #include guards
5: d1dee56fcc = 5: 5add01f8ff msvc: ignore some libraries when linking
6: c978c76599 = 6: 5c880f923e msvc: handle DEVELOPER=1
7: 0590514e43 = 7: 1f2245a228 msvc: work around a bug in GetEnvironmentVariable()
8: f7d5d1a1bc = 8: 582b299634 vcxproj: only copy `git-remote-http.exe` once it was built
9: 6adfc63e98 = 9: 38ccf999e7 vcxproj: include more generated files
10: ad9ab10ce0 = 10: 24b1c7bff3 test-tool run-command: learn to run (parts of) the testsuite
11: 99724f6a1e ! 11: 7be13d19e1 tests: let --immediate and --write-junit-xml play well together
@@ -11,12 +11,34 @@
Pipelines, where the JUnit-style XML is consumed to present the test
results in an informative and helpful way.
+ While at it, also handle the `error()` code path.
+
+ The only remaining code path that sets `GIT_EXIT_OK` happens whenever
+ the trash directory could not be set up, i.e. long before the JUnit XML
+ was written, therefore we should _not_ try to finalize that XML in that
+ case.
+
+ It is tempting to change the `immediate` code path to just hand off to
+ `error`, simplifying the code in the process. That would, however,
+ result in a change of behavior (an additional error message) in the test
+ suite, which is outside of the purview of the current patch series: its
+ goal is to allow building Git with Visual Studio and testing it with a
+ portable version of Git for Windows.
+
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
diff --git a/t/test-lib.sh b/t/test-lib.sh
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@
+
+ error () {
+ say_color error "error: $*"
++ finalize_junit_xml
+ GIT_EXIT_OK=t
+ exit 1
+ }
+@@
say_color error "not ok $test_count - $1"
shift
printf '%s\n' "$*" | sed -e 's/^/# /'
12: 29dceaae5e = 12: bde1e8ef65 ci: really use shallow clones on Azure Pipelines
13: ece296922b = 13: 7af1c01a08 ci: also build and test with MS Visual Studio on Azure Pipelines
--
gitgitgadget
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH v2 01/13] push: do not pretend to return `int` from `die_push_simple()`
2019-09-30 9:55 ` [PATCH v2 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 02/13] msvc: avoid using minus operator on unsigned types Johannes Schindelin via GitGitGadget
@ 2019-09-30 9:55 ` Johannes Schindelin via GitGitGadget
2019-10-03 22:37 ` Junio C Hamano
2019-09-30 9:55 ` [PATCH v2 03/13] winansi: use FLEX_ARRAY to avoid compiler warning Johannes Schindelin via GitGitGadget
` (11 subsequent siblings)
13 siblings, 1 reply; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-30 9:55 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
This function is marked as `NORETURN`, and it indeed does not want to
return anything. So let's not declare it with the return type `int`.
This fixes the following warning when building with MSVC:
C4646: function declared with 'noreturn' has non-void return type
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/push.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/push.c b/builtin/push.c
index 021dd3b1e4..d216270d5f 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -143,8 +143,8 @@ static int push_url_of_remote(struct remote *remote, const char ***url_p)
return remote->url_nr;
}
-static NORETURN int die_push_simple(struct branch *branch,
- struct remote *remote)
+static NORETURN void die_push_simple(struct branch *branch,
+ struct remote *remote)
{
/*
* There's no point in using shorten_unambiguous_ref here,
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH v2 02/13] msvc: avoid using minus operator on unsigned types
2019-09-30 9:55 ` [PATCH v2 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
@ 2019-09-30 9:55 ` Johannes Schindelin via GitGitGadget
2019-10-03 22:44 ` Junio C Hamano
2019-09-30 9:55 ` [PATCH v2 01/13] push: do not pretend to return `int` from `die_push_simple()` Johannes Schindelin via GitGitGadget
` (12 subsequent siblings)
13 siblings, 1 reply; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-30 9:55 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
MSVC complains about this with `-Wall`, which can be taken as a sign
that this is indeed a real bug. The symptom is:
C4146: unary minus operator applied to unsigned type, result
still unsigned
Let's avoid this warning in the minimal way, e.g. writing `-1 -
<unsigned value>` instead of `-<unsigned value> - 1`.
Note that the change in the `estimate_cache_size()` function is
needed because MSVC considers the "return type" of the `sizeof()`
operator to be `size_t`, i.e. unsigned, and therefore it cannot be
negated using the unary minus operator.
Even worse, that arithmetic is doing extra work, in vain. We want to
calculate the entry extra cache size as the difference between the
size of the `cache_entry` structure minus the size of the
`ondisk_cache_entry` structure, padded to the appropriate alignment
boundary.
To that end, we start by assigning that difference to the `per_entry`
variable, and then abuse the `len` parameter of the
`align_padding_size()` macro to take the negative size of the ondisk
entry size. Essentially, we try to avoid passing the already calculated
difference to that macro by passing the operands of that difference
instead, when the macro expects operands of an addition:
#define align_padding_size(size, len) \
((size + (len) + 8) & ~7) - (size + len)
Currently, we pass A and -B to that macro instead of passing A - B and
0, where A - B is already stored in the `per_entry` variable, ready to
be used.
This is neither necessary, nor intuitive. Let's fix this, and have code
that is both easier to read and that also does not trigger MSVC's
warning.
While at it, we take care of reporting overflows (which are unlikely,
but hey, defensive programming is good!).
We _also_ take pains of casting the unsigned value to signed: otherwise,
the signed operand (i.e. the `-1`) would be cast to unsigned before
doing the arithmetic.
Helped-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
read-cache.c | 9 ++++++---
sha1-lookup.c | 12 +++++++++---
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index c701f7f8b8..97745c2a31 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1275,8 +1275,11 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e
* we can avoid searching for it.
*/
if (istate->cache_nr > 0 &&
- strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)
- pos = -istate->cache_nr - 1;
+ strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0) {
+ if (istate->cache_nr > INT_MAX)
+ die("overflow: -1 - %u", istate->cache_nr);
+ pos = -1 - (int)istate->cache_nr;
+ }
else
pos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce));
@@ -1894,7 +1897,7 @@ static size_t estimate_cache_size(size_t ondisk_size, unsigned int entries)
/*
* Account for potential alignment differences.
*/
- per_entry += align_padding_size(sizeof(struct cache_entry), -sizeof(struct ondisk_cache_entry));
+ per_entry += align_padding_size(per_entry, 0);
return ondisk_size + entries * per_entry;
}
diff --git a/sha1-lookup.c b/sha1-lookup.c
index 796ab68da8..bb786b5420 100644
--- a/sha1-lookup.c
+++ b/sha1-lookup.c
@@ -69,8 +69,12 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,
miv = take2(sha1 + ofs);
if (miv < lov)
return -1;
- if (hiv < miv)
- return -1 - nr;
+ if (hiv < miv) {
+ if (nr > INT_MAX)
+ die("overflow: -1 - %"PRIuMAX,
+ (uintmax_t)nr);
+ return -1 - (int)nr;
+ }
if (lov != hiv) {
/*
* At this point miv could be equal
@@ -97,7 +101,9 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,
lo = mi + 1;
mi = lo + (hi - lo) / 2;
} while (lo < hi);
- return -lo-1;
+ if (nr > INT_MAX)
+ die("overflow: -1 - %"PRIuMAX, (uintmax_t)lo);
+ return -1 - (int)lo;
}
int bsearch_hash(const unsigned char *sha1, const uint32_t *fanout_nbo,
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH v2 03/13] winansi: use FLEX_ARRAY to avoid compiler warning
2019-09-30 9:55 ` [PATCH v2 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 02/13] msvc: avoid using minus operator on unsigned types Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 01/13] push: do not pretend to return `int` from `die_push_simple()` Johannes Schindelin via GitGitGadget
@ 2019-09-30 9:55 ` Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 05/13] msvc: ignore some libraries when linking Johannes Schindelin via GitGitGadget
` (10 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-30 9:55 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
MSVC would complain thusly:
C4200: nonstandard extension used: zero-sized array in struct/union
Let's just use the `FLEX_ARRAY` constant that we introduced for exactly
this type of scenario.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
compat/winansi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compat/winansi.c b/compat/winansi.c
index cacd82c833..54fd701cbf 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -546,7 +546,7 @@ static HANDLE swap_osfhnd(int fd, HANDLE new_handle)
typedef struct _OBJECT_NAME_INFORMATION
{
UNICODE_STRING Name;
- WCHAR NameBuffer[0];
+ WCHAR NameBuffer[FLEX_ARRAY];
} OBJECT_NAME_INFORMATION, *POBJECT_NAME_INFORMATION;
#define ObjectNameInformation 1
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH v2 05/13] msvc: ignore some libraries when linking
2019-09-30 9:55 ` [PATCH v2 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (2 preceding siblings ...)
2019-09-30 9:55 ` [PATCH v2 03/13] winansi: use FLEX_ARRAY to avoid compiler warning Johannes Schindelin via GitGitGadget
@ 2019-09-30 9:55 ` Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 04/13] compat/win32/path-utils.h: add #include guards Johannes Schindelin via GitGitGadget
` (9 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-30 9:55 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
To build with MSVC, we "translate" GCC options to MSVC options, and part
of those options refer to the libraries to link into the final
executable. Currently, this part looks somewhat like this on Windows:
-lcurl -lnghttp2 -lidn2 -lssl -lcrypto -lssl -lcrypto -lgdi32
-lcrypt32 -lwldap32 -lz -lws2_32 -lexpat
Some of those are direct dependencies (such as curl and ssl) and others
are indirect (nghttp2 and idn2, for example, are dependencies of curl,
but need to be linked in for reasons).
We already handle the direct dependencies, e.g. `-liconv` is already
handled as adding `libiconv.lib` to the list of libraries to link
against.
Let's just ignore the remaining `-l*` options so that MSVC does not have
to warn us that it ignored e.g. the `/lnghttp2` option. We do that by
extending the clause that already "eats" the `-R*` options to also eat
the `-l*` options.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
compat/vcbuild/scripts/clink.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compat/vcbuild/scripts/clink.pl b/compat/vcbuild/scripts/clink.pl
index c7b021bfac..00fc339cba 100755
--- a/compat/vcbuild/scripts/clink.pl
+++ b/compat/vcbuild/scripts/clink.pl
@@ -68,7 +68,7 @@
} elsif ("$arg" =~ /^-L/ && "$arg" ne "-LTCG") {
$arg =~ s/^-L/-LIBPATH:/;
push(@lflags, $arg);
- } elsif ("$arg" =~ /^-R/) {
+ } elsif ("$arg" =~ /^-[Rl]/) {
# eat
} else {
push(@args, $arg);
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH v2 04/13] compat/win32/path-utils.h: add #include guards
2019-09-30 9:55 ` [PATCH v2 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (3 preceding siblings ...)
2019-09-30 9:55 ` [PATCH v2 05/13] msvc: ignore some libraries when linking Johannes Schindelin via GitGitGadget
@ 2019-09-30 9:55 ` Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 06/13] msvc: handle DEVELOPER=1 Johannes Schindelin via GitGitGadget
` (8 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-30 9:55 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
This adds the common guards that allow headers to be #include'd multiple
times.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
compat/win32/path-utils.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/compat/win32/path-utils.h b/compat/win32/path-utils.h
index 0f70d43920..8ed062a6b7 100644
--- a/compat/win32/path-utils.h
+++ b/compat/win32/path-utils.h
@@ -1,3 +1,6 @@
+#ifndef WIN32_PATH_UTILS_H
+#define WIN32_PATH_UTILS_H
+
#define has_dos_drive_prefix(path) \
(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
int win32_skip_dos_drive_prefix(char **path);
@@ -18,3 +21,5 @@ static inline char *win32_find_last_dir_sep(const char *path)
#define find_last_dir_sep win32_find_last_dir_sep
int win32_offset_1st_component(const char *path);
#define offset_1st_component win32_offset_1st_component
+
+#endif
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH v2 06/13] msvc: handle DEVELOPER=1
2019-09-30 9:55 ` [PATCH v2 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (4 preceding siblings ...)
2019-09-30 9:55 ` [PATCH v2 04/13] compat/win32/path-utils.h: add #include guards Johannes Schindelin via GitGitGadget
@ 2019-09-30 9:55 ` Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 07/13] msvc: work around a bug in GetEnvironmentVariable() Johannes Schindelin via GitGitGadget
` (7 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-30 9:55 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
We frequently build Git using the `DEVELOPER=1` make setting as a
shortcut to enable all kinds of more stringent compiler warnings.
Those compiler warnings are relatively specific to GCC, though, so let's
try our best to translate them to the equivalent options to pass to MS
Visual C++'s `cl.exe`.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
compat/vcbuild/scripts/clink.pl | 46 +++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/compat/vcbuild/scripts/clink.pl b/compat/vcbuild/scripts/clink.pl
index 00fc339cba..ec95a3b2d0 100755
--- a/compat/vcbuild/scripts/clink.pl
+++ b/compat/vcbuild/scripts/clink.pl
@@ -70,6 +70,52 @@
push(@lflags, $arg);
} elsif ("$arg" =~ /^-[Rl]/) {
# eat
+ } elsif ("$arg" eq "-Werror") {
+ push(@cflags, "-WX");
+ } elsif ("$arg" eq "-Wall") {
+ # cl.exe understands -Wall, but it is really overzealous
+ push(@cflags, "-W4");
+ # disable the "signed/unsigned mismatch" warnings; our source code violates that
+ push(@cflags, "-wd4018");
+ push(@cflags, "-wd4245");
+ push(@cflags, "-wd4389");
+ # disable the "unreferenced formal parameter" warning; our source code violates that
+ push(@cflags, "-wd4100");
+ # disable the "conditional expression is constant" warning; our source code violates that
+ push(@cflags, "-wd4127");
+ # disable the "const object should be initialized" warning; these warnings affect only objects that are `static`
+ push(@cflags, "-wd4132");
+ # disable the "function/data pointer conversion in expression" warning; our source code violates that
+ push(@cflags, "-wd4152");
+ # disable the "non-constant aggregate initializer" warning; our source code violates that
+ push(@cflags, "-wd4204");
+ # disable the "cannot be initialized using address of automatic variable" warning; our source code violates that
+ push(@cflags, "-wd4221");
+ # disable the "possible loss of data" warnings; our source code violates that
+ push(@cflags, "-wd4244");
+ push(@cflags, "-wd4267");
+ # disable the "array is too small to include a terminating null character" warning; we ab-use strings to initialize OIDs
+ push(@cflags, "-wd4295");
+ # disable the "'<<': result of 32-bit shift implicitly converted to 64 bits" warning; our source code violates that
+ push(@cflags, "-wd4334");
+ # disable the "declaration hides previous local declaration" warning; our source code violates that
+ push(@cflags, "-wd4456");
+ # disable the "declaration hides function parameter" warning; our source code violates that
+ push(@cflags, "-wd4457");
+ # disable the "declaration hides global declaration" warning; our source code violates that
+ push(@cflags, "-wd4459");
+ # disable the "potentially uninitialized local variable '<name>' used" warning; our source code violates that
+ push(@cflags, "-wd4701");
+ # disable the "unreachable code" warning; our source code violates that
+ push(@cflags, "-wd4702");
+ # disable the "potentially uninitialized local pointer variable used" warning; our source code violates that
+ push(@cflags, "-wd4703");
+ # disable the "assignment within conditional expression" warning; our source code violates that
+ push(@cflags, "-wd4706");
+ # disable the "'inet_ntoa': Use inet_ntop() or InetNtop() instead" warning; our source code violates that
+ push(@cflags, "-wd4996");
+ } elsif ("$arg" =~ /^-W[a-z]/) {
+ # let's ignore those
} else {
push(@args, $arg);
}
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH v2 07/13] msvc: work around a bug in GetEnvironmentVariable()
2019-09-30 9:55 ` [PATCH v2 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (5 preceding siblings ...)
2019-09-30 9:55 ` [PATCH v2 06/13] msvc: handle DEVELOPER=1 Johannes Schindelin via GitGitGadget
@ 2019-09-30 9:55 ` Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 08/13] vcxproj: only copy `git-remote-http.exe` once it was built Johannes Schindelin via GitGitGadget
` (6 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-30 9:55 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The return value of that function is 0 both for variables that are
unset, as well as for variables whose values are empty. To discern those
two cases, one has to call `GetLastError()`, whose return value is
`ERROR_ENVVAR_NOT_FOUND` and `ERROR_SUCCESS`, respectively.
Except that it is not actually set to `ERROR_SUCCESS` in the latter
case, apparently, but the last error value seems to be simply unchanged.
To work around this, let's just re-set the last error value just before
inspecting the environment variable.
This fixes a problem that triggers failures in t3301-notes.sh (where we
try to override config settings by passing empty values for certain
environment variables).
This problem is hidden in the MINGW build by the fact that older
MSVC runtimes (such as the one used by MINGW builds) have a `calloc()`
that re-sets the last error value in case of success, while newer
runtimes set the error value only if `NULL` is returned by that
function.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
compat/mingw.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/compat/mingw.c b/compat/mingw.c
index 4891789771..7d8cb814ba 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1661,6 +1661,8 @@ char *mingw_getenv(const char *name)
if (!w_key)
die("Out of memory, (tried to allocate %u wchar_t's)", len_key);
xutftowcs(w_key, name, len_key);
+ /* GetEnvironmentVariableW() only sets the last error upon failure */
+ SetLastError(ERROR_SUCCESS);
len_value = GetEnvironmentVariableW(w_key, w_value, ARRAY_SIZE(w_value));
if (!len_value && GetLastError() == ERROR_ENVVAR_NOT_FOUND) {
free(w_key);
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH v2 09/13] vcxproj: include more generated files
2019-09-30 9:55 ` [PATCH v2 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (7 preceding siblings ...)
2019-09-30 9:55 ` [PATCH v2 08/13] vcxproj: only copy `git-remote-http.exe` once it was built Johannes Schindelin via GitGitGadget
@ 2019-09-30 9:55 ` Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 10/13] test-tool run-command: learn to run (parts of) the testsuite Johannes Schindelin via GitGitGadget
` (4 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-30 9:55 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
In the CI builds, we bundle all generated files into a so-called
artifacts `.tar` file, so that the test phase can fan out into multiple
parallel builds.
This patch makes sure that all files are included in the `vcxproj`
target which are needed for that artifacts `.tar` file.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
config.mak.uname | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/config.mak.uname b/config.mak.uname
index 701aad62b1..cc8efd95b1 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -728,11 +728,10 @@ vcxproj:
# Add scripts
rm -f perl/perl.mak
- $(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 \
- $(SCRIPT_LIB) $(SCRIPT_SH_GEN) $(SCRIPT_PERL_GEN)
+ $(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 $(SCRIPT_LIB) $(SCRIPTS)
# Strip out the sane tool path, needed only for building
sed -i '/^git_broken_path_fix ".*/d' git-sh-setup
- git add -f $(SCRIPT_LIB) $(SCRIPT_SH_GEN) $(SCRIPT_PERL_GEN)
+ git add -f $(SCRIPT_LIB) $(SCRIPTS)
# Add Perl module
$(MAKE) $(LIB_PERL_GEN)
@@ -762,6 +761,10 @@ vcxproj:
$(MAKE) -C templates
git add -f templates/boilerplates.made templates/blt/
+ # Add the translated messages
+ make MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 $(MOFILES)
+ git add -f $(MOFILES)
+
# Add build options
$(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 GIT-BUILD-OPTIONS
git add -f GIT-BUILD-OPTIONS
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH v2 08/13] vcxproj: only copy `git-remote-http.exe` once it was built
2019-09-30 9:55 ` [PATCH v2 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (6 preceding siblings ...)
2019-09-30 9:55 ` [PATCH v2 07/13] msvc: work around a bug in GetEnvironmentVariable() Johannes Schindelin via GitGitGadget
@ 2019-09-30 9:55 ` Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 09/13] vcxproj: include more generated files Johannes Schindelin via GitGitGadget
` (5 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-30 9:55 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
In b18ae14a8f6 (vcxproj: also link-or-copy builtins, 2019-07-29), we
started to copy or hard-link the built-ins as a post-build step of the
`git` project.
At the same time, we tried to copy or hard-link `git-remote-http.exe`,
but it is quite possible that it was not built at that time.
Let's move that latter task into a post-install step of the
`git-remote-http` project instead.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
config.mak.uname | 10 +++++++---
contrib/buildsystems/Generators/Vcxproj.pm | 3 +++
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/config.mak.uname b/config.mak.uname
index db7f06b95f..701aad62b1 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -703,20 +703,24 @@ vcxproj:
perl contrib/buildsystems/generate -g Vcxproj
git add -f git.sln {*,*/lib,t/helper/*}/*.vcxproj
- # Generate the LinkOrCopyBuiltins.targets file
+ # Generate the LinkOrCopyBuiltins.targets and LinkOrCopyRemoteHttp.targets file
(echo '<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">' && \
echo ' <Target Name="CopyBuiltins_AfterBuild" AfterTargets="AfterBuild">' && \
for name in $(BUILT_INS);\
do \
echo ' <Copy SourceFiles="$$(OutDir)\git.exe" DestinationFiles="$$(OutDir)\'"$$name"'" SkipUnchangedFiles="true" UseHardlinksIfPossible="true" />'; \
done && \
+ echo ' </Target>' && \
+ echo '</Project>') >git/LinkOrCopyBuiltins.targets
+ (echo '<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">' && \
+ echo ' <Target Name="CopyBuiltins_AfterBuild" AfterTargets="AfterBuild">' && \
for name in $(REMOTE_CURL_ALIASES); \
do \
echo ' <Copy SourceFiles="$$(OutDir)\'"$(REMOTE_CURL_PRIMARY)"'" DestinationFiles="$$(OutDir)\'"$$name"'" SkipUnchangedFiles="true" UseHardlinksIfPossible="true" />'; \
done && \
echo ' </Target>' && \
- echo '</Project>') >git/LinkOrCopyBuiltins.targets
- git add -f git/LinkOrCopyBuiltins.targets
+ echo '</Project>') >git-remote-http/LinkOrCopyRemoteHttp.targets
+ git add -f git/LinkOrCopyBuiltins.targets git-remote-http/LinkOrCopyRemoteHttp.targets
# Add command-list.h
$(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 command-list.h
diff --git a/contrib/buildsystems/Generators/Vcxproj.pm b/contrib/buildsystems/Generators/Vcxproj.pm
index 576ccabe1d..868b787855 100644
--- a/contrib/buildsystems/Generators/Vcxproj.pm
+++ b/contrib/buildsystems/Generators/Vcxproj.pm
@@ -277,6 +277,9 @@ sub createProject {
if ($target eq 'git') {
print F " <Import Project=\"LinkOrCopyBuiltins.targets\" />\n";
}
+ if ($target eq 'git-remote-http') {
+ print F " <Import Project=\"LinkOrCopyRemoteHttp.targets\" />\n";
+ }
print F << "EOM";
</Project>
EOM
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH v2 10/13] test-tool run-command: learn to run (parts of) the testsuite
2019-09-30 9:55 ` [PATCH v2 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (8 preceding siblings ...)
2019-09-30 9:55 ` [PATCH v2 09/13] vcxproj: include more generated files Johannes Schindelin via GitGitGadget
@ 2019-09-30 9:55 ` Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 11/13] tests: let --immediate and --write-junit-xml play well together Johannes Schindelin via GitGitGadget
` (3 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-30 9:55 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Git for Windows jumps through hoops to provide a development environment
that allows to build Git and to run its test suite. To that end, an
entire MSYS2 system, including GNU make and GCC is offered as "the Git
for Windows SDK". It does come at a price: an initial download of said
SDK weighs in with several hundreds of megabytes, and the unpacked SDK
occupies ~2GB of disk space.
A much more native development environment on Windows is Visual Studio.
To help contributors use that environment, we already have a Makefile
target `vcxproj` that generates a commit with project files (and other
generated files), and Git for Windows' `vs/master` branch is
continuously re-generated using that target.
The idea is to allow building Git in Visual Studio, and to run
individual tests using a Portable Git.
The one missing thing is a way to run the entire test suite: neither
`make` nor `prove` are required to run Git, therefore Git for Windows
does not support those commands in the Portable Git.
To help with that, add a simple test helper that exercises the
`run_processes_parallel()` function to allow for running test scripts in
parallel (which is really necessary, especially on Windows, as Git's
test suite takes such a long time to run).
This will also come in handy for the upcoming change to our Azure
Pipeline: we will use this helper in a Portable Git to test the Visual
Studio build of Git.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/helper/test-run-command.c | 153 ++++++++++++++++++++++++++++++++++++
1 file changed, 153 insertions(+)
diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c
index 2cc93bb69c..ead6dc611a 100644
--- a/t/helper/test-run-command.c
+++ b/t/helper/test-run-command.c
@@ -10,9 +10,14 @@
#include "test-tool.h"
#include "git-compat-util.h"
+#include "cache.h"
#include "run-command.h"
#include "argv-array.h"
#include "strbuf.h"
+#include "parse-options.h"
+#include "string-list.h"
+#include "thread-utils.h"
+#include "wildmatch.h"
#include <string.h>
#include <errno.h>
@@ -50,11 +55,159 @@ static int task_finished(int result,
return 1;
}
+struct testsuite {
+ struct string_list tests, failed;
+ int next;
+ int quiet, immediate, verbose, verbose_log, trace, write_junit_xml;
+};
+#define TESTSUITE_INIT \
+ { STRING_LIST_INIT_DUP, STRING_LIST_INIT_DUP, -1, 0, 0, 0, 0, 0, 0 }
+
+static int next_test(struct child_process *cp, struct strbuf *err, void *cb,
+ void **task_cb)
+{
+ struct testsuite *suite = cb;
+ const char *test;
+ if (suite->next >= suite->tests.nr)
+ return 0;
+
+ test = suite->tests.items[suite->next++].string;
+ argv_array_pushl(&cp->args, "sh", test, NULL);
+ if (suite->quiet)
+ argv_array_push(&cp->args, "--quiet");
+ if (suite->immediate)
+ argv_array_push(&cp->args, "-i");
+ if (suite->verbose)
+ argv_array_push(&cp->args, "-v");
+ if (suite->verbose_log)
+ argv_array_push(&cp->args, "-V");
+ if (suite->trace)
+ argv_array_push(&cp->args, "-x");
+ if (suite->write_junit_xml)
+ argv_array_push(&cp->args, "--write-junit-xml");
+
+ strbuf_addf(err, "Output of '%s':\n", test);
+ *task_cb = (void *)test;
+
+ return 1;
+}
+
+static int test_finished(int result, struct strbuf *err, void *cb,
+ void *task_cb)
+{
+ struct testsuite *suite = cb;
+ const char *name = (const char *)task_cb;
+
+ if (result)
+ string_list_append(&suite->failed, name);
+
+ strbuf_addf(err, "%s: '%s'\n", result ? "FAIL" : "SUCCESS", name);
+
+ return 0;
+}
+
+static int test_failed(struct strbuf *out, void *cb, void *task_cb)
+{
+ struct testsuite *suite = cb;
+ const char *name = (const char *)task_cb;
+
+ string_list_append(&suite->failed, name);
+ strbuf_addf(out, "FAILED TO START: '%s'\n", name);
+
+ return 0;
+}
+
+static const char * const testsuite_usage[] = {
+ "test-run-command testsuite [<options>] [<pattern>...]",
+ NULL
+};
+
+static int testsuite(int argc, const char **argv)
+{
+ struct testsuite suite = TESTSUITE_INIT;
+ int max_jobs = 1, i, ret;
+ DIR *dir;
+ struct dirent *d;
+ struct option options[] = {
+ OPT_BOOL('i', "immediate", &suite.immediate,
+ "stop at first failed test case(s)"),
+ OPT_INTEGER('j', "jobs", &max_jobs, "run <N> jobs in parallel"),
+ OPT_BOOL('q', "quiet", &suite.quiet, "be terse"),
+ OPT_BOOL('v', "verbose", &suite.verbose, "be verbose"),
+ OPT_BOOL('V', "verbose-log", &suite.verbose_log,
+ "be verbose, redirected to a file"),
+ OPT_BOOL('x', "trace", &suite.trace, "trace shell commands"),
+ OPT_BOOL(0, "write-junit-xml", &suite.write_junit_xml,
+ "write JUnit-style XML files"),
+ OPT_END()
+ };
+
+ memset(&suite, 0, sizeof(suite));
+ suite.tests.strdup_strings = suite.failed.strdup_strings = 1;
+
+ argc = parse_options(argc, argv, NULL, options,
+ testsuite_usage, PARSE_OPT_STOP_AT_NON_OPTION);
+
+ if (max_jobs <= 0)
+ max_jobs = online_cpus();
+
+ dir = opendir(".");
+ if (!dir)
+ die("Could not open the current directory");
+ while ((d = readdir(dir))) {
+ const char *p = d->d_name;
+
+ if (*p != 't' || !isdigit(p[1]) || !isdigit(p[2]) ||
+ !isdigit(p[3]) || !isdigit(p[4]) || p[5] != '-' ||
+ !ends_with(p, ".sh"))
+ continue;
+
+ /* No pattern: match all */
+ if (!argc) {
+ string_list_append(&suite.tests, p);
+ continue;
+ }
+
+ for (i = 0; i < argc; i++)
+ if (!wildmatch(argv[i], p, 0)) {
+ string_list_append(&suite.tests, p);
+ break;
+ }
+ }
+ closedir(dir);
+
+ if (!suite.tests.nr)
+ die("No tests match!");
+ if (max_jobs > suite.tests.nr)
+ max_jobs = suite.tests.nr;
+
+ fprintf(stderr, "Running %d tests (%d at a time)\n",
+ suite.tests.nr, max_jobs);
+
+ ret = run_processes_parallel(max_jobs, next_test, test_failed,
+ test_finished, &suite);
+
+ if (suite.failed.nr > 0) {
+ ret = 1;
+ fprintf(stderr, "%d tests failed:\n\n", suite.failed.nr);
+ for (i = 0; i < suite.failed.nr; i++)
+ fprintf(stderr, "\t%s\n", suite.failed.items[i].string);
+ }
+
+ string_list_clear(&suite.tests, 0);
+ string_list_clear(&suite.failed, 0);
+
+ return !!ret;
+}
+
int cmd__run_command(int argc, const char **argv)
{
struct child_process proc = CHILD_PROCESS_INIT;
int jobs;
+ if (argc > 1 && !strcmp(argv[1], "testsuite"))
+ exit(testsuite(argc - 1, argv + 1));
+
if (argc < 3)
return 1;
while (!strcmp(argv[1], "env")) {
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH v2 11/13] tests: let --immediate and --write-junit-xml play well together
2019-09-30 9:55 ` [PATCH v2 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (9 preceding siblings ...)
2019-09-30 9:55 ` [PATCH v2 10/13] test-tool run-command: learn to run (parts of) the testsuite Johannes Schindelin via GitGitGadget
@ 2019-09-30 9:55 ` Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 12/13] ci: really use shallow clones on Azure Pipelines Johannes Schindelin via GitGitGadget
` (2 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-30 9:55 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
When the `--immediate` option is in effect, any test failure will
immediately exit the test script. Together with `--write-junit-xml`, we
will want the JUnit-style `.xml` file to be finalized (and not leave the
XML incomplete). Let's make it so.
This comes in particularly handy when trying to debug via Azure
Pipelines, where the JUnit-style XML is consumed to present the test
results in an informative and helpful way.
While at it, also handle the `error()` code path.
The only remaining code path that sets `GIT_EXIT_OK` happens whenever
the trash directory could not be set up, i.e. long before the JUnit XML
was written, therefore we should _not_ try to finalize that XML in that
case.
It is tempting to change the `immediate` code path to just hand off to
`error`, simplifying the code in the process. That would, however,
result in a change of behavior (an additional error message) in the test
suite, which is outside of the purview of the current patch series: its
goal is to allow building Git with Visual Studio and testing it with a
portable version of Git for Windows.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/test-lib.sh | 38 ++++++++++++++++++++++----------------
1 file changed, 22 insertions(+), 16 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index d1ba33745a..86b5e6133b 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -567,6 +567,7 @@ export TERM
error () {
say_color error "error: $*"
+ finalize_junit_xml
GIT_EXIT_OK=t
exit 1
}
@@ -695,7 +696,7 @@ test_failure_ () {
say_color error "not ok $test_count - $1"
shift
printf '%s\n' "$*" | sed -e 's/^/# /'
- test "$immediate" = "" || { GIT_EXIT_OK=t; exit 1; }
+ test "$immediate" = "" || { finalize_junit_xml; GIT_EXIT_OK=t; exit 1; }
}
test_known_broken_ok_ () {
@@ -1063,6 +1064,25 @@ write_junit_xml_testcase () {
junit_have_testcase=t
}
+finalize_junit_xml () {
+ if test -n "$write_junit_xml" && test -n "$junit_xml_path"
+ then
+ test -n "$junit_have_testcase" || {
+ junit_start=$(test-tool date getnanos)
+ write_junit_xml_testcase "all tests skipped"
+ }
+
+ # adjust the overall time
+ junit_time=$(test-tool date getnanos $junit_suite_start)
+ sed "s/<testsuite [^>]*/& time=\"$junit_time\"/" \
+ <"$junit_xml_path" >"$junit_xml_path.new"
+ mv "$junit_xml_path.new" "$junit_xml_path"
+
+ write_junit_xml " </testsuite>" "</testsuites>"
+ write_junit_xml=
+ fi
+}
+
test_atexit_cleanup=:
test_atexit_handler () {
# In a succeeding test script 'test_atexit_handler' is invoked
@@ -1085,21 +1105,7 @@ test_done () {
# removed, so the commands can access pidfiles and socket files.
test_atexit_handler
- if test -n "$write_junit_xml" && test -n "$junit_xml_path"
- then
- test -n "$junit_have_testcase" || {
- junit_start=$(test-tool date getnanos)
- write_junit_xml_testcase "all tests skipped"
- }
-
- # adjust the overall time
- junit_time=$(test-tool date getnanos $junit_suite_start)
- sed "s/<testsuite [^>]*/& time=\"$junit_time\"/" \
- <"$junit_xml_path" >"$junit_xml_path.new"
- mv "$junit_xml_path.new" "$junit_xml_path"
-
- write_junit_xml " </testsuite>" "</testsuites>"
- fi
+ finalize_junit_xml
if test -z "$HARNESS_ACTIVE"
then
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH v2 13/13] ci: also build and test with MS Visual Studio on Azure Pipelines
2019-09-30 9:55 ` [PATCH v2 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (11 preceding siblings ...)
2019-09-30 9:55 ` [PATCH v2 12/13] ci: really use shallow clones on Azure Pipelines Johannes Schindelin via GitGitGadget
@ 2019-09-30 9:55 ` Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-30 9:55 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
... because we can, now. Technically, we actually build using `MSBuild`,
which is however pretty close to building interactively in Visual
Studio.
As there is no convenient way to run Git's test suite in Visual Studio,
we unpack a Portable Git to run it, using the just-added test helper to
allow running test scripts in parallel.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Makefile | 4 ++
azure-pipelines.yml | 159 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 163 insertions(+)
diff --git a/Makefile b/Makefile
index 3716dadc08..3f6dcec54b 100644
--- a/Makefile
+++ b/Makefile
@@ -3025,6 +3025,10 @@ rpm::
@false
.PHONY: rpm
+ifneq ($(INCLUDE_DLLS_IN_ARTIFACTS),)
+OTHER_PROGRAMS += $(shell echo *.dll t/helper/*.dll)
+endif
+
artifacts-tar:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) \
GIT-BUILD-OPTIONS $(TEST_PROGRAMS) $(test_bindir_programs) \
$(MOFILES)
diff --git a/azure-pipelines.yml b/azure-pipelines.yml
index 55ee23ad0f..875e63cac1 100644
--- a/azure-pipelines.yml
+++ b/azure-pipelines.yml
@@ -130,6 +130,165 @@ jobs:
PathtoPublish: t/failed-test-artifacts
ArtifactName: failed-test-artifacts
+- job: vs_build
+ displayName: Visual Studio Build
+ condition: succeeded()
+ pool: Hosted VS2017
+ timeoutInMinutes: 240
+ steps:
+ - powershell: |
+ if ("$GITFILESHAREPWD" -ne "" -and "$GITFILESHAREPWD" -ne "`$`(gitfileshare.pwd)") {
+ net use s: \\gitfileshare.file.core.windows.net\test-cache "$GITFILESHAREPWD" /user:AZURE\gitfileshare /persistent:no
+ cmd /c mklink /d "$(Build.SourcesDirectory)\test-cache" S:\
+ }
+ displayName: 'Mount test-cache'
+ env:
+ GITFILESHAREPWD: $(gitfileshare.pwd)
+ - powershell: |
+ $urlbase = "https://dev.azure.com/git-for-windows/git/_apis/build/builds"
+ $id = ((Invoke-WebRequest -UseBasicParsing "${urlbase}?definitions=22&statusFilter=completed&resultFilter=succeeded&`$top=1").content | ConvertFrom-JSON).value[0].id
+ $downloadUrl = ((Invoke-WebRequest -UseBasicParsing "${urlbase}/$id/artifacts").content | ConvertFrom-JSON).value[1].resource.downloadUrl
+ (New-Object Net.WebClient).DownloadFile($downloadUrl,"git-sdk-64-minimal.zip")
+ Expand-Archive git-sdk-64-minimal.zip -DestinationPath . -Force
+ Remove-Item git-sdk-64-minimal.zip
+
+ # Let Git ignore the SDK and the test-cache
+ "/git-sdk-64-minimal/`n/test-cache/`n" | Out-File -NoNewLine -Encoding ascii -Append "$(Build.SourcesDirectory)\.git\info\exclude"
+ displayName: 'Download git-sdk-64-minimal'
+ - powershell: |
+ & git-sdk-64-minimal\usr\bin\bash.exe -lc @"
+ make vcxproj
+ "@
+ if (!$?) { exit(1) }
+ displayName: Generate Visual Studio Solution
+ env:
+ HOME: $(Build.SourcesDirectory)
+ MSYSTEM: MINGW64
+ DEVELOPER: 1
+ NO_PERL: 1
+ GIT_CONFIG_PARAMETERS: "'user.name=CI' 'user.email=ci@git'"
+ - powershell: |
+ $urlbase = "https://dev.azure.com/git/git/_apis/build/builds"
+ $id = ((Invoke-WebRequest -UseBasicParsing "${urlbase}?definitions=9&statusFilter=completed&resultFilter=succeeded&`$top=1").content | ConvertFrom-JSON).value[0].id
+ $downloadUrl = ((Invoke-WebRequest -UseBasicParsing "${urlbase}/$id/artifacts").content | ConvertFrom-JSON).value[0].resource.downloadUrl
+ (New-Object Net.WebClient).DownloadFile($downloadUrl, "compat.zip")
+ Expand-Archive compat.zip -DestinationPath . -Force
+ Remove-Item compat.zip
+ displayName: 'Download vcpkg artifacts'
+ - task: MSBuild@1
+ inputs:
+ solution: git.sln
+ platform: x64
+ configuration: Release
+ maximumCpuCount: 4
+ - powershell: |
+ & compat\vcbuild\vcpkg_copy_dlls.bat release
+ if (!$?) { exit(1) }
+ & git-sdk-64-minimal\usr\bin\bash.exe -lc @"
+ mkdir -p artifacts &&
+ eval \"`$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts | grep ^tar)\"
+ "@
+ if (!$?) { exit(1) }
+ displayName: Bundle artifact tar
+ env:
+ HOME: $(Build.SourcesDirectory)
+ MSYSTEM: MINGW64
+ DEVELOPER: 1
+ NO_PERL: 1
+ MSVC: 1
+ VCPKG_ROOT: $(Build.SourcesDirectory)\compat\vcbuild\vcpkg
+ - powershell: |
+ $tag = (Invoke-WebRequest -UseBasicParsing "https://gitforwindows.org/latest-tag.txt").content
+ $version = (Invoke-WebRequest -UseBasicParsing "https://gitforwindows.org/latest-version.txt").content
+ $url = "https://github.com/git-for-windows/git/releases/download/${tag}/PortableGit-${version}-64-bit.7z.exe"
+ (New-Object Net.WebClient).DownloadFile($url,"PortableGit.exe")
+ & .\PortableGit.exe -y -oartifacts\PortableGit
+ # Wait until it is unpacked
+ while (-not @(Remove-Item -ErrorAction SilentlyContinue PortableGit.exe; $?)) { sleep 1 }
+ displayName: Download & extract portable Git
+ - task: PublishPipelineArtifact@0
+ displayName: 'Publish Pipeline Artifact: MSVC test artifacts'
+ inputs:
+ artifactName: 'vs-artifacts'
+ targetPath: '$(Build.SourcesDirectory)\artifacts'
+ - powershell: |
+ if ("$GITFILESHAREPWD" -ne "" -and "$GITFILESHAREPWD" -ne "`$`(gitfileshare.pwd)") {
+ cmd /c rmdir "$(Build.SourcesDirectory)\test-cache"
+ }
+ displayName: 'Unmount test-cache'
+ condition: true
+ env:
+ GITFILESHAREPWD: $(gitfileshare.pwd)
+
+- job: vs_test
+ displayName: Visual Studio Test
+ dependsOn: vs_build
+ condition: succeeded()
+ pool: Hosted
+ timeoutInMinutes: 240
+ strategy:
+ parallel: 10
+ steps:
+ - powershell: |
+ if ("$GITFILESHAREPWD" -ne "" -and "$GITFILESHAREPWD" -ne "`$`(gitfileshare.pwd)") {
+ net use s: \\gitfileshare.file.core.windows.net\test-cache "$GITFILESHAREPWD" /user:AZURE\gitfileshare /persistent:no
+ cmd /c mklink /d "$(Build.SourcesDirectory)\test-cache" S:\
+ }
+ displayName: 'Mount test-cache'
+ env:
+ GITFILESHAREPWD: $(gitfileshare.pwd)
+ - task: DownloadPipelineArtifact@0
+ displayName: 'Download Pipeline Artifact: VS test artifacts'
+ inputs:
+ artifactName: 'vs-artifacts'
+ targetPath: '$(Build.SourcesDirectory)'
+ - powershell: |
+ & PortableGit\git-cmd.exe --command=usr\bin\bash.exe -lc @"
+ test -f artifacts.tar.gz || {
+ echo No test artifacts found\; skipping >&2
+ exit 0
+ }
+ tar xf artifacts.tar.gz || exit 1
+
+ # Let Git ignore the SDK and the test-cache
+ printf '%s\n' /PortableGit/ /test-cache/ >>.git/info/exclude
+
+ cd t &&
+ PATH=\"`$PWD/helper:`$PATH\" &&
+ test-tool.exe run-command testsuite -V -x --write-junit-xml \
+ `$(test-tool.exe path-utils slice-tests \
+ `$SYSTEM_JOBPOSITIONINPHASE `$SYSTEM_TOTALJOBSINPHASE t[0-9]*.sh)
+ "@
+ if (!$?) { exit(1) }
+ displayName: 'Test (parallel)'
+ env:
+ HOME: $(Build.SourcesDirectory)
+ MSYSTEM: MINGW64
+ NO_SVN_TESTS: 1
+ GIT_TEST_SKIP_REBASE_P: 1
+ - powershell: |
+ if ("$GITFILESHAREPWD" -ne "" -and "$GITFILESHAREPWD" -ne "`$`(gitfileshare.pwd)") {
+ cmd /c rmdir "$(Build.SourcesDirectory)\test-cache"
+ }
+ displayName: 'Unmount test-cache'
+ condition: true
+ env:
+ GITFILESHAREPWD: $(gitfileshare.pwd)
+ - task: PublishTestResults@2
+ displayName: 'Publish Test Results **/TEST-*.xml'
+ inputs:
+ mergeTestResults: true
+ testRunTitle: 'vs'
+ platform: Windows
+ publishRunAttachments: false
+ condition: succeededOrFailed()
+ - task: PublishBuildArtifacts@1
+ displayName: 'Publish trash directories of failed tests'
+ condition: failed()
+ inputs:
+ PathtoPublish: t/failed-test-artifacts
+ ArtifactName: failed-vs-test-artifacts
+
- job: linux_clang
displayName: linux-clang
condition: succeeded()
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH v2 12/13] ci: really use shallow clones on Azure Pipelines
2019-09-30 9:55 ` [PATCH v2 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (10 preceding siblings ...)
2019-09-30 9:55 ` [PATCH v2 11/13] tests: let --immediate and --write-junit-xml play well together Johannes Schindelin via GitGitGadget
@ 2019-09-30 9:55 ` Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 13/13] ci: also build and test with MS Visual Studio " Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-09-30 9:55 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
This was a left-over from the previous YAML schema, and it no longer
works. The problem was noticed while editing `azure-pipelines.yml` in VS
Code with the very helpful "Azure Pipelines" extension (syntax
highlighting and intellisense for `azure-pipelines.yml`...).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
azure-pipelines.yml | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/azure-pipelines.yml b/azure-pipelines.yml
index c329b7218b..55ee23ad0f 100644
--- a/azure-pipelines.yml
+++ b/azure-pipelines.yml
@@ -1,6 +1,5 @@
-resources:
-- repo: self
- fetchDepth: 1
+variables:
+ Agent.Source.Git.ShallowFetchDepth: 1
jobs:
- job: windows_build
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* Re: [PATCH v2 01/13] push: do not pretend to return `int` from `die_push_simple()`
2019-09-30 9:55 ` [PATCH v2 01/13] push: do not pretend to return `int` from `die_push_simple()` Johannes Schindelin via GitGitGadget
@ 2019-10-03 22:37 ` Junio C Hamano
2019-10-04 9:36 ` Johannes Schindelin
0 siblings, 1 reply; 72+ messages in thread
From: Junio C Hamano @ 2019-10-03 22:37 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Denton Liu, Johannes Schindelin
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> This function is marked as `NORETURN`, and it indeed does not want to
> return anything. So let's not declare it with the return type `int`.
> This fixes the following warning when building with MSVC:
>
> C4646: function declared with 'noreturn' has non-void return type
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> builtin/push.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/push.c b/builtin/push.c
> index 021dd3b1e4..d216270d5f 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -143,8 +143,8 @@ static int push_url_of_remote(struct remote *remote, const char ***url_p)
> return remote->url_nr;
> }
>
> -static NORETURN int die_push_simple(struct branch *branch,
> - struct remote *remote)
> +static NORETURN void die_push_simple(struct branch *branch,
> + struct remote *remote)
Haha ;-) "I won't return and I'll return int" sounds like an
oxymoron.
> {
> /*
> * There's no point in using shorten_unambiguous_ref here,
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v2 02/13] msvc: avoid using minus operator on unsigned types
2019-09-30 9:55 ` [PATCH v2 02/13] msvc: avoid using minus operator on unsigned types Johannes Schindelin via GitGitGadget
@ 2019-10-03 22:44 ` Junio C Hamano
2019-10-04 9:55 ` Johannes Schindelin
0 siblings, 1 reply; 72+ messages in thread
From: Junio C Hamano @ 2019-10-03 22:44 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Denton Liu, Johannes Schindelin
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> While at it, we take care of reporting overflows (which are unlikely,
> but hey, defensive programming is good!).
>
> We _also_ take pains of casting the unsigned value to signed: otherwise,
> the signed operand (i.e. the `-1`) would be cast to unsigned before
> doing the arithmetic.
These three look good and too similar to each other, which makes me
wonder if we want to allow them simply write
return insert_pos_as_negative_offset(nr);
with something like
static int insert_pos_as_negative_offset(uintmax_t nr)
{
if (INT_MAX < nr)
die("overflow: -1 - %"PRIuMAX, nr);
return -1 - (int)nr;
}
to avoid repetition.
> Helped-by: Denton Liu <liu.denton@gmail.com>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> read-cache.c | 9 ++++++---
> sha1-lookup.c | 12 +++++++++---
> 2 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/read-cache.c b/read-cache.c
> index c701f7f8b8..97745c2a31 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -1275,8 +1275,11 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e
> * we can avoid searching for it.
> */
> if (istate->cache_nr > 0 &&
> - strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)
> - pos = -istate->cache_nr - 1;
> + strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0) {
> + if (istate->cache_nr > INT_MAX)
> + die("overflow: -1 - %u", istate->cache_nr);
> + pos = -1 - (int)istate->cache_nr;
> + }
> else
> pos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce));
>
> @@ -1894,7 +1897,7 @@ static size_t estimate_cache_size(size_t ondisk_size, unsigned int entries)
> /*
> * Account for potential alignment differences.
> */
> - per_entry += align_padding_size(sizeof(struct cache_entry), -sizeof(struct ondisk_cache_entry));
> + per_entry += align_padding_size(per_entry, 0);
> return ondisk_size + entries * per_entry;
> }
>
> diff --git a/sha1-lookup.c b/sha1-lookup.c
> index 796ab68da8..bb786b5420 100644
> --- a/sha1-lookup.c
> +++ b/sha1-lookup.c
> @@ -69,8 +69,12 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,
> miv = take2(sha1 + ofs);
> if (miv < lov)
> return -1;
> - if (hiv < miv)
> - return -1 - nr;
> + if (hiv < miv) {
> + if (nr > INT_MAX)
> + die("overflow: -1 - %"PRIuMAX,
> + (uintmax_t)nr);
> + return -1 - (int)nr;
> + }
> if (lov != hiv) {
> /*
> * At this point miv could be equal
> @@ -97,7 +101,9 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,
> lo = mi + 1;
> mi = lo + (hi - lo) / 2;
> } while (lo < hi);
> - return -lo-1;
> + if (nr > INT_MAX)
> + die("overflow: -1 - %"PRIuMAX, (uintmax_t)lo);
> + return -1 - (int)lo;
> }
>
> int bsearch_hash(const unsigned char *sha1, const uint32_t *fanout_nbo,
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v2 01/13] push: do not pretend to return `int` from `die_push_simple()`
2019-10-03 22:37 ` Junio C Hamano
@ 2019-10-04 9:36 ` Johannes Schindelin
0 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin @ 2019-10-04 9:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin via GitGitGadget, git, Denton Liu
Hi Junio,
On Fri, 4 Oct 2019, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > This function is marked as `NORETURN`, and it indeed does not want to
> > return anything. So let's not declare it with the return type `int`.
> > This fixes the following warning when building with MSVC:
> >
> > C4646: function declared with 'noreturn' has non-void return type
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > builtin/push.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/builtin/push.c b/builtin/push.c
> > index 021dd3b1e4..d216270d5f 100644
> > --- a/builtin/push.c
> > +++ b/builtin/push.c
> > @@ -143,8 +143,8 @@ static int push_url_of_remote(struct remote *remote, const char ***url_p)
> > return remote->url_nr;
> > }
> >
> > -static NORETURN int die_push_simple(struct branch *branch,
> > - struct remote *remote)
> > +static NORETURN void die_push_simple(struct branch *branch,
> > + struct remote *remote)
>
> Haha ;-) "I won't return and I'll return int" sounds like an
> oxymoron.
Funny, right? ;-)
I was a bit concerned that GCC did not catch it, nor Clang, but hey,
that's the benefit of compiling with more than just one compiler.
Ciao,
Dscho
>
> > {
> > /*
> > * There's no point in using shorten_unambiguous_ref here,
>
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v2 02/13] msvc: avoid using minus operator on unsigned types
2019-10-03 22:44 ` Junio C Hamano
@ 2019-10-04 9:55 ` Johannes Schindelin
2019-10-04 17:09 ` Johannes Sixt
0 siblings, 1 reply; 72+ messages in thread
From: Johannes Schindelin @ 2019-10-04 9:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin via GitGitGadget, git, Denton Liu
Hi Junio,
On Fri, 4 Oct 2019, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > While at it, we take care of reporting overflows (which are unlikely,
> > but hey, defensive programming is good!).
> >
> > We _also_ take pains of casting the unsigned value to signed: otherwise,
> > the signed operand (i.e. the `-1`) would be cast to unsigned before
> > doing the arithmetic.
>
> These three look good and too similar to each other, which makes me
> wonder if we want to allow them simply write
>
> return insert_pos_as_negative_offset(nr);
>
> with something like
>
> static int insert_pos_as_negative_offset(uintmax_t nr)
> {
> if (INT_MAX < nr)
> die("overflow: -1 - %"PRIuMAX, nr);
> return -1 - (int)nr;
> }
>
> to avoid repetition.
I tried not to do that because there are two different data types in
play: `unsigned int` and `size_t`. But I guess by making this an
`inline` function, compilers can optimize for the common case and avoid
casting _twice_.
Will be fixed in v2,
Dscho
>
> > Helped-by: Denton Liu <liu.denton@gmail.com>
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > read-cache.c | 9 ++++++---
> > sha1-lookup.c | 12 +++++++++---
> > 2 files changed, 15 insertions(+), 6 deletions(-)
> >
> > diff --git a/read-cache.c b/read-cache.c
> > index c701f7f8b8..97745c2a31 100644
> > --- a/read-cache.c
> > +++ b/read-cache.c
> > @@ -1275,8 +1275,11 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e
> > * we can avoid searching for it.
> > */
> > if (istate->cache_nr > 0 &&
> > - strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)
> > - pos = -istate->cache_nr - 1;
> > + strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0) {
> > + if (istate->cache_nr > INT_MAX)
> > + die("overflow: -1 - %u", istate->cache_nr);
> > + pos = -1 - (int)istate->cache_nr;
> > + }
> > else
> > pos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce));
> >
> > @@ -1894,7 +1897,7 @@ static size_t estimate_cache_size(size_t ondisk_size, unsigned int entries)
> > /*
> > * Account for potential alignment differences.
> > */
> > - per_entry += align_padding_size(sizeof(struct cache_entry), -sizeof(struct ondisk_cache_entry));
> > + per_entry += align_padding_size(per_entry, 0);
> > return ondisk_size + entries * per_entry;
> > }
> >
> > diff --git a/sha1-lookup.c b/sha1-lookup.c
> > index 796ab68da8..bb786b5420 100644
> > --- a/sha1-lookup.c
> > +++ b/sha1-lookup.c
> > @@ -69,8 +69,12 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,
> > miv = take2(sha1 + ofs);
> > if (miv < lov)
> > return -1;
> > - if (hiv < miv)
> > - return -1 - nr;
> > + if (hiv < miv) {
> > + if (nr > INT_MAX)
> > + die("overflow: -1 - %"PRIuMAX,
> > + (uintmax_t)nr);
> > + return -1 - (int)nr;
> > + }
> > if (lov != hiv) {
> > /*
> > * At this point miv could be equal
> > @@ -97,7 +101,9 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,
> > lo = mi + 1;
> > mi = lo + (hi - lo) / 2;
> > } while (lo < hi);
> > - return -lo-1;
> > + if (nr > INT_MAX)
> > + die("overflow: -1 - %"PRIuMAX, (uintmax_t)lo);
> > + return -1 - (int)lo;
> > }
> >
> > int bsearch_hash(const unsigned char *sha1, const uint32_t *fanout_nbo,
>
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline
2019-09-30 9:55 ` [PATCH v2 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (12 preceding siblings ...)
2019-09-30 9:55 ` [PATCH v2 13/13] ci: also build and test with MS Visual Studio " Johannes Schindelin via GitGitGadget
@ 2019-10-04 15:09 ` Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 01/13] push: do not pretend to return `int` from `die_push_simple()` Johannes Schindelin via GitGitGadget
` (13 more replies)
13 siblings, 14 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-10-04 15:09 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano
Git's Continuous Integration (CI) includes an Azure Pipeline that builds Git
on Linux, macOS and Windows, in the former two cases even in multiple
configurations (using GCC vs clang, 32-bit vs 64-bit, etc). On Windows, we
only build using GCC, using (a subset of) Git for Windows' SDK.
Recently, a patch series made it into Git that re-instates the ability to
generate project files for use with Visual Studio. The idea there being:
contributors can check out a special branch that has those generated files
in one generated commit on top of e.g. Git for Windows' master, allowing the
contributors to build Git in Visual Studio, without the need for downloading
Git for Windows' SDK (which weighs quite a bit: ~600MB download, ~2GB disk
footprint). The tests can then be run from a regular Git for Windows Bash.
This patch series adds that axis to Git's Azure Pipeline: the project files
are generated, MSBuild (which is kind of the command-line equivalent of
Visual Studio's "Build" operation) is used to build Git, and then a
parallelized test job runs the test suite in a Portable Git.
These patches are based on js/visual-studio.
Changes since v2:
* The overflow check introduced in v1 was consolidated into a single
helper.
Changes since v1:
* "While at it", we now also check for overflows when doing that -1 -
<unsigned> arithmetic.
* The JUnit-style XML is finalized also in case that the script aborts e.g.
due to an incorrect number of arguments in a test_expect_success call.
Johannes Schindelin (13):
push: do not pretend to return `int` from `die_push_simple()`
msvc: avoid using minus operator on unsigned types
winansi: use FLEX_ARRAY to avoid compiler warning
compat/win32/path-utils.h: add #include guards
msvc: ignore some libraries when linking
msvc: handle DEVELOPER=1
msvc: work around a bug in GetEnvironmentVariable()
vcxproj: only copy `git-remote-http.exe` once it was built
vcxproj: include more generated files
test-tool run-command: learn to run (parts of) the testsuite
tests: let --immediate and --write-junit-xml play well together
ci: really use shallow clones on Azure Pipelines
ci: also build and test with MS Visual Studio on Azure Pipelines
Makefile | 4 +
azure-pipelines.yml | 164 ++++++++++++++++++++-
builtin/push.c | 4 +-
cache.h | 13 ++
compat/mingw.c | 2 +
compat/vcbuild/scripts/clink.pl | 48 +++++-
compat/win32/path-utils.h | 5 +
compat/winansi.c | 2 +-
config.mak.uname | 19 ++-
contrib/buildsystems/Generators/Vcxproj.pm | 3 +
read-cache.c | 4 +-
sha1-lookup.c | 4 +-
t/helper/test-run-command.c | 153 +++++++++++++++++++
t/test-lib.sh | 38 +++--
14 files changed, 430 insertions(+), 33 deletions(-)
base-commit: aac6ff7b5beeea9bca66ecda6eec60fc1dd447ec
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-288%2Fdscho%2Fazure-pipelines-msvc-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-288/dscho/azure-pipelines-msvc-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/288
Range-diff vs v2:
1: 4d0b38125a = 1: 4d0b38125a push: do not pretend to return `int` from `die_push_simple()`
2: 8800320590 < -: ---------- msvc: avoid using minus operator on unsigned types
-: ---------- > 2: 7fe2a85506 msvc: avoid using minus operator on unsigned types
3: 8512a3e96d = 3: e632a4eef4 winansi: use FLEX_ARRAY to avoid compiler warning
4: 0345b08f54 = 4: cd96d7ff70 compat/win32/path-utils.h: add #include guards
5: 5add01f8ff = 5: bf09257f65 msvc: ignore some libraries when linking
6: 5c880f923e = 6: 39c707464c msvc: handle DEVELOPER=1
7: 1f2245a228 = 7: 91b09cfdd8 msvc: work around a bug in GetEnvironmentVariable()
8: 582b299634 = 8: cca891450d vcxproj: only copy `git-remote-http.exe` once it was built
9: 38ccf999e7 = 9: e6e60b3c2b vcxproj: include more generated files
10: 24b1c7bff3 = 10: d2c4973431 test-tool run-command: learn to run (parts of) the testsuite
11: 7be13d19e1 = 11: 0644a2f8da tests: let --immediate and --write-junit-xml play well together
12: bde1e8ef65 = 12: 4495c392fd ci: really use shallow clones on Azure Pipelines
13: 7af1c01a08 = 13: b1ff8eff4d ci: also build and test with MS Visual Studio on Azure Pipelines
--
gitgitgadget
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH v3 01/13] push: do not pretend to return `int` from `die_push_simple()`
2019-10-04 15:09 ` [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
@ 2019-10-04 15:09 ` Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 02/13] msvc: avoid using minus operator on unsigned types Johannes Schindelin via GitGitGadget
` (12 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-10-04 15:09 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
This function is marked as `NORETURN`, and it indeed does not want to
return anything. So let's not declare it with the return type `int`.
This fixes the following warning when building with MSVC:
C4646: function declared with 'noreturn' has non-void return type
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/push.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/push.c b/builtin/push.c
index 021dd3b1e4..d216270d5f 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -143,8 +143,8 @@ static int push_url_of_remote(struct remote *remote, const char ***url_p)
return remote->url_nr;
}
-static NORETURN int die_push_simple(struct branch *branch,
- struct remote *remote)
+static NORETURN void die_push_simple(struct branch *branch,
+ struct remote *remote)
{
/*
* There's no point in using shorten_unambiguous_ref here,
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH v3 02/13] msvc: avoid using minus operator on unsigned types
2019-10-04 15:09 ` [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 01/13] push: do not pretend to return `int` from `die_push_simple()` Johannes Schindelin via GitGitGadget
@ 2019-10-04 15:09 ` Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 03/13] winansi: use FLEX_ARRAY to avoid compiler warning Johannes Schindelin via GitGitGadget
` (11 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-10-04 15:09 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
MSVC complains about this with `-Wall`, which can be taken as a sign
that this is indeed a real bug. The symptom is:
C4146: unary minus operator applied to unsigned type, result
still unsigned
Let's avoid this warning in the minimal way, e.g. writing `-1 -
<unsigned value>` instead of `-<unsigned value> - 1`.
Note that the change in the `estimate_cache_size()` function is
needed because MSVC considers the "return type" of the `sizeof()`
operator to be `size_t`, i.e. unsigned, and therefore it cannot be
negated using the unary minus operator.
Even worse, that arithmetic is doing extra work, in vain. We want to
calculate the entry extra cache size as the difference between the
size of the `cache_entry` structure minus the size of the
`ondisk_cache_entry` structure, padded to the appropriate alignment
boundary.
To that end, we start by assigning that difference to the `per_entry`
variable, and then abuse the `len` parameter of the
`align_padding_size()` macro to take the negative size of the ondisk
entry size. Essentially, we try to avoid passing the already calculated
difference to that macro by passing the operands of that difference
instead, when the macro expects operands of an addition:
#define align_padding_size(size, len) \
((size + (len) + 8) & ~7) - (size + len)
Currently, we pass A and -B to that macro instead of passing A - B and
0, where A - B is already stored in the `per_entry` variable, ready to
be used.
This is neither necessary, nor intuitive. Let's fix this, and have code
that is both easier to read and that also does not trigger MSVC's
warning.
While at it, we take care of reporting overflows (which are unlikely,
but hey, defensive programming is good!).
We _also_ take pains of casting the unsigned value to signed: otherwise,
the signed operand (i.e. the `-1`) would be cast to unsigned before
doing the arithmetic.
Helped-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
cache.h | 13 +++++++++++++
read-cache.c | 4 ++--
sha1-lookup.c | 4 ++--
3 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/cache.h b/cache.h
index 3167585cab..850e7b945a 100644
--- a/cache.h
+++ b/cache.h
@@ -725,6 +725,19 @@ struct cache_entry *index_file_exists(struct index_state *istate, const char *na
*/
int index_name_pos(const struct index_state *, const char *name, int namelen);
+/*
+ * Some functions return the negative complement of an insert position when a
+ * precise match was not found but a position was found where the entry would
+ * need to be inserted. This helper protects that logic from any integer
+ * underflow.
+ */
+static inline int index_pos_to_insert_pos(uintmax_t pos)
+{
+ if (pos > INT_MAX)
+ die("overflow: -1 - %"PRIuMAX, pos);
+ return -1 - (int)pos;
+}
+
#define ADD_CACHE_OK_TO_ADD 1 /* Ok to add */
#define ADD_CACHE_OK_TO_REPLACE 2 /* Ok to replace file/directory */
#define ADD_CACHE_SKIP_DFCHECK 4 /* Ok to skip DF conflict checks */
diff --git a/read-cache.c b/read-cache.c
index c701f7f8b8..ec13953a21 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1276,7 +1276,7 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e
*/
if (istate->cache_nr > 0 &&
strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)
- pos = -istate->cache_nr - 1;
+ pos = index_pos_to_insert_pos(istate->cache_nr);
else
pos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce));
@@ -1894,7 +1894,7 @@ static size_t estimate_cache_size(size_t ondisk_size, unsigned int entries)
/*
* Account for potential alignment differences.
*/
- per_entry += align_padding_size(sizeof(struct cache_entry), -sizeof(struct ondisk_cache_entry));
+ per_entry += align_padding_size(per_entry, 0);
return ondisk_size + entries * per_entry;
}
diff --git a/sha1-lookup.c b/sha1-lookup.c
index 796ab68da8..8590aac254 100644
--- a/sha1-lookup.c
+++ b/sha1-lookup.c
@@ -70,7 +70,7 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,
if (miv < lov)
return -1;
if (hiv < miv)
- return -1 - nr;
+ return index_pos_to_insert_pos(nr);
if (lov != hiv) {
/*
* At this point miv could be equal
@@ -97,7 +97,7 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,
lo = mi + 1;
mi = lo + (hi - lo) / 2;
} while (lo < hi);
- return -lo-1;
+ return index_pos_to_insert_pos(lo);
}
int bsearch_hash(const unsigned char *sha1, const uint32_t *fanout_nbo,
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH v3 03/13] winansi: use FLEX_ARRAY to avoid compiler warning
2019-10-04 15:09 ` [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 01/13] push: do not pretend to return `int` from `die_push_simple()` Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 02/13] msvc: avoid using minus operator on unsigned types Johannes Schindelin via GitGitGadget
@ 2019-10-04 15:09 ` Johannes Schindelin via GitGitGadget
2019-10-07 19:16 ` Alban Gruin
2019-10-04 15:09 ` [PATCH v3 04/13] compat/win32/path-utils.h: add #include guards Johannes Schindelin via GitGitGadget
` (10 subsequent siblings)
13 siblings, 1 reply; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-10-04 15:09 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
MSVC would complain thusly:
C4200: nonstandard extension used: zero-sized array in struct/union
Let's just use the `FLEX_ARRAY` constant that we introduced for exactly
this type of scenario.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
compat/winansi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compat/winansi.c b/compat/winansi.c
index cacd82c833..54fd701cbf 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -546,7 +546,7 @@ static HANDLE swap_osfhnd(int fd, HANDLE new_handle)
typedef struct _OBJECT_NAME_INFORMATION
{
UNICODE_STRING Name;
- WCHAR NameBuffer[0];
+ WCHAR NameBuffer[FLEX_ARRAY];
} OBJECT_NAME_INFORMATION, *POBJECT_NAME_INFORMATION;
#define ObjectNameInformation 1
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH v3 04/13] compat/win32/path-utils.h: add #include guards
2019-10-04 15:09 ` [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (2 preceding siblings ...)
2019-10-04 15:09 ` [PATCH v3 03/13] winansi: use FLEX_ARRAY to avoid compiler warning Johannes Schindelin via GitGitGadget
@ 2019-10-04 15:09 ` Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 05/13] msvc: ignore some libraries when linking Johannes Schindelin via GitGitGadget
` (9 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-10-04 15:09 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
This adds the common guards that allow headers to be #include'd multiple
times.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
compat/win32/path-utils.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/compat/win32/path-utils.h b/compat/win32/path-utils.h
index 0f70d43920..8ed062a6b7 100644
--- a/compat/win32/path-utils.h
+++ b/compat/win32/path-utils.h
@@ -1,3 +1,6 @@
+#ifndef WIN32_PATH_UTILS_H
+#define WIN32_PATH_UTILS_H
+
#define has_dos_drive_prefix(path) \
(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
int win32_skip_dos_drive_prefix(char **path);
@@ -18,3 +21,5 @@ static inline char *win32_find_last_dir_sep(const char *path)
#define find_last_dir_sep win32_find_last_dir_sep
int win32_offset_1st_component(const char *path);
#define offset_1st_component win32_offset_1st_component
+
+#endif
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH v3 05/13] msvc: ignore some libraries when linking
2019-10-04 15:09 ` [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (3 preceding siblings ...)
2019-10-04 15:09 ` [PATCH v3 04/13] compat/win32/path-utils.h: add #include guards Johannes Schindelin via GitGitGadget
@ 2019-10-04 15:09 ` Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 07/13] msvc: work around a bug in GetEnvironmentVariable() Johannes Schindelin via GitGitGadget
` (8 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-10-04 15:09 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
To build with MSVC, we "translate" GCC options to MSVC options, and part
of those options refer to the libraries to link into the final
executable. Currently, this part looks somewhat like this on Windows:
-lcurl -lnghttp2 -lidn2 -lssl -lcrypto -lssl -lcrypto -lgdi32
-lcrypt32 -lwldap32 -lz -lws2_32 -lexpat
Some of those are direct dependencies (such as curl and ssl) and others
are indirect (nghttp2 and idn2, for example, are dependencies of curl,
but need to be linked in for reasons).
We already handle the direct dependencies, e.g. `-liconv` is already
handled as adding `libiconv.lib` to the list of libraries to link
against.
Let's just ignore the remaining `-l*` options so that MSVC does not have
to warn us that it ignored e.g. the `/lnghttp2` option. We do that by
extending the clause that already "eats" the `-R*` options to also eat
the `-l*` options.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
compat/vcbuild/scripts/clink.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compat/vcbuild/scripts/clink.pl b/compat/vcbuild/scripts/clink.pl
index c7b021bfac..00fc339cba 100755
--- a/compat/vcbuild/scripts/clink.pl
+++ b/compat/vcbuild/scripts/clink.pl
@@ -68,7 +68,7 @@
} elsif ("$arg" =~ /^-L/ && "$arg" ne "-LTCG") {
$arg =~ s/^-L/-LIBPATH:/;
push(@lflags, $arg);
- } elsif ("$arg" =~ /^-R/) {
+ } elsif ("$arg" =~ /^-[Rl]/) {
# eat
} else {
push(@args, $arg);
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH v3 07/13] msvc: work around a bug in GetEnvironmentVariable()
2019-10-04 15:09 ` [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (4 preceding siblings ...)
2019-10-04 15:09 ` [PATCH v3 05/13] msvc: ignore some libraries when linking Johannes Schindelin via GitGitGadget
@ 2019-10-04 15:09 ` Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 06/13] msvc: handle DEVELOPER=1 Johannes Schindelin via GitGitGadget
` (7 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-10-04 15:09 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The return value of that function is 0 both for variables that are
unset, as well as for variables whose values are empty. To discern those
two cases, one has to call `GetLastError()`, whose return value is
`ERROR_ENVVAR_NOT_FOUND` and `ERROR_SUCCESS`, respectively.
Except that it is not actually set to `ERROR_SUCCESS` in the latter
case, apparently, but the last error value seems to be simply unchanged.
To work around this, let's just re-set the last error value just before
inspecting the environment variable.
This fixes a problem that triggers failures in t3301-notes.sh (where we
try to override config settings by passing empty values for certain
environment variables).
This problem is hidden in the MINGW build by the fact that older
MSVC runtimes (such as the one used by MINGW builds) have a `calloc()`
that re-sets the last error value in case of success, while newer
runtimes set the error value only if `NULL` is returned by that
function.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
compat/mingw.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/compat/mingw.c b/compat/mingw.c
index 4891789771..7d8cb814ba 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1661,6 +1661,8 @@ char *mingw_getenv(const char *name)
if (!w_key)
die("Out of memory, (tried to allocate %u wchar_t's)", len_key);
xutftowcs(w_key, name, len_key);
+ /* GetEnvironmentVariableW() only sets the last error upon failure */
+ SetLastError(ERROR_SUCCESS);
len_value = GetEnvironmentVariableW(w_key, w_value, ARRAY_SIZE(w_value));
if (!len_value && GetLastError() == ERROR_ENVVAR_NOT_FOUND) {
free(w_key);
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH v3 06/13] msvc: handle DEVELOPER=1
2019-10-04 15:09 ` [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (5 preceding siblings ...)
2019-10-04 15:09 ` [PATCH v3 07/13] msvc: work around a bug in GetEnvironmentVariable() Johannes Schindelin via GitGitGadget
@ 2019-10-04 15:09 ` Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 08/13] vcxproj: only copy `git-remote-http.exe` once it was built Johannes Schindelin via GitGitGadget
` (6 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-10-04 15:09 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
We frequently build Git using the `DEVELOPER=1` make setting as a
shortcut to enable all kinds of more stringent compiler warnings.
Those compiler warnings are relatively specific to GCC, though, so let's
try our best to translate them to the equivalent options to pass to MS
Visual C++'s `cl.exe`.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
compat/vcbuild/scripts/clink.pl | 46 +++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/compat/vcbuild/scripts/clink.pl b/compat/vcbuild/scripts/clink.pl
index 00fc339cba..ec95a3b2d0 100755
--- a/compat/vcbuild/scripts/clink.pl
+++ b/compat/vcbuild/scripts/clink.pl
@@ -70,6 +70,52 @@
push(@lflags, $arg);
} elsif ("$arg" =~ /^-[Rl]/) {
# eat
+ } elsif ("$arg" eq "-Werror") {
+ push(@cflags, "-WX");
+ } elsif ("$arg" eq "-Wall") {
+ # cl.exe understands -Wall, but it is really overzealous
+ push(@cflags, "-W4");
+ # disable the "signed/unsigned mismatch" warnings; our source code violates that
+ push(@cflags, "-wd4018");
+ push(@cflags, "-wd4245");
+ push(@cflags, "-wd4389");
+ # disable the "unreferenced formal parameter" warning; our source code violates that
+ push(@cflags, "-wd4100");
+ # disable the "conditional expression is constant" warning; our source code violates that
+ push(@cflags, "-wd4127");
+ # disable the "const object should be initialized" warning; these warnings affect only objects that are `static`
+ push(@cflags, "-wd4132");
+ # disable the "function/data pointer conversion in expression" warning; our source code violates that
+ push(@cflags, "-wd4152");
+ # disable the "non-constant aggregate initializer" warning; our source code violates that
+ push(@cflags, "-wd4204");
+ # disable the "cannot be initialized using address of automatic variable" warning; our source code violates that
+ push(@cflags, "-wd4221");
+ # disable the "possible loss of data" warnings; our source code violates that
+ push(@cflags, "-wd4244");
+ push(@cflags, "-wd4267");
+ # disable the "array is too small to include a terminating null character" warning; we ab-use strings to initialize OIDs
+ push(@cflags, "-wd4295");
+ # disable the "'<<': result of 32-bit shift implicitly converted to 64 bits" warning; our source code violates that
+ push(@cflags, "-wd4334");
+ # disable the "declaration hides previous local declaration" warning; our source code violates that
+ push(@cflags, "-wd4456");
+ # disable the "declaration hides function parameter" warning; our source code violates that
+ push(@cflags, "-wd4457");
+ # disable the "declaration hides global declaration" warning; our source code violates that
+ push(@cflags, "-wd4459");
+ # disable the "potentially uninitialized local variable '<name>' used" warning; our source code violates that
+ push(@cflags, "-wd4701");
+ # disable the "unreachable code" warning; our source code violates that
+ push(@cflags, "-wd4702");
+ # disable the "potentially uninitialized local pointer variable used" warning; our source code violates that
+ push(@cflags, "-wd4703");
+ # disable the "assignment within conditional expression" warning; our source code violates that
+ push(@cflags, "-wd4706");
+ # disable the "'inet_ntoa': Use inet_ntop() or InetNtop() instead" warning; our source code violates that
+ push(@cflags, "-wd4996");
+ } elsif ("$arg" =~ /^-W[a-z]/) {
+ # let's ignore those
} else {
push(@args, $arg);
}
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH v3 08/13] vcxproj: only copy `git-remote-http.exe` once it was built
2019-10-04 15:09 ` [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (6 preceding siblings ...)
2019-10-04 15:09 ` [PATCH v3 06/13] msvc: handle DEVELOPER=1 Johannes Schindelin via GitGitGadget
@ 2019-10-04 15:09 ` Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 09/13] vcxproj: include more generated files Johannes Schindelin via GitGitGadget
` (5 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-10-04 15:09 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
In b18ae14a8f6 (vcxproj: also link-or-copy builtins, 2019-07-29), we
started to copy or hard-link the built-ins as a post-build step of the
`git` project.
At the same time, we tried to copy or hard-link `git-remote-http.exe`,
but it is quite possible that it was not built at that time.
Let's move that latter task into a post-install step of the
`git-remote-http` project instead.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
config.mak.uname | 10 +++++++---
contrib/buildsystems/Generators/Vcxproj.pm | 3 +++
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/config.mak.uname b/config.mak.uname
index db7f06b95f..701aad62b1 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -703,20 +703,24 @@ vcxproj:
perl contrib/buildsystems/generate -g Vcxproj
git add -f git.sln {*,*/lib,t/helper/*}/*.vcxproj
- # Generate the LinkOrCopyBuiltins.targets file
+ # Generate the LinkOrCopyBuiltins.targets and LinkOrCopyRemoteHttp.targets file
(echo '<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">' && \
echo ' <Target Name="CopyBuiltins_AfterBuild" AfterTargets="AfterBuild">' && \
for name in $(BUILT_INS);\
do \
echo ' <Copy SourceFiles="$$(OutDir)\git.exe" DestinationFiles="$$(OutDir)\'"$$name"'" SkipUnchangedFiles="true" UseHardlinksIfPossible="true" />'; \
done && \
+ echo ' </Target>' && \
+ echo '</Project>') >git/LinkOrCopyBuiltins.targets
+ (echo '<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">' && \
+ echo ' <Target Name="CopyBuiltins_AfterBuild" AfterTargets="AfterBuild">' && \
for name in $(REMOTE_CURL_ALIASES); \
do \
echo ' <Copy SourceFiles="$$(OutDir)\'"$(REMOTE_CURL_PRIMARY)"'" DestinationFiles="$$(OutDir)\'"$$name"'" SkipUnchangedFiles="true" UseHardlinksIfPossible="true" />'; \
done && \
echo ' </Target>' && \
- echo '</Project>') >git/LinkOrCopyBuiltins.targets
- git add -f git/LinkOrCopyBuiltins.targets
+ echo '</Project>') >git-remote-http/LinkOrCopyRemoteHttp.targets
+ git add -f git/LinkOrCopyBuiltins.targets git-remote-http/LinkOrCopyRemoteHttp.targets
# Add command-list.h
$(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 command-list.h
diff --git a/contrib/buildsystems/Generators/Vcxproj.pm b/contrib/buildsystems/Generators/Vcxproj.pm
index 576ccabe1d..868b787855 100644
--- a/contrib/buildsystems/Generators/Vcxproj.pm
+++ b/contrib/buildsystems/Generators/Vcxproj.pm
@@ -277,6 +277,9 @@ sub createProject {
if ($target eq 'git') {
print F " <Import Project=\"LinkOrCopyBuiltins.targets\" />\n";
}
+ if ($target eq 'git-remote-http') {
+ print F " <Import Project=\"LinkOrCopyRemoteHttp.targets\" />\n";
+ }
print F << "EOM";
</Project>
EOM
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH v3 09/13] vcxproj: include more generated files
2019-10-04 15:09 ` [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (7 preceding siblings ...)
2019-10-04 15:09 ` [PATCH v3 08/13] vcxproj: only copy `git-remote-http.exe` once it was built Johannes Schindelin via GitGitGadget
@ 2019-10-04 15:09 ` Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 10/13] test-tool run-command: learn to run (parts of) the testsuite Johannes Schindelin via GitGitGadget
` (4 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-10-04 15:09 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
In the CI builds, we bundle all generated files into a so-called
artifacts `.tar` file, so that the test phase can fan out into multiple
parallel builds.
This patch makes sure that all files are included in the `vcxproj`
target which are needed for that artifacts `.tar` file.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
config.mak.uname | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/config.mak.uname b/config.mak.uname
index 701aad62b1..cc8efd95b1 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -728,11 +728,10 @@ vcxproj:
# Add scripts
rm -f perl/perl.mak
- $(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 \
- $(SCRIPT_LIB) $(SCRIPT_SH_GEN) $(SCRIPT_PERL_GEN)
+ $(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 $(SCRIPT_LIB) $(SCRIPTS)
# Strip out the sane tool path, needed only for building
sed -i '/^git_broken_path_fix ".*/d' git-sh-setup
- git add -f $(SCRIPT_LIB) $(SCRIPT_SH_GEN) $(SCRIPT_PERL_GEN)
+ git add -f $(SCRIPT_LIB) $(SCRIPTS)
# Add Perl module
$(MAKE) $(LIB_PERL_GEN)
@@ -762,6 +761,10 @@ vcxproj:
$(MAKE) -C templates
git add -f templates/boilerplates.made templates/blt/
+ # Add the translated messages
+ make MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 $(MOFILES)
+ git add -f $(MOFILES)
+
# Add build options
$(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 GIT-BUILD-OPTIONS
git add -f GIT-BUILD-OPTIONS
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH v3 10/13] test-tool run-command: learn to run (parts of) the testsuite
2019-10-04 15:09 ` [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (8 preceding siblings ...)
2019-10-04 15:09 ` [PATCH v3 09/13] vcxproj: include more generated files Johannes Schindelin via GitGitGadget
@ 2019-10-04 15:09 ` Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 11/13] tests: let --immediate and --write-junit-xml play well together Johannes Schindelin via GitGitGadget
` (3 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-10-04 15:09 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Git for Windows jumps through hoops to provide a development environment
that allows to build Git and to run its test suite. To that end, an
entire MSYS2 system, including GNU make and GCC is offered as "the Git
for Windows SDK". It does come at a price: an initial download of said
SDK weighs in with several hundreds of megabytes, and the unpacked SDK
occupies ~2GB of disk space.
A much more native development environment on Windows is Visual Studio.
To help contributors use that environment, we already have a Makefile
target `vcxproj` that generates a commit with project files (and other
generated files), and Git for Windows' `vs/master` branch is
continuously re-generated using that target.
The idea is to allow building Git in Visual Studio, and to run
individual tests using a Portable Git.
The one missing thing is a way to run the entire test suite: neither
`make` nor `prove` are required to run Git, therefore Git for Windows
does not support those commands in the Portable Git.
To help with that, add a simple test helper that exercises the
`run_processes_parallel()` function to allow for running test scripts in
parallel (which is really necessary, especially on Windows, as Git's
test suite takes such a long time to run).
This will also come in handy for the upcoming change to our Azure
Pipeline: we will use this helper in a Portable Git to test the Visual
Studio build of Git.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/helper/test-run-command.c | 153 ++++++++++++++++++++++++++++++++++++
1 file changed, 153 insertions(+)
diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c
index 2cc93bb69c..ead6dc611a 100644
--- a/t/helper/test-run-command.c
+++ b/t/helper/test-run-command.c
@@ -10,9 +10,14 @@
#include "test-tool.h"
#include "git-compat-util.h"
+#include "cache.h"
#include "run-command.h"
#include "argv-array.h"
#include "strbuf.h"
+#include "parse-options.h"
+#include "string-list.h"
+#include "thread-utils.h"
+#include "wildmatch.h"
#include <string.h>
#include <errno.h>
@@ -50,11 +55,159 @@ static int task_finished(int result,
return 1;
}
+struct testsuite {
+ struct string_list tests, failed;
+ int next;
+ int quiet, immediate, verbose, verbose_log, trace, write_junit_xml;
+};
+#define TESTSUITE_INIT \
+ { STRING_LIST_INIT_DUP, STRING_LIST_INIT_DUP, -1, 0, 0, 0, 0, 0, 0 }
+
+static int next_test(struct child_process *cp, struct strbuf *err, void *cb,
+ void **task_cb)
+{
+ struct testsuite *suite = cb;
+ const char *test;
+ if (suite->next >= suite->tests.nr)
+ return 0;
+
+ test = suite->tests.items[suite->next++].string;
+ argv_array_pushl(&cp->args, "sh", test, NULL);
+ if (suite->quiet)
+ argv_array_push(&cp->args, "--quiet");
+ if (suite->immediate)
+ argv_array_push(&cp->args, "-i");
+ if (suite->verbose)
+ argv_array_push(&cp->args, "-v");
+ if (suite->verbose_log)
+ argv_array_push(&cp->args, "-V");
+ if (suite->trace)
+ argv_array_push(&cp->args, "-x");
+ if (suite->write_junit_xml)
+ argv_array_push(&cp->args, "--write-junit-xml");
+
+ strbuf_addf(err, "Output of '%s':\n", test);
+ *task_cb = (void *)test;
+
+ return 1;
+}
+
+static int test_finished(int result, struct strbuf *err, void *cb,
+ void *task_cb)
+{
+ struct testsuite *suite = cb;
+ const char *name = (const char *)task_cb;
+
+ if (result)
+ string_list_append(&suite->failed, name);
+
+ strbuf_addf(err, "%s: '%s'\n", result ? "FAIL" : "SUCCESS", name);
+
+ return 0;
+}
+
+static int test_failed(struct strbuf *out, void *cb, void *task_cb)
+{
+ struct testsuite *suite = cb;
+ const char *name = (const char *)task_cb;
+
+ string_list_append(&suite->failed, name);
+ strbuf_addf(out, "FAILED TO START: '%s'\n", name);
+
+ return 0;
+}
+
+static const char * const testsuite_usage[] = {
+ "test-run-command testsuite [<options>] [<pattern>...]",
+ NULL
+};
+
+static int testsuite(int argc, const char **argv)
+{
+ struct testsuite suite = TESTSUITE_INIT;
+ int max_jobs = 1, i, ret;
+ DIR *dir;
+ struct dirent *d;
+ struct option options[] = {
+ OPT_BOOL('i', "immediate", &suite.immediate,
+ "stop at first failed test case(s)"),
+ OPT_INTEGER('j', "jobs", &max_jobs, "run <N> jobs in parallel"),
+ OPT_BOOL('q', "quiet", &suite.quiet, "be terse"),
+ OPT_BOOL('v', "verbose", &suite.verbose, "be verbose"),
+ OPT_BOOL('V', "verbose-log", &suite.verbose_log,
+ "be verbose, redirected to a file"),
+ OPT_BOOL('x', "trace", &suite.trace, "trace shell commands"),
+ OPT_BOOL(0, "write-junit-xml", &suite.write_junit_xml,
+ "write JUnit-style XML files"),
+ OPT_END()
+ };
+
+ memset(&suite, 0, sizeof(suite));
+ suite.tests.strdup_strings = suite.failed.strdup_strings = 1;
+
+ argc = parse_options(argc, argv, NULL, options,
+ testsuite_usage, PARSE_OPT_STOP_AT_NON_OPTION);
+
+ if (max_jobs <= 0)
+ max_jobs = online_cpus();
+
+ dir = opendir(".");
+ if (!dir)
+ die("Could not open the current directory");
+ while ((d = readdir(dir))) {
+ const char *p = d->d_name;
+
+ if (*p != 't' || !isdigit(p[1]) || !isdigit(p[2]) ||
+ !isdigit(p[3]) || !isdigit(p[4]) || p[5] != '-' ||
+ !ends_with(p, ".sh"))
+ continue;
+
+ /* No pattern: match all */
+ if (!argc) {
+ string_list_append(&suite.tests, p);
+ continue;
+ }
+
+ for (i = 0; i < argc; i++)
+ if (!wildmatch(argv[i], p, 0)) {
+ string_list_append(&suite.tests, p);
+ break;
+ }
+ }
+ closedir(dir);
+
+ if (!suite.tests.nr)
+ die("No tests match!");
+ if (max_jobs > suite.tests.nr)
+ max_jobs = suite.tests.nr;
+
+ fprintf(stderr, "Running %d tests (%d at a time)\n",
+ suite.tests.nr, max_jobs);
+
+ ret = run_processes_parallel(max_jobs, next_test, test_failed,
+ test_finished, &suite);
+
+ if (suite.failed.nr > 0) {
+ ret = 1;
+ fprintf(stderr, "%d tests failed:\n\n", suite.failed.nr);
+ for (i = 0; i < suite.failed.nr; i++)
+ fprintf(stderr, "\t%s\n", suite.failed.items[i].string);
+ }
+
+ string_list_clear(&suite.tests, 0);
+ string_list_clear(&suite.failed, 0);
+
+ return !!ret;
+}
+
int cmd__run_command(int argc, const char **argv)
{
struct child_process proc = CHILD_PROCESS_INIT;
int jobs;
+ if (argc > 1 && !strcmp(argv[1], "testsuite"))
+ exit(testsuite(argc - 1, argv + 1));
+
if (argc < 3)
return 1;
while (!strcmp(argv[1], "env")) {
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH v3 11/13] tests: let --immediate and --write-junit-xml play well together
2019-10-04 15:09 ` [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (9 preceding siblings ...)
2019-10-04 15:09 ` [PATCH v3 10/13] test-tool run-command: learn to run (parts of) the testsuite Johannes Schindelin via GitGitGadget
@ 2019-10-04 15:09 ` Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 12/13] ci: really use shallow clones on Azure Pipelines Johannes Schindelin via GitGitGadget
` (2 subsequent siblings)
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-10-04 15:09 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
When the `--immediate` option is in effect, any test failure will
immediately exit the test script. Together with `--write-junit-xml`, we
will want the JUnit-style `.xml` file to be finalized (and not leave the
XML incomplete). Let's make it so.
This comes in particularly handy when trying to debug via Azure
Pipelines, where the JUnit-style XML is consumed to present the test
results in an informative and helpful way.
While at it, also handle the `error()` code path.
The only remaining code path that sets `GIT_EXIT_OK` happens whenever
the trash directory could not be set up, i.e. long before the JUnit XML
was written, therefore we should _not_ try to finalize that XML in that
case.
It is tempting to change the `immediate` code path to just hand off to
`error`, simplifying the code in the process. That would, however,
result in a change of behavior (an additional error message) in the test
suite, which is outside of the purview of the current patch series: its
goal is to allow building Git with Visual Studio and testing it with a
portable version of Git for Windows.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/test-lib.sh | 38 ++++++++++++++++++++++----------------
1 file changed, 22 insertions(+), 16 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index d1ba33745a..86b5e6133b 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -567,6 +567,7 @@ export TERM
error () {
say_color error "error: $*"
+ finalize_junit_xml
GIT_EXIT_OK=t
exit 1
}
@@ -695,7 +696,7 @@ test_failure_ () {
say_color error "not ok $test_count - $1"
shift
printf '%s\n' "$*" | sed -e 's/^/# /'
- test "$immediate" = "" || { GIT_EXIT_OK=t; exit 1; }
+ test "$immediate" = "" || { finalize_junit_xml; GIT_EXIT_OK=t; exit 1; }
}
test_known_broken_ok_ () {
@@ -1063,6 +1064,25 @@ write_junit_xml_testcase () {
junit_have_testcase=t
}
+finalize_junit_xml () {
+ if test -n "$write_junit_xml" && test -n "$junit_xml_path"
+ then
+ test -n "$junit_have_testcase" || {
+ junit_start=$(test-tool date getnanos)
+ write_junit_xml_testcase "all tests skipped"
+ }
+
+ # adjust the overall time
+ junit_time=$(test-tool date getnanos $junit_suite_start)
+ sed "s/<testsuite [^>]*/& time=\"$junit_time\"/" \
+ <"$junit_xml_path" >"$junit_xml_path.new"
+ mv "$junit_xml_path.new" "$junit_xml_path"
+
+ write_junit_xml " </testsuite>" "</testsuites>"
+ write_junit_xml=
+ fi
+}
+
test_atexit_cleanup=:
test_atexit_handler () {
# In a succeeding test script 'test_atexit_handler' is invoked
@@ -1085,21 +1105,7 @@ test_done () {
# removed, so the commands can access pidfiles and socket files.
test_atexit_handler
- if test -n "$write_junit_xml" && test -n "$junit_xml_path"
- then
- test -n "$junit_have_testcase" || {
- junit_start=$(test-tool date getnanos)
- write_junit_xml_testcase "all tests skipped"
- }
-
- # adjust the overall time
- junit_time=$(test-tool date getnanos $junit_suite_start)
- sed "s/<testsuite [^>]*/& time=\"$junit_time\"/" \
- <"$junit_xml_path" >"$junit_xml_path.new"
- mv "$junit_xml_path.new" "$junit_xml_path"
-
- write_junit_xml " </testsuite>" "</testsuites>"
- fi
+ finalize_junit_xml
if test -z "$HARNESS_ACTIVE"
then
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH v3 12/13] ci: really use shallow clones on Azure Pipelines
2019-10-04 15:09 ` [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (10 preceding siblings ...)
2019-10-04 15:09 ` [PATCH v3 11/13] tests: let --immediate and --write-junit-xml play well together Johannes Schindelin via GitGitGadget
@ 2019-10-04 15:09 ` Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 13/13] ci: also build and test with MS Visual Studio " Johannes Schindelin via GitGitGadget
2019-10-06 0:19 ` [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Junio C Hamano
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-10-04 15:09 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
This was a left-over from the previous YAML schema, and it no longer
works. The problem was noticed while editing `azure-pipelines.yml` in VS
Code with the very helpful "Azure Pipelines" extension (syntax
highlighting and intellisense for `azure-pipelines.yml`...).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
azure-pipelines.yml | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/azure-pipelines.yml b/azure-pipelines.yml
index c329b7218b..55ee23ad0f 100644
--- a/azure-pipelines.yml
+++ b/azure-pipelines.yml
@@ -1,6 +1,5 @@
-resources:
-- repo: self
- fetchDepth: 1
+variables:
+ Agent.Source.Git.ShallowFetchDepth: 1
jobs:
- job: windows_build
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH v3 13/13] ci: also build and test with MS Visual Studio on Azure Pipelines
2019-10-04 15:09 ` [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (11 preceding siblings ...)
2019-10-04 15:09 ` [PATCH v3 12/13] ci: really use shallow clones on Azure Pipelines Johannes Schindelin via GitGitGadget
@ 2019-10-04 15:09 ` Johannes Schindelin via GitGitGadget
2019-10-06 0:19 ` [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Junio C Hamano
13 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-10-04 15:09 UTC (permalink / raw)
To: git; +Cc: Denton Liu, Johannes Schindelin, Junio C Hamano,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
... because we can, now. Technically, we actually build using `MSBuild`,
which is however pretty close to building interactively in Visual
Studio.
As there is no convenient way to run Git's test suite in Visual Studio,
we unpack a Portable Git to run it, using the just-added test helper to
allow running test scripts in parallel.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Makefile | 4 ++
azure-pipelines.yml | 159 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 163 insertions(+)
diff --git a/Makefile b/Makefile
index 3716dadc08..3f6dcec54b 100644
--- a/Makefile
+++ b/Makefile
@@ -3025,6 +3025,10 @@ rpm::
@false
.PHONY: rpm
+ifneq ($(INCLUDE_DLLS_IN_ARTIFACTS),)
+OTHER_PROGRAMS += $(shell echo *.dll t/helper/*.dll)
+endif
+
artifacts-tar:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) \
GIT-BUILD-OPTIONS $(TEST_PROGRAMS) $(test_bindir_programs) \
$(MOFILES)
diff --git a/azure-pipelines.yml b/azure-pipelines.yml
index 55ee23ad0f..875e63cac1 100644
--- a/azure-pipelines.yml
+++ b/azure-pipelines.yml
@@ -130,6 +130,165 @@ jobs:
PathtoPublish: t/failed-test-artifacts
ArtifactName: failed-test-artifacts
+- job: vs_build
+ displayName: Visual Studio Build
+ condition: succeeded()
+ pool: Hosted VS2017
+ timeoutInMinutes: 240
+ steps:
+ - powershell: |
+ if ("$GITFILESHAREPWD" -ne "" -and "$GITFILESHAREPWD" -ne "`$`(gitfileshare.pwd)") {
+ net use s: \\gitfileshare.file.core.windows.net\test-cache "$GITFILESHAREPWD" /user:AZURE\gitfileshare /persistent:no
+ cmd /c mklink /d "$(Build.SourcesDirectory)\test-cache" S:\
+ }
+ displayName: 'Mount test-cache'
+ env:
+ GITFILESHAREPWD: $(gitfileshare.pwd)
+ - powershell: |
+ $urlbase = "https://dev.azure.com/git-for-windows/git/_apis/build/builds"
+ $id = ((Invoke-WebRequest -UseBasicParsing "${urlbase}?definitions=22&statusFilter=completed&resultFilter=succeeded&`$top=1").content | ConvertFrom-JSON).value[0].id
+ $downloadUrl = ((Invoke-WebRequest -UseBasicParsing "${urlbase}/$id/artifacts").content | ConvertFrom-JSON).value[1].resource.downloadUrl
+ (New-Object Net.WebClient).DownloadFile($downloadUrl,"git-sdk-64-minimal.zip")
+ Expand-Archive git-sdk-64-minimal.zip -DestinationPath . -Force
+ Remove-Item git-sdk-64-minimal.zip
+
+ # Let Git ignore the SDK and the test-cache
+ "/git-sdk-64-minimal/`n/test-cache/`n" | Out-File -NoNewLine -Encoding ascii -Append "$(Build.SourcesDirectory)\.git\info\exclude"
+ displayName: 'Download git-sdk-64-minimal'
+ - powershell: |
+ & git-sdk-64-minimal\usr\bin\bash.exe -lc @"
+ make vcxproj
+ "@
+ if (!$?) { exit(1) }
+ displayName: Generate Visual Studio Solution
+ env:
+ HOME: $(Build.SourcesDirectory)
+ MSYSTEM: MINGW64
+ DEVELOPER: 1
+ NO_PERL: 1
+ GIT_CONFIG_PARAMETERS: "'user.name=CI' 'user.email=ci@git'"
+ - powershell: |
+ $urlbase = "https://dev.azure.com/git/git/_apis/build/builds"
+ $id = ((Invoke-WebRequest -UseBasicParsing "${urlbase}?definitions=9&statusFilter=completed&resultFilter=succeeded&`$top=1").content | ConvertFrom-JSON).value[0].id
+ $downloadUrl = ((Invoke-WebRequest -UseBasicParsing "${urlbase}/$id/artifacts").content | ConvertFrom-JSON).value[0].resource.downloadUrl
+ (New-Object Net.WebClient).DownloadFile($downloadUrl, "compat.zip")
+ Expand-Archive compat.zip -DestinationPath . -Force
+ Remove-Item compat.zip
+ displayName: 'Download vcpkg artifacts'
+ - task: MSBuild@1
+ inputs:
+ solution: git.sln
+ platform: x64
+ configuration: Release
+ maximumCpuCount: 4
+ - powershell: |
+ & compat\vcbuild\vcpkg_copy_dlls.bat release
+ if (!$?) { exit(1) }
+ & git-sdk-64-minimal\usr\bin\bash.exe -lc @"
+ mkdir -p artifacts &&
+ eval \"`$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts | grep ^tar)\"
+ "@
+ if (!$?) { exit(1) }
+ displayName: Bundle artifact tar
+ env:
+ HOME: $(Build.SourcesDirectory)
+ MSYSTEM: MINGW64
+ DEVELOPER: 1
+ NO_PERL: 1
+ MSVC: 1
+ VCPKG_ROOT: $(Build.SourcesDirectory)\compat\vcbuild\vcpkg
+ - powershell: |
+ $tag = (Invoke-WebRequest -UseBasicParsing "https://gitforwindows.org/latest-tag.txt").content
+ $version = (Invoke-WebRequest -UseBasicParsing "https://gitforwindows.org/latest-version.txt").content
+ $url = "https://github.com/git-for-windows/git/releases/download/${tag}/PortableGit-${version}-64-bit.7z.exe"
+ (New-Object Net.WebClient).DownloadFile($url,"PortableGit.exe")
+ & .\PortableGit.exe -y -oartifacts\PortableGit
+ # Wait until it is unpacked
+ while (-not @(Remove-Item -ErrorAction SilentlyContinue PortableGit.exe; $?)) { sleep 1 }
+ displayName: Download & extract portable Git
+ - task: PublishPipelineArtifact@0
+ displayName: 'Publish Pipeline Artifact: MSVC test artifacts'
+ inputs:
+ artifactName: 'vs-artifacts'
+ targetPath: '$(Build.SourcesDirectory)\artifacts'
+ - powershell: |
+ if ("$GITFILESHAREPWD" -ne "" -and "$GITFILESHAREPWD" -ne "`$`(gitfileshare.pwd)") {
+ cmd /c rmdir "$(Build.SourcesDirectory)\test-cache"
+ }
+ displayName: 'Unmount test-cache'
+ condition: true
+ env:
+ GITFILESHAREPWD: $(gitfileshare.pwd)
+
+- job: vs_test
+ displayName: Visual Studio Test
+ dependsOn: vs_build
+ condition: succeeded()
+ pool: Hosted
+ timeoutInMinutes: 240
+ strategy:
+ parallel: 10
+ steps:
+ - powershell: |
+ if ("$GITFILESHAREPWD" -ne "" -and "$GITFILESHAREPWD" -ne "`$`(gitfileshare.pwd)") {
+ net use s: \\gitfileshare.file.core.windows.net\test-cache "$GITFILESHAREPWD" /user:AZURE\gitfileshare /persistent:no
+ cmd /c mklink /d "$(Build.SourcesDirectory)\test-cache" S:\
+ }
+ displayName: 'Mount test-cache'
+ env:
+ GITFILESHAREPWD: $(gitfileshare.pwd)
+ - task: DownloadPipelineArtifact@0
+ displayName: 'Download Pipeline Artifact: VS test artifacts'
+ inputs:
+ artifactName: 'vs-artifacts'
+ targetPath: '$(Build.SourcesDirectory)'
+ - powershell: |
+ & PortableGit\git-cmd.exe --command=usr\bin\bash.exe -lc @"
+ test -f artifacts.tar.gz || {
+ echo No test artifacts found\; skipping >&2
+ exit 0
+ }
+ tar xf artifacts.tar.gz || exit 1
+
+ # Let Git ignore the SDK and the test-cache
+ printf '%s\n' /PortableGit/ /test-cache/ >>.git/info/exclude
+
+ cd t &&
+ PATH=\"`$PWD/helper:`$PATH\" &&
+ test-tool.exe run-command testsuite -V -x --write-junit-xml \
+ `$(test-tool.exe path-utils slice-tests \
+ `$SYSTEM_JOBPOSITIONINPHASE `$SYSTEM_TOTALJOBSINPHASE t[0-9]*.sh)
+ "@
+ if (!$?) { exit(1) }
+ displayName: 'Test (parallel)'
+ env:
+ HOME: $(Build.SourcesDirectory)
+ MSYSTEM: MINGW64
+ NO_SVN_TESTS: 1
+ GIT_TEST_SKIP_REBASE_P: 1
+ - powershell: |
+ if ("$GITFILESHAREPWD" -ne "" -and "$GITFILESHAREPWD" -ne "`$`(gitfileshare.pwd)") {
+ cmd /c rmdir "$(Build.SourcesDirectory)\test-cache"
+ }
+ displayName: 'Unmount test-cache'
+ condition: true
+ env:
+ GITFILESHAREPWD: $(gitfileshare.pwd)
+ - task: PublishTestResults@2
+ displayName: 'Publish Test Results **/TEST-*.xml'
+ inputs:
+ mergeTestResults: true
+ testRunTitle: 'vs'
+ platform: Windows
+ publishRunAttachments: false
+ condition: succeededOrFailed()
+ - task: PublishBuildArtifacts@1
+ displayName: 'Publish trash directories of failed tests'
+ condition: failed()
+ inputs:
+ PathtoPublish: t/failed-test-artifacts
+ ArtifactName: failed-vs-test-artifacts
+
- job: linux_clang
displayName: linux-clang
condition: succeeded()
--
gitgitgadget
^ permalink raw reply related [flat|nested] 72+ messages in thread
* Re: [PATCH v2 02/13] msvc: avoid using minus operator on unsigned types
2019-10-04 9:55 ` Johannes Schindelin
@ 2019-10-04 17:09 ` Johannes Sixt
2019-10-04 21:24 ` Johannes Schindelin
0 siblings, 1 reply; 72+ messages in thread
From: Johannes Sixt @ 2019-10-04 17:09 UTC (permalink / raw)
To: Johannes Schindelin, Junio C Hamano
Cc: Johannes Schindelin via GitGitGadget, git, Denton Liu
Am 04.10.19 um 11:55 schrieb Johannes Schindelin:
> On Fri, 4 Oct 2019, Junio C Hamano wrote:
>> These three look good and too similar to each other, which makes me
>> wonder if we want to allow them simply write
>>
>> return insert_pos_as_negative_offset(nr);
>>
>> with something like
>>
>> static int insert_pos_as_negative_offset(uintmax_t nr)
>> {
>> if (INT_MAX < nr)
>> die("overflow: -1 - %"PRIuMAX, nr);
>> return -1 - (int)nr;
>> }
>>
>> to avoid repetition.
>
> I tried not to do that because there are two different data types in
> play: `unsigned int` and `size_t`. But I guess by making this an
> `inline` function, compilers can optimize for the common case and avoid
> casting _twice_.
>
> Will be fixed in v2,
IMHO, if you don't accompany insert_pos_as_negative_offset() with a
corresponding extract_pos_and_found_condition() and use it everywhere,
it is more obfuscating than necessary.
The *real* problem to solve is to ensure that the index/cache does not
grow so large that 32-bit indexes would be needed. Then the calculation
that you want to hide here cannot overflow.
-- Hannes
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v2 02/13] msvc: avoid using minus operator on unsigned types
2019-10-04 17:09 ` Johannes Sixt
@ 2019-10-04 21:24 ` Johannes Schindelin
2019-10-06 0:02 ` Junio C Hamano
0 siblings, 1 reply; 72+ messages in thread
From: Johannes Schindelin @ 2019-10-04 21:24 UTC (permalink / raw)
To: Johannes Sixt
Cc: Junio C Hamano, Johannes Schindelin via GitGitGadget, git,
Denton Liu
Hi Hannes,
On Fri, 4 Oct 2019, Johannes Sixt wrote:
> Am 04.10.19 um 11:55 schrieb Johannes Schindelin:
> > On Fri, 4 Oct 2019, Junio C Hamano wrote:
> >> These three look good and too similar to each other, which makes me
> >> wonder if we want to allow them simply write
> >>
> >> return insert_pos_as_negative_offset(nr);
> >>
> >> with something like
> >>
> >> static int insert_pos_as_negative_offset(uintmax_t nr)
> >> {
> >> if (INT_MAX < nr)
> >> die("overflow: -1 - %"PRIuMAX, nr);
> >> return -1 - (int)nr;
> >> }
> >>
> >> to avoid repetition.
> >
> > I tried not to do that because there are two different data types in
> > play: `unsigned int` and `size_t`. But I guess by making this an
> > `inline` function, compilers can optimize for the common case and avoid
> > casting _twice_.
> >
> > Will be fixed in v2,
>
> IMHO, if you don't accompany insert_pos_as_negative_offset() with a
> corresponding extract_pos_and_found_condition() and use it everywhere,
> it is more obfuscating than necessary.
I do disagree here. No overflow checking needs to be performed for `-1 -
<int-value>`. And that's what the opposite of this function really boils
down to.
> The *real* problem to solve is to ensure that the index/cache does not
> grow so large that 32-bit indexes would be needed. Then the calculation
> that you want to hide here cannot overflow.
Well, that may be the real problem of another patch series. This patch
series' problem is to add a job to our Azure Pipeline that builds Git
with Visual Studio, and it patches the code minimally so that it builds
even in `DEVELOPER=1` mode, for good measure.
So I'd like not to dilute the purpose of this patch series.
Thanks,
Dscho
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v2 02/13] msvc: avoid using minus operator on unsigned types
2019-10-04 21:24 ` Johannes Schindelin
@ 2019-10-06 0:02 ` Junio C Hamano
2019-10-06 10:53 ` Johannes Sixt
0 siblings, 1 reply; 72+ messages in thread
From: Junio C Hamano @ 2019-10-06 0:02 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Johannes Sixt, Johannes Schindelin via GitGitGadget, git,
Denton Liu
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> IMHO, if you don't accompany insert_pos_as_negative_offset() with a
>> corresponding extract_pos_and_found_condition() and use it everywhere,
>> it is more obfuscating than necessary.
>
> I do disagree here. No overflow checking needs to be performed for `-1 -
> <int-value>`. And that's what the opposite of this function really boils
> down to.
I do not think j6t is referring to the over/underflow issues at all.
The suggestion is that, because insert-pos-as-negative-offset
abstracts away (in addition to the overflow checks) the fact that
"does not exist but here is the location it would be inserted" is
encoded in a certain way (i.e. not just the array index negated, but
also is offset by -1, because we wouldn't be able to say "at the
very beginning at index 0" without the -1 offset), the side that
consumes the encoded "pos" (i.e. "we got a negative, so we know the
element does not exist, and the index into the array we would insert
a new element is computed this way") should be abstracted away, as
it must know that the extra negative offset used when encoding is
"-1".
I think that is a reasonable thing to consider; it is not necessary
for correctness, but contributes to the conceptual clarity (iow, it
can be left as a separate clean-up step done after the series is
done).
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline
2019-10-04 15:09 ` [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
` (12 preceding siblings ...)
2019-10-04 15:09 ` [PATCH v3 13/13] ci: also build and test with MS Visual Studio " Johannes Schindelin via GitGitGadget
@ 2019-10-06 0:19 ` Junio C Hamano
2019-10-06 10:45 ` Johannes Schindelin
13 siblings, 1 reply; 72+ messages in thread
From: Junio C Hamano @ 2019-10-06 0:19 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Denton Liu, Johannes Schindelin
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> Changes since v2:
>
> * The overflow check introduced in v1 was consolidated into a single
> helper.
Looks good to me.
> Range-diff vs v2:
>
> 1: 4d0b38125a = 1: 4d0b38125a push: do not pretend to return `int` from `die_push_simple()`
> 2: 8800320590 < -: ---------- msvc: avoid using minus operator on unsigned types
> -: ---------- > 2: 7fe2a85506 msvc: avoid using minus operator on unsigned types
> 3: 8512a3e96d = 3: e632a4eef4 winansi: use FLEX_ARRAY to avoid compiler warning
This is less useful than it could be.
With a larger creation-factor (and we can afford using a larger one,
simply because the user of GGG _knows_ that the two series being
compared are closely related), what is output is entirely readable
(attached at the end).
Oh, while I am suggesting possible improvements on GGG, can we
please tweak the sender date like git-send-email does so that two
messages in the same series do not share the same timestamp? When
multi-patch series are displayed in MUA or public-inbox News feed
out of order, it almost always is from GGG that gave the same
timestamp to adjacent messages in a series, and it prevents me from
applying them in one go (or saving in one action to a mbox).
What send-email does is, at the beginning for N patch series, to
take the current wallclock time and subtract N seconds from it, and
then give that timestamp to the first message it sends out, and
after that, it increments the timestamp by 1 seconds.
Note that there is no need for any "sleep"---the timestamps are
given by explicitly generating the "Date: " header. The last time
we looked into this issue, I think the code was trying to do almost
the right thing but it was giving a malformatted timezone and forcing
the sending MTA to override it with the wallclock time or something.
Thanks.
1: 9629f3c751 ! 1: c097b95a26 msvc: avoid using minus operator on unsigned types
@@ Commit message
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
+ ## cache.h ##
+@@ cache.h: struct cache_entry *index_file_exists(struct index_state *istate, const char *na
+ */
+ int index_name_pos(const struct index_state *, const char *name, int namelen);
+
++/*
++ * Some functions return the negative complement of an insert position when a
++ * precise match was not found but a position was found where the entry would
++ * need to be inserted. This helper protects that logic from any integer
++ * underflow.
++ */
++static inline int index_pos_to_insert_pos(uintmax_t pos)
++{
++ if (pos > INT_MAX)
++ die("overflow: -1 - %"PRIuMAX, pos);
++ return -1 - (int)pos;
++}
++
+ #define ADD_CACHE_OK_TO_ADD 1 /* Ok to add */
+ #define ADD_CACHE_OK_TO_REPLACE 2 /* Ok to replace file/directory */
+ #define ADD_CACHE_SKIP_DFCHECK 4 /* Ok to skip DF conflict checks */
+
## read-cache.c ##
@@ read-cache.c: static int add_index_entry_with_check(struct index_state *istate, struct cache_e
- * we can avoid searching for it.
*/
if (istate->cache_nr > 0 &&
-- strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)
+ strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)
- pos = -istate->cache_nr - 1;
-+ strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0) {
-+ if (istate->cache_nr > INT_MAX)
-+ die("overflow: -1 - %u", istate->cache_nr);
-+ pos = -1 - (int)istate->cache_nr;
-+ }
++ pos = index_pos_to_insert_pos(istate->cache_nr);
else
pos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce));
@@ read-cache.c: static size_t estimate_cache_size(size_t ondisk_size, unsigned int
## sha1-lookup.c ##
@@ sha1-lookup.c: int sha1_pos(const unsigned char *sha1, void *table, size_t nr,
- miv = take2(sha1 + ofs);
if (miv < lov)
return -1;
-- if (hiv < miv)
+ if (hiv < miv)
- return -1 - nr;
-+ if (hiv < miv) {
-+ if (nr > INT_MAX)
-+ die("overflow: -1 - %"PRIuMAX,
-+ (uintmax_t)nr);
-+ return -1 - (int)nr;
-+ }
++ return index_pos_to_insert_pos(nr);
if (lov != hiv) {
/*
* At this point miv could be equal
@@ sha1-lookup.c: int sha1_pos(const unsigned char *sha1, void *table, size_t nr,
mi = lo + (hi - lo) / 2;
} while (lo < hi);
- return -lo-1;
-+ if (nr > INT_MAX)
-+ die("overflow: -1 - %"PRIuMAX, (uintmax_t)lo);
-+ return -1 - (int)lo;
++ return index_pos_to_insert_pos(lo);
}
int bsearch_hash(const unsigned char *sha1, const uint32_t *fanout_nbo,
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline
2019-10-06 0:19 ` [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Junio C Hamano
@ 2019-10-06 10:45 ` Johannes Schindelin
2019-10-06 20:38 ` Johannes Schindelin
2019-10-07 0:59 ` Junio C Hamano
0 siblings, 2 replies; 72+ messages in thread
From: Johannes Schindelin @ 2019-10-06 10:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin via GitGitGadget, git, Denton Liu
Hi Junio,
On Sun, 6 Oct 2019, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > Range-diff vs v2:
> >
> > 1: 4d0b38125a = 1: 4d0b38125a push: do not pretend to return `int` from `die_push_simple()`
> > 2: 8800320590 < -: ---------- msvc: avoid using minus operator on unsigned types
> > -: ---------- > 2: 7fe2a85506 msvc: avoid using minus operator on unsigned types
> > 3: 8512a3e96d = 3: e632a4eef4 winansi: use FLEX_ARRAY to avoid compiler warning
>
> This is less useful than it could be.
>
> With a larger creation-factor (and we can afford using a larger one,
> simply because the user of GGG _knows_ that the two series being
> compared are closely related), what is output is entirely readable
> (attached at the end).
I just implemented this here:
https://github.com/gitgitgadget/gitgitgadget/pull/128 (it still needs to
be reviewed and merged before it takes effect).
> Oh, while I am suggesting possible improvements on GGG, can we
> please tweak the sender date like git-send-email does so that two
> messages in the same series do not share the same timestamp? When
> multi-patch series are displayed in MUA or public-inbox News feed
> out of order, it almost always is from GGG that gave the same
> timestamp to adjacent messages in a series, and it prevents me from
> applying them in one go (or saving in one action to a mbox).
>
> What send-email does is, at the beginning for N patch series, to
> take the current wallclock time and subtract N seconds from it, and
> then give that timestamp to the first message it sends out, and
> after that, it increments the timestamp by 1 seconds.
>
> Note that there is no need for any "sleep"---the timestamps are
> given by explicitly generating the "Date: " header. The last time
> we looked into this issue, I think the code was trying to do almost
> the right thing but it was giving a malformatted timezone and forcing
> the sending MTA to override it with the wallclock time or something.
You mentioned this before, and I implemented it. But GMail ignores the
`Date:` header sent by GitGitGadget, and I don't know why. See e.g.
https://public-inbox.org/git/4d0b38125a13d85963be5e524becf48271893e2b.1570201763.git.gitgitgadget@gmail.com/raw
[...]
Date: Fri, 04 Oct 2019 08:09:25 -0700 (PDT)
[...]
X-Google-Original-Date: Fri, 04 Oct 2019 15:09:10 GMT
[...]
I am fairly certain that the latter is the actual `Date:` line sent to
GMail, and GMail just decides that it will not respect it.
Once https://github.com/gitgitgadget/gitgitgadget/pull/125 makes it into
GitGitGadget (adding the `/preview` command that allows to send patch
series to the PR owner as a test), it should be easier to start
investigating further.
Unless anybody here knows why GMail rejects the header. Maybe it is the
`GMT`?
Ciao,
Dscho
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v2 02/13] msvc: avoid using minus operator on unsigned types
2019-10-06 0:02 ` Junio C Hamano
@ 2019-10-06 10:53 ` Johannes Sixt
2019-10-08 12:04 ` Johannes Schindelin
0 siblings, 1 reply; 72+ messages in thread
From: Johannes Sixt @ 2019-10-06 10:53 UTC (permalink / raw)
To: Junio C Hamano, Johannes Schindelin
Cc: Johannes Schindelin via GitGitGadget, git, Denton Liu
Am 06.10.19 um 02:02 schrieb Junio C Hamano:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>>> IMHO, if you don't accompany insert_pos_as_negative_offset() with a
>>> corresponding extract_pos_and_found_condition() and use it everywhere,
>>> it is more obfuscating than necessary.
>>
>> I do disagree here. No overflow checking needs to be performed for `-1 -
>> <int-value>`. And that's what the opposite of this function really boils
>> down to.
>
> I do not think j6t is referring to the over/underflow issues at all.
>
> The suggestion is that, because insert-pos-as-negative-offset
> abstracts away [...] the fact that
> "does not exist but here is the location it would be inserted" is
> encoded in a certain way [...], the side that
> consumes the encoded "pos" [...] should be abstracted away [...].
Thank you, that summarizes my thoughts very well.
> I think that is a reasonable thing to consider; it is not necessary
> for correctness, but contributes to the conceptual clarity (iow, it
> can be left as a separate clean-up step done after the series is
> done).
Thanks, but I disagree with the course of action: I think we should do
both sides at the same time. (And if we aren't ready to do the decoding
side now, then we should delay the encoding side).
Consider someone is looking at the call site without knowing the
detailed meaning of the return value ("What the heck is this -1
business?"), it is a matter to look at the function being called to know
what it is. With another function that does the encoding, it is one
additional hop. That is my reason for saying "more obfuscating than
necessary".
That the helper function would reduce some small code duplication does
not outweigh the obfuscation in my book. That's just MHO, of course.
-- Hannes
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline
2019-10-06 10:45 ` Johannes Schindelin
@ 2019-10-06 20:38 ` Johannes Schindelin
2019-10-07 1:14 ` Junio C Hamano
2019-10-07 0:59 ` Junio C Hamano
1 sibling, 1 reply; 72+ messages in thread
From: Johannes Schindelin @ 2019-10-06 20:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin via GitGitGadget, git, Denton Liu
Hi Junio,
On Sun, 6 Oct 2019, Johannes Schindelin wrote:
> On Sun, 6 Oct 2019, Junio C Hamano wrote:
>
> > "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> > writes:
> >
> > > Range-diff vs v2:
> > >
> > > 1: 4d0b38125a = 1: 4d0b38125a push: do not pretend to return `int` from `die_push_simple()`
> > > 2: 8800320590 < -: ---------- msvc: avoid using minus operator on unsigned types
> > > -: ---------- > 2: 7fe2a85506 msvc: avoid using minus operator on unsigned types
> > > 3: 8512a3e96d = 3: e632a4eef4 winansi: use FLEX_ARRAY to avoid compiler warning
> >
> > This is less useful than it could be.
> >
> > With a larger creation-factor (and we can afford using a larger one,
> > simply because the user of GGG _knows_ that the two series being
> > compared are closely related), what is output is entirely readable
> > (attached at the end).
>
> I just implemented this here:
> https://github.com/gitgitgadget/gitgitgadget/pull/128 (it still needs to
> be reviewed and merged before it takes effect).
FWIW this is now merged.
Ciao,
Dscho
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline
2019-10-06 10:45 ` Johannes Schindelin
2019-10-06 20:38 ` Johannes Schindelin
@ 2019-10-07 0:59 ` Junio C Hamano
2019-10-07 16:08 ` Thomas Gummerer
1 sibling, 1 reply; 72+ messages in thread
From: Junio C Hamano @ 2019-10-07 0:59 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Johannes Schindelin via GitGitGadget, git, Denton Liu
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Date: Fri, 04 Oct 2019 08:09:25 -0700 (PDT)
> [...]
> X-Google-Original-Date: Fri, 04 Oct 2019 15:09:10 GMT
> [...]
>
> I am fairly certain that the latter is the actual `Date:` line sent to
> GMail, and GMail just decides that it will not respect it.
If the submitting program said "Fri, 04 Oct 2019 15:09:10 +0000
(GMT)" instead of "Fri, 04 Oct 2019 15:09:10 GMT", that would match
the format the MTA produced itself, I guess. I am kind-of surprised
if the problem is the use of the obs-zone format (RFC 2822 page 31),
but anything is possible with GMail X-<.
How does send-email write that date header? Matching that would be
probably the most appropriate, if possible, given that GGG was
written for send-email refugees, I guess ;-)
Here is what its format_2822_time sub does, so +0000 without any
textual zone name, it is.
return sprintf("%s, %2d %s %d %02d:%02d:%02d %s%02d%02d",
qw(Sun Mon Tue Wed Thu Fri Sat)[$localtm[6]],
$localtm[3],
qw(Jan Feb Mar Apr May Jun
Jul Aug Sep Oct Nov Dec)[$localtm[4]],
$localtm[5]+1900,
$localtm[2],
$localtm[1],
$localtm[0],
($offset >= 0) ? '+' : '-',
abs($offhour),
$offmin,
);
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline
2019-10-06 20:38 ` Johannes Schindelin
@ 2019-10-07 1:14 ` Junio C Hamano
2019-10-07 21:51 ` Johannes Schindelin
0 siblings, 1 reply; 72+ messages in thread
From: Junio C Hamano @ 2019-10-07 1:14 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Johannes Schindelin via GitGitGadget, git, Denton Liu
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> I just implemented this here:
>> https://github.com/gitgitgadget/gitgitgadget/pull/128 (it still needs to
>> be reviewed and merged before it takes effect).
>
> FWIW this is now merged.
Nice.
I didn't quite understand this part, though.
The default creation factor is 60 (roughly speaking, it wants 60% of
the lines to match between two patches, otherwise it considers the
patches to be unrelated).
Would the updated creation factor used which is 95 (roughly
speaking) want 95% of the lines to match between two patches?
That would make the matching logic even pickier and reject more
paring, so I must be reading the statement wrong X-<.
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline
2019-10-07 0:59 ` Junio C Hamano
@ 2019-10-07 16:08 ` Thomas Gummerer
2019-10-11 22:06 ` Johannes Schindelin
0 siblings, 1 reply; 72+ messages in thread
From: Thomas Gummerer @ 2019-10-07 16:08 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Johannes Schindelin via GitGitGadget, git,
Denton Liu
On 10/07, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > Date: Fri, 04 Oct 2019 08:09:25 -0700 (PDT)
> > [...]
> > X-Google-Original-Date: Fri, 04 Oct 2019 15:09:10 GMT
> > [...]
> >
> > I am fairly certain that the latter is the actual `Date:` line sent to
> > GMail, and GMail just decides that it will not respect it.
>
> If the submitting program said "Fri, 04 Oct 2019 15:09:10 +0000
> (GMT)" instead of "Fri, 04 Oct 2019 15:09:10 GMT", that would match
> the format the MTA produced itself, I guess. I am kind-of surprised
> if the problem is the use of the obs-zone format (RFC 2822 page 31),
> but anything is possible with GMail X-<.
Yeah, the obs-zone format did seem to be the problem. I just dug up
the previous thread we had about this, where I confirmed that +0000 as
the timezone worked just fine in my setup through GMail [*1*]. Note
sure if the (GMT) would cause any problems, but I'd agree with
avoiding it as you mention below to make sure GMail doesn't do
anything funny with it.
*1*: https://public-inbox.org/git/20190318214842.GA32487@hank.intra.tgummerer.com/
> How does send-email write that date header? Matching that would be
> probably the most appropriate, if possible, given that GGG was
> written for send-email refugees, I guess ;-)
>
> Here is what its format_2822_time sub does, so +0000 without any
> textual zone name, it is.
>
> return sprintf("%s, %2d %s %d %02d:%02d:%02d %s%02d%02d",
> qw(Sun Mon Tue Wed Thu Fri Sat)[$localtm[6]],
> $localtm[3],
> qw(Jan Feb Mar Apr May Jun
> Jul Aug Sep Oct Nov Dec)[$localtm[4]],
> $localtm[5]+1900,
> $localtm[2],
> $localtm[1],
> $localtm[0],
> ($offset >= 0) ? '+' : '-',
> abs($offhour),
> $offmin,
> );
>
>
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 03/13] winansi: use FLEX_ARRAY to avoid compiler warning
2019-10-04 15:09 ` [PATCH v3 03/13] winansi: use FLEX_ARRAY to avoid compiler warning Johannes Schindelin via GitGitGadget
@ 2019-10-07 19:16 ` Alban Gruin
0 siblings, 0 replies; 72+ messages in thread
From: Alban Gruin @ 2019-10-07 19:16 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget, git
Cc: Denton Liu, Johannes Schindelin, Junio C Hamano
Hi Johannes,
Le 04/10/2019 à 17:09, Johannes Schindelin via GitGitGadget a écrit :
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> MSVC would complain thusly:
>
> C4200: nonstandard extension used: zero-sized array in struct/union
>
> Let's just use the `FLEX_ARRAY` constant that we introduced for exactly
> this type of scenario.
Perhaps this is a good candidate for a semantic patch?
Cheers,
Alban
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline
2019-10-07 1:14 ` Junio C Hamano
@ 2019-10-07 21:51 ` Johannes Schindelin
2019-10-08 2:19 ` Junio C Hamano
0 siblings, 1 reply; 72+ messages in thread
From: Johannes Schindelin @ 2019-10-07 21:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin via GitGitGadget, git, Denton Liu
Hi Junio,
On Mon, 7 Oct 2019, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> I just implemented this here:
> >> https://github.com/gitgitgadget/gitgitgadget/pull/128 (it still needs to
> >> be reviewed and merged before it takes effect).
> >
> > FWIW this is now merged.
>
> Nice.
>
> I didn't quite understand this part, though.
>
> The default creation factor is 60 (roughly speaking, it wants 60% of
> the lines to match between two patches, otherwise it considers the
> patches to be unrelated).
>
> Would the updated creation factor used which is 95 (roughly
> speaking) want 95% of the lines to match between two patches?
>
> That would make the matching logic even pickier and reject more
> paring, so I must be reading the statement wrong X-<.
No, I must have written the opposite of what I tried to say, is all.
Sorry about the confusion.
Ciao,
Dscho
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline
2019-10-07 21:51 ` Johannes Schindelin
@ 2019-10-08 2:19 ` Junio C Hamano
2019-10-08 12:46 ` Johannes Schindelin
0 siblings, 1 reply; 72+ messages in thread
From: Junio C Hamano @ 2019-10-08 2:19 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Johannes Schindelin via GitGitGadget, git, Denton Liu
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> I didn't quite understand this part, though.
>>
>> The default creation factor is 60 (roughly speaking, it wants 60% of
>> the lines to match between two patches, otherwise it considers the
>> patches to be unrelated).
>>
>> Would the updated creation factor used which is 95 (roughly
>> speaking) want 95% of the lines to match between two patches?
>>
>> That would make the matching logic even pickier and reject more
>> paring, so I must be reading the statement wrong X-<.
>
> No, I must have written the opposite of what I tried to say, is all.
So, cfactor of 60 means at most 60% is allowed to differ and the
two patches are still considered to be related, while 95 means only
5% needs to be common? That would make more sense to me.
Thanks.
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v2 02/13] msvc: avoid using minus operator on unsigned types
2019-10-06 10:53 ` Johannes Sixt
@ 2019-10-08 12:04 ` Johannes Schindelin
2019-10-08 21:13 ` Johannes Sixt
0 siblings, 1 reply; 72+ messages in thread
From: Johannes Schindelin @ 2019-10-08 12:04 UTC (permalink / raw)
To: Johannes Sixt
Cc: Junio C Hamano, Johannes Schindelin via GitGitGadget, git,
Denton Liu
Hi Hannes,
On Sun, 6 Oct 2019, Johannes Sixt wrote:
> Am 06.10.19 um 02:02 schrieb Junio C Hamano:
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >
> >>> IMHO, if you don't accompany insert_pos_as_negative_offset() with a
> >>> corresponding extract_pos_and_found_condition() and use it everywhere,
> >>> it is more obfuscating than necessary.
> >>
> >> I do disagree here. No overflow checking needs to be performed for `-1 -
> >> <int-value>`. And that's what the opposite of this function really boils
> >> down to.
> >
> > I do not think j6t is referring to the over/underflow issues at all.
> >
> > The suggestion is that, because insert-pos-as-negative-offset
> > abstracts away [...] the fact that
> > "does not exist but here is the location it would be inserted" is
> > encoded in a certain way [...], the side that
> > consumes the encoded "pos" [...] should be abstracted away [...].
>
> Thank you, that summarizes my thoughts very well.
>
> > I think that is a reasonable thing to consider; it is not necessary
> > for correctness, but contributes to the conceptual clarity (iow, it
> > can be left as a separate clean-up step done after the series is
> > done).
>
> Thanks, but I disagree with the course of action: I think we should do
> both sides at the same time. (And if we aren't ready to do the decoding
> side now, then we should delay the encoding side).
>
> Consider someone is looking at the call site without knowing the
> detailed meaning of the return value ("What the heck is this -1
> business?"), it is a matter to look at the function being called to know
> what it is. With another function that does the encoding, it is one
> additional hop. That is my reason for saying "more obfuscating than
> necessary".
>
> That the helper function would reduce some small code duplication does
> not outweigh the obfuscation in my book. That's just MHO, of course.
So you got what you wished for:
https://public-inbox.org/git/pull.378.git.gitgitgadget@gmail.com
Care to review?
Thanks,
Dscho
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline
2019-10-08 2:19 ` Junio C Hamano
@ 2019-10-08 12:46 ` Johannes Schindelin
2019-10-09 13:57 ` Philip Oakley
0 siblings, 1 reply; 72+ messages in thread
From: Johannes Schindelin @ 2019-10-08 12:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin via GitGitGadget, git, Denton Liu
Hi Junio,
On Tue, 8 Oct 2019, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> I didn't quite understand this part, though.
> >>
> >> The default creation factor is 60 (roughly speaking, it wants 60% of
> >> the lines to match between two patches, otherwise it considers the
> >> patches to be unrelated).
> >>
> >> Would the updated creation factor used which is 95 (roughly
> >> speaking) want 95% of the lines to match between two patches?
> >>
> >> That would make the matching logic even pickier and reject more
> >> paring, so I must be reading the statement wrong X-<.
> >
> > No, I must have written the opposite of what I tried to say, is all.
>
> So, cfactor of 60 means at most 60% is allowed to differ and the
> two patches are still considered to be related, while 95 means only
> 5% needs to be common? That would make more sense to me.
Okay, I not only wrote the opposite of what I wanted to say, I also
misremembered.
When `range-diff` tries to determine matching pairs of patches, it
builds an `(m+n)x(m+n)` cost matrix, where `m` is the number of patches
in the first commit range and `n` is the number of patches in the second
one.
Why not `m x n`? Well, that's the obvious matrix, and that's what it
starts with, essentially assigning the number of lines of the diff
between the diffs as "cost".
But then `git range-diff` extends the cost matrix to allow for _all_ of
the `m` patches to be considered deleted, and _all_ of the `n` patches
to be added. As cost, it cannot use a "diff of diffs" because there is
no second diff. So it uses the number of lines of the one diff it has,
multiplied by the creation factor interpreted as a percentage.
The naive creation factor would be 100%, which is (almost) as if we
assumed an empty diff for the missing diff. But that would make the
range-diff too eager to dismiss rewrites, as experience obviously showed
(not my experience, but Thomas Rast's, who came up with `tbdiff` after
all): the diff of diffs includes a diff header, for example.
The interpretation I offered (although I inverted what I wanted to say)
is similar in spirit to that metric (which is not actually a metric, I
believe, because I expect it to violate the triangle inequality) is
obviously inaccurate: the number of lines of the diff of diffs does not
say anything about the number of matching lines, quite to the contrary,
it correlates somewhat to the number of non-matching lines.
So a better interpretation would have been:
The default creation factor is 60 (roughly speaking, it wants at
most 60% of the diffs' lines to differ, otherwise it considers
them not to be a match.
This is still inaccurate, but at least it gets the idea of the
range-diff across.
Of course, I will never be able to amend the commit message in
GitGitGadget anyway, as I have merged that PR already.
Ciao,
Dscho
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v2 02/13] msvc: avoid using minus operator on unsigned types
2019-10-08 12:04 ` Johannes Schindelin
@ 2019-10-08 21:13 ` Johannes Sixt
0 siblings, 0 replies; 72+ messages in thread
From: Johannes Sixt @ 2019-10-08 21:13 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Junio C Hamano, Johannes Schindelin via GitGitGadget, git,
Denton Liu
Am 08.10.19 um 14:04 schrieb Johannes Schindelin:
> So you got what you wished for:
> https://public-inbox.org/git/pull.378.git.gitgitgadget@gmail.com
After having seen the result I do not wish for it anymore. (Not that I
had "wished" for it in the first place...) It does not make the result
any more readable than the original.
I do wish you had rejected Junio's suggestion to introduce
index_pos_to_insert_pos(). It doesn't make the code a lot more readable,
either, in my eyes.
-- Hannes
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline
2019-10-08 12:46 ` Johannes Schindelin
@ 2019-10-09 13:57 ` Philip Oakley
2019-10-10 9:03 ` Johannes Schindelin
0 siblings, 1 reply; 72+ messages in thread
From: Philip Oakley @ 2019-10-09 13:57 UTC (permalink / raw)
To: Johannes Schindelin, Junio C Hamano
Cc: Johannes Schindelin via GitGitGadget, git, Denton Liu
Hi Dscho,
On 08/10/2019 13:46, Johannes Schindelin wrote:
> Hi Junio,
>
> On Tue, 8 Oct 2019, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>
>>>> I didn't quite understand this part, though.
>>>>
>>>> The default creation factor is 60 (roughly speaking, it wants 60% of
>>>> the lines to match between two patches, otherwise it considers the
>>>> patches to be unrelated).
>>>>
>>>> Would the updated creation factor used which is 95 (roughly
>>>> speaking) want 95% of the lines to match between two patches?
>>>>
>>>> That would make the matching logic even pickier and reject more
>>>> paring, so I must be reading the statement wrong X-<.
>>> No, I must have written the opposite of what I tried to say, is all.
>> So, cfactor of 60 means at most 60% is allowed to differ and the
>> two patches are still considered to be related, while 95 means only
>> 5% needs to be common? That would make more sense to me.
> Okay, I not only wrote the opposite of what I wanted to say, I also
> misremembered.
>
> When `range-diff` tries to determine matching pairs of patches, it
> builds an `(m+n)x(m+n)` cost matrix, where `m` is the number of patches
> in the first commit range and `n` is the number of patches in the second
> one.
>
> Why not `m x n`? Well, that's the obvious matrix, and that's what it
> starts with, essentially assigning the number of lines of the diff
> between the diffs as "cost".
>
> But then `git range-diff` extends the cost matrix to allow for _all_ of
> the `m` patches to be considered deleted, and _all_ of the `n` patches
> to be added. As cost, it cannot use a "diff of diffs" because there is
> no second diff. So it uses the number of lines of the one diff it has,
> multiplied by the creation factor interpreted as a percentage.
>
> The naive creation factor would be 100%, which is (almost) as if we
> assumed an empty diff for the missing diff. But that would make the
> range-diff too eager to dismiss rewrites, as experience obviously showed
> (not my experience, but Thomas Rast's, who came up with `tbdiff` after
> all): the diff of diffs includes a diff header, for example.
>
> The interpretation I offered (although I inverted what I wanted to say)
> is similar in spirit to that metric (which is not actually a metric, I
> believe, because I expect it to violate the triangle inequality) is
> obviously inaccurate: the number of lines of the diff of diffs does not
> say anything about the number of matching lines, quite to the contrary,
> it correlates somewhat to the number of non-matching lines.
>
> So a better interpretation would have been:
>
> The default creation factor is 60 (roughly speaking, it wants at
> most 60% of the diffs' lines to differ, otherwise it considers
> them not to be a match.
>
> This is still inaccurate, but at least it gets the idea of the
> range-diff across.
>
> Of course, I will never be able to amend the commit message in
> GitGitGadget anyway, as I have merged that PR already.
>
> Ciao,
> Dscho
Medium term, is this something that could go in the algorithms section
of the range-diff man page, especially if the upstream commit message is
already in place.
#leftoverdocs ?
Philip
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline
2019-10-09 13:57 ` Philip Oakley
@ 2019-10-10 9:03 ` Johannes Schindelin
2019-10-10 10:12 ` Philip Oakley
0 siblings, 1 reply; 72+ messages in thread
From: Johannes Schindelin @ 2019-10-10 9:03 UTC (permalink / raw)
To: Philip Oakley
Cc: Junio C Hamano, Johannes Schindelin via GitGitGadget, git,
Denton Liu
Hi Philip,
On Wed, 9 Oct 2019, Philip Oakley wrote:
> On 08/10/2019 13:46, Johannes Schindelin wrote:
> > Hi Junio,
> >
> > On Tue, 8 Oct 2019, Junio C Hamano wrote:
> >
> > > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> > >
> > > > > I didn't quite understand this part, though.
> > > > >
> > > > > The default creation factor is 60 (roughly speaking, it wants 60%
> > > > > of
> > > > > the lines to match between two patches, otherwise it considers
> > > > > the
> > > > > patches to be unrelated).
> > > > >
> > > > > Would the updated creation factor used which is 95 (roughly
> > > > > speaking) want 95% of the lines to match between two patches?
> > > > >
> > > > > That would make the matching logic even pickier and reject more
> > > > > paring, so I must be reading the statement wrong X-<.
> > > > No, I must have written the opposite of what I tried to say, is all.
> > > So, cfactor of 60 means at most 60% is allowed to differ and the
> > > two patches are still considered to be related, while 95 means only
> > > 5% needs to be common? That would make more sense to me.
> > Okay, I not only wrote the opposite of what I wanted to say, I also
> > misremembered.
> >
> > When `range-diff` tries to determine matching pairs of patches, it
> > builds an `(m+n)x(m+n)` cost matrix, where `m` is the number of patches
> > in the first commit range and `n` is the number of patches in the second
> > one.
> >
> > Why not `m x n`? Well, that's the obvious matrix, and that's what it
> > starts with, essentially assigning the number of lines of the diff
> > between the diffs as "cost".
> >
> > But then `git range-diff` extends the cost matrix to allow for _all_ of
> > the `m` patches to be considered deleted, and _all_ of the `n` patches
> > to be added. As cost, it cannot use a "diff of diffs" because there is
> > no second diff. So it uses the number of lines of the one diff it has,
> > multiplied by the creation factor interpreted as a percentage.
> >
> > The naive creation factor would be 100%, which is (almost) as if we
> > assumed an empty diff for the missing diff. But that would make the
> > range-diff too eager to dismiss rewrites, as experience obviously showed
> > (not my experience, but Thomas Rast's, who came up with `tbdiff` after
> > all): the diff of diffs includes a diff header, for example.
> >
> > The interpretation I offered (although I inverted what I wanted to say)
> > is similar in spirit to that metric (which is not actually a metric, I
> > believe, because I expect it to violate the triangle inequality) is
> > obviously inaccurate: the number of lines of the diff of diffs does not
> > say anything about the number of matching lines, quite to the contrary,
> > it correlates somewhat to the number of non-matching lines.
> >
> > So a better interpretation would have been:
> >
> > The default creation factor is 60 (roughly speaking, it wants at
> > most 60% of the diffs' lines to differ, otherwise it considers
> > them not to be a match.
> >
> > This is still inaccurate, but at least it gets the idea of the
> > range-diff across.
> >
> > Of course, I will never be able to amend the commit message in
> > GitGitGadget anyway, as I have merged that PR already.
> >
> > Ciao,
> > Dscho
> Medium term, is this something that could go in the algorithms section of the
> range-diff man page, especially if the upstream commit message is already in
> place.
>
> #leftoverdocs ?
Sure. How about giving it a try while our memory is still fresh? You
would help me immensely if you could take that task off of my plate...
Thanks,
Dscho
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline
2019-10-10 9:03 ` Johannes Schindelin
@ 2019-10-10 10:12 ` Philip Oakley
0 siblings, 0 replies; 72+ messages in thread
From: Philip Oakley @ 2019-10-10 10:12 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Junio C Hamano, Johannes Schindelin via GitGitGadget, git,
Denton Liu
Hi Dscho,
On 10/10/2019 10:03, Johannes Schindelin wrote:
>>> So a better interpretation would have been:
>>>
>>> The default creation factor is 60 (roughly speaking, it wants at
>>> most 60% of the diffs' lines to differ, otherwise it considers
>>> them not to be a match.
>>>
>>> This is still inaccurate, but at least it gets the idea of the
>>> range-diff across.
>>>
>>> Of course, I will never be able to amend the commit message in
>>> GitGitGadget anyway, as I have merged that PR already.
>>>
>>> Ciao,
>>> Dscho
>> Medium term, is this something that could go in the algorithms section of the
>> range-diff man page, especially if the upstream commit message is already in
>> place.
>>
>> #leftoverdocs ?
> Sure. How about giving it a try while our memory is still fresh? You
> would help me immensely if you could take that task off of my plate...
My sneak think was to introduce the new hash tag for useful explanations
from the list that could be copied into the man/guide pages, especially
for the project pages.
At the moment I'm trying to get the >4Gb on Window' series sorted, which
though working has been a load of bear traps for testing and configs and
VS compiling, etc.
But yes, it is on the back-list.
--
Philip
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline
2019-10-07 16:08 ` Thomas Gummerer
@ 2019-10-11 22:06 ` Johannes Schindelin
0 siblings, 0 replies; 72+ messages in thread
From: Johannes Schindelin @ 2019-10-11 22:06 UTC (permalink / raw)
To: Thomas Gummerer
Cc: Junio C Hamano, Johannes Schindelin via GitGitGadget, git,
Denton Liu
Hi Thomas & Junio,
On Mon, 7 Oct 2019, Thomas Gummerer wrote:
> On 10/07, Junio C Hamano wrote:
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >
> > > Date: Fri, 04 Oct 2019 08:09:25 -0700 (PDT)
> > > [...]
> > > X-Google-Original-Date: Fri, 04 Oct 2019 15:09:10 GMT
> > > [...]
> > >
> > > I am fairly certain that the latter is the actual `Date:` line sent to
> > > GMail, and GMail just decides that it will not respect it.
> >
> > If the submitting program said "Fri, 04 Oct 2019 15:09:10 +0000
> > (GMT)" instead of "Fri, 04 Oct 2019 15:09:10 GMT", that would match
> > the format the MTA produced itself, I guess. I am kind-of surprised
> > if the problem is the use of the obs-zone format (RFC 2822 page 31),
> > but anything is possible with GMail X-<.
>
> Yeah, the obs-zone format did seem to be the problem. I just dug up
> the previous thread we had about this, where I confirmed that +0000 as
> the timezone worked just fine in my setup through GMail [*1*]. Note
> sure if the (GMT) would cause any problems, but I'd agree with
> avoiding it as you mention below to make sure GMail doesn't do
> anything funny with it.
>
> *1*: https://public-inbox.org/git/20190318214842.GA32487@hank.intra.tgummerer.com/
Urg. Sorry for dropping the ball on that one.
I changed the GitGitGadget code accordingly (here is the commit:
https://github.com/gitgitgadget/gitgitgadget/commit/6117c61be5c2d13b46229ed12b2a9f63ef80f3c9).
Thanks,
Dscho
>
> > How does send-email write that date header? Matching that would be
> > probably the most appropriate, if possible, given that GGG was
> > written for send-email refugees, I guess ;-)
> >
> > Here is what its format_2822_time sub does, so +0000 without any
> > textual zone name, it is.
> >
> > return sprintf("%s, %2d %s %d %02d:%02d:%02d %s%02d%02d",
> > qw(Sun Mon Tue Wed Thu Fri Sat)[$localtm[6]],
> > $localtm[3],
> > qw(Jan Feb Mar Apr May Jun
> > Jul Aug Sep Oct Nov Dec)[$localtm[4]],
> > $localtm[5]+1900,
> > $localtm[2],
> > $localtm[1],
> > $localtm[0],
> > ($offset >= 0) ? '+' : '-',
> > abs($offhour),
> > $offmin,
> > );
> >
> >
>
^ permalink raw reply [flat|nested] 72+ messages in thread
end of thread, other threads:[~2019-10-11 22:07 UTC | newest]
Thread overview: 72+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-26 8:30 [PATCH 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
2019-09-26 8:30 ` [PATCH 01/13] push: do not pretend to return `int` from `die_push_simple()` Johannes Schindelin via GitGitGadget
2019-09-26 8:30 ` [PATCH 02/13] msvc: avoid using minus operator on unsigned types Johannes Schindelin via GitGitGadget
2019-09-26 17:20 ` Denton Liu
2019-09-26 21:01 ` Johannes Schindelin
2019-09-26 23:57 ` Denton Liu
2019-09-30 9:50 ` Johannes Schindelin
2019-09-26 8:30 ` [PATCH 03/13] winansi: use FLEX_ARRAY to avoid compiler warning Johannes Schindelin via GitGitGadget
2019-09-26 8:30 ` [PATCH 04/13] compat/win32/path-utils.h: add #include guards Johannes Schindelin via GitGitGadget
2019-09-26 8:30 ` [PATCH 05/13] msvc: ignore some libraries when linking Johannes Schindelin via GitGitGadget
2019-09-26 8:30 ` [PATCH 06/13] msvc: handle DEVELOPER=1 Johannes Schindelin via GitGitGadget
2019-09-26 8:30 ` [PATCH 07/13] msvc: work around a bug in GetEnvironmentVariable() Johannes Schindelin via GitGitGadget
2019-09-26 8:30 ` [PATCH 08/13] vcxproj: only copy `git-remote-http.exe` once it was built Johannes Schindelin via GitGitGadget
2019-09-26 8:30 ` [PATCH 09/13] vcxproj: include more generated files Johannes Schindelin via GitGitGadget
2019-09-26 8:30 ` [PATCH 10/13] test-tool run-command: learn to run (parts of) the testsuite Johannes Schindelin via GitGitGadget
2019-09-26 8:30 ` [PATCH 11/13] tests: let --immediate and --write-junit-xml play well together Johannes Schindelin via GitGitGadget
2019-09-28 22:22 ` Junio C Hamano
2019-09-30 9:52 ` Johannes Schindelin
2019-09-26 8:30 ` [PATCH 12/13] ci: really use shallow clones on Azure Pipelines Johannes Schindelin via GitGitGadget
2019-09-26 8:30 ` [PATCH 13/13] ci: also build and test with MS Visual Studio " Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 02/13] msvc: avoid using minus operator on unsigned types Johannes Schindelin via GitGitGadget
2019-10-03 22:44 ` Junio C Hamano
2019-10-04 9:55 ` Johannes Schindelin
2019-10-04 17:09 ` Johannes Sixt
2019-10-04 21:24 ` Johannes Schindelin
2019-10-06 0:02 ` Junio C Hamano
2019-10-06 10:53 ` Johannes Sixt
2019-10-08 12:04 ` Johannes Schindelin
2019-10-08 21:13 ` Johannes Sixt
2019-09-30 9:55 ` [PATCH v2 01/13] push: do not pretend to return `int` from `die_push_simple()` Johannes Schindelin via GitGitGadget
2019-10-03 22:37 ` Junio C Hamano
2019-10-04 9:36 ` Johannes Schindelin
2019-09-30 9:55 ` [PATCH v2 03/13] winansi: use FLEX_ARRAY to avoid compiler warning Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 05/13] msvc: ignore some libraries when linking Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 04/13] compat/win32/path-utils.h: add #include guards Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 06/13] msvc: handle DEVELOPER=1 Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 07/13] msvc: work around a bug in GetEnvironmentVariable() Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 08/13] vcxproj: only copy `git-remote-http.exe` once it was built Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 09/13] vcxproj: include more generated files Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 10/13] test-tool run-command: learn to run (parts of) the testsuite Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 11/13] tests: let --immediate and --write-junit-xml play well together Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 12/13] ci: really use shallow clones on Azure Pipelines Johannes Schindelin via GitGitGadget
2019-09-30 9:55 ` [PATCH v2 13/13] ci: also build and test with MS Visual Studio " Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 01/13] push: do not pretend to return `int` from `die_push_simple()` Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 02/13] msvc: avoid using minus operator on unsigned types Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 03/13] winansi: use FLEX_ARRAY to avoid compiler warning Johannes Schindelin via GitGitGadget
2019-10-07 19:16 ` Alban Gruin
2019-10-04 15:09 ` [PATCH v3 04/13] compat/win32/path-utils.h: add #include guards Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 05/13] msvc: ignore some libraries when linking Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 07/13] msvc: work around a bug in GetEnvironmentVariable() Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 06/13] msvc: handle DEVELOPER=1 Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 08/13] vcxproj: only copy `git-remote-http.exe` once it was built Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 09/13] vcxproj: include more generated files Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 10/13] test-tool run-command: learn to run (parts of) the testsuite Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 11/13] tests: let --immediate and --write-junit-xml play well together Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 12/13] ci: really use shallow clones on Azure Pipelines Johannes Schindelin via GitGitGadget
2019-10-04 15:09 ` [PATCH v3 13/13] ci: also build and test with MS Visual Studio " Johannes Schindelin via GitGitGadget
2019-10-06 0:19 ` [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Junio C Hamano
2019-10-06 10:45 ` Johannes Schindelin
2019-10-06 20:38 ` Johannes Schindelin
2019-10-07 1:14 ` Junio C Hamano
2019-10-07 21:51 ` Johannes Schindelin
2019-10-08 2:19 ` Junio C Hamano
2019-10-08 12:46 ` Johannes Schindelin
2019-10-09 13:57 ` Philip Oakley
2019-10-10 9:03 ` Johannes Schindelin
2019-10-10 10:12 ` Philip Oakley
2019-10-07 0:59 ` Junio C Hamano
2019-10-07 16:08 ` Thomas Gummerer
2019-10-11 22:06 ` Johannes Schindelin
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).