git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Matheus Tavares <matheus.bernardino@usp.br>
To: gitster@pobox.com
Cc: git@vger.kernel.org, avarab@gmail.com
Subject: [PATCH v2 2/3] checkout: show bug about failed entries being included in final report
Date: Thu, 14 Jul 2022 08:49:11 -0300	[thread overview]
Message-ID: <4541e90224f5e79b16717b94f37edd10a4fdbbb1.1657799213.git.matheus.bernardino@usp.br> (raw)
In-Reply-To: <cover.1657799213.git.matheus.bernardino@usp.br>

After checkout, git usually reports how many entries were updated at
that operation. However, because we count the entries too soon during
the checkout process, we may actually include entries that do not get
properly checked out in the end. This can lead to an inaccurate final
report if the user expects it to show only the *successful* updates.
This will be fixed in the next commit, but for now let's document it
with a test that cover all checkout modes.

Note that `test_checkout_workers` have to be slightly adjusted in order
to use the construct `test_checkout_workers ...  test_must_fail git
checkout`. The function runs the command given to it with an assignment
prefix to set the GIT_TRACE2 variable. However, this this assignment has
an undefined behavior when the command is a shell function (like
`test_must_fail`). As POSIX specifies:

  If the command name is a function that is not a standard utility
  implemented as a function, variable assignments shall affect the
  current execution environment during the execution of the function. It
  is unspecified:

    - Whether or not the variable assignments persist after the
      completion of the function

    - Whether or not the variables gain the export attribute during the
      execution of the function

Thus, in order to make sure the GIT_TRACE2 value gets visible to the git
command executed by `test_must_fail`, export the variable and run git in
a subshell.

[1]: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html
     (Vol. 3: Shell and Utilities, Section 2.9.1: Simple Commands)

Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
---
 t/lib-parallel-checkout.sh          |  6 +++-
 t/t2080-parallel-checkout-basics.sh | 48 +++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/t/lib-parallel-checkout.sh b/t/lib-parallel-checkout.sh
index 83b279a846..acaee9cbb6 100644
--- a/t/lib-parallel-checkout.sh
+++ b/t/lib-parallel-checkout.sh
@@ -25,7 +25,11 @@ test_checkout_workers () {
 
 	local trace_file=trace-test-checkout-workers &&
 	rm -f "$trace_file" &&
-	GIT_TRACE2="$(pwd)/$trace_file" "$@" 2>&8 &&
+	(
+		GIT_TRACE2="$(pwd)/$trace_file" &&
+		export GIT_TRACE2 &&
+		"$@" 2>&8
+	) &&
 
 	local workers="$(grep "child_start\[..*\] git checkout--worker" "$trace_file" | wc -l)" &&
 	test $workers -eq $expected_workers &&
diff --git a/t/t2080-parallel-checkout-basics.sh b/t/t2080-parallel-checkout-basics.sh
index 3e0f8c675f..7d6d26e1a4 100755
--- a/t/t2080-parallel-checkout-basics.sh
+++ b/t/t2080-parallel-checkout-basics.sh
@@ -226,4 +226,52 @@ test_expect_success SYMLINKS 'parallel checkout checks for symlinks in leading d
 	)
 '
 
+# This test is here (and not in e.g. t2022-checkout-paths.sh), because we
+# check the final report including sequential, parallel, and delayed entries
+# all at the same time. So we must have finer control of the parallel checkout
+# variables.
+test_expect_failure PERL '"git checkout ." report should not include failed entries' '
+	write_script rot13-filter.pl "$PERL_PATH" \
+		<"$TEST_DIRECTORY"/t0021/rot13-filter.pl &&
+
+	test_config_global filter.delay.process \
+		"\"$(pwd)/rot13-filter.pl\" --always-delay delayed.log clean smudge delay" &&
+	test_config_global filter.delay.required true &&
+	test_config_global filter.cat.clean cat  &&
+	test_config_global filter.cat.smudge cat  &&
+	test_config_global filter.cat.required true  &&
+
+	set_checkout_config 2 0 &&
+	git init failed_entries &&
+	(
+		cd failed_entries &&
+		cat >.gitattributes <<-EOF &&
+		*delay*              filter=delay
+		parallel-ineligible* filter=cat
+		EOF
+		echo a >missing-delay.a &&
+		echo a >parallel-ineligible.a &&
+		echo a >parallel-eligible.a &&
+		echo b >success-delay.b &&
+		echo b >parallel-ineligible.b &&
+		echo b >parallel-eligible.b &&
+		git add -A &&
+		git commit -m files &&
+
+		a_blob="$(git rev-parse :parallel-ineligible.a)" &&
+		rm .git/objects/$(test_oid_to_path $a_blob) &&
+		rm *.a *.b &&
+
+		test_checkout_workers 2 test_must_fail git checkout . 2>err &&
+
+		# All *.b entries should succeed and all *.a entries should fail:
+		#  - missing-delay.a: the delay filter will drop this path
+		#  - parallel-*.a: the blob will be missing
+		#
+		grep "Updated 3 paths from the index" err &&
+		test_stdout_line_count = 3 ls *.b &&
+		! ls *.a
+	)
+'
+
 test_done
-- 
2.37.0


  parent reply	other threads:[~2022-07-14 11:49 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-13  4:19 [PATCH 0/3] checkout: fix two bugs on count of updated entries Matheus Tavares
2022-07-13  4:19 ` [PATCH 1/3] checkout: document bug where delayed checkout counts entries twice Matheus Tavares
2022-07-13 17:57   ` Junio C Hamano
2022-07-13  4:19 ` [PATCH 2/3] checkout: show bug about failed entries being included in final report Matheus Tavares
2022-07-13 11:14   ` Ævar Arnfjörð Bjarmason
     [not found]     ` <CAHd-oW6AeOGv=zQ=9Udtzwau=5XbQkhuctVDa0=4PoMTSU20HQ@mail.gmail.com>
2022-07-13 13:00       ` Matheus Tavares
2022-07-13  4:19 ` [PATCH 3/3] checkout: fix two bugs on the final count of updated entries Matheus Tavares
2022-07-14 11:49 ` [PATCH v2 0/3] checkout: fix two bugs on " Matheus Tavares
2022-07-14 11:49   ` [PATCH v2 1/3] checkout: document bug where delayed checkout counts entries twice Matheus Tavares
2022-07-14 11:49   ` Matheus Tavares [this message]
2022-07-14 11:49   ` [PATCH v2 3/3] checkout: fix two bugs on the final count of updated entries Matheus Tavares

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=4541e90224f5e79b16717b94f37edd10a4fdbbb1.1657799213.git.matheus.bernardino@usp.br \
    --to=matheus.bernardino@usp.br \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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.
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).