gitgitgadget / git

GitGitGadget's Git fork. Open Pull Requests here to submit them to the Git mailing list
https://gitgitgadget.github.io/
Other
208 stars 135 forks source link

Use a "best effort" strategy in scheduled maintenance #1719

Closed dscho closed 4 months ago

dscho commented 4 months ago

Over in https://github.com/microsoft/git/issues/623, it was pointed out that scheduled maintenance will error out when it encounters a missing repository. The scheduled maintenance should exit with an error, all right, but what about the remaining repositories for which maintenance was scheduled, and that may not be missing?

This patch series addresses this by introducing a new for-each-repo option and then using it in the command that is run via scheduled maintenance.

Changes since v2 (thanks Patrick, Jeff and Junio):

Changes since v1 (thanks Eric!):

cc: Eric Sunshine sunshine@sunshineco.com cc: Patrick Steinhardt ps@pks.im cc: Jeff King peff@peff.net

dscho commented 4 months ago

/submit

gitgitgadget[bot] commented 4 months ago

Submitted as pull.1719.git.1713342535.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1719/dscho/for-each-repo-stop-on-error-2.44-v1

To fetch this version to local tag pr-1719/dscho/for-each-repo-stop-on-error-2.44-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1719/dscho/for-each-repo-stop-on-error-2.44-v1
gitgitgadget[bot] commented 4 months ago

User Eric Sunshine <sunshine@sunshineco.com> has been added to the cc: list.

gitgitgadget[bot] commented 4 months ago

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> Over in https://github.com/microsoft/git/issues/623, it was pointed out that
> scheduled maintenance will error out when it encounters a missing
> repository. The scheduled maintenance should exit with an error, all right,
> but what about the remaining repositories for which maintenance was
> scheduled, and that may not be missing?

Interesting.

As the order of the maintenance run across repositories is not
controlled by the end-user, I think it is reasonable to keep going
and finish the others, at least by default (if you could control the
ordering, you could make an arrangement where having ran maintenance
on repository A successfully would be necessary to do maintenance on
repository B, but we do not give that power to the end users).

> This patch series addresses this by introducing a new for-each-repo option
> and then using it in the command that is run via scheduled maintenance.

OK.
gitgitgadget[bot] commented 4 months ago

On the Git mailing list, Junio C Hamano wrote (reply to this), regarding a86bcf2e1a0aadaaaeb5bfe5b0a4a0fa12594921 (outdated):

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> In https://github.com/microsoft/git/issues/623, it was reported that
> maintenance stops on a missing repository, omitting the remaining
> repositories that were scheduled for maintenance.
>
> This is undesirable, as it should be a best effort type of operation.
>
> It should still fail due to the missing repository, of course, but not
> leave the non-missing repositories in unmaintained shapes.
>
> Let's use `for-each-repo`'s shiny new `--keep-going` option that we just
> introduced for that very purpose.
>
> This change will be picked up when running `git maintenance start`,
> which is run implicitly by `scalar reconfigure`.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  builtin/gc.c           | 7 ++++---
>  t/t7900-maintenance.sh | 6 +++---
>  2 files changed, 7 insertions(+), 6 deletions(-)

Other than the N_("") thing Eric noticed, I didn't find anything
glaringly wrong in these two patches.  Nicely done.

We may want to fold overly long lines we see in the patch, but I'd
prefer to see it done as a post-cleanup task (#leftoverbits), as the
lines in the preimage of the patch are already overly long.

Thanks.
gitgitgadget[bot] commented 4 months ago

This branch is now known as js/for-each-repo-keep-going.

gitgitgadget[bot] commented 4 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/a402d1374373eef1e662ce606a1e84b21cc689ed.

gitgitgadget[bot] commented 4 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/b07c629c1cf47b20ff85e813a9e5cb8589c35128.

gitgitgadget[bot] commented 4 months ago

There was a status update in the "New Topics" section about the branch js/for-each-repo-keep-going on the Git mailing list:

A scheduled "git maintenance" job is expected to work on all
repositories it knows about, but it stopped at the first one that
errored out.  Now it keeps going.

Expecting a (hopefully minor and final) reroll.
cf. <CAPig+cSjoGe7Eeynz=jGSaNYWXQ-VkvWv7mv1NDeCXPFEtdqOA@mail.gmail.com>
source: <pull.1719.git.1713342535.gitgitgadget@gmail.com>
dscho commented 4 months ago

/submit

gitgitgadget[bot] commented 4 months ago

Submitted as pull.1719.v2.git.1713444783.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1719/dscho/for-each-repo-stop-on-error-2.44-v2

To fetch this version to local tag pr-1719/dscho/for-each-repo-stop-on-error-2.44-v2:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1719/dscho/for-each-repo-stop-on-error-2.44-v2
gitgitgadget[bot] commented 4 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/f4daeda576e6295c47f16db0b78fd7b3dae0a56b.

gitgitgadget[bot] commented 4 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/344516ae4365e92d0b8fcdb88c232b0abf14451a.

gitgitgadget[bot] commented 4 months ago

User Patrick Steinhardt <ps@pks.im> has been added to the cc: list.

gitgitgadget[bot] commented 4 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/428855c312caab8e048d66fb2f734b0e1278b057.

gitgitgadget[bot] commented 4 months ago

There was a status update in the "Cooking" section about the branch js/for-each-repo-keep-going on the Git mailing list:

A scheduled "git maintenance" job is expected to work on all
repositories it knows about, but it stopped at the first one that
errored out.  Now it keeps going.

Will merge to 'next'?
source: <pull.1719.v2.git.1713444783.gitgitgadget@gmail.com>
gitgitgadget[bot] commented 4 months ago

User Jeff King <peff@peff.net> has been added to the cc: list.

gitgitgadget[bot] commented 4 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/f04847b80e2f79f04159665a91dee373fa2804a8.

gitgitgadget[bot] commented 4 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/4f0b57dbafc947c06237ec8258ec783ad041509c.

gitgitgadget[bot] commented 4 months ago

There was a status update in the "Cooking" section about the branch js/for-each-repo-keep-going on the Git mailing list:

A scheduled "git maintenance" job is expected to work on all
repositories it knows about, but it stopped at the first one that
errored out.  Now it keeps going.

Expecting a hopefully small and final reroll.
Can change exit condition, which needs fixing.
cf. <20240419175621.GB14309@coredump.intra.peff.net>
source: <pull.1719.v2.git.1713444783.gitgitgadget@gmail.com>
gitgitgadget[bot] commented 4 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/9e92166a169bf4697d0828ecce09cb6c57e0c175.

dscho commented 4 months ago

/submit

gitgitgadget[bot] commented 4 months ago

Submitted as pull.1719.v3.git.1713975299.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1719/dscho/for-each-repo-stop-on-error-2.44-v3

To fetch this version to local tag pr-1719/dscho/for-each-repo-stop-on-error-2.44-v3:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1719/dscho/for-each-repo-stop-on-error-2.44-v3
gitgitgadget[bot] commented 4 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/449488ec8d5b6ed1641dd4b9ffe202eb2c9fcbd7.

gitgitgadget[bot] commented 4 months ago

There was a status update in the "Cooking" section about the branch js/for-each-repo-keep-going on the Git mailing list:

A scheduled "git maintenance" job is expected to work on all
repositories it knows about, but it stopped at the first one that
errored out.  Now it keeps going.

Expecting a hopefully small and final reroll.
Can change exit condition, which needs fixing.
cf. <20240419175621.GB14309@coredump.intra.peff.net>
source: <pull.1719.v2.git.1713444783.gitgitgadget@gmail.com>
gitgitgadget[bot] commented 4 months ago

On the Git mailing list, Patrick Steinhardt wrote (reply to this):

On Wed, Apr 24, 2024 at 04:14:57PM +0000, Johannes Schindelin via GitGitGadget wrote:
> Over in https://github.com/microsoft/git/issues/623, it was pointed out that
> scheduled maintenance will error out when it encounters a missing
> repository. The scheduled maintenance should exit with an error, all right,
> but what about the remaining repositories for which maintenance was
> scheduled, and that may not be missing?
> 
> This patch series addresses this by introducing a new for-each-repo option
> and then using it in the command that is run via scheduled maintenance.
> 
> Changes since v2 (thanks Patrick, Jeff and Junio):
> 
>  * When not passing the new --keep-going option, the exit code is the same
>    as before.
>  * Clarified in the documentation of the --keep-going option that it is 0
>    for success, 1 for failure, no matter the exact exit code of the failing
>    command invocation(s).
> 
> Changes since v1 (thanks Eric!):
> 
>  * Changed the option's documentation to reflect the current state (instead
>    of the original design)
>  * Fixed grammar issues

Thanks, this version looks good to me!

Patrick
gitgitgadget[bot] commented 4 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/218c997f02b3d5fff343d268d629ad8a40db1904.

gitgitgadget[bot] commented 4 months ago

This patch series was integrated into next via https://github.com/git/git/commit/d33253d919a7e789b5de507c5763e3fcdcb376e7.

gitgitgadget[bot] commented 4 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/86e44abfde3d6a87c09d968a685221ba8792e5f7.

gitgitgadget[bot] commented 4 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/36af2aec03ce7a4ce45f82ff73da48eb031ce6ab.

gitgitgadget[bot] commented 4 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/64c18164c83c6730b66d9da022754de0e19a910f.

gitgitgadget[bot] commented 4 months ago

There was a status update in the "Cooking" section about the branch js/for-each-repo-keep-going on the Git mailing list:

A scheduled "git maintenance" job is expected to work on all
repositories it knows about, but it stopped at the first one that
errored out.  Now it keeps going.

Will cook in 'next'.
source: <pull.1719.v3.git.1713975299.gitgitgadget@gmail.com>
gitgitgadget[bot] commented 4 months ago

This patch series was integrated into next via https://github.com/git/git/commit/75b182d34ed1ed9aad81dca32430e2d4a5aa49eb.

gitgitgadget[bot] commented 4 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/75b182d34ed1ed9aad81dca32430e2d4a5aa49eb.

gitgitgadget[bot] commented 4 months ago

This patch series was integrated into master via https://github.com/git/git/commit/75b182d34ed1ed9aad81dca32430e2d4a5aa49eb.

gitgitgadget[bot] commented 4 months ago

Closed via 75b182d34ed1ed9aad81dca32430e2d4a5aa49eb.