git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / Atom feed
From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, jrnieder@gmail.com,
	jonathantanmy@google.com, sluongng@gmail.com,
	congdanhqx@gmail.com, Derrick Stolee <derrickstolee@github.com>,
	Derrick Stolee <dstolee@microsoft.com>,
	John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>,
	Todd Zullinger <tmz@pobox.com>
Subject: Re: [PATCH 5/7] maintenance: add start/stop subcommands
Date: Tue, 8 Sep 2020 08:29:14 +0200
Message-ID: <20200908062914.GC6209@szeder.dev> (raw)
In-Reply-To: <e02641881d998d1e6a31e941b61eb6f89d0519f7.1599234127.git.gitgitgadget@gmail.com>

On Fri, Sep 04, 2020 at 03:42:04PM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <dstolee@microsoft.com>
> 
> Add new subcommands to 'git maintenance' that start or stop background
> maintenance using 'cron', when available. This integration is as simple
> as I could make it, barring some implementation complications.
> 
> The schedule is laid out as follows:
> 
>   0 1-23 * * *   $cmd maintenance run --schedule=hourly
>   0 0    * * 1-6 $cmd maintenance run --schedule=daily
>   0 0    * * 0   $cmd maintenance run --schedule=weekly
> 
> where $cmd is a properly-qualified 'git for-each-repo' execution:
> 
> $cmd=$path/git --exec-path=$path for-each-repo --config=maintenance.repo
> 
> where $path points to the location of the Git executable running 'git
> maintenance start'. This is critical for systems with multiple versions
> of Git. Specifically, macOS has a system version at '/usr/bin/git' while
> the version that users can install resides at '/usr/local/bin/git'
> (symlinked to '/usr/local/libexec/git-core/git'). This will also use
> your locally-built version if you build and run this in your development
> environment without installing first.
> 
> This conditional schedule avoids having cron launch multiple 'git
> for-each-repo' commands in parallel. Such parallel commands would likely
> lead to the 'hourly' and 'daily' tasks competing over the object
> database lock. This could lead to to some tasks never being run! Since
> the --schedule=<frequency> argument will run all tasks with _at least_
> the given frequency, the daily runs will also run the hourly tasks.
> Similarly, the weekly runs will also run the daily and hourly tasks.
> 
> The GIT_TEST_CRONTAB environment variable is not intended for users to
> edit, but instead as a way to mock the 'crontab [-l]' command. This
> variable is set in test-lib.sh to avoid a future test from accidentally
> running anything with the cron integration from modifying the user's
> schedule. We use GIT_TEST_CRONTAB='test-tool crontab <file>' in our
> tests to check how the schedule is modified in 'git maintenance
> (start|stop)' commands.
> 
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---


> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
> index 272d1605d2..8803fcf621 100755
> --- a/t/t7900-maintenance.sh
> +++ b/t/t7900-maintenance.sh
> @@ -319,4 +319,32 @@ test_expect_success 'register and unregister' '
>  	test_cmp before actual
>  '
>  
> +test_expect_success 'start from empty cron table' '
> +	GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance start &&

This command hangs when run on Travis CI's s390x arch.  Now, Travis
CI's multi-arch support is labelled as an alpha feature and isn't
exactly bug free, so Cc-ing Adrian and Todd, who reported and tested
big-endian issues and fixes in the past, in the hope that they can
confirm.

> +
> +	# start registers the repo
> +	git config --get --global maintenance.repo "$(pwd)" &&
> +
> +	grep "for-each-repo --config=maintenance.repo maintenance run --schedule=daily" cron.txt &&
> +	grep "for-each-repo --config=maintenance.repo maintenance run --schedule=hourly" cron.txt &&
> +	grep "for-each-repo --config=maintenance.repo maintenance run --schedule=weekly" cron.txt
> +'
> +
> +test_expect_success 'stop from existing schedule' '
> +	GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance stop &&
> +
> +	# stop does not unregister the repo
> +	git config --get --global maintenance.repo "$(pwd)" &&
> +
> +	# Operation is idempotent
> +	GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance stop &&
> +	test_must_be_empty cron.txt
> +'
> +
> +test_expect_success 'start preserves existing schedule' '
> +	echo "Important information!" >cron.txt &&
> +	GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance start &&
> +	grep "Important information!" cron.txt
> +'
> +
>  test_done


> diff --git a/t/helper/test-crontab.c b/t/helper/test-crontab.c
> new file mode 100644
> index 0000000000..f5db6319c6
> --- /dev/null
> +++ b/t/helper/test-crontab.c
> @@ -0,0 +1,35 @@
> +#include "test-tool.h"
> +#include "cache.h"
> +
> +/*
> + * Usage: test-tool cron <file> [-l]
> + *
> + * If -l is specified, then write the contents of <file> to stdou.

s/stdou/stdout/

> + * Otherwise, write from stdin into <file>.
> + */
> +int cmd__crontab(int argc, const char **argv)
> +{
> +	char a;

So 'a' is a char...

> +	FILE *from, *to;
> +
> +	if (argc == 3 && !strcmp(argv[2], "-l")) {
> +		from = fopen(argv[1], "r");
> +		if (!from)
> +			return 0;
> +		to = stdout;
> +	} else if (argc == 2) {
> +		from = stdin;
> +		to = fopen(argv[1], "w");
> +	} else
> +		return error("unknown arguments");
> +
> +	while ((a = fgetc(from)) != EOF)

fgetc() returns an int, which is assigned to a char, which is then
compared to whatever EOF might be on the platform.  Apparently this
casting and comparison doesn't work as expected on s390x (I haven't
even tried to think it through...), and instead of detecting EOF and
exiting we end up in an endless loop writing 0xff bytes to 'cron.txt',
while 'git maintenance start' in vain waits for 'test-crontab' to
exit.

Changing the type of 'a' to int fixes this issue, and all these tests
pass.

> +		fputc(a, to);
> +
> +	if (argc == 3)
> +		fclose(from);
> +	else
> +		fclose(to);
> +
> +	return 0;
> +}

  reply	other threads:[~2020-09-08  6:29 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-04 15:41 [PATCH 0/7] Maintenance III: Background maintenance Derrick Stolee via GitGitGadget
2020-09-04 15:42 ` [PATCH 1/7] maintenance: optionally skip --auto process Derrick Stolee via GitGitGadget
2020-09-04 15:42 ` [PATCH 2/7] maintenance: add --schedule option and config Derrick Stolee via GitGitGadget
2020-09-08 13:07   ` Đoàn Trần Công Danh
2020-09-09 12:14     ` Derrick Stolee
2020-09-04 15:42 ` [PATCH 3/7] for-each-repo: run subcommands on configured repos Derrick Stolee via GitGitGadget
2020-09-04 15:42 ` [PATCH 4/7] maintenance: add [un]register subcommands Derrick Stolee via GitGitGadget
2020-09-04 15:42 ` [PATCH 5/7] maintenance: add start/stop subcommands Derrick Stolee via GitGitGadget
2020-09-08  6:29   ` SZEDER Gábor [this message]
2020-09-08 12:43     ` Derrick Stolee
2020-09-08 19:31     ` Junio C Hamano
2020-09-04 15:42 ` [PATCH 6/7] maintenance: recommended schedule in register/start Derrick Stolee via GitGitGadget
2020-09-04 15:42 ` [PATCH 7/7] maintenance: add troubleshooting guide to docs Derrick Stolee via GitGitGadget
2020-09-11 17:49 ` [PATCH v2 0/7] Maintenance III: Background maintenance Derrick Stolee via GitGitGadget
2020-09-11 17:49   ` [PATCH v2 1/7] maintenance: optionally skip --auto process Derrick Stolee via GitGitGadget
2020-09-11 17:49   ` [PATCH v2 2/7] maintenance: add --schedule option and config Derrick Stolee via GitGitGadget
2020-09-11 17:49   ` [PATCH v2 3/7] for-each-repo: run subcommands on configured repos Derrick Stolee via GitGitGadget
2020-09-11 17:49   ` [PATCH v2 4/7] maintenance: add [un]register subcommands Derrick Stolee via GitGitGadget
2020-09-17 14:05     ` Đoàn Trần Công Danh
2020-09-11 17:49   ` [PATCH v2 5/7] maintenance: add start/stop subcommands Derrick Stolee via GitGitGadget
2020-09-11 17:49   ` [PATCH v2 6/7] maintenance: recommended schedule in register/start Derrick Stolee via GitGitGadget
2020-09-29 19:48     ` Martin Ågren
2020-09-30 20:11       ` Derrick Stolee
2020-10-01 20:38         ` Derrick Stolee
2020-10-02  0:38           ` Đoàn Trần Công Danh
2020-10-02  1:55             ` Derrick Stolee
2020-10-05 13:16               ` Đoàn Trần Công Danh
2020-10-05 18:17                 ` Derrick Stolee
2020-09-11 17:49   ` [PATCH v2 7/7] maintenance: add troubleshooting guide to docs Derrick Stolee via GitGitGadget
2020-10-05 12:57   ` [PATCH v3 0/7] Maintenance III: Background maintenance Derrick Stolee via GitGitGadget
2020-10-05 12:57     ` [PATCH v3 1/7] maintenance: optionally skip --auto process Derrick Stolee via GitGitGadget
2020-10-05 12:57     ` [PATCH v3 2/7] maintenance: add --schedule option and config Derrick Stolee via GitGitGadget
2020-10-05 12:57     ` [PATCH v3 3/7] for-each-repo: run subcommands on configured repos Derrick Stolee via GitGitGadget
2020-10-05 12:57     ` [PATCH v3 4/7] maintenance: add [un]register subcommands Derrick Stolee via GitGitGadget
2020-10-05 12:57     ` [PATCH v3 5/7] maintenance: add start/stop subcommands Derrick Stolee via GitGitGadget
2020-10-05 12:57     ` [PATCH v3 6/7] maintenance: use default schedule if not configured Derrick Stolee via GitGitGadget
2020-10-05 19:57       ` Martin Ågren
2020-10-08 13:32         ` Derrick Stolee
2020-10-05 12:57     ` [PATCH v3 7/7] maintenance: add troubleshooting guide to docs Derrick Stolee via GitGitGadget
2020-10-15 17:21     ` [PATCH v4 0/8] Maintenance III: Background maintenance Derrick Stolee via GitGitGadget
2020-10-15 17:21       ` [PATCH v4 1/8] maintenance: optionally skip --auto process Derrick Stolee via GitGitGadget
2020-10-15 17:21       ` [PATCH v4 2/8] maintenance: add --schedule option and config Derrick Stolee via GitGitGadget
2020-10-15 17:21       ` [PATCH v4 3/8] for-each-repo: run subcommands on configured repos Derrick Stolee via GitGitGadget
2020-10-15 17:22       ` [PATCH v4 4/8] maintenance: add [un]register subcommands Derrick Stolee via GitGitGadget
2020-10-15 17:22       ` [PATCH v4 5/8] maintenance: add start/stop subcommands Derrick Stolee via GitGitGadget
2020-12-09 18:51         ` Josh Steadmon
2020-12-09 19:16           ` Josh Steadmon
2020-12-09 21:59             ` Derrick Stolee
2020-12-10  0:13             ` Junio C Hamano
2020-12-10  1:52               ` Derrick Stolee
2020-12-10  6:54                 ` Junio C Hamano
2020-10-15 17:22       ` [PATCH v4 6/8] maintenance: create maintenance.strategy config Derrick Stolee via GitGitGadget
2020-10-15 17:22       ` [PATCH v4 7/8] maintenance: use 'incremental' strategy by default Derrick Stolee via GitGitGadget
2020-10-15 17:22       ` [PATCH v4 8/8] maintenance: add troubleshooting guide to docs Derrick Stolee via GitGitGadget

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=20200908062914.GC6209@szeder.dev \
    --to=szeder.dev@gmail.com \
    --cc=congdanhqx@gmail.com \
    --cc=derrickstolee@github.com \
    --cc=dstolee@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=glaubitz@physik.fu-berlin.de \
    --cc=jonathantanmy@google.com \
    --cc=jrnieder@gmail.com \
    --cc=sluongng@gmail.com \
    --cc=tmz@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

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for the project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git