@matheusbsbs / matheus-bagliano / commits / 50a044f1e4

gc: replace config subprocesses with API calls

The 'git maintenance [un]register' commands set or unset the multi- valued maintenance.repo config key with the absolute path of the current repository. These are set in the global config file. Instead of calling a subcommand and creating a new process, create the proper API calls to git_config_set_multivar_in_file_gently(). It requires loading the filename for the global config file (and erroring out if now $HOME value is set). We also need to be careful about using CONFIG_REGEX_NONE when adding the value and using CONFIG_FLAGS_FIXED_VALUE when removing the value. In both cases, we check that the value already exists (this check already existed for 'unregister'). Also, remove the transparent translation of the error code from the config API to the exit code of 'git maintenance'. Instead, use die() to recover from failures at that level. In the case of 'unregister --force', allow the CONFIG_NOTHING_SET error code to be a success. This allows a possible race where another process removes the config value. The end result is that the config value is not set anymore, so we can treat this as a success. Reported-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Derrick Stolee committed Sep 27, 2022 at 13:57 UTC 50a044f1e407ef217cb5d057af08e9e87daddc78
1 file changed +44 -31
builtin/gc.c
+44 -31
@@ -1470,11 +1470,12 @@ static int maintenance_register(int argc, const char **argv, const char *prefix)
1470 struct option options[] = {
1471 OPT_END(),
1472 };
1473 - int rc;
1473 + int found = 0;
1474 + const char *key = "maintenance.repo";
1475 char *config_value;
1475 - struct child_process config_set = CHILD_PROCESS_INIT;
1476 - struct child_process config_get = CHILD_PROCESS_INIT;
1476 char *maintpath = get_maintpath();
1477 + struct string_list_item *item;
1478 + const struct string_list *list;
1479
1480 argc = parse_options(argc, argv, prefix, options,
1481 builtin_maintenance_register_usage, 0);
@@ -1491,31 +1492,35 @@ static int maintenance_register(int argc, const char **argv, const char *prefix)
1492 else
1493 git_config_set("maintenance.strategy", "incremental");
1494
1494 - config_get.git_cmd = 1;
1495 - strvec_pushl(&config_get.args, "config", "--global", "--get",
1496 - "--fixed-value", "maintenance.repo", maintpath, NULL);
1497 - config_get.out = -1;
1498 -
1499 - if (start_command(&config_get)) {
1500 - rc = error(_("failed to run 'git config'"));
1501 - goto done;
1495 + list = git_config_get_value_multi(key);
1496 + if (list) {
1497 + for_each_string_list_item(item, list) {
1498 + if (!strcmp(maintpath, item->string)) {
1499 + found = 1;
1500 + break;
1501 + }
1502 + }
1503 }
1504
1504 - /* We already have this value in our config! */
1505 - if (!finish_command(&config_get)) {
1506 - rc = 0;
1507 - goto done;
1505 + if (!found) {
1506 + int rc;
1507 + char *user_config, *xdg_config;
1508 + git_global_config(&user_config, &xdg_config);
1509 + if (!user_config)
1510 + die(_("$HOME not set"));
1511 + rc = git_config_set_multivar_in_file_gently(
1512 + user_config, "maintenance.repo", maintpath,
1513 + CONFIG_REGEX_NONE, 0);
1514 + free(user_config);
1515 + free(xdg_config);
1516 +
1517 + if (rc)
1518 + die(_("unable to add '%s' value of '%s'"),
1519 + key, maintpath);
1520 }
1521
1510 - config_set.git_cmd = 1;
1511 - strvec_pushl(&config_set.args, "config", "--add", "--global", "maintenance.repo",
1512 - maintpath, NULL);
1513 -
1514 - rc = run_command(&config_set);
1515 -
1516 -done:
1522 free(maintpath);
1518 - return rc;
1523 + return 0;
1524 }
1525
1526 static char const * const builtin_maintenance_unregister_usage[] = {
@@ -1533,8 +1538,6 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi
1538 OPT_END(),
1539 };
1540 const char *key = "maintenance.repo";
1536 - int rc = 0;
1537 - struct child_process config_unset = CHILD_PROCESS_INIT;
1541 char *maintpath = get_maintpath();
1542 int found = 0;
1543 struct string_list_item *item;
@@ -1557,17 +1560,27 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi
1560 }
1561
1562 if (found) {
1560 - config_unset.git_cmd = 1;
1561 - strvec_pushl(&config_unset.args, "config", "--global", "--unset",
1562 - "--fixed-value", key, maintpath, NULL);
1563 -
1564 - rc = run_command(&config_unset);
1563 + int rc;
1564 + char *user_config, *xdg_config;
1565 + git_global_config(&user_config, &xdg_config);
1566 + if (!user_config)
1567 + die(_("$HOME not set"));
1568 + rc = git_config_set_multivar_in_file_gently(
1569 + user_config, key, NULL, maintpath,
1570 + CONFIG_FLAGS_MULTI_REPLACE | CONFIG_FLAGS_FIXED_VALUE);
1571 + free(user_config);
1572 + free(xdg_config);
1573 +
1574 + if (rc &&
1575 + (!force || rc == CONFIG_NOTHING_SET))
1576 + die(_("unable to unset '%s' value of '%s'"),
1577 + key, maintpath);
1578 } else if (!force) {
1579 die(_("repository '%s' is not registered"), maintpath);
1580 }
1581
1582 free(maintpath);
1570 - return rc;
1583 + return 0;
1584 }
1585
1586 static const char *get_frequency(enum schedule_priority schedule)