From patchwork Fri Aug 16 10:44:57 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13765894 Received: from fout8-smtp.messagingengine.com (fout8-smtp.messagingengine.com [103.168.172.151]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8814013B783 for ; Fri, 16 Aug 2024 10:45:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.151 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723805103; cv=none; b=cNp8xW5mNSlusVR7g7P0ZuhwAowwyh1Kkw0PdqETZ2RDylUaQr/8twygcM+u8UIfvusknn7tYHVORgsoSUOz0l4MznzbkUcpRNtINshFZsW5n32/rioVAwi6gXqtE1l968Nz8hnISVl4t5EKZd4oZZ9pkdql6iOJOWrexd3EUu8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723805103; c=relaxed/simple; bh=TSy4hW9tqsU10Gu3DsDfyJ5qj8UbJdsDpUvI9jQtP7Q=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=ZVlFig7k2On/FYaaOdDHEVVb7g7gyRAulYHyNG5D8eaYez32ToS/uRhCj+ixKvj5tvU1xEr9oYP+AmbPcLAJaVtIuFTulVVrFFd5Hu2oYbV9zSpCxy9RydkLQ/KmssILII2zJ6uwwNjtZciFcbXhwpVslH7T/IAtnUy3tOLX3h0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=CVt3FC3F; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=HkC5gFQT; arc=none smtp.client-ip=103.168.172.151 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="CVt3FC3F"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="HkC5gFQT" Received: from phl-compute-08.internal (phl-compute-08.nyi.internal [10.202.2.48]) by mailfout.nyi.internal (Postfix) with ESMTP id 9B1CE138FF9B; Fri, 16 Aug 2024 06:45:00 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-08.internal (MEProxy); Fri, 16 Aug 2024 06:45:00 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm3; t=1723805100; x=1723891500; bh=bQynbxy3ZS cPk+bnalMGOTQzDIBCdG9RCnZWRnMufJg=; b=CVt3FC3FO4t7uMtVrCRGoFw3pn GOyFPMqxw8Hh3K1CSDx61j5uxfbWFjiX6JuGzQWH/r3+ZkTGzhnbBhE/ibVx7JRb Xt3UQUntULyxNutz/DiSCmpWGgWhSoLS4F7ngBZWT5pYcbJxpUXbebOiy/H/6KQ+ tjCj6MVX4GSwdSRS3t2A0n+/m7DPJJzybkZu/ypuex6zCopyOMLPPl7Ozru8gsPv WK+JN+IdZhcZhr6JLjqcrUknXthVQQ5QfG3hHyEoUbkO3vj7xCnypqq9EY/XSjkS PNiVqYdOKocVMz44fwrkT3ngUXt5EjxK3uE4h95bFeorxSrwiNFaRfFFhbcw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1723805100; x=1723891500; bh=bQynbxy3ZScPk+bnalMGOTQzDIBC dG9RCnZWRnMufJg=; b=HkC5gFQTdXBeiokoBRSP1lOWme46X0Gm2WQUqxC33tSe 7j9Vu9n+mu4ZwcSOvRlbw4HxH5AbFxMwGWxw7GpyLa1xT/7F0weWY1Fn7YUpqWrR DW7xKsdvcwAyjGKA5hZHzWbqaJaRNpBqWQaJ3K0/qGSzTOy9QV95rIexrcUWS1jd t55uW52m9MlzTcgLauKDqOEjFNhpdM89i73wBAoh4tT1Dk2XyavDSe3eIro3sxjR bFqtO8HhJFPGYync8SdQyog/nhTvMThCokOvF5UZHaY9bbvRn+bdp3jVa3DzHygB Sxcj3d23s9Ep3MmUl8hc8+aVK1a+gsPoYEsVyDbN7g== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddruddtkedgfeduucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhepfffhvfevuffkfhggtggujgesthdtredttddtvden ucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimh eqnecuggftrfgrthhtvghrnhepveekkeffhfeitdeludeigfejtdetvdelvdduhefgueeg udfghfeukefhjedvkedtnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrg hilhhfrhhomhepphhssehpkhhsrdhimhdpnhgspghrtghpthhtohepiedpmhhouggvpehs mhhtphhouhhtpdhrtghpthhtohepshhtohhlvggvsehgmhgrihhlrdgtohhmpdhrtghpth htohepjhgrmhgvshesjhgrmhgvshhlihhurdhiohdprhgtphhtthhopehphhhilhhlihhp rdifohhougesughunhgvlhhmrdhorhhgrdhukhdprhgtphhtthhopehgihhtshhtvghrse hpohgsohigrdgtohhmpdhrtghpthhtohepghhithesvhhgvghrrdhkvghrnhgvlhdrohhr ghdprhgtphhtthhopehphhhilhhlihhprdifohhougduvdefsehgmhgrihhlrdgtohhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 16 Aug 2024 06:44:59 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 4b01eb3d (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 16 Aug 2024 10:44:36 +0000 (UTC) Date: Fri, 16 Aug 2024 12:44:57 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Phillip Wood , phillip.wood@dunelm.org.uk, James Liu , Derrick Stolee , Junio C Hamano Subject: [PATCH v3 1/7] config: fix constness of out parameter for `git_config_get_expiry()` Message-ID: <040453f27f0b2755bb359fe07a703ce66e071ab6.1723804990.git.ps@pks.im> References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: The type of the out parameter of `git_config_get_expiry()` is a pointer to a constant string, which creates the impression that ownership of the returned data wasn't transferred to the caller. This isn't true though and thus quite misleading. Adapt the parameter to be of type `char **` and adjust callers accordingly. While at it, refactor `get_shared_index_expire_date()` to drop the static `shared_index_expire` variable. It is only used in that function, and furthermore we would only hit the code where we parse the expiry date a single time because we already use a static `prepared` variable to track whether we did parse it. Signed-off-by: Patrick Steinhardt --- builtin/gc.c | 6 +++--- config.c | 4 ++-- config.h | 2 +- read-cache.c | 12 +++++++++--- 4 files changed, 15 insertions(+), 9 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 72bac2554f..e7406bf667 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -167,9 +167,9 @@ static void gc_config(void) git_config_get_bool("gc.autodetach", &detach_auto); git_config_get_bool("gc.cruftpacks", &cruft_packs); git_config_get_ulong("gc.maxcruftsize", &max_cruft_size); - git_config_get_expiry("gc.pruneexpire", &prune_expire); - git_config_get_expiry("gc.worktreepruneexpire", &prune_worktrees_expire); - git_config_get_expiry("gc.logexpiry", &gc_log_expire); + git_config_get_expiry("gc.pruneexpire", (char **) &prune_expire); + git_config_get_expiry("gc.worktreepruneexpire", (char **) &prune_worktrees_expire); + git_config_get_expiry("gc.logexpiry", (char **) &gc_log_expire); git_config_get_ulong("gc.bigpackthreshold", &big_pack_threshold); git_config_get_ulong("pack.deltacachesize", &max_delta_cache_size); diff --git a/config.c b/config.c index 6421894614..dfa4df1417 100644 --- a/config.c +++ b/config.c @@ -2766,9 +2766,9 @@ int git_config_get_pathname(const char *key, char **dest) return repo_config_get_pathname(the_repository, key, dest); } -int git_config_get_expiry(const char *key, const char **output) +int git_config_get_expiry(const char *key, char **output) { - int ret = git_config_get_string(key, (char **)output); + int ret = git_config_get_string(key, output); if (ret) return ret; if (strcmp(*output, "now")) { diff --git a/config.h b/config.h index 54b47dec9e..4801391c32 100644 --- a/config.h +++ b/config.h @@ -701,7 +701,7 @@ int git_config_get_split_index(void); int git_config_get_max_percent_split_change(void); /* This dies if the configured or default date is in the future */ -int git_config_get_expiry(const char *key, const char **output); +int git_config_get_expiry(const char *key, char **output); /* parse either "this many days" integer, or "5.days.ago" approxidate */ int git_config_get_expiry_in_days(const char *key, timestamp_t *, timestamp_t now); diff --git a/read-cache.c b/read-cache.c index 48bf24f87c..7f393ee687 100644 --- a/read-cache.c +++ b/read-cache.c @@ -3176,18 +3176,24 @@ static int write_split_index(struct index_state *istate, return ret; } -static const char *shared_index_expire = "2.weeks.ago"; - static unsigned long get_shared_index_expire_date(void) { static unsigned long shared_index_expire_date; static int shared_index_expire_date_prepared; if (!shared_index_expire_date_prepared) { + const char *shared_index_expire = "2.weeks.ago"; + char *value = NULL; + git_config_get_expiry("splitindex.sharedindexexpire", - &shared_index_expire); + &value); + if (value) + shared_index_expire = value; + shared_index_expire_date = approxidate(shared_index_expire); shared_index_expire_date_prepared = 1; + + free(value); } return shared_index_expire_date; From patchwork Fri Aug 16 10:45:00 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13765895 Received: from fout8-smtp.messagingengine.com (fout8-smtp.messagingengine.com [103.168.172.151]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 64916198846 for ; Fri, 16 Aug 2024 10:45:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.151 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723805106; cv=none; b=tJID130K/wULVFvCcYySmGd6EUffpfLFfcuCvJ+U9kMR3NGB67s+0bDQrJpL+YoV9pmyF6pgJY3vLn2yQYr7+Yyq1wVQaNyg3h4hEz/1YfqDgI6VAKHFULdMmsbCMDZoJEVcWRE0IcBNg3ILD3NAFTd5vyCpg3tmHZBtjqSpysA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723805106; c=relaxed/simple; bh=7oVC+fJ4YyTKYG94rfjADQoTrNgBR4nUnPS33bxvvfk=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=i9cvCdt2U1OkEVyJI2lIJ6XvzxUHzVOdWYG/S/QPWfEzuonE9sI5g9KsbSRUrB0G/UrlbANY9bgcOmIaI6TuglpZcLsc729y5OCuO665NPjng49iCE7EMLXArXiDPxSVRRksLZ1vyqh1QDrwtenM88wDaDOHq2wsOayWrmnYyqc= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=R4VB4XcR; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=jz1DcA0J; arc=none smtp.client-ip=103.168.172.151 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="R4VB4XcR"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="jz1DcA0J" Received: from phl-compute-05.internal (phl-compute-05.nyi.internal [10.202.2.45]) by mailfout.nyi.internal (Postfix) with ESMTP id 69279138FF9F; Fri, 16 Aug 2024 06:45:03 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-05.internal (MEProxy); Fri, 16 Aug 2024 06:45:03 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm3; t=1723805103; x=1723891503; bh=p9CwnAe3yB IO3CTb23KgLYHWq940TsLWUFKqGQhtdtw=; b=R4VB4XcRLo3nApb+7jb7IQ1Ejl ZFkczYvfaqRqVlJgeh+yyePbESxcrwYOaxQ9k8iDqAGOxxfAFRkGw8g5qsM6kL0N Qth4j0cI0HQQ2p4oZVN6Q99lQsZogehHohN5VvmaMP90raysZcM5usjCkUFW700O +h8QZcdsLITKXE20fz+rCbW0nONMcl8ZCqDj+nb8yDGfHLK8SlasMZjnWW/HT3PW XxGenZBlywA4P3iDFReiNENtcYx9OY7qrpKZ43H43Ilo23VnikYILwi41hrz4cDb bTQNDmxt/X/xXItA9Y4f4uQqKjxyyc5ksZj6Mz7EMqnvkSU2gO7soTEDaWzg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1723805103; x=1723891503; bh=p9CwnAe3yBIO3CTb23KgLYHWq940 TsLWUFKqGQhtdtw=; b=jz1DcA0JE0FMGjQc7vD7HVxinTOPlvkQJX8IptG64WWg zGO06AvBQUS8iB5YK++Rp09yO3PLQIVqHTFOtIqkzoM86SiNW1TktsTYaBylJZyt J1tVT+ud+dRU+SN+6UqjBsUICIxt0n1cX7xx0Y5/Wd31D1q4wJlfzKVb7/HAyUNY x7h83UVO2CaS17c2dslGbA8aE4k1gxb2NmPl9InlWPLN8nRIdf9jSl7MvlJ7l9/V gm9sFIF9mmc8OVGYqO4bew+6YY3TpQAqS8aQYfTR+3rqISvyD5lehapxSIJKLs/N xm46pvfC4LInplliKpoCLg6+Dp8kqleQ/LJLMl4YSA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddruddtkedgfeduucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhepfffhvfevuffkfhggtggujgesthdtredttddtvden ucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimh eqnecuggftrfgrthhtvghrnhepveekkeffhfeitdeludeigfejtdetvdelvdduhefgueeg udfghfeukefhjedvkedtnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrg hilhhfrhhomhepphhssehpkhhsrdhimhdpnhgspghrtghpthhtohepiedpmhhouggvpehs mhhtphhouhhtpdhrtghpthhtohepghhithhsthgvrhesphhosghogidrtghomhdprhgtph htthhopehphhhilhhlihhprdifohhougduvdefsehgmhgrihhlrdgtohhmpdhrtghpthht ohepjhgrmhgvshesjhgrmhgvshhlihhurdhiohdprhgtphhtthhopehsthholhgvvgesgh hmrghilhdrtghomhdprhgtphhtthhopehgihhtsehvghgvrhdrkhgvrhhnvghlrdhorhhg pdhrtghpthhtohepphhhihhllhhiphdrfihoohguseguuhhnvghlmhdrohhrghdruhhk X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 16 Aug 2024 06:45:01 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 1923e4b6 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 16 Aug 2024 10:44:39 +0000 (UTC) Date: Fri, 16 Aug 2024 12:45:00 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Phillip Wood , phillip.wood@dunelm.org.uk, James Liu , Derrick Stolee , Junio C Hamano Subject: [PATCH v3 2/7] builtin/gc: refactor to read config into structure Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: The git-gc(1) command knows to read a bunch of config keys to tweak its own behaviour. The values are parsed into global variables, which makes it hard to correctly manage the lifecycle of values that may require a memory allocation. Refactor the code to use a `struct gc_config` that gets populated and passed around. For one, this makes previously-implicit dependencies on these config values clear. Second, it will allow us to properly manage the lifecycle in the next commit. Signed-off-by: Patrick Steinhardt --- builtin/gc.c | 255 +++++++++++++++++++++++++++++---------------------- 1 file changed, 143 insertions(+), 112 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index e7406bf667..eee7401647 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -49,23 +49,7 @@ static const char * const builtin_gc_usage[] = { NULL }; -static int pack_refs = 1; -static int prune_reflogs = 1; -static int cruft_packs = 1; -static unsigned long max_cruft_size; -static int aggressive_depth = 50; -static int aggressive_window = 250; -static int gc_auto_threshold = 6700; -static int gc_auto_pack_limit = 50; -static int detach_auto = 1; static timestamp_t gc_log_expire_time; -static const char *gc_log_expire = "1.day.ago"; -static const char *prune_expire = "2.weeks.ago"; -static const char *prune_worktrees_expire = "3.months.ago"; -static char *repack_filter; -static char *repack_filter_to; -static unsigned long big_pack_threshold; -static unsigned long max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE; static struct strvec reflog = STRVEC_INIT; static struct strvec repack = STRVEC_INIT; @@ -145,37 +129,71 @@ static int gc_config_is_timestamp_never(const char *var) return 0; } -static void gc_config(void) +struct gc_config { + int pack_refs; + int prune_reflogs; + int cruft_packs; + unsigned long max_cruft_size; + int aggressive_depth; + int aggressive_window; + int gc_auto_threshold; + int gc_auto_pack_limit; + int detach_auto; + const char *gc_log_expire; + const char *prune_expire; + const char *prune_worktrees_expire; + char *repack_filter; + char *repack_filter_to; + unsigned long big_pack_threshold; + unsigned long max_delta_cache_size; +}; + +#define GC_CONFIG_INIT { \ + .pack_refs = 1, \ + .prune_reflogs = 1, \ + .cruft_packs = 1, \ + .aggressive_depth = 50, \ + .aggressive_window = 250, \ + .gc_auto_threshold = 6700, \ + .gc_auto_pack_limit = 50, \ + .detach_auto = 1, \ + .gc_log_expire = "1.day.ago", \ + .prune_expire = "2.weeks.ago", \ + .prune_worktrees_expire = "3.months.ago", \ + .max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE, \ +} + +static void gc_config(struct gc_config *cfg) { const char *value; if (!git_config_get_value("gc.packrefs", &value)) { if (value && !strcmp(value, "notbare")) - pack_refs = -1; + cfg->pack_refs = -1; else - pack_refs = git_config_bool("gc.packrefs", value); + cfg->pack_refs = git_config_bool("gc.packrefs", value); } if (gc_config_is_timestamp_never("gc.reflogexpire") && gc_config_is_timestamp_never("gc.reflogexpireunreachable")) - prune_reflogs = 0; + cfg->prune_reflogs = 0; - git_config_get_int("gc.aggressivewindow", &aggressive_window); - git_config_get_int("gc.aggressivedepth", &aggressive_depth); - git_config_get_int("gc.auto", &gc_auto_threshold); - git_config_get_int("gc.autopacklimit", &gc_auto_pack_limit); - git_config_get_bool("gc.autodetach", &detach_auto); - git_config_get_bool("gc.cruftpacks", &cruft_packs); - git_config_get_ulong("gc.maxcruftsize", &max_cruft_size); - git_config_get_expiry("gc.pruneexpire", (char **) &prune_expire); - git_config_get_expiry("gc.worktreepruneexpire", (char **) &prune_worktrees_expire); - git_config_get_expiry("gc.logexpiry", (char **) &gc_log_expire); + git_config_get_int("gc.aggressivewindow", &cfg->aggressive_window); + git_config_get_int("gc.aggressivedepth", &cfg->aggressive_depth); + git_config_get_int("gc.auto", &cfg->gc_auto_threshold); + git_config_get_int("gc.autopacklimit", &cfg->gc_auto_pack_limit); + git_config_get_bool("gc.autodetach", &cfg->detach_auto); + git_config_get_bool("gc.cruftpacks", &cfg->cruft_packs); + git_config_get_ulong("gc.maxcruftsize", &cfg->max_cruft_size); + git_config_get_expiry("gc.pruneexpire", (char **) &cfg->prune_expire); + git_config_get_expiry("gc.worktreepruneexpire", (char **) &cfg->prune_worktrees_expire); + git_config_get_expiry("gc.logexpiry", (char **) &cfg->gc_log_expire); - git_config_get_ulong("gc.bigpackthreshold", &big_pack_threshold); - git_config_get_ulong("pack.deltacachesize", &max_delta_cache_size); + git_config_get_ulong("gc.bigpackthreshold", &cfg->big_pack_threshold); + git_config_get_ulong("pack.deltacachesize", &cfg->max_delta_cache_size); - git_config_get_string("gc.repackfilter", &repack_filter); - git_config_get_string("gc.repackfilterto", &repack_filter_to); + git_config_get_string("gc.repackfilter", &cfg->repack_filter); + git_config_get_string("gc.repackfilterto", &cfg->repack_filter_to); git_config(git_default_config, NULL); } @@ -206,7 +224,7 @@ struct maintenance_run_opts { enum schedule_priority schedule; }; -static int pack_refs_condition(void) +static int pack_refs_condition(UNUSED struct gc_config *cfg) { /* * The auto-repacking logic for refs is handled by the ref backends and @@ -216,7 +234,8 @@ static int pack_refs_condition(void) return 1; } -static int maintenance_task_pack_refs(MAYBE_UNUSED struct maintenance_run_opts *opts) +static int maintenance_task_pack_refs(MAYBE_UNUSED struct maintenance_run_opts *opts, + UNUSED struct gc_config *cfg) { struct child_process cmd = CHILD_PROCESS_INIT; @@ -228,7 +247,7 @@ static int maintenance_task_pack_refs(MAYBE_UNUSED struct maintenance_run_opts * return run_command(&cmd); } -static int too_many_loose_objects(void) +static int too_many_loose_objects(struct gc_config *cfg) { /* * Quickly check if a "gc" is needed, by estimating how @@ -247,7 +266,7 @@ static int too_many_loose_objects(void) if (!dir) return 0; - auto_threshold = DIV_ROUND_UP(gc_auto_threshold, 256); + auto_threshold = DIV_ROUND_UP(cfg->gc_auto_threshold, 256); while ((ent = readdir(dir)) != NULL) { if (strspn(ent->d_name, "0123456789abcdef") != hexsz_loose || ent->d_name[hexsz_loose] != '\0') @@ -283,12 +302,12 @@ static struct packed_git *find_base_packs(struct string_list *packs, return base; } -static int too_many_packs(void) +static int too_many_packs(struct gc_config *cfg) { struct packed_git *p; int cnt; - if (gc_auto_pack_limit <= 0) + if (cfg->gc_auto_pack_limit <= 0) return 0; for (cnt = 0, p = get_all_packs(the_repository); p; p = p->next) { @@ -302,7 +321,7 @@ static int too_many_packs(void) */ cnt++; } - return gc_auto_pack_limit < cnt; + return cfg->gc_auto_pack_limit < cnt; } static uint64_t total_ram(void) @@ -336,7 +355,8 @@ static uint64_t total_ram(void) return 0; } -static uint64_t estimate_repack_memory(struct packed_git *pack) +static uint64_t estimate_repack_memory(struct gc_config *cfg, + struct packed_git *pack) { unsigned long nr_objects = repo_approximate_object_count(the_repository); size_t os_cache, heap; @@ -373,7 +393,7 @@ static uint64_t estimate_repack_memory(struct packed_git *pack) */ heap += delta_base_cache_limit; /* and of course pack-objects has its own delta cache */ - heap += max_delta_cache_size; + heap += cfg->max_delta_cache_size; return os_cache + heap; } @@ -384,30 +404,31 @@ static int keep_one_pack(struct string_list_item *item, void *data UNUSED) return 0; } -static void add_repack_all_option(struct string_list *keep_pack) +static void add_repack_all_option(struct gc_config *cfg, + struct string_list *keep_pack) { - if (prune_expire && !strcmp(prune_expire, "now")) + if (cfg->prune_expire && !strcmp(cfg->prune_expire, "now")) strvec_push(&repack, "-a"); - else if (cruft_packs) { + else if (cfg->cruft_packs) { strvec_push(&repack, "--cruft"); - if (prune_expire) - strvec_pushf(&repack, "--cruft-expiration=%s", prune_expire); - if (max_cruft_size) + if (cfg->prune_expire) + strvec_pushf(&repack, "--cruft-expiration=%s", cfg->prune_expire); + if (cfg->max_cruft_size) strvec_pushf(&repack, "--max-cruft-size=%lu", - max_cruft_size); + cfg->max_cruft_size); } else { strvec_push(&repack, "-A"); - if (prune_expire) - strvec_pushf(&repack, "--unpack-unreachable=%s", prune_expire); + if (cfg->prune_expire) + strvec_pushf(&repack, "--unpack-unreachable=%s", cfg->prune_expire); } if (keep_pack) for_each_string_list(keep_pack, keep_one_pack, NULL); - if (repack_filter && *repack_filter) - strvec_pushf(&repack, "--filter=%s", repack_filter); - if (repack_filter_to && *repack_filter_to) - strvec_pushf(&repack, "--filter-to=%s", repack_filter_to); + if (cfg->repack_filter && *cfg->repack_filter) + strvec_pushf(&repack, "--filter=%s", cfg->repack_filter); + if (cfg->repack_filter_to && *cfg->repack_filter_to) + strvec_pushf(&repack, "--filter-to=%s", cfg->repack_filter_to); } static void add_repack_incremental_option(void) @@ -415,13 +436,13 @@ static void add_repack_incremental_option(void) strvec_push(&repack, "--no-write-bitmap-index"); } -static int need_to_gc(void) +static int need_to_gc(struct gc_config *cfg) { /* * Setting gc.auto to 0 or negative can disable the * automatic gc. */ - if (gc_auto_threshold <= 0) + if (cfg->gc_auto_threshold <= 0) return 0; /* @@ -430,13 +451,13 @@ static int need_to_gc(void) * we run "repack -A -d -l". Otherwise we tell the caller * there is no need. */ - if (too_many_packs()) { + if (too_many_packs(cfg)) { struct string_list keep_pack = STRING_LIST_INIT_NODUP; - if (big_pack_threshold) { - find_base_packs(&keep_pack, big_pack_threshold); - if (keep_pack.nr >= gc_auto_pack_limit) { - big_pack_threshold = 0; + if (cfg->big_pack_threshold) { + find_base_packs(&keep_pack, cfg->big_pack_threshold); + if (keep_pack.nr >= cfg->gc_auto_pack_limit) { + cfg->big_pack_threshold = 0; string_list_clear(&keep_pack, 0); find_base_packs(&keep_pack, 0); } @@ -445,7 +466,7 @@ static int need_to_gc(void) uint64_t mem_have, mem_want; mem_have = total_ram(); - mem_want = estimate_repack_memory(p); + mem_want = estimate_repack_memory(cfg, p); /* * Only allow 1/2 of memory for pack-objects, leave @@ -456,9 +477,9 @@ static int need_to_gc(void) string_list_clear(&keep_pack, 0); } - add_repack_all_option(&keep_pack); + add_repack_all_option(cfg, &keep_pack); string_list_clear(&keep_pack, 0); - } else if (too_many_loose_objects()) + } else if (too_many_loose_objects(cfg)) add_repack_incremental_option(); else return 0; @@ -585,7 +606,8 @@ static int report_last_gc_error(void) return ret; } -static void gc_before_repack(struct maintenance_run_opts *opts) +static void gc_before_repack(struct maintenance_run_opts *opts, + struct gc_config *cfg) { /* * We may be called twice, as both the pre- and @@ -596,10 +618,10 @@ static void gc_before_repack(struct maintenance_run_opts *opts) if (done++) return; - if (pack_refs && maintenance_task_pack_refs(opts)) + if (cfg->pack_refs && maintenance_task_pack_refs(opts, cfg)) die(FAILED_RUN, "pack-refs"); - if (prune_reflogs) { + if (cfg->prune_reflogs) { struct child_process cmd = CHILD_PROCESS_INIT; cmd.git_cmd = 1; @@ -621,14 +643,15 @@ int cmd_gc(int argc, const char **argv, const char *prefix) timestamp_t dummy; struct child_process rerere_cmd = CHILD_PROCESS_INIT; struct maintenance_run_opts opts = {0}; + struct gc_config cfg = GC_CONFIG_INIT; struct option builtin_gc_options[] = { OPT__QUIET(&quiet, N_("suppress progress reporting")), - { OPTION_STRING, 0, "prune", &prune_expire, N_("date"), + { OPTION_STRING, 0, "prune", &cfg.prune_expire, N_("date"), N_("prune unreferenced objects"), - PARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire }, - OPT_BOOL(0, "cruft", &cruft_packs, N_("pack unreferenced objects separately")), - OPT_MAGNITUDE(0, "max-cruft-size", &max_cruft_size, + PARSE_OPT_OPTARG, NULL, (intptr_t)cfg.prune_expire }, + OPT_BOOL(0, "cruft", &cfg.cruft_packs, N_("pack unreferenced objects separately")), + OPT_MAGNITUDE(0, "max-cruft-size", &cfg.max_cruft_size, N_("with --cruft, limit the size of new cruft packs")), OPT_BOOL(0, "aggressive", &aggressive, N_("be more thorough (increased runtime)")), OPT_BOOL_F(0, "auto", &opts.auto_flag, N_("enable auto-gc mode"), @@ -651,27 +674,27 @@ int cmd_gc(int argc, const char **argv, const char *prefix) strvec_pushl(&rerere, "rerere", "gc", NULL); /* default expiry time, overwritten in gc_config */ - gc_config(); - if (parse_expiry_date(gc_log_expire, &gc_log_expire_time)) - die(_("failed to parse gc.logExpiry value %s"), gc_log_expire); + gc_config(&cfg); + if (parse_expiry_date(cfg.gc_log_expire, &gc_log_expire_time)) + die(_("failed to parse gc.logExpiry value %s"), cfg.gc_log_expire); - if (pack_refs < 0) - pack_refs = !is_bare_repository(); + if (cfg.pack_refs < 0) + cfg.pack_refs = !is_bare_repository(); argc = parse_options(argc, argv, prefix, builtin_gc_options, builtin_gc_usage, 0); if (argc > 0) usage_with_options(builtin_gc_usage, builtin_gc_options); - if (prune_expire && parse_expiry_date(prune_expire, &dummy)) - die(_("failed to parse prune expiry value %s"), prune_expire); + if (cfg.prune_expire && parse_expiry_date(cfg.prune_expire, &dummy)) + die(_("failed to parse prune expiry value %s"), cfg.prune_expire); if (aggressive) { strvec_push(&repack, "-f"); - if (aggressive_depth > 0) - strvec_pushf(&repack, "--depth=%d", aggressive_depth); - if (aggressive_window > 0) - strvec_pushf(&repack, "--window=%d", aggressive_window); + if (cfg.aggressive_depth > 0) + strvec_pushf(&repack, "--depth=%d", cfg.aggressive_depth); + if (cfg.aggressive_window > 0) + strvec_pushf(&repack, "--window=%d", cfg.aggressive_window); } if (quiet) strvec_push(&repack, "-q"); @@ -680,16 +703,16 @@ int cmd_gc(int argc, const char **argv, const char *prefix) /* * Auto-gc should be least intrusive as possible. */ - if (!need_to_gc()) + if (!need_to_gc(&cfg)) return 0; if (!quiet) { - if (detach_auto) + if (cfg.detach_auto) fprintf(stderr, _("Auto packing the repository in background for optimum performance.\n")); else fprintf(stderr, _("Auto packing the repository for optimum performance.\n")); fprintf(stderr, _("See \"git help gc\" for manual housekeeping.\n")); } - if (detach_auto) { + if (cfg.detach_auto) { int ret = report_last_gc_error(); if (ret == 1) @@ -701,7 +724,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix) if (lock_repo_for_gc(force, &pid)) return 0; - gc_before_repack(&opts); /* dies on failure */ + gc_before_repack(&opts, &cfg); /* dies on failure */ delete_tempfile(&pidfile); /* @@ -716,11 +739,11 @@ int cmd_gc(int argc, const char **argv, const char *prefix) if (keep_largest_pack != -1) { if (keep_largest_pack) find_base_packs(&keep_pack, 0); - } else if (big_pack_threshold) { - find_base_packs(&keep_pack, big_pack_threshold); + } else if (cfg.big_pack_threshold) { + find_base_packs(&keep_pack, cfg.big_pack_threshold); } - add_repack_all_option(&keep_pack); + add_repack_all_option(&cfg, &keep_pack); string_list_clear(&keep_pack, 0); } @@ -741,7 +764,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix) atexit(process_log_file_at_exit); } - gc_before_repack(&opts); + gc_before_repack(&opts, &cfg); if (!repository_format_precious_objects) { struct child_process repack_cmd = CHILD_PROCESS_INIT; @@ -752,11 +775,11 @@ int cmd_gc(int argc, const char **argv, const char *prefix) if (run_command(&repack_cmd)) die(FAILED_RUN, repack.v[0]); - if (prune_expire) { + if (cfg.prune_expire) { struct child_process prune_cmd = CHILD_PROCESS_INIT; /* run `git prune` even if using cruft packs */ - strvec_push(&prune, prune_expire); + strvec_push(&prune, cfg.prune_expire); if (quiet) strvec_push(&prune, "--no-progress"); if (repo_has_promisor_remote(the_repository)) @@ -769,10 +792,10 @@ int cmd_gc(int argc, const char **argv, const char *prefix) } } - if (prune_worktrees_expire) { + if (cfg.prune_worktrees_expire) { struct child_process prune_worktrees_cmd = CHILD_PROCESS_INIT; - strvec_push(&prune_worktrees, prune_worktrees_expire); + strvec_push(&prune_worktrees, cfg.prune_worktrees_expire); prune_worktrees_cmd.git_cmd = 1; strvec_pushv(&prune_worktrees_cmd.args, prune_worktrees.v); if (run_command(&prune_worktrees_cmd)) @@ -796,7 +819,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix) !quiet && !daemonized ? COMMIT_GRAPH_WRITE_PROGRESS : 0, NULL); - if (opts.auto_flag && too_many_loose_objects()) + if (opts.auto_flag && too_many_loose_objects(&cfg)) warning(_("There are too many unreachable loose objects; " "run 'git prune' to remove them.")); @@ -892,7 +915,7 @@ static int dfs_on_ref(const char *refname UNUSED, return result; } -static int should_write_commit_graph(void) +static int should_write_commit_graph(struct gc_config *cfg) { int result; struct cg_auto_data data; @@ -929,7 +952,8 @@ static int run_write_commit_graph(struct maintenance_run_opts *opts) return !!run_command(&child); } -static int maintenance_task_commit_graph(struct maintenance_run_opts *opts) +static int maintenance_task_commit_graph(struct maintenance_run_opts *opts, + struct gc_config *cfg) { prepare_repo_settings(the_repository); if (!the_repository->settings.core_commit_graph) @@ -963,7 +987,8 @@ static int fetch_remote(struct remote *remote, void *cbdata) return !!run_command(&child); } -static int maintenance_task_prefetch(struct maintenance_run_opts *opts) +static int maintenance_task_prefetch(struct maintenance_run_opts *opts, + struct gc_config *cfg) { if (for_each_remote(fetch_remote, opts)) { error(_("failed to prefetch remotes")); @@ -973,7 +998,8 @@ static int maintenance_task_prefetch(struct maintenance_run_opts *opts) return 0; } -static int maintenance_task_gc(struct maintenance_run_opts *opts) +static int maintenance_task_gc(struct maintenance_run_opts *opts, + struct gc_config *cfg) { struct child_process child = CHILD_PROCESS_INIT; @@ -1021,7 +1047,7 @@ static int loose_object_count(const struct object_id *oid UNUSED, return 0; } -static int loose_object_auto_condition(void) +static int loose_object_auto_condition(struct gc_config *cfg) { int count = 0; @@ -1106,12 +1132,13 @@ static int pack_loose(struct maintenance_run_opts *opts) return result; } -static int maintenance_task_loose_objects(struct maintenance_run_opts *opts) +static int maintenance_task_loose_objects(struct maintenance_run_opts *opts, + struct gc_config *cfg) { return prune_packed(opts) || pack_loose(opts); } -static int incremental_repack_auto_condition(void) +static int incremental_repack_auto_condition(struct gc_config *cfg) { struct packed_git *p; int incremental_repack_auto_limit = 10; @@ -1230,7 +1257,8 @@ static int multi_pack_index_repack(struct maintenance_run_opts *opts) return 0; } -static int maintenance_task_incremental_repack(struct maintenance_run_opts *opts) +static int maintenance_task_incremental_repack(struct maintenance_run_opts *opts, + struct gc_config *cfg) { prepare_repo_settings(the_repository); if (!the_repository->settings.core_multi_pack_index) { @@ -1247,14 +1275,15 @@ static int maintenance_task_incremental_repack(struct maintenance_run_opts *opts return 0; } -typedef int maintenance_task_fn(struct maintenance_run_opts *opts); +typedef int maintenance_task_fn(struct maintenance_run_opts *opts, + struct gc_config *cfg); /* * An auto condition function returns 1 if the task should run * and 0 if the task should NOT run. See needs_to_gc() for an * example. */ -typedef int maintenance_auto_fn(void); +typedef int maintenance_auto_fn(struct gc_config *cfg); struct maintenance_task { const char *name; @@ -1321,7 +1350,8 @@ static int compare_tasks_by_selection(const void *a_, const void *b_) return b->selected_order - a->selected_order; } -static int maintenance_run_tasks(struct maintenance_run_opts *opts) +static int maintenance_run_tasks(struct maintenance_run_opts *opts, + struct gc_config *cfg) { int i, found_selected = 0; int result = 0; @@ -1360,14 +1390,14 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts) if (opts->auto_flag && (!tasks[i].auto_condition || - !tasks[i].auto_condition())) + !tasks[i].auto_condition(cfg))) continue; if (opts->schedule && tasks[i].schedule < opts->schedule) continue; trace2_region_enter("maintenance", tasks[i].name, r); - if (tasks[i].fn(opts)) { + if (tasks[i].fn(opts, cfg)) { error(_("task '%s' failed"), tasks[i].name); result = 1; } @@ -1404,7 +1434,6 @@ static void initialize_task_config(int schedule) { int i; struct strbuf config_name = STRBUF_INIT; - gc_config(); if (schedule) initialize_maintenance_strategy(); @@ -1468,6 +1497,7 @@ static int maintenance_run(int argc, const char **argv, const char *prefix) { int i; struct maintenance_run_opts opts; + struct gc_config cfg = GC_CONFIG_INIT; struct option builtin_maintenance_run_options[] = { OPT_BOOL(0, "auto", &opts.auto_flag, N_("run tasks based on the state of the repository")), @@ -1496,12 +1526,13 @@ static int maintenance_run(int argc, const char **argv, const char *prefix) if (opts.auto_flag && opts.schedule) die(_("use at most one of --auto and --schedule=")); + gc_config(&cfg); initialize_task_config(opts.schedule); if (argc != 0) usage_with_options(builtin_maintenance_run_usage, builtin_maintenance_run_options); - return maintenance_run_tasks(&opts); + return maintenance_run_tasks(&opts, &cfg); } static char *get_maintpath(void) From patchwork Fri Aug 16 10:45:03 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13765896 Received: from fhigh3-smtp.messagingengine.com (fhigh3-smtp.messagingengine.com [103.168.172.154]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id ECAC9198A2F for ; Fri, 16 Aug 2024 10:45:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.154 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723805109; cv=none; b=FyCZAAFnZjgwDAmKC/j1d49bK7IhQ+qlFOTjbVGR1b945xfJqgtDDXC8k+/mwl6inhLvBER+i0eHpZOQCpRS/GUu7S1HaxWrXUHJD6rxyP+fpDFixF9v72GRyLUHDBlLGs71CcOCusMrmORKuTcz78MFHweefjJ0UY8bpuzOOZs= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723805109; c=relaxed/simple; bh=4Wzv36psV92fF6A3OxDi5LlfStXVgt9cTn++ujBmkMQ=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=cxW2wIXXo4590Wfx76S67qFLx3REwwMAS36nV3Gqm+hNvXYeypUyk8EinXBKZPI8f5K4mNZz4XK4xOPepdnRQS7QugJDe49eyi9FtQmE2E6KeuIsB3lBogLhiDUleFV9tD0ZmWZ3Xi5b1iRZvHuLNmZjLnyHqAvhuJ2OMDl+SDQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=DZVvyNHZ; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=qh6SlzaC; arc=none smtp.client-ip=103.168.172.154 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="DZVvyNHZ"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="qh6SlzaC" Received: from phl-compute-01.internal (phl-compute-01.nyi.internal [10.202.2.41]) by mailfhigh.nyi.internal (Postfix) with ESMTP id 1022A1151BD3; Fri, 16 Aug 2024 06:45:07 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-01.internal (MEProxy); Fri, 16 Aug 2024 06:45:07 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm3; t=1723805107; x=1723891507; bh=jX9yp/VkT4 lRnv0IDOV2dC1huardv/smssO30VI+W9A=; b=DZVvyNHZfvZKf5ys8bXrk+BV7w ex+f1SAWwRcUrdiR7ZVs5624dkSahKGiAW3k/74bAxd9+G3MaxlBCrMGA+qhAjtF VtmMqiw+3G9lZT0DhSyTqhCGxq2AjVTrkCweKPZ0HrcL4HiGDi+AeFFNOomXCykk HoRfKTNAnUyY5Zm0oHXPysH9Cu5gmpQMZ0a7iGXcMjiqYaw/FES2SVsOj0AfTT+t m2zMxPmSA4ce1+rGezQxjH2XwuDTM52lk3483nVb7TaQRl14hxMAQX70NQ5F3BlQ T6yUDyIc15YMka06iFcyifT86Nx4GunVf6FwsfHRkbyfGw0iRfhHEC/y9yyw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1723805107; x=1723891507; bh=jX9yp/VkT4lRnv0IDOV2dC1huard v/smssO30VI+W9A=; b=qh6SlzaCHucd1c+3xyjPP3SmUCv18D/Ia4NEnQBjH836 7NZ8M4l9qr/gUvV3RibspOJWTiRrp/nm2XhGq8jxK+jtsmd/MSduK5LqQEzmjEyx 6Jf6D4yzifh6djdZsj+x/m+ofs0/F/z+DDwQ44OYKhBLtJzNxjMFq3Jchrpm0k+p 7KVIbCEbx0lcnNk5aVw4x7couOu99xIAjmFURaT5iYplD8nHyO/c2xeCxX5Ej7mF /n0XfcD48ATIuxj9P1c7XjXX9iQltmNNftHzit02iT5YHr+TDxqwYPUB7OcvAHJ6 qaTrOiwDBE8mM9MfbfTdfGajhBz73EO/ltINOOsdSw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddruddtkedgfedtucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhepfffhvfevuffkfhggtggujgesthdtredttddtvden ucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimh eqnecuggftrfgrthhtvghrnhepveekkeffhfeitdeludeigfejtdetvdelvdduhefgueeg udfghfeukefhjedvkedtnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrg hilhhfrhhomhepphhssehpkhhsrdhimhdpnhgspghrtghpthhtohepiedpmhhouggvpehs mhhtphhouhhtpdhrtghpthhtohepshhtohhlvggvsehgmhgrihhlrdgtohhmpdhrtghpth htohepghhithesvhhgvghrrdhkvghrnhgvlhdrohhrghdprhgtphhtthhopehjrghmvghs sehjrghmvghslhhiuhdrihhopdhrtghpthhtohepghhithhsthgvrhesphhosghogidrtg homhdprhgtphhtthhopehphhhilhhlihhprdifohhougduvdefsehgmhgrihhlrdgtohhm pdhrtghpthhtohepphhhihhllhhiphdrfihoohguseguuhhnvghlmhdrohhrghdruhhk X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 16 Aug 2024 06:45:05 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 8d1352ab (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 16 Aug 2024 10:44:42 +0000 (UTC) Date: Fri, 16 Aug 2024 12:45:03 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Phillip Wood , phillip.wood@dunelm.org.uk, James Liu , Derrick Stolee , Junio C Hamano Subject: [PATCH v3 3/7] builtin/gc: fix leaking config values Message-ID: <310e361371efc156c3aaac94439bdbeaa965155f.1723804990.git.ps@pks.im> References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: We're leaking config values in git-gc(1) when those values are tracked as strings. Introduce a new `gc_config_release()` function that releases this memory to plug those leaks and release old values before populating the config fields via `git_config_string()` et al. Note that there is one small gotcha here with the "--prune" option. Next to passing a string, this option also accepts the "--no-prune" option that overrides the default or configured value. We thus need to discern between the option not having been passed by the user and the negative variant of it. This is done by using a simple sentinel value that lets us discern these cases. Signed-off-by: Patrick Steinhardt --- builtin/gc.c | 108 +++++++++++++++++++++++++++++++++++------------ t/t5304-prune.sh | 1 + 2 files changed, 82 insertions(+), 27 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index eee7401647..a93cfa147e 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -139,9 +139,9 @@ struct gc_config { int gc_auto_threshold; int gc_auto_pack_limit; int detach_auto; - const char *gc_log_expire; - const char *prune_expire; - const char *prune_worktrees_expire; + char *gc_log_expire; + char *prune_expire; + char *prune_worktrees_expire; char *repack_filter; char *repack_filter_to; unsigned long big_pack_threshold; @@ -157,15 +157,25 @@ struct gc_config { .gc_auto_threshold = 6700, \ .gc_auto_pack_limit = 50, \ .detach_auto = 1, \ - .gc_log_expire = "1.day.ago", \ - .prune_expire = "2.weeks.ago", \ - .prune_worktrees_expire = "3.months.ago", \ + .gc_log_expire = xstrdup("1.day.ago"), \ + .prune_expire = xstrdup("2.weeks.ago"), \ + .prune_worktrees_expire = xstrdup("3.months.ago"), \ .max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE, \ } +static void gc_config_release(struct gc_config *cfg) +{ + free(cfg->gc_log_expire); + free(cfg->prune_expire); + free(cfg->prune_worktrees_expire); + free(cfg->repack_filter); + free(cfg->repack_filter_to); +} + static void gc_config(struct gc_config *cfg) { const char *value; + char *owned = NULL; if (!git_config_get_value("gc.packrefs", &value)) { if (value && !strcmp(value, "notbare")) @@ -185,15 +195,34 @@ static void gc_config(struct gc_config *cfg) git_config_get_bool("gc.autodetach", &cfg->detach_auto); git_config_get_bool("gc.cruftpacks", &cfg->cruft_packs); git_config_get_ulong("gc.maxcruftsize", &cfg->max_cruft_size); - git_config_get_expiry("gc.pruneexpire", (char **) &cfg->prune_expire); - git_config_get_expiry("gc.worktreepruneexpire", (char **) &cfg->prune_worktrees_expire); - git_config_get_expiry("gc.logexpiry", (char **) &cfg->gc_log_expire); + + if (!git_config_get_expiry("gc.pruneexpire", &owned)) { + free(cfg->prune_expire); + cfg->prune_expire = owned; + } + + if (!git_config_get_expiry("gc.worktreepruneexpire", &owned)) { + free(cfg->prune_worktrees_expire); + cfg->prune_worktrees_expire = owned; + } + + if (!git_config_get_expiry("gc.logexpiry", &owned)) { + free(cfg->gc_log_expire); + cfg->gc_log_expire = owned; + } git_config_get_ulong("gc.bigpackthreshold", &cfg->big_pack_threshold); git_config_get_ulong("pack.deltacachesize", &cfg->max_delta_cache_size); - git_config_get_string("gc.repackfilter", &cfg->repack_filter); - git_config_get_string("gc.repackfilterto", &cfg->repack_filter_to); + if (!git_config_get_string("gc.repackfilter", &owned)) { + free(cfg->repack_filter); + cfg->repack_filter = owned; + } + + if (!git_config_get_string("gc.repackfilterto", &owned)) { + free(cfg->repack_filter_to); + cfg->repack_filter_to = owned; + } git_config(git_default_config, NULL); } @@ -644,12 +673,15 @@ int cmd_gc(int argc, const char **argv, const char *prefix) struct child_process rerere_cmd = CHILD_PROCESS_INIT; struct maintenance_run_opts opts = {0}; struct gc_config cfg = GC_CONFIG_INIT; + const char *prune_expire_sentinel = "sentinel"; + const char *prune_expire_arg = prune_expire_sentinel; + int ret; struct option builtin_gc_options[] = { OPT__QUIET(&quiet, N_("suppress progress reporting")), - { OPTION_STRING, 0, "prune", &cfg.prune_expire, N_("date"), + { OPTION_STRING, 0, "prune", &prune_expire_arg, N_("date"), N_("prune unreferenced objects"), - PARSE_OPT_OPTARG, NULL, (intptr_t)cfg.prune_expire }, + PARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire_arg }, OPT_BOOL(0, "cruft", &cfg.cruft_packs, N_("pack unreferenced objects separately")), OPT_MAGNITUDE(0, "max-cruft-size", &cfg.max_cruft_size, N_("with --cruft, limit the size of new cruft packs")), @@ -673,8 +705,8 @@ int cmd_gc(int argc, const char **argv, const char *prefix) strvec_pushl(&prune_worktrees, "worktree", "prune", "--expire", NULL); strvec_pushl(&rerere, "rerere", "gc", NULL); - /* default expiry time, overwritten in gc_config */ gc_config(&cfg); + if (parse_expiry_date(cfg.gc_log_expire, &gc_log_expire_time)) die(_("failed to parse gc.logExpiry value %s"), cfg.gc_log_expire); @@ -686,6 +718,10 @@ int cmd_gc(int argc, const char **argv, const char *prefix) if (argc > 0) usage_with_options(builtin_gc_usage, builtin_gc_options); + if (prune_expire_arg != prune_expire_sentinel) { + free(cfg.prune_expire); + cfg.prune_expire = xstrdup_or_null(prune_expire_arg); + } if (cfg.prune_expire && parse_expiry_date(cfg.prune_expire, &dummy)) die(_("failed to parse prune expiry value %s"), cfg.prune_expire); @@ -703,8 +739,11 @@ int cmd_gc(int argc, const char **argv, const char *prefix) /* * Auto-gc should be least intrusive as possible. */ - if (!need_to_gc(&cfg)) - return 0; + if (!need_to_gc(&cfg)) { + ret = 0; + goto out; + } + if (!quiet) { if (cfg.detach_auto) fprintf(stderr, _("Auto packing the repository in background for optimum performance.\n")); @@ -713,17 +752,22 @@ int cmd_gc(int argc, const char **argv, const char *prefix) fprintf(stderr, _("See \"git help gc\" for manual housekeeping.\n")); } if (cfg.detach_auto) { - int ret = report_last_gc_error(); - - if (ret == 1) + ret = report_last_gc_error(); + if (ret == 1) { /* Last gc --auto failed. Skip this one. */ - return 0; - else if (ret) + ret = 0; + goto out; + + } else if (ret) { /* an I/O error occurred, already reported */ - return ret; + goto out; + } + + if (lock_repo_for_gc(force, &pid)) { + ret = 0; + goto out; + } - if (lock_repo_for_gc(force, &pid)) - return 0; gc_before_repack(&opts, &cfg); /* dies on failure */ delete_tempfile(&pidfile); @@ -749,8 +793,11 @@ int cmd_gc(int argc, const char **argv, const char *prefix) name = lock_repo_for_gc(force, &pid); if (name) { - if (opts.auto_flag) - return 0; /* be quiet on --auto */ + if (opts.auto_flag) { + ret = 0; + goto out; /* be quiet on --auto */ + } + die(_("gc is already running on machine '%s' pid %"PRIuMAX" (use --force if not)"), name, (uintmax_t)pid); } @@ -826,6 +873,8 @@ int cmd_gc(int argc, const char **argv, const char *prefix) if (!daemonized) unlink(git_path("gc.log")); +out: + gc_config_release(&cfg); return 0; } @@ -1511,6 +1560,8 @@ static int maintenance_run(int argc, const char **argv, const char *prefix) PARSE_OPT_NONEG, task_option_parse), OPT_END() }; + int ret; + memset(&opts, 0, sizeof(opts)); opts.quiet = !isatty(2); @@ -1532,7 +1583,10 @@ static int maintenance_run(int argc, const char **argv, const char *prefix) if (argc != 0) usage_with_options(builtin_maintenance_run_usage, builtin_maintenance_run_options); - return maintenance_run_tasks(&opts, &cfg); + + ret = maintenance_run_tasks(&opts, &cfg); + gc_config_release(&cfg); + return ret; } static char *get_maintpath(void) diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh index 1f1f664871..e641df0116 100755 --- a/t/t5304-prune.sh +++ b/t/t5304-prune.sh @@ -7,6 +7,7 @@ test_description='prune' GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh day=$((60*60*24)) From patchwork Fri Aug 16 10:45:06 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13765897 Received: from fhigh3-smtp.messagingengine.com (fhigh3-smtp.messagingengine.com [103.168.172.154]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9C887199E81 for ; Fri, 16 Aug 2024 10:45:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.154 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723805114; cv=none; b=OxICFNG4ldZdJWnQOkqyer8fN0ArpB1p/+k8Mpl63s3MFHOxueWVRpxDYdSEqZ6+TSMmP+M+vMmw9GsV0W2h5vzD+5nfQEjpj673yor3XcEhwbCawBgHE4yQxZXGRatyksWk/1TwglxWpcbCf2/pKdqMRD44l8ywmBzC0iKW+tI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723805114; c=relaxed/simple; bh=XmkTB6jxtXARBaWDG5rPAnVGrArllhsJEigdfU6PNxE=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=ZZ6NQKgJfIxFPfAkskT0U6XtAkfxFUE88wNmgwq1EtuRiskEBqEFzJnvbxikPLlMrMSRiWNXr1HOyVJ6Z3uplo58M8geSFxcgJkH9Sa7ydCrCcqRDYNm8tKzVvo8Xm3TvLd/TP6IzSEWQH64uflgorhJcATiaTmLFZzVBo1876k= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=fnopqSRE; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=mXiO7oHx; arc=none smtp.client-ip=103.168.172.154 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="fnopqSRE"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="mXiO7oHx" Received: from phl-compute-06.internal (phl-compute-06.nyi.internal [10.202.2.46]) by mailfhigh.nyi.internal (Postfix) with ESMTP id ADA1B1151BD7; Fri, 16 Aug 2024 06:45:11 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-06.internal (MEProxy); Fri, 16 Aug 2024 06:45:11 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm3; t=1723805111; x=1723891511; bh=5tYMlbLDwv m982XMJggbaMnOEuW6BOecxZBgQpD8Tug=; b=fnopqSREJrevQsY2bbViubn9/S ogCfdQA+WzkHTOJV/GxUC5wqsG6JHWroBSUymFl8HEnxZzWZk+7rhIZUOtUtTlnD 6QardTTyT+ZN+z08ku6TYbOo/pOVltTLAGVjV5s8b7qaojXG8xYSotmJslwyRzJI BqoASDJxbEvMEXfFBN5ZE2NQqwjLU6dv+hjzDZY+00CnSFYbMzE9FKZTD9hdgMy3 tZd6f6UI6B4dzw6S8ZFV/qHCO7794Kq4QVIWIYfC3JLq2eYadFVYrjx+CvH1mJdR E1BuyuXdFRprjySGnyT536toReti7KukzJYgsJwZwpp/mzxzUIWG3zGpBPkA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1723805111; x=1723891511; bh=5tYMlbLDwvm982XMJggbaMnOEuW6 BOecxZBgQpD8Tug=; b=mXiO7oHxl983UC0l4Fy5lIvaa06AKf2FcbhwYn53tQdO DAnKJF2d/xpIRVNb5hmzDYR98BD/16W4WClkWfeIeEjCSLqmpofFjOF9xtA76mAh UR5FqgldPU5rnpYSDDf2hcSgEY0BNJgyHNbNiBc0isgLyP6zzsd94r50QYSB6Vy4 tLCyPw/TbTpnCoNC4nkNflG7NcTRbYudWgwrNIF1ScjZlTYblGhAu12f2wZ0i9VW CC4FXbsx2e5OkZXn6n3fBtmK+HmzTolVCBXEHwXqUqgVtTUQ/GFGX9q+pLoWmL9y wwz7hrD2B+gLXImqQMMghW6xAW8VviBmP15CEaBGDQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddruddtkedgfedtucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhepfffhvfevuffkfhggtggujgesthdtredttddtvden ucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimh eqnecuggftrfgrthhtvghrnhepveekkeffhfeitdeludeigfejtdetvdelvdduhefgueeg udfghfeukefhjedvkedtnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrg hilhhfrhhomhepphhssehpkhhsrdhimhdpnhgspghrtghpthhtohepiedpmhhouggvpehs mhhtphhouhhtpdhrtghpthhtohepphhhihhllhhiphdrfihoohguseguuhhnvghlmhdroh hrghdruhhkpdhrtghpthhtohepshhtohhlvggvsehgmhgrihhlrdgtohhmpdhrtghpthht ohepghhithhsthgvrhesphhosghogidrtghomhdprhgtphhtthhopehjrghmvghssehjrg hmvghslhhiuhdrihhopdhrtghpthhtohepphhhihhllhhiphdrfihoohguuddvfeesghhm rghilhdrtghomhdprhgtphhtthhopehgihhtsehvghgvrhdrkhgvrhhnvghlrdhorhhg X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 16 Aug 2024 06:45:10 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id ea2fbe53 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 16 Aug 2024 10:44:47 +0000 (UTC) Date: Fri, 16 Aug 2024 12:45:06 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Phillip Wood , phillip.wood@dunelm.org.uk, James Liu , Derrick Stolee , Junio C Hamano Subject: [PATCH v3 4/7] builtin/gc: stop processing log file on signal Message-ID: <812c61c9b66d7608e41c6c1d00a6e22e995cef06.1723804990.git.ps@pks.im> References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: When detaching, git-gc(1) will redirect its stderr to a "gc.log" log file, which is then used to surface errors of a backgrounded process to the user. To ensure that the file is properly managed on abnormal exit paths, we install both signal and exit handlers that try to either commit the underlying lock file or roll it back in case there wasn't any error. This logic is severly broken when handling signals though, as we end up calling all kinds of functions that are not signal safe. This includes malloc(3P) via `git_path()`, fprintf(3P), fflush(3P) and many more functions. The consequence can be anything, from deadlocks to crashes. Unfortunately, we cannot really do much about this without a larger refactoring. The least-worst thing we can do is to not set up the signal handler in the first place. This will still cause us to remove the lockfile, as the underlying tempfile subsystem already knows to unlink locks when receiving a signal. But it may cause us to remove the lock even in the case where it would have contained actual errors, which is a change in behaviour. The consequence is that "gc.log" will not be committed, and thus subsequent calls to `git gc --auto` won't bail out because of this. Arguably though, it is better to retry garbage collection rather than having the process run into a potentially-corrupted state. Signed-off-by: Patrick Steinhardt --- builtin/gc.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index a93cfa147e..f815557081 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -109,13 +109,6 @@ static void process_log_file_at_exit(void) process_log_file(); } -static void process_log_file_on_signal(int signo) -{ - process_log_file(); - sigchain_pop(signo); - raise(signo); -} - static int gc_config_is_timestamp_never(const char *var) { const char *value; @@ -807,7 +800,6 @@ int cmd_gc(int argc, const char **argv, const char *prefix) git_path("gc.log"), LOCK_DIE_ON_ERROR); dup2(get_lock_file_fd(&log_lock), 2); - sigchain_push_common(process_log_file_on_signal); atexit(process_log_file_at_exit); } From patchwork Fri Aug 16 10:45:11 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13765898 Received: from fout8-smtp.messagingengine.com (fout8-smtp.messagingengine.com [103.168.172.151]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 33E681922D6 for ; Fri, 16 Aug 2024 10:45:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.151 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723805117; cv=none; b=ArGvYUOH7LD4qhbO4hL+Bnvo64tI7Ph3yyH0wjaUBJK/HxFTlu2mfvGXfmRuWakJgua9mqm5B5WT/k6srkQdi3h8sDMI2O+Lw1FGMqHRElYbHhpQJptbhS/2VxQuwJ0/13CUcmMQ7CjpZrk4B5edl2qtnvPmrHxQb8k/s/vUVx0= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723805117; c=relaxed/simple; bh=xl2Q4Xzqx/ofjO32pNNvVUe73vXl2KDIYZa91QhQJv0=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=QlRCkDG6ikWN3bRZXN3/3UHiV+BDIqp6Ee9uOYKdiRPpX8mWnm68wDYl6XmqbJdD8k8bpqEuoxXs2PYiYXztcq+nKmNvWqKMEMcj4RmH1xCc8WNs14hWt1UcftiUGgCtTNWxT35L4GySWgjH2wMtALPM/a0dEO6C7iPsHpjA49E= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=bAu+oYoM; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=fHeVtQS3; arc=none smtp.client-ip=103.168.172.151 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="bAu+oYoM"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="fHeVtQS3" Received: from phl-compute-03.internal (phl-compute-03.nyi.internal [10.202.2.43]) by mailfout.nyi.internal (Postfix) with ESMTP id 46B0C138FF9D; Fri, 16 Aug 2024 06:45:15 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-03.internal (MEProxy); Fri, 16 Aug 2024 06:45:15 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm3; t=1723805115; x=1723891515; bh=R9MhfufrW3 MyBfp8JOpMiCQ1QiXUwzbYdOupl+IX0LA=; b=bAu+oYoM7asiAMs77dT2KVsxie WoPSE67UPlX0i0+buDgUZ5UgHU+p9fq+0k/Il+H5asowMFQGLCsoWB9VafEqiQLi Z6bIKr/C+eJmJZzRQskEy5vUdlDV2+Wx5PC4WbH7ORKf/U6p5yDubXphHZGZN4Tl lMNGgoMXfV7dyd1Jug+AYAoAtsiXFDqj/BLNA4j7kh/m9/1FzdkiOmyh+mY8uxwY BGrzDpnCCq1ImZ1qDdGqz+cleZPMKeU8aGO808mWTvI+HnonDNJv0RZGqWRUE+t7 3zUQtcLmSbz/Irm5nyAy7mFxaMIzcxt+D6Z4sWPWeeU/64l42jAe6I4mTzZA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1723805115; x=1723891515; bh=R9MhfufrW3MyBfp8JOpMiCQ1QiXU wzbYdOupl+IX0LA=; b=fHeVtQS30Q7hkAt+HD8caJMRvZpvdCOmPIZEKuUP/AL2 lIfc4g06sZTIVB0KI0k+hQi88Ug/Cy2TZ8T7htAUBc6hYDySL7neh4nTt10RMCpd ZTIwEzYSqDCLQWPIP813Z2rl8ml3E4o3cCJm3KYgXlFkoIJIgXcaZcfTyJeI3RTw jW7puuI8Oa+c4gtfydWUDV0GJXLtvoZT+4Ww+8M2GB6eh9CI3c3w/OWPsy6qUjSW WBZMC0uZDbyIrcIA9nSl9XDv2eMjtUlGha3B9x1s8sYANDrl6O4UL9fS4njzrKrp Y/ZeGuaKlE91eDnRxNhOmlJt5nGvY3ncG9upkZzmyw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddruddtkedgfedtucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhepfffhvfevuffkfhggtggujgesthdtredttddtvden ucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimh eqnecuggftrfgrthhtvghrnhepveekkeffhfeitdeludeigfejtdetvdelvdduhefgueeg udfghfeukefhjedvkedtnecuvehluhhsthgvrhfuihiivgepudenucfrrghrrghmpehmrg hilhhfrhhomhepphhssehpkhhsrdhimhdpnhgspghrtghpthhtohepiedpmhhouggvpehs mhhtphhouhhtpdhrtghpthhtohepjhgrmhgvshesjhgrmhgvshhlihhurdhiohdprhgtph htthhopehsthholhgvvgesghhmrghilhdrtghomhdprhgtphhtthhopehgihhtsehvghgv rhdrkhgvrhhnvghlrdhorhhgpdhrtghpthhtohepphhhihhllhhiphdrfihoohguuddvfe esghhmrghilhdrtghomhdprhgtphhtthhopehphhhilhhlihhprdifohhougesughunhgv lhhmrdhorhhgrdhukhdprhgtphhtthhopehgihhtshhtvghrsehpohgsohigrdgtohhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 16 Aug 2024 06:45:13 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 47a5c405 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 16 Aug 2024 10:44:50 +0000 (UTC) Date: Fri, 16 Aug 2024 12:45:11 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Phillip Wood , phillip.wood@dunelm.org.uk, James Liu , Derrick Stolee , Junio C Hamano Subject: [PATCH v3 5/7] builtin/gc: add a `--detach` flag Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: When running `git gc --auto`, the command will by default detach and continue running in the background. This behaviour can be tweaked via the `gc.autoDetach` config, but not via a command line switch. We need that in a subsequent commit though, where git-maintenance(1) will want to ask its git-gc(1) child process to not detach anymore. Add a `--[no-]detach` flag that does this for us. Signed-off-by: Patrick Steinhardt --- Documentation/git-gc.txt | 5 ++- builtin/gc.c | 70 ++++++++++++++++++++++------------------ t/t6500-gc.sh | 45 +++++++++++++++++++++++--- 3 files changed, 84 insertions(+), 36 deletions(-) diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt index b5561c458a..370e22faae 100644 --- a/Documentation/git-gc.txt +++ b/Documentation/git-gc.txt @@ -9,7 +9,7 @@ git-gc - Cleanup unnecessary files and optimize the local repository SYNOPSIS -------- [verse] -'git gc' [--aggressive] [--auto] [--quiet] [--prune= | --no-prune] [--force] [--keep-largest-pack] +'git gc' [--aggressive] [--auto] [--[no-]detach] [--quiet] [--prune= | --no-prune] [--force] [--keep-largest-pack] DESCRIPTION ----------- @@ -53,6 +53,9 @@ configuration options such as `gc.auto` and `gc.autoPackLimit`, all other housekeeping tasks (e.g. rerere, working trees, reflog...) will be performed as well. +--[no-]detach:: + Run in the background if the system supports it. This option overrides + the `gc.autoDetach` config. --[no-]cruft:: When expiring unreachable objects, pack them separately into a diff --git a/builtin/gc.c b/builtin/gc.c index f815557081..269a77960f 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -242,9 +242,13 @@ static enum schedule_priority parse_schedule(const char *value) struct maintenance_run_opts { int auto_flag; + int detach; int quiet; enum schedule_priority schedule; }; +#define MAINTENANCE_RUN_OPTS_INIT { \ + .detach = -1, \ +} static int pack_refs_condition(UNUSED struct gc_config *cfg) { @@ -664,7 +668,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix) int keep_largest_pack = -1; timestamp_t dummy; struct child_process rerere_cmd = CHILD_PROCESS_INIT; - struct maintenance_run_opts opts = {0}; + struct maintenance_run_opts opts = MAINTENANCE_RUN_OPTS_INIT; struct gc_config cfg = GC_CONFIG_INIT; const char *prune_expire_sentinel = "sentinel"; const char *prune_expire_arg = prune_expire_sentinel; @@ -681,6 +685,8 @@ int cmd_gc(int argc, const char **argv, const char *prefix) OPT_BOOL(0, "aggressive", &aggressive, N_("be more thorough (increased runtime)")), OPT_BOOL_F(0, "auto", &opts.auto_flag, N_("enable auto-gc mode"), PARSE_OPT_NOCOMPLETE), + OPT_BOOL(0, "detach", &opts.detach, + N_("perform garbage collection in the background")), OPT_BOOL_F(0, "force", &force, N_("force running gc even if there may be another gc running"), PARSE_OPT_NOCOMPLETE), @@ -729,6 +735,9 @@ int cmd_gc(int argc, const char **argv, const char *prefix) strvec_push(&repack, "-q"); if (opts.auto_flag) { + if (cfg.detach_auto && opts.detach < 0) + opts.detach = 1; + /* * Auto-gc should be least intrusive as possible. */ @@ -738,38 +747,12 @@ int cmd_gc(int argc, const char **argv, const char *prefix) } if (!quiet) { - if (cfg.detach_auto) + if (opts.detach > 0) fprintf(stderr, _("Auto packing the repository in background for optimum performance.\n")); else fprintf(stderr, _("Auto packing the repository for optimum performance.\n")); fprintf(stderr, _("See \"git help gc\" for manual housekeeping.\n")); } - if (cfg.detach_auto) { - ret = report_last_gc_error(); - if (ret == 1) { - /* Last gc --auto failed. Skip this one. */ - ret = 0; - goto out; - - } else if (ret) { - /* an I/O error occurred, already reported */ - goto out; - } - - if (lock_repo_for_gc(force, &pid)) { - ret = 0; - goto out; - } - - gc_before_repack(&opts, &cfg); /* dies on failure */ - delete_tempfile(&pidfile); - - /* - * failure to daemonize is ok, we'll continue - * in foreground - */ - daemonized = !daemonize(); - } } else { struct string_list keep_pack = STRING_LIST_INIT_NODUP; @@ -784,6 +767,33 @@ int cmd_gc(int argc, const char **argv, const char *prefix) string_list_clear(&keep_pack, 0); } + if (opts.detach > 0) { + ret = report_last_gc_error(); + if (ret == 1) { + /* Last gc --auto failed. Skip this one. */ + ret = 0; + goto out; + + } else if (ret) { + /* an I/O error occurred, already reported */ + goto out; + } + + if (lock_repo_for_gc(force, &pid)) { + ret = 0; + goto out; + } + + gc_before_repack(&opts, &cfg); /* dies on failure */ + delete_tempfile(&pidfile); + + /* + * failure to daemonize is ok, we'll continue + * in foreground + */ + daemonized = !daemonize(); + } + name = lock_repo_for_gc(force, &pid); if (name) { if (opts.auto_flag) { @@ -1537,7 +1547,7 @@ static int task_option_parse(const struct option *opt UNUSED, static int maintenance_run(int argc, const char **argv, const char *prefix) { int i; - struct maintenance_run_opts opts; + struct maintenance_run_opts opts = MAINTENANCE_RUN_OPTS_INIT; struct gc_config cfg = GC_CONFIG_INIT; struct option builtin_maintenance_run_options[] = { OPT_BOOL(0, "auto", &opts.auto_flag, @@ -1554,8 +1564,6 @@ static int maintenance_run(int argc, const char **argv, const char *prefix) }; int ret; - memset(&opts, 0, sizeof(opts)); - opts.quiet = !isatty(2); for (i = 0; i < TASK__COUNT; i++) diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh index 1b5909d1b7..5378455968 100755 --- a/t/t6500-gc.sh +++ b/t/t6500-gc.sh @@ -338,14 +338,14 @@ test_expect_success 'gc.maxCruftSize sets appropriate repack options' ' test_subcommand $cruft_max_size_opts --max-cruft-size=3145728 &1) + doesnt_matter=$(git gc "$@" 9>&1) } test_expect_success 'background auto gc does not run if gc.log is present and recent but does if it is old' ' @@ -361,7 +361,7 @@ test_expect_success 'background auto gc does not run if gc.log is present and re test-tool chmtime =-345600 .git/gc.log && git gc --auto && test_config gc.logexpiry 2.days && - run_and_wait_for_auto_gc && + run_and_wait_for_gc --auto && ls .git/objects/pack/pack-*.pack >packs && test_line_count = 1 packs ' @@ -391,11 +391,48 @@ test_expect_success 'background auto gc respects lock for all operations' ' printf "%d %s" "$shell_pid" "$hostname" >.git/gc.pid && # our gc should exit zero without doing anything - run_and_wait_for_auto_gc && + run_and_wait_for_gc --auto && (ls -1 .git/refs/heads .git/reftable >actual || true) && test_cmp expect actual ' +test_expect_success '--detach overrides gc.autoDetach=false' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + + # Prepare the repository such that git-gc(1) ends up repacking. + test_commit "$(test_oid blob17_1)" && + test_commit "$(test_oid blob17_2)" && + git config gc.autodetach false && + git config gc.auto 2 && + + # Note that we cannot use `test_cmp` here to compare stderr + # because it may contain output from `set -x`. + run_and_wait_for_gc --auto --detach 2>actual && + test_grep "Auto packing the repository in background for optimum performance." actual + ) +' + +test_expect_success '--no-detach overrides gc.autoDetach=true' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + + # Prepare the repository such that git-gc(1) ends up repacking. + test_commit "$(test_oid blob17_1)" && + test_commit "$(test_oid blob17_2)" && + git config gc.autodetach true && + git config gc.auto 2 && + + GIT_PROGRESS_DELAY=0 git gc --auto --no-detach 2>output && + test_grep "Auto packing the repository for optimum performance." output && + test_grep "Collecting referenced commits: 2, done." output + ) +' + # 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'. From patchwork Fri Aug 16 10:45:15 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13765899 Received: from fout8-smtp.messagingengine.com (fout8-smtp.messagingengine.com [103.168.172.151]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 92275198853 for ; Fri, 16 Aug 2024 10:45:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.151 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723805120; cv=none; b=Fl/ljpqBgcnqxRbseALwVNozwtdqiqPWfgnO0Oyq+ypAHmx8cHmB7Ooz5BabxweN3g6ctHwMwSNpY+WNnXkidu+fSb51Wj3r+7M79EJHSmhSHHdVeA3SqFE0ik2hAB3HCQxCS/geDtpWY3kyQ6Y6jt1lqwxduNl0911kDMg88dg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723805120; c=relaxed/simple; bh=KSWsfajIcUFRTvw5OXCvMJXWv8lmrp53J2EwVytGMyI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Qte/wj8qxPpNjF1OKA/RPfZnCFgvPO8sg1jxIRHF6rabUPHnxdaFEKdSBeNB7/dyMap69ZyHHcgsFGZlvcXK7Gh+2gRHWF62WqB0zrrsgCuECvbzMGa5vGQJ0G69YmCm6vMTXb5ruvmI2JH0DZ+kuk0frcX3Xkn23ft8y9qOUFQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=R+eOFL4B; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=tBCBVpi1; arc=none smtp.client-ip=103.168.172.151 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="R+eOFL4B"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="tBCBVpi1" Received: from phl-compute-07.internal (phl-compute-07.nyi.internal [10.202.2.47]) by mailfout.nyi.internal (Postfix) with ESMTP id E7AA4138FE48; Fri, 16 Aug 2024 06:45:17 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-07.internal (MEProxy); Fri, 16 Aug 2024 06:45:17 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm3; t=1723805117; x=1723891517; bh=KgsNH8wJ5B XMfgoqLPY8FcTf3d+ImoI1XBkyOliaKJQ=; b=R+eOFL4BAz82Xum+GLu1Nd7Kik K8TCjIco5YiMOwbDfCuhOS629rYg4r+DVjp4hiwvpw7qu/Y03tswINqmEWumRDWQ BJ8Ws57cTj7UIE+76WUE+UnKvEPXiEe23t2y73l2h9/DqpnkeXLEifMnSDhvNL58 xC3PzsAsNWMn4UO4vKT54QUjpQnOMFgOteZf/GSkrHJKoUt2f4pLUaI3mncYn9XE 0sBIhKqnGawHE1LcM8F7JC9vY4hqrZmtbuQOWY49TPfJF9cz+SK1U5cx73jSHPcO gfSakd88NGtQr9YodS2A4XhXMP9VP+zCsXpBQoZXFXz73VkrqE7+ZRBwk2xw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1723805117; x=1723891517; bh=KgsNH8wJ5BXMfgoqLPY8FcTf3d+I moI1XBkyOliaKJQ=; b=tBCBVpi1YyGSbZurWOuWpufqb3i1LeZyJhAk7SmodH6T kvL4T7YN3Tma0yPvpS0VmUAu+yNw8VFTxsn4F/D4w2Hz6dd1HdnJ2xcY3XE7u1qF O/iksEnZrJ1sEiugyNZDXa1j6Xn5zeu2scmHK503Ut0Qh8R6KqUfBOF9sh2MzSZM ziJyTz6b+9AivBDnAXBvlWK21PU37pel/I2UbO/KuWPM4HflpxgcDv+rtBIuh/is bhla4rPZVAbrVo4kLvE5QEUFIhhHhSfm5pCQyewO4u+HbHAP8QL1yFz1M5vft8Ir EQXrE+qYdO4fLnDT20QqgHY4Z4KHxRUNe/uOsB0GGQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddruddtkedgfedtucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhepfffhvfevuffkfhggtggujgesthdtredttddtvden ucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimh eqnecuggftrfgrthhtvghrnhepleeuffeggeehvedtteekiefftddtheetkeeutdekgeei tddvhfeuveduleekgefhnecuffhomhgrihhnpehlohhoshgvqdhosghjvggtthhsrdgruh htohenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehp shesphhkshdrihhmpdhnsggprhgtphhtthhopeeipdhmohguvgepshhmthhpohhuthdprh gtphhtthhopehphhhilhhlihhprdifohhougesughunhgvlhhmrdhorhhgrdhukhdprhgt phhtthhopehjrghmvghssehjrghmvghslhhiuhdrihhopdhrtghpthhtohepshhtohhlvg gvsehgmhgrihhlrdgtohhmpdhrtghpthhtohepghhithesvhhgvghrrdhkvghrnhgvlhdr ohhrghdprhgtphhtthhopehgihhtshhtvghrsehpohgsohigrdgtohhmpdhrtghpthhtoh epphhhihhllhhiphdrfihoohguuddvfeesghhmrghilhdrtghomh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 16 Aug 2024 06:45:16 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 223a9146 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 16 Aug 2024 10:44:53 +0000 (UTC) Date: Fri, 16 Aug 2024 12:45:15 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Phillip Wood , phillip.wood@dunelm.org.uk, James Liu , Derrick Stolee , Junio C Hamano Subject: [PATCH v3 6/7] builtin/maintenance: add a `--detach` flag Message-ID: <347d0a200201ce215f5b2c46d23de0cdd0181956.1723804990.git.ps@pks.im> References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Same as the preceding commit, add a `--[no-]detach` flag to the git-maintenance(1) command. This will be used in a subsequent commit to fix backgrounding of that command when configured with a non-standard set of tasks. Signed-off-by: Patrick Steinhardt --- builtin/gc.c | 6 ++++++ t/t7900-maintenance.sh | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/builtin/gc.c b/builtin/gc.c index 269a77960f..63106e2028 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1426,6 +1426,10 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts, } free(lock_path); + /* Failure to daemonize is ok, we'll continue in foreground. */ + if (opts->detach > 0) + daemonize(); + for (i = 0; !found_selected && i < TASK__COUNT; i++) found_selected = tasks[i].selected_order >= 0; @@ -1552,6 +1556,8 @@ static int maintenance_run(int argc, const char **argv, const char *prefix) struct option builtin_maintenance_run_options[] = { OPT_BOOL(0, "auto", &opts.auto_flag, N_("run tasks based on the state of the repository")), + OPT_BOOL(0, "detach", &opts.detach, + N_("perform maintenance in the background")), OPT_CALLBACK(0, "schedule", &opts.schedule, N_("frequency"), N_("run tasks based on frequency"), maintenance_opt_schedule), diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index 8595489ceb..771525aa4b 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -908,4 +908,43 @@ test_expect_success 'failed schedule prevents config change' ' done ' +test_expect_success '--no-detach causes maintenance to not run in background' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + + # Prepare the repository such that git-maintenance(1) ends up + # outputting something. + test_commit something && + git config set maintenance.gc.enabled false && + git config set maintenance.loose-objects.enabled true && + git config set maintenance.loose-objects.auto 1 && + git config set maintenance.incremental-repack.enabled true && + + # We have no better way to check whether or not the task ran in + # the background than to verify whether it output anything. The + # next testcase checks the reverse, making this somewhat safer. + git maintenance run --no-detach >out 2>&1 && + test_line_count = 1 out + ) +' + +test_expect_success '--detach causes maintenance to run in background' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + + test_commit something && + git config set maintenance.gc.enabled false && + git config set maintenance.loose-objects.enabled true && + git config set maintenance.loose-objects.auto 1 && + git config set maintenance.incremental-repack.enabled true && + + git maintenance run --detach >out 2>&1 && + test_must_be_empty out + ) +' + test_done From patchwork Fri Aug 16 10:45:17 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13765900 Received: from fout8-smtp.messagingengine.com (fout8-smtp.messagingengine.com [103.168.172.151]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6065E19CCFA for ; Fri, 16 Aug 2024 10:45:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.151 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723805123; cv=none; b=ChvhTz84AjQum/FbbNr/KSqekc9HxTV4IvR/97lIr6BEaU6SnSCeNzhdFSegWg4nRTGEQA4WFEf2OF3G40jNDHkWdGAk93ROheYUjqpqvuCqwUew5MT4G/s6jo7MC8AlDcFdV3q+5MmzjHAwmDwgZ058xzwFTh0e4oeJO//YvQ4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723805123; c=relaxed/simple; bh=dbrmvS4VnmROPhPGk/ck6RhA+qBaahr3/hKeix3JC/g=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=C3pCv9y9kLt9NyAmCncV3Ecf5EnZGoF2ICJdbWqfITc/o4cL4WRXWecRmaoNcn+Gi3ueYKeQtkU240pZ3/kKkGWTBjIhePjYZfmCR4IM9TKyZbgnCI1qJw41SQsa/mADYsjTPP+AS/pw6lwfR5LfcMlojpfGDBVi/rZuQ8P3QAQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=NcfM3Opw; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=hSuX3CBU; arc=none smtp.client-ip=103.168.172.151 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="NcfM3Opw"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="hSuX3CBU" Received: from phl-compute-05.internal (phl-compute-05.nyi.internal [10.202.2.45]) by mailfout.nyi.internal (Postfix) with ESMTP id 997DE138FFA0; Fri, 16 Aug 2024 06:45:20 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-05.internal (MEProxy); Fri, 16 Aug 2024 06:45:20 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm3; t=1723805120; x=1723891520; bh=/3dMGy5TVU C7xDyrk7D0BGzHLBdOpms3x2V9CUbjhv4=; b=NcfM3OpwdP7KyJFh+YLSrbMskL b03zsC9SHxFQ8f4pLqTnAekVrSynhKWEt9zzN5eEbeYFe7soV3PaQVfjfEQO4Ezr fRvrhYQ4RZT290lagSo5oT4KI4pWJ+9OxN1QIjqUFfWsbbI/xK7qiBGcX3snFaJd hV0WPun7gQrvGTcOchDYMucmzMLx5CXFpAGcPRTnONktTBewary6EoM95+EPV6BF MLDM4JM+ilGsZ8ufnqgSx2RFjFb0O67uW3HJ5nKIJSuw7kbPb0uTzsXmJR4f/Wcd sAasgIErulX9JqL6Yb2fOzL+dkZciGv4rHTwcKqijAqhkFqo2Db3+BmWe1wQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1723805120; x=1723891520; bh=/3dMGy5TVUC7xDyrk7D0BGzHLBdO pms3x2V9CUbjhv4=; b=hSuX3CBUZjnG5gwa7+S7+4nkPoK7bpBHZE9ecOH0bwx7 Dbl6PQhNZMnyvwtTpfFk3ot+abGx4lJ0VW5jARd/VUxXezGBY0hWL2Gpeex3Cu6l E9HXAmQYnJvsgJp0EsI2FnPPYVKxsM3W4tT2IEHAm3Mm8Yh2KP3JCmlLJon0pz53 zB+FUI/My2GSIiGNz/LHf6kfcnAelYGeFrbO2bGWisivIeWJLX5H0bnMU24VUDB8 d2q60S0MBNQkz7CCq6k1acrSLpJL+7seOvO6n/71dlkZXFGyvShhERKzQPHDA2bn Sm3BdWxKfrWRvOvn+2Bso+iFSGG4562EfVgHSX0aQQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddruddtkedgfeduucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhepfffhvfevuffkfhggtggujgesthdtredttddtvden ucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimh eqnecuggftrfgrthhtvghrnhepudeviefffeekjeeghfdtkefgteegveevjeeiffeukedv gedukedvvddtveffieefnecuffhomhgrihhnpehinhgtrhgvmhgvnhhtrghlqdhrvghprg gtkhdrrghuthhonecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhf rhhomhepphhssehpkhhsrdhimhdpnhgspghrtghpthhtohepiedpmhhouggvpehsmhhtph houhhtpdhrtghpthhtohepghhithesvhhgvghrrdhkvghrnhgvlhdrohhrghdprhgtphht thhopehjrghmvghssehjrghmvghslhhiuhdrihhopdhrtghpthhtohepghhithhsthgvrh esphhosghogidrtghomhdprhgtphhtthhopehphhhilhhlihhprdifohhougesughunhgv lhhmrdhorhhgrdhukhdprhgtphhtthhopehphhhilhhlihhprdifohhougduvdefsehgmh grihhlrdgtohhmpdhrtghpthhtohepshhtohhlvggvsehgmhgrihhlrdgtohhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 16 Aug 2024 06:45:19 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 20c9e390 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 16 Aug 2024 10:44:56 +0000 (UTC) Date: Fri, 16 Aug 2024 12:45:17 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Phillip Wood , phillip.wood@dunelm.org.uk, James Liu , Derrick Stolee , Junio C Hamano Subject: [PATCH v3 7/7] run-command: fix detaching when running auto maintenance Message-ID: <9befef7c1f7520d58af2b2db17174b8dbc493d56.1723804990.git.ps@pks.im> References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: In the past, we used to execute `git gc --auto` as part of our automatic housekeeping routines. As git-gc(1) may require quite some time to perform the housekeeping, it knows to detach itself and run in the background so that the user can continue their work. Eventually, we refactored our automatic housekeeping to instead use the more flexible git-maintenance(1) command. The upside of this new infra is that the user can configure which maintenance tasks are performed, at least to a certain degree. So while it continues to run git-gc(1) by default, it can also be adapted to e.g. use git-multi-pack-index(1) for maintenance of the object database. The auto-detach of the new infra is somewhat broken though once the user configures non-standard tasks. The problem is essentially that we detach at the wrong level in the process hierarchy: git-maintenance(1) never detaches itself, but instead it continues to be git-gc(1) which does. When configured to only run the git-gc(1) maintenance task, then the result is basically the same as before. But when configured to run other tasks, then git-maintenance(1) will wait for these to run to completion. Even worse, it may be that git-gc(1) runs concurrently with other housekeeping tasks, stomping on each others feet. Fix this bug by asking git-gc(1) to not detach when it is being invoked via git-maintenance(1). Instead, git-maintenance(1) now respects a new config "maintenance.autoDetach", the equivalent of "gc.autoDetach", and detaches itself into the background when running as part of our auto maintenance. This should continue to behave the same for all users which use the git-gc(1) task, only. For others though, it means that we now properly perform all tasks in the background. The default behaviour of git-maintenance(1) when executed by the user does not change, it will remain in the foreground unless they pass the `--detach` option. Signed-off-by: Patrick Steinhardt --- Documentation/config/gc.txt | 3 +- Documentation/config/maintenance.txt | 11 +++++++ builtin/gc.c | 1 + run-command.c | 12 +++++++- t/t5616-partial-clone.sh | 6 ++-- t/t7900-maintenance.sh | 43 ++++++++++++++++++++++------ 6 files changed, 62 insertions(+), 14 deletions(-) diff --git a/Documentation/config/gc.txt b/Documentation/config/gc.txt index 664a3c2874..1d4f9470ea 100644 --- a/Documentation/config/gc.txt +++ b/Documentation/config/gc.txt @@ -40,7 +40,8 @@ use, it'll affect how the auto pack limit works. gc.autoDetach:: Make `git gc --auto` return immediately and run in the background - if the system supports it. Default is true. + if the system supports it. Default is true. This config variable acts + as a fallback in case `maintenance.autoDetach` is not set. gc.bigPackThreshold:: If non-zero, all non-cruft packs larger than this limit are kept diff --git a/Documentation/config/maintenance.txt b/Documentation/config/maintenance.txt index 69a4f05153..72a9d6cf81 100644 --- a/Documentation/config/maintenance.txt +++ b/Documentation/config/maintenance.txt @@ -3,6 +3,17 @@ maintenance.auto:: `git maintenance run --auto` after doing their normal work. Defaults to true. +maintenance.autoDetach:: + Many Git commands trigger automatic maintenance after they have + written data into the repository. This boolean config option + controls whether this automatic maintenance shall happen in the + foreground or whether the maintenance process shall detach and + continue to run in the background. ++ +If unset, the value of `gc.autoDetach` is used as a fallback. Defaults +to true if both are unset, meaning that the maintenance process will +detach. + maintenance.strategy:: This string config option provides a way to specify one of a few recommended schedules for background maintenance. This only affects diff --git a/builtin/gc.c b/builtin/gc.c index 63106e2028..bafee330a2 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1063,6 +1063,7 @@ static int maintenance_task_gc(struct maintenance_run_opts *opts, strvec_push(&child.args, "--quiet"); else strvec_push(&child.args, "--no-quiet"); + strvec_push(&child.args, "--no-detach"); return run_command(&child); } diff --git a/run-command.c b/run-command.c index 45ba544932..94f2f3079f 100644 --- a/run-command.c +++ b/run-command.c @@ -1808,16 +1808,26 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts) int prepare_auto_maintenance(int quiet, struct child_process *maint) { - int enabled; + int enabled, auto_detach; if (!git_config_get_bool("maintenance.auto", &enabled) && !enabled) return 0; + /* + * When `maintenance.autoDetach` isn't set, then we fall back to + * honoring `gc.autoDetach`. This is somewhat weird, but required to + * retain behaviour from when we used to run git-gc(1) here. + */ + if (git_config_get_bool("maintenance.autodetach", &auto_detach) && + git_config_get_bool("gc.autodetach", &auto_detach)) + auto_detach = 1; + maint->git_cmd = 1; maint->close_object_store = 1; strvec_pushl(&maint->args, "maintenance", "run", "--auto", NULL); strvec_push(&maint->args, quiet ? "--quiet" : "--no-quiet"); + strvec_push(&maint->args, auto_detach ? "--detach" : "--no-detach"); return 1; } diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh index 2da7291e37..8415884754 100755 --- a/t/t5616-partial-clone.sh +++ b/t/t5616-partial-clone.sh @@ -229,7 +229,7 @@ test_expect_success 'fetch --refetch triggers repacking' ' GIT_TRACE2_EVENT="$PWD/trace1.event" \ git -C pc1 fetch --refetch origin && - test_subcommand git maintenance run --auto --no-quiet /dev/null && GIT_TRACE2_EVENT="$(pwd)/run-no-quiet.txt" \ git maintenance run --no-quiet 2>/dev/null && - test_subcommand git gc --quiet ' ' git maintenance run --task=commit-graph 2>/dev/null && GIT_TRACE2_EVENT="$(pwd)/run-both.txt" \ git maintenance run --task=commit-graph --task=gc 2>/dev/null && - test_subcommand ! git gc --quiet