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

[PATCH 2/3] gc: fix handling of crontab magic markers

From
MÅMartin Ågren <martin.agren@gmail.com>
Date
Dec 21, 2020, 21:26 UTC
Message-ID
<689d3150e9822eeccac0e1d07c2ba26dac47b4c9.1608585497.git.martin.agren@gmail.com>
In-Reply-To
<cover.1608585497.git.martin.agren@gmail.com>

On `git maintenance start`, we add a few entries to the user's cron table. We wrap our entries using two magic markers, "# BEGIN GIT MAINTENANCE SCHEDULE" and "# END GIT MAINTENANCE SCHEDULE". At a later `git maintenance stop`, we will go through the table and remove these lines. Or rather, we will remove the "BEGIN" marker, the "END" marker and everything between them.

Alas, we have a bug in how we detect the "END" marker: we don't. As we loop through all the lines of the crontab, if we are in the "old region", i.e., the region we're aiming to remove, we make an early `continue` and don't get as far as checking for the "END" marker. Thus, once we've seen our "BEGIN", we remove everything until the end of the file.

Rewrite the logic for identifying these markers. There are four cases that are mutually exclusive: The current line starts a region or it ends it, or it's firmly within the region, or it's outside of it (and should be printed).

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 t/t7900-maintenance.sh | 7 +++++++
 builtin/gc.c           | 7 +++----
 2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index d1e0c8f830..4bbfce31e9 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -446,6 +446,13 @@ test_expect_success 'start preserves existing schedule' '
 	grep "Important information!" cron.txt
 '
 
+test_expect_success 'stop preserves surrounding schedule' '
+	echo "Crucial information!" >>cron.txt &&
+	GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance stop &&
+	grep "Important information!" cron.txt &&
+	grep "Crucial information!" cron.txt
+'
+
 test_expect_success 'register preserves existing strategy' '
 	git config maintenance.strategy none &&
 	git maintenance register &&
diff --git a/builtin/gc.c b/builtin/gc.c
index b57fda4924..4c24f41852 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1554,11 +1554,10 @@ static int update_background_schedule(int run_maintenance)
 	while (!strbuf_getline_lf(&line, cron_list)) {
 		if (!in_old_region && !strcmp(line.buf, BEGIN_LINE))
 			in_old_region = 1;
-		if (in_old_region)
-			continue;
-		fprintf(cron_in, "%s\n", line.buf);
-		if (in_old_region && !strcmp(line.buf, END_LINE))
+		else if (in_old_region && !strcmp(line.buf, END_LINE))
 			in_old_region = 0;
+		else if (!in_old_region)
+			fprintf(cron_in, "%s\n", line.buf);
 	}
 
 	if (run_maintenance) {
-- 
2.30.0.rc1
Previous: Martin ÅgrenNext: Junio C Hamano
Message 4 of 10 in “gc: fix handling of crontab magic markers”
  1. 0/3 gc: fix handling of crontab magic markersMartin Ågren, Dec 21, 2020
  2. 1/3 git-maintenance.txt: add missing wordMartin Ågren, Dec 21, 2020
  3. 3/3 t7900-maintenance: test for magic markersMartin Ågren, Dec 21, 2020
  4. 2/3 gc: fix handling of crontab magic markersMartin Ågren, Dec 21, 2020
  5. Junio C HamanoDec 22, 2020
  6. Junio C HamanoDec 22, 2020
  7. Eric SunshineDec 23, 2020
  8. Martin ÅgrenDec 23, 2020
  9. Junio C HamanoDec 23, 2020
  10. Derrick StoleeDec 21, 2020

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.