Skip to content

Commit

Permalink
maintenance: update schedule before config
Browse files Browse the repository at this point in the history
When running 'git maintenance start', the current pattern is to
configure global config settings to enable maintenance on the current
repository and set 'maintenance.auto' to false and _then_ to set up the
schedule with the system scheduler.

This has a problematic error condition: if the scheduler fails to
initialize, the repository still will not use automatic maintenance due
to the 'maintenance.auto' setting.

Fix this gap by swapping the order of operations. If Git fails to
initialize maintenance, then the config changes should never happen.

Reported-by: Phillip Wood <phillip.wood123@gmail.com>
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
  • Loading branch information
derrickstolee committed Aug 9, 2023
1 parent fae78b9 commit c09bde0
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 1 deletion.
5 changes: 4 additions & 1 deletion builtin/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -2728,9 +2728,12 @@ static int maintenance_start(int argc, const char **argv, const char *prefix)
opts.scheduler = resolve_scheduler(opts.scheduler);
validate_scheduler(opts.scheduler);

if (update_background_schedule(&opts, 1))
die(_("failed to set up maintenance schedule"));

if (maintenance_register(ARRAY_SIZE(register_args)-1, register_args, NULL))
warning(_("failed to add repo to global config"));
return update_background_schedule(&opts, 1);
return 0;
}

static const char *const builtin_maintenance_stop_usage[] = {
Expand Down
11 changes: 11 additions & 0 deletions t/t7900-maintenance.sh
Original file line number Diff line number Diff line change
Expand Up @@ -843,4 +843,15 @@ test_expect_success 'register and unregister bare repo' '
)
'

test_expect_success 'failed schedule prevents config change' '
git init --bare failcase &&
for scheduler in crontab launchctl schtasks systemctl
do
GIT_TEST_MAINT_SCHEDULER="$scheduler:false" test_must_fail \
git -C failcase maintenance start &&
test_must_fail git -C failcase config maintenance.auto || return 1
done
'

test_done

0 comments on commit c09bde0

Please sign in to comment.