From: "Kyle Lippincott via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Patrick Steinhardt <ps@pks.im>, Kyle Lippincott <spectral@google.com>
Subject: [PATCH v2 0/3] Small fixes for issues detected during internal CI runs
Date: Fri, 02 Aug 2024 20:58:04 +0000 [thread overview]
Message-ID: <pull.1756.v2.git.git.1722632287.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1756.git.git.1722571853.gitgitgadget@gmail.com>
I'm attempting to get the git test suite running automatically during our
weekly import. I have this mostly working, including with Address Sanitizer
and Memory Sanitizer, but ran into a few issues:
* several tests were failing due to strbuf_getcwd not clearing errno on
success after it internally looped due to the path being >128 bytes. This
is resolved in depth; though either one of the commits alone would
resolve our issues:
* modify locations that call strtoX and check for ERANGE to set errno =
0; prior to calling the conversion function. This is the typical way
that these functions are invoked, and may indicate that we want
compatibility helpers in git-compat-util.h to ensure that this happens
correctly (and add these functions to the banned list).
* have strbuf_getcwd set errno = 0; prior to a successful exit. This
isn't very common for most functions in the codebase, but some other
examples of this were found.
* t6421-merge-partial-clone.sh had >10% flakiness. This is due to our build
system using paths that contain a 64-hex-char hash, which had a 12.5%
chance of containing the substring d0.
Kyle Lippincott (3):
set errno=0 before strtoX calls
strbuf: set errno to 0 after strbuf_getcwd
t6421: fix test to work when repo dir contains d0
builtin/get-tar-commit-id.c | 1 +
ref-filter.c | 1 +
strbuf.c | 1 +
t/helper/test-json-writer.c | 2 ++
t/helper/test-trace2.c | 1 +
t/t6421-merge-partial-clone.sh | 15 +++++++++------
6 files changed, 15 insertions(+), 6 deletions(-)
base-commit: e559c4bf1a306cf5814418d318cc0fea070da3c7
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1756%2Fspectral54%2Fstrbuf_getcwd-clear-errno-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1756/spectral54/strbuf_getcwd-clear-errno-v2
Pull-Request: https://github.com/git/git/pull/1756
Range-diff vs v1:
1: 4dbd0bec40a = 1: 4dbd0bec40a set errno=0 before strtoX calls
2: 0ed09e9abb8 = 2: 0ed09e9abb8 strbuf: set errno to 0 after strbuf_getcwd
3: 6c08b8ceb2b ! 3: 818dc9e6b3e t6421: fix test to work when repo dir contains d0
@@ Commit message
## t/t6421-merge-partial-clone.sh ##
@@ t/t6421-merge-partial-clone.sh: test_expect_merge_algorithm failure success 'Objects downloaded for single relev
+ grep fetch_count trace.output | cut -d "|" -f 9 | tr -d " ." >actual &&
test_cmp expect actual &&
- # Check the number of fetch commands exec-ed
+- # Check the number of fetch commands exec-ed
- grep d0.*fetch.negotiationAlgorithm trace.output >fetches &&
++ # Check the number of fetch commands exec-ed by filtering trace to
++ # child_start events by the top-level program (2nd field == d0)
+ grep " d0 .* child_start .*fetch.negotiationAlgorithm" trace.output >fetches &&
test_line_count = 2 fetches &&
git rev-list --objects --all --missing=print |
@@ t/t6421-merge-partial-clone.sh: test_expect_merge_algorithm failure success 'Objects downloaded when a directory
+ grep fetch_count trace.output | cut -d "|" -f 9 | tr -d " ." >actual &&
test_cmp expect actual &&
- # Check the number of fetch commands exec-ed
+- # Check the number of fetch commands exec-ed
- grep d0.*fetch.negotiationAlgorithm trace.output >fetches &&
++ # Check the number of fetch commands exec-ed by filtering trace to
++ # child_start events by the top-level program (2nd field == d0)
+ grep " d0 .* child_start .*fetch.negotiationAlgorithm" trace.output >fetches &&
test_line_count = 1 fetches &&
git rev-list --objects --all --missing=print |
@@ t/t6421-merge-partial-clone.sh: test_expect_merge_algorithm failure success 'Objects downloaded with lots of ren
+ grep fetch_count trace.output | cut -d "|" -f 9 | tr -d " ." >actual &&
test_cmp expect actual &&
- # Check the number of fetch commands exec-ed
+- # Check the number of fetch commands exec-ed
- grep d0.*fetch.negotiationAlgorithm trace.output >fetches &&
++ # Check the number of fetch commands exec-ed by filtering trace to
++ # child_start events by the top-level program (2nd field == d0)
+ grep " d0 .* child_start .*fetch.negotiationAlgorithm" trace.output >fetches &&
test_line_count = 4 fetches &&
--
gitgitgadget
next prev parent reply other threads:[~2024-08-02 20:58 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-02 4:10 [PATCH 0/3] Small fixes for issues detected during internal CI runs Kyle Lippincott via GitGitGadget
2024-08-02 4:10 ` [PATCH 1/3] set errno=0 before strtoX calls Kyle Lippincott via GitGitGadget
2024-08-02 5:12 ` Patrick Steinhardt
2024-08-02 6:15 ` Kyle Lippincott
2024-08-02 15:01 ` Junio C Hamano
2024-08-02 4:10 ` [PATCH 2/3] strbuf: set errno to 0 after strbuf_getcwd Kyle Lippincott via GitGitGadget
2024-08-02 15:10 ` Junio C Hamano
2024-08-02 17:56 ` Kyle Lippincott
2024-08-02 4:10 ` [PATCH 3/3] t6421: fix test to work when repo dir contains d0 Kyle Lippincott via GitGitGadget
2024-08-02 15:13 ` Junio C Hamano
2024-08-02 20:58 ` Kyle Lippincott via GitGitGadget [this message]
2024-08-02 20:58 ` [PATCH v2 1/3] set errno=0 before strtoX calls Kyle Lippincott via GitGitGadget
2024-08-02 21:18 ` Junio C Hamano
2024-08-02 20:58 ` [PATCH v2 2/3] strbuf: set errno to 0 after strbuf_getcwd Kyle Lippincott via GitGitGadget
2024-08-02 21:32 ` Junio C Hamano
2024-08-02 21:54 ` Eric Sunshine
2024-08-05 15:51 ` Junio C Hamano
2024-08-06 6:26 ` Patrick Steinhardt
2024-08-06 7:04 ` Kyle Lippincott
2024-08-02 23:51 ` Kyle Lippincott
2024-08-05 17:12 ` Kyle Lippincott
2024-08-02 20:58 ` [PATCH v2 3/3] t6421: fix test to work when repo dir contains d0 Kyle Lippincott via GitGitGadget
2024-08-02 21:41 ` Junio C Hamano
2024-08-03 0:03 ` Kyle Lippincott
2024-08-03 0:27 ` Junio C Hamano
2024-08-05 17:10 ` [PATCH v3 0/2] Small fixes for issues detected during internal CI runs Kyle Lippincott via GitGitGadget
2024-08-05 17:10 ` [PATCH v3 1/2] set errno=0 before strtoX calls Kyle Lippincott via GitGitGadget
2024-08-05 17:10 ` [PATCH v3 2/2] t6421: fix test to work when repo dir contains d0 Kyle Lippincott via GitGitGadget
2024-08-05 18:37 ` [PATCH v3 0/2] Small fixes for issues detected during internal CI runs 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.1756.v2.git.git.1722632287.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=git@vger.kernel.org \
--cc=ps@pks.im \
--cc=spectral@google.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.