git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCHv2.1] t6500: wait for detached auto gc at the end of the test script

From
SZEDER Gábor <szeder.dev@gmail.com>
Date
Apr 13, 2017, 10:31 UTC
Message-ID
<20170413103138.12717-1-szeder.dev@gmail.com>
In-Reply-To
<20170412220717.7357-1-szeder.dev@gmail.com>

The last test in 't6500-gc', 'background auto gc does not run if gc.log is present and recent but does if it is old', added in a831c06a2 (gc: ignore old gc.log files, 2017-02-10), may sporadically trigger an error message from the test harness:

  rm: cannot remove 'trash directory.t6500-gc/.git/objects': Directory not empty

The test in question ends with executing an auto gc in the backround, which occasionally takes so long that it's still running when 'test_done' is about to remove the trash directory. This 'rm -rf $trash' in the foreground might race with the detached auto gc to create and delete files and directories, and gc might (re-)create a path that 'rm' already visited and removed, triggering the above error message when 'rm' attempts to remove its parent directory.

Commit bb05510e5 (t5510: run auto-gc in the foreground, 2016-05-01) fixed the same problem in a different test script by simply disallowing background gc. Unfortunately, what worked there is not applicable here, because the purpose of this test is to check the behavior of a detached auto gc.

Make sure that the test doesn't continue before the gc is finished in the background with a clever bit of shell trickery:

  - Open fd 9 in the shell, to be inherited by the background gc
    process, because our daemonize() only closes the standard fds 0,
    1 and 2.
  - Duplicate this fd 9 to stdout.
  - Read 'git gc's stdout, and thus fd 9, through a command
    substitution.  We don't actually care about gc's output, but this
    construct has two useful properties:
  - This read blocks until stdout or fd 9 are open.  While stdout is
    closed after the main gc process creates the background process
    and exits, fd 9 remains open until the backround process exits.
  - The variable assignment from the command substitution gets its
    exit status from the command executed within the command
    substitution, i.e. a failing main gc process will cause the test
    to fail.

Note, that this fd trickery doesn't work on Windows, because due to MSYS limitations the git process only inherits the standard fds 0, 1 and 2 from the shell. Luckily, it doesn't matter in this case, because on Windows daemonize() is basically a noop, thus 'git gc --auto' always runs in the foreground.

And since we can now continue the test reliably after the detached gc finished, check that there is only a single packfile left at the end, i.e. that the detached gc actually did what it was supposed to do. Also add a comment at the end of the test script to warn developers of future tests about this issue of long running detached gc processes.

Helped-by: Jeff King <peff@peff.net>
Helped-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
Updated subject line, but otherwise the same as v2.
 t/t6500-gc.sh | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index 08de2e8ab..cc7acd101 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -67,6 +67,16 @@ test_expect_success 'auto gc with too many loose objects does not attempt to cre
 	test_line_count = 2 new # There is one new pack and its .idx
 '
 
+run_and_wait_for_auto_gc () {
+	# We read stdout from gc for the side effect of waiting until the
+	# background gc process exits, closing its fd 9.  Furthermore, the
+	# variable assignment from a command substitution preserves the
+	# exit status of the main gc process.
+	# Note: this fd trickery doesn't work on Windows, but there is no
+	# need to, because on Win the auto gc always runs in the foreground.
+	doesnt_matter=$(git gc --auto 9>&1)
+}
+
 test_expect_success 'background auto gc does not run if gc.log is present and recent but does if it is old' '
 	test_commit foo &&
 	test_commit bar &&
@@ -80,7 +90,13 @@ test_expect_success 'background auto gc does not run if gc.log is present and re
 	test-chmtime =-345600 .git/gc.log &&
 	test_must_fail git gc --auto &&
 	test_config gc.logexpiry 2.days &&
-	git gc --auto
+	run_and_wait_for_auto_gc &&
+	ls .git/objects/pack/pack-*.pack >packs &&
+	test_line_count = 1 packs
 '
 
+# DO NOT leave a detached auto gc process running near the end of the
+# test script: it can run long enough in the background to racily
+# interfere with the cleanup in 'test_done'.
+
 test_done
-- 
2.12.2.613.g9c5b79913
Previous: SZEDER GáborNext: David Turner
Message 12 of 41 in “t6500: don't run detached auto gc at the end of the test script”
  1. t6500: don't run detached auto gc at the end of the test scriptSZEDER Gábor, Apr 10, 2017
  2. Jeff KingApr 10, 2017
  3. SZEDER GáborApr 10, 2017
  4. Jeff KingApr 10, 2017
  5. SZEDER GáborApr 10, 2017
  6. Jeff KingApr 10, 2017
  7. Johannes SixtApr 11, 2017
  8. SZEDER GáborApr 12, 2017
  9. Jeff KingApr 12, 2017
  10. SZEDER GáborApr 12, 2017
  11. [PATCHv2] t6500: don't run detached auto gc at the end of the test scriptSZEDER Gábor, Apr 12, 2017
  12. [PATCHv2.1] t6500: wait for detached auto gc at the end of the test scriptSZEDER Gábor, Apr 13, 2017
  13. David TurnerApr 13, 2017
  14. Jeff KingApr 13, 2017
  15. SZEDER GáborApr 13, 2017
  16. Jeff KingApr 13, 2017
  17. Jeff KingApr 13, 2017
  18. Stefan BellerApr 13, 2017
  19. Jeff KingApr 13, 2017
  20. SZEDER GáborApr 13, 2017
  21. Jeff KingApr 13, 2017
  22. SZEDER GáborApr 13, 2017
  23. Jeff KingApr 14, 2017
  24. SZEDER GáborApr 20, 2017
  25. Jeff KingApr 20, 2017
  26. test-lib: abort when can't remove trash directorySZEDER Gábor, Apr 20, 2017
  27. Jeff KingApr 20, 2017
  28. Junio C HamanoApr 21, 2017
  29. SZEDER GáborApr 21, 2017
  30. Jeff KingApr 21, 2017
  31. Junio C HamanoApr 24, 2017
  32. Jeff KingApr 24, 2017
  33. Junio C HamanoApr 24, 2017
  34. Junio C HamanoApr 24, 2017
  35. Jeff KingApr 24, 2017
  36. Torsten BögershausenApr 24, 2017
  37. Jeff KingApr 24, 2017
  38. Junio C HamanoApr 25, 2017
  39. Junio C HamanoApr 25, 2017
  40. Jeff KingApr 25, 2017
  41. Junio C HamanoApr 25, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.