git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: "Jeff King" <peff@peff.net>,
	"SZEDER Gábor" <szeder.dev@gmail.com>,
	"Alexandr Miloslavskiy" <alexandr.miloslavskiy@syntevo.com>,
	"Johannes Schindelin" <johannes.schindelin@gmx.de>,
	"Junio C Hamano" <gitster@pobox.com>
Subject: [PATCH v4 0/1]  Fix t5516 flakiness in Visual Studio builds
Date: Wed, 30 Oct 2019 10:44:35 +0000	[thread overview]
Message-ID: <pull.428.v4.git.1572432276.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.428.v3.git.1572379280.gitgitgadget@gmail.com>

Among the flaky tests, it seems that the Azure Pipeline suffers relatively
frequently from t5516 failing with the Visual Studio builds. Essentially, we
grep for an error message, but that error message is produced twice, once by
a fetch and once by the upload-pack spawned from it, and those error
messages are usually interleaved because of MSVC runtime fprintf() 
idiosyncracies.

As a consequence, I have to re-run about half a dozen failed builds a day,
which I would like to avoid. My plan is therefore to merge this patch into
Git for Windows v2.24.0-rc2.

The commit message of this patch is based, in part, on 
https://github.com/gitgitgadget/git/pull/407. 

This fixes https://github.com/gitgitgadget/git/issues/240.

Changes since v3:

 * Reworded the part of the commit message that talks about the need for 
   fflush(stderr); for accuracy, as suggested by Junio.
 * Simplified the flow of the prefix handling as well as providing a proper 
   BUG message and abort()ing when the prefix is too long. Thanks Junio!

Changes since v2:

 * Using write_in_full() instead of xwrite() again (to make sure that the
   entire message is printed).
 * When vsnprintf() fails, now we at least print the prefix.
 * The code to check whether prefix was too long no longer tests an
   inequality between pointers, but between sizes.

Changes since v1:

 * Changed the oneline to be more accurate (thanks Junio).
 * Improved the commit message (e.g. talking about the xwrite() function
   this patch uses, rather than the write_in_full() function used by an
   earlier iteration, thanks Gábor).
 * Revamped the actual code to account for insanely long prefixes (thanks
   for the advice, Junio).

Johannes Schindelin (1):
  vreportf(): avoid relying on stdio buffering

 usage.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)


base-commit: 566a1439f6f56c2171b8853ddbca0ad3f5098770
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-428%2Fdscho%2Ffix-t5516-flakiness-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-428/dscho/fix-t5516-flakiness-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/428

Range-diff vs v3:

 1:  fce0894ee4 ! 1:  f6d6c8122a vreportf(): avoid relying on stdio buffering
     @@ -42,9 +42,9 @@
          the error message; There is little we can do in that case, and it should
          not happen anyway.
      
     -    Also please note that we `fflush(stderr)` here to help when running in a
     -    Git Bash on Windows: in this case, `stderr` is not actually truly
     -    unbuffered, and needs the extra help.
     +    The process may have written to `stderr` and there may be something left
     +    in the buffer kept in the stdio layer. Call `fflush(stderr)` before
     +    writing the message we prepare in this function.
      
          Helped-by: Jeff King <peff@peff.net>
          Helped-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
     @@ -60,12 +60,17 @@
       {
       	char msg[4096];
      -	char *p;
     -+	size_t off = strlcpy(msg, prefix, sizeof(msg));
      +	char *p, *pend = msg + sizeof(msg);
     ++	size_t prefix_len = strlen(prefix);
       
      -	vsnprintf(msg, sizeof(msg), err, params);
      -	for (p = msg; *p; p++) {
     -+	p = off < pend - msg ? msg + off : pend - 1;
     ++	if (sizeof(msg) <= prefix_len) {
     ++		fprintf(stderr, "BUG!!! too long a prefix '%s'\n", prefix);
     ++		abort();
     ++	}
     ++	memcpy(msg, prefix, prefix_len);
     ++	p = msg + prefix_len;
      +	if (vsnprintf(p, pend - p, err, params) < 0)
      +		*p = '\0'; /* vsnprintf() failed, clip at prefix */
      +

-- 
gitgitgadget

  parent reply	other threads:[~2019-10-30 10:44 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-28 15:00 [PATCH " Johannes Schindelin via GitGitGadget
2019-10-28 15:00 ` [PATCH 1/1] vreportf(): avoid buffered write in favor of unbuffered one Johannes Schindelin via GitGitGadget
2019-10-29  3:18   ` Junio C Hamano
2019-10-29 12:30     ` Johannes Schindelin
2019-10-29 13:49       ` Jeff King
2019-10-29 14:13         ` Johannes Schindelin
2019-10-29 14:32           ` Jeff King
2019-10-29 20:09             ` Johannes Schindelin
2019-10-30  1:43               ` Junio C Hamano
2019-10-29 16:44         ` Junio C Hamano
2019-10-29 10:38   ` SZEDER Gábor
2019-10-29 12:38     ` Johannes Schindelin
2019-10-29 13:52       ` Jeff King
2019-10-29 14:18         ` Johannes Schindelin
2019-10-29 13:37 ` [PATCH v2 0/1] Fix t5516 flakiness in Visual Studio builds Johannes Schindelin via GitGitGadget
2019-10-29 13:37   ` [PATCH v2 1/1] vreportf(): avoid relying on stdio buffering Johannes Schindelin via GitGitGadget
2019-10-29 14:21     ` Alexandr Miloslavskiy
2019-10-29 19:57       ` Johannes Schindelin
2019-10-29 20:09         ` Jeff King
2019-10-29 20:24           ` Alexandr Miloslavskiy
2019-10-29 20:11         ` Alexandr Miloslavskiy
2019-10-29 20:01   ` [PATCH v3 0/1] Fix t5516 flakiness in Visual Studio builds Johannes Schindelin via GitGitGadget
2019-10-29 20:01     ` [PATCH v3 1/1] vreportf(): avoid relying on stdio buffering Johannes Schindelin via GitGitGadget
2019-10-29 20:32       ` Jeff King
2019-10-30  8:54         ` Johannes Schindelin
2019-10-31  6:24           ` Jeff King
2019-10-31 10:26             ` Johannes Schindelin
2019-10-31 15:48               ` Jeff King
2019-11-01 18:41                 ` Johannes Schindelin
2019-10-30  2:01       ` Junio C Hamano
2019-10-30  9:13         ` Johannes Schindelin
2019-10-30 10:44     ` Johannes Schindelin via GitGitGadget [this message]
2019-10-30 10:44       ` [PATCH v4 " Johannes Schindelin via GitGitGadget
2019-11-02  4:05         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.428.v4.git.1572432276.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=alexandr.miloslavskiy@syntevo.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=peff@peff.net \
    --cc=szeder.dev@gmail.com \
    --subject='Re: [PATCH v4 0/1]  Fix t5516 flakiness in Visual Studio builds' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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 NNTP newsgroup(s).