From patchwork Tue Nov 24 04:16:42 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Derrick Stolee X-Patchwork-Id: 11927041 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 04182C64E69 for ; Tue, 24 Nov 2020 04:16:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 812EB2085B for ; Tue, 24 Nov 2020 04:16:51 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="qlGaOHEd" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728835AbgKXEQu (ORCPT ); Mon, 23 Nov 2020 23:16:50 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:32898 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727988AbgKXEQu (ORCPT ); Mon, 23 Nov 2020 23:16:50 -0500 Received: from mail-wm1-x341.google.com (mail-wm1-x341.google.com [IPv6:2a00:1450:4864:20::341]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 24CD1C0613CF for ; Mon, 23 Nov 2020 20:16:50 -0800 (PST) Received: by mail-wm1-x341.google.com with SMTP id a186so1241701wme.1 for ; Mon, 23 Nov 2020 20:16:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=GeHMVAQUD8d6JoZr3OOSyoRumStUgEOOmfk4MjdgbTM=; b=qlGaOHEdzorrh5EdNGZjjg9PGcJwFZVgzsjid1jg+oiuGMN8Mp0qmg/NW3iU6z3QEC ugNwZai9iziSJJtU6L8sbBkcKXFQdEs2Cm8D4GXeCEnO5YAFg7bQfPrQOzrWr6Eli3KD 2ueuW5DDeT9Z+z586tkY+cpVeqcgYQvSo6VNrFuQ/5PizgWAQsZ0HZROVh7BjlSxGLdf H0AuQCBxQVJV+RIm9GzFO8iMDh0i+Ye/r7E+vjUtRaQOYyVxQNsQsmVU+litSQThrRzH KPruaXB4jogU8jDQwkergU5bWPeM8um2o1SCWqKa0ctAdZ0MfMJEdHtad1xJwHsO93Ib UV5g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=GeHMVAQUD8d6JoZr3OOSyoRumStUgEOOmfk4MjdgbTM=; b=cwjtjHtRCWfmgnHrbqHp6Z/Jy3E4RH/RmReiLA134TD3/Mlsq9tT6MsfYMm0ipWraa jpwZEfzZCTJUDDvHYPmbZAuN9bHK+HAMLJmj19T2R9sGOyGzYSmKglqMB+jdlveTuINH ETVryJ4HhGZzviqFxNOoEYbCxsFdRaXUsAefan+o73ScKrlsuVcVdLSVUsOM106fWCjF MEyeiNDa7CtSakAjYj5z1EiqG5k0diPVXBlimsgXTkG4+GnZIdPGBK8+b+Vt9uH02Zq2 iShpVvr+H8xBstJlnZMUJ+U6cewOVpqQjh9UWCzpbpxWMxF3Q0yHf669SgBC3aJmkyPL ASHQ== X-Gm-Message-State: AOAM533KZUA67oajGEhDk0o2YUok83A3xiJjKM8mBDz/LGuE0vGOALsR xlQja5ffct1fOFf50/n10KUbLKNviPE= X-Google-Smtp-Source: ABdhPJzBIH0V258lkvUIpIAhvO7j2fnQ/tuZX4aZbPCJzYGtZz1d2BnoTzUhPoF4hYJtuaM0JKyoXg== X-Received: by 2002:a1c:44f:: with SMTP id 76mr2037997wme.181.1606191408600; Mon, 23 Nov 2020 20:16:48 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id b4sm2925901wmc.1.2020.11.23.20.16.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 23 Nov 2020 20:16:47 -0800 (PST) Message-Id: <4807342b0019be29bb369ed3403a485f0ce9c15d.1606191405.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Tue, 24 Nov 2020 04:16:42 +0000 Subject: [PATCH v5 1/4] maintenance: extract platform-specific scheduling Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: jrnieder@gmail.com, jonathantanmy@google.com, sluongng@gmail.com, =?utf-8?b?xJBvw6BuIFRy4bqnbiBDw7RuZw==?= Danh , Martin =?utf-8?b?w4VncmVu?= , Eric Sunshine , Derrick Stolee , Derrick Stolee , Derrick Stolee Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Derrick Stolee From: Derrick Stolee The existing schedule mechanism using 'cron' is supported by POSIX platforms, but not Windows. It also works slightly differently on macOS to significant detriment of the user experience. To allow for new implementations on these platforms, extract a method that performs the platform-specific scheduling mechanism. This will be swapped at compile time with new implementations on specialized platforms. As we add this generality, rename GIT_TEST_CRONTAB to GIT_TEST_MAINT_SCHEDULER. Further, this variable is now parsed as ":" so we can test platform-specific scheduling logic even when not on the correct platform. By specifying the in this string, we will be able to test all three sets of Git logic from a Linux machine. Co-authored-by: Eric Sunshine Signed-off-by: Eric Sunshine Signed-off-by: Derrick Stolee --- builtin/gc.c | 70 ++++++++++++++++++++++++++---------------- t/t7900-maintenance.sh | 8 ++--- t/test-lib.sh | 7 +++-- 3 files changed, 51 insertions(+), 34 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index e3098ef6a1..18ae7f7138 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1494,35 +1494,23 @@ static int maintenance_unregister(void) #define BEGIN_LINE "# BEGIN GIT MAINTENANCE SCHEDULE" #define END_LINE "# END GIT MAINTENANCE SCHEDULE" -static int update_background_schedule(int run_maintenance) +static int crontab_update_schedule(int run_maintenance, int fd, const char *cmd) { int result = 0; int in_old_region = 0; struct child_process crontab_list = CHILD_PROCESS_INIT; struct child_process crontab_edit = CHILD_PROCESS_INIT; FILE *cron_list, *cron_in; - const char *crontab_name; struct strbuf line = STRBUF_INIT; - struct lock_file lk; - char *lock_path = xstrfmt("%s/schedule", the_repository->objects->odb->path); - if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF) < 0) - return error(_("another process is scheduling background maintenance")); - - crontab_name = getenv("GIT_TEST_CRONTAB"); - if (!crontab_name) - crontab_name = "crontab"; - - strvec_split(&crontab_list.args, crontab_name); + strvec_split(&crontab_list.args, cmd); strvec_push(&crontab_list.args, "-l"); crontab_list.in = -1; - crontab_list.out = dup(lk.tempfile->fd); + crontab_list.out = dup(fd); crontab_list.git_cmd = 0; - if (start_command(&crontab_list)) { - result = error(_("failed to run 'crontab -l'; your system might not support 'cron'")); - goto cleanup; - } + if (start_command(&crontab_list)) + return error(_("failed to run 'crontab -l'; your system might not support 'cron'")); /* Ignore exit code, as an empty crontab will return error. */ finish_command(&crontab_list); @@ -1531,17 +1519,15 @@ static int update_background_schedule(int run_maintenance) * Read from the .lock file, filtering out the old * schedule while appending the new schedule. */ - cron_list = fdopen(lk.tempfile->fd, "r"); + cron_list = fdopen(fd, "r"); rewind(cron_list); - strvec_split(&crontab_edit.args, crontab_name); + strvec_split(&crontab_edit.args, cmd); crontab_edit.in = -1; crontab_edit.git_cmd = 0; - if (start_command(&crontab_edit)) { - result = error(_("failed to run 'crontab'; your system might not support 'cron'")); - goto cleanup; - } + if (start_command(&crontab_edit)) + return error(_("failed to run 'crontab'; your system might not support 'cron'")); cron_in = fdopen(crontab_edit.in, "w"); if (!cron_in) { @@ -1586,14 +1572,44 @@ static int update_background_schedule(int run_maintenance) close(crontab_edit.in); done_editing: - if (finish_command(&crontab_edit)) { + if (finish_command(&crontab_edit)) result = error(_("'crontab' died")); - goto cleanup; + else + fclose(cron_list); + return result; +} + +static const char platform_scheduler[] = "crontab"; + +static int update_background_schedule(int enable) +{ + int result; + const char *scheduler = platform_scheduler; + const char *cmd = scheduler; + char *testing; + struct lock_file lk; + char *lock_path = xstrfmt("%s/schedule", the_repository->objects->odb->path); + + testing = xstrdup_or_null(getenv("GIT_TEST_MAINT_SCHEDULER")); + if (testing) { + char *sep = strchr(testing, ':'); + if (!sep) + die("GIT_TEST_MAINT_SCHEDULER unparseable: %s", testing); + *sep = '\0'; + scheduler = testing; + cmd = sep + 1; } - fclose(cron_list); -cleanup: + if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF) < 0) + return error(_("another process is scheduling background maintenance")); + + if (!strcmp(scheduler, "crontab")) + result = crontab_update_schedule(enable, lk.tempfile->fd, cmd); + else + die("unknown background scheduler: %s", scheduler); + rollback_lock_file(&lk); + free(testing); return result; } diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index 20184e96e1..eeb939168d 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -368,7 +368,7 @@ test_expect_success 'register and unregister' ' ' test_expect_success 'start from empty cron table' ' - GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance start && + GIT_TEST_MAINT_SCHEDULER="crontab:test-tool crontab cron.txt" git maintenance start && # start registers the repo git config --get --global maintenance.repo "$(pwd)" && @@ -379,19 +379,19 @@ test_expect_success 'start from empty cron table' ' ' test_expect_success 'stop from existing schedule' ' - GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance stop && + GIT_TEST_MAINT_SCHEDULER="crontab:test-tool crontab cron.txt" git maintenance stop && # stop does not unregister the repo git config --get --global maintenance.repo "$(pwd)" && # Operation is idempotent - GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance stop && + GIT_TEST_MAINT_SCHEDULER="crontab:test-tool crontab cron.txt" git maintenance stop && test_must_be_empty cron.txt ' test_expect_success 'start preserves existing schedule' ' echo "Important information!" >cron.txt && - GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance start && + GIT_TEST_MAINT_SCHEDULER="crontab:test-tool crontab cron.txt" git maintenance start && grep "Important information!" cron.txt ' diff --git a/t/test-lib.sh b/t/test-lib.sh index 4a60d1ed76..ddbeee1f5e 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -1704,7 +1704,8 @@ test_lazy_prereq REBASE_P ' ' # Ensure that no test accidentally triggers a Git command -# that runs 'crontab', affecting a user's cron schedule. -# Tests that verify the cron integration must set this locally +# that runs the actual maintenance scheduler, affecting a user's +# system permanently. +# Tests that verify the scheduler integration must set this locally # to avoid errors. -GIT_TEST_CRONTAB="exit 1" +GIT_TEST_MAINT_SCHEDULER="none:exit 1" From patchwork Tue Nov 24 04:16:43 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Derrick Stolee X-Patchwork-Id: 11927037 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 136F2C64E75 for ; Tue, 24 Nov 2020 04:16:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id AA4442085B for ; Tue, 24 Nov 2020 04:16:53 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="oIp2eyl0" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728840AbgKXEQw (ORCPT ); Mon, 23 Nov 2020 23:16:52 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:32902 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727988AbgKXEQv (ORCPT ); Mon, 23 Nov 2020 23:16:51 -0500 Received: from mail-wr1-x441.google.com (mail-wr1-x441.google.com [IPv6:2a00:1450:4864:20::441]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2D69BC0613CF for ; Mon, 23 Nov 2020 20:16:51 -0800 (PST) Received: by mail-wr1-x441.google.com with SMTP id 23so20866138wrc.8 for ; Mon, 23 Nov 2020 20:16:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=c3ckQj1fIW3U8Ln1uReV5M8x5QIwk6yY6Ii5rpTvUG4=; b=oIp2eyl0hpCXgR5GoFuDoGu4IENbZ48KmkF7lcRvjRd5Ua2hYi7o9jbXHOf+eO9lK6 EDKYrDpwfANUoG3npR39tavIW3oD3qJwiox1y+bpKOCXYiubWWknU5Cuga29m+cabXOv Mp7ROC/Z8+FK/gP85bNNrzF0kzwiLNmwOCgKNkUCLTHAWTGVjgLt3548a3qjCwK4NrX2 i1njBMy7nfzA0JxY5vIBVDK+YuBbAbI4z2U9ULSWg1OXNapfXGPvL1CSwhSqF3+6Elgx 1Bc8buJdw8LmOzqQpB3MfWU/Q4SaBKaAOQhmLFBTnJYWkbdFhuy2hCleTsZT5eHHrasZ vzLQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=c3ckQj1fIW3U8Ln1uReV5M8x5QIwk6yY6Ii5rpTvUG4=; b=puB5WgsaxhiD3fHdwNXH8qtdPvM52XwWcwbMSHrTcvlIU8/doySI+p3nMeLlh0yJ/d NF6w1EVFeoAm6kwtfUNWA2rTF5tE888HqcpXuC57jO43C5jd9a89Hba+mcZCbtmoS8PE 2Q4nadyhZpBJ34A681+StrJTGatv90QR7NjJIB03myg+uzo8jIXGll73cBxWxMyRh/P9 xaUV0Di1V7YpUfCVkQAcKGwH3BtwUjs81pGF29SBM8CqUBsjup5K8henCbblFSaj+jOS U9ZwajbNJ2Q1UHg99H9AERHEkY+kGET03U6mU9cyOUPO1HkcoAfMDb2sMqQQGQbYO2xL 6Kkg== X-Gm-Message-State: AOAM530sCIMoIvq6mnyAYKmYxw1at0MZtJQmUqz+X4bHBPAadhIgN9Nc s4vNx7KetKZ03ktsXkyTfGyqidiRRyM= X-Google-Smtp-Source: ABdhPJx889nW0H7jtNZJEAKWg3NEnLZiRDtPSyZxs9YMYgOtSaGbj1X/SsCWB/WDXztgy8h7r8BMcA== X-Received: by 2002:adf:f241:: with SMTP id b1mr2989968wrp.248.1606191409702; Mon, 23 Nov 2020 20:16:49 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id l3sm4104967wrr.89.2020.11.23.20.16.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 23 Nov 2020 20:16:49 -0800 (PST) Message-Id: <7cc70a8fe7bad92179961b31520147af39d5353c.1606191405.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Tue, 24 Nov 2020 04:16:43 +0000 Subject: [PATCH v5 2/4] maintenance: include 'cron' details in docs Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: jrnieder@gmail.com, jonathantanmy@google.com, sluongng@gmail.com, =?utf-8?b?xJBvw6BuIFRy4bqnbiBDw7RuZw==?= Danh , Martin =?utf-8?b?w4VncmVu?= , Eric Sunshine , Derrick Stolee , Derrick Stolee , Derrick Stolee Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Derrick Stolee From: Derrick Stolee Advanced and expert users may want to know how 'git maintenance start' schedules background maintenance in order to customize their own schedules beyond what the maintenance.* config values allow. Start a new set of sections in git-maintenance.txt that describe how 'cron' is used to run these tasks. This is particularly valuable for users who want to inspect what Git is doing or for users who want to customize the schedule further. Having a baseline can provide a way forward for users who have never worked with cron schedules. Helped-by: Eric Sunshine Signed-off-by: Derrick Stolee --- Documentation/git-maintenance.txt | 54 +++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt index 6fec1eb8dc..1aa1112418 100644 --- a/Documentation/git-maintenance.txt +++ b/Documentation/git-maintenance.txt @@ -218,6 +218,60 @@ Further, the `git gc` command should not be combined with but does not take the lock in the same way as `git maintenance run`. If possible, use `git maintenance run --task=gc` instead of `git gc`. +The following sections describe the mechanisms put in place to run +background maintenance by `git maintenance start` and how to customize +them. + +BACKGROUND MAINTENANCE ON POSIX SYSTEMS +--------------------------------------- + +The standard mechanism for scheduling background tasks on POSIX systems +is cron(8). This tool executes commands based on a given schedule. The +current list of user-scheduled tasks can be found by running `crontab -l`. +The schedule written by `git maintenance start` is similar to this: + +----------------------------------------------------------------------- +# BEGIN GIT MAINTENANCE SCHEDULE +# The following schedule was created by Git +# Any edits made in this region might be +# replaced in the future by a Git command. + +0 1-23 * * * "//git" --exec-path="/" for-each-repo --config=maintenance.repo maintenance run --schedule=hourly +0 0 * * 1-6 "//git" --exec-path="/" for-each-repo --config=maintenance.repo maintenance run --schedule=daily +0 0 * * 0 "//git" --exec-path="/" for-each-repo --config=maintenance.repo maintenance run --schedule=weekly + +# END GIT MAINTENANCE SCHEDULE +----------------------------------------------------------------------- + +The comments are used as a region to mark the schedule as written by Git. +Any modifications within this region will be completely deleted by +`git maintenance stop` or overwritten by `git maintenance start`. + +The `crontab` entry specifies the full path of the `git` executable to +ensure that the executed `git` command is the same one with which +`git maintenance start` was issued independent of `PATH`. If the same user +runs `git maintenance start` with multiple Git executables, then only the +latest executable is used. + +These commands use `git for-each-repo --config=maintenance.repo` to run +`git maintenance run --schedule=` on each repository listed in +the multi-valued `maintenance.repo` config option. These are typically +loaded from the user-specific global config. The `git maintenance` process +then determines which maintenance tasks are configured to run on each +repository with each `` using the `maintenance..schedule` +config options. These values are loaded from the global or repository +config values. + +If the config values are insufficient to achieve your desired background +maintenance schedule, then you can create your own schedule. If you run +`crontab -e`, then an editor will load with your user-specific `cron` +schedule. In that editor, you can add your own schedule lines. You could +start by adapting the default schedule listed earlier, or you could read +the crontab(5) documentation for advanced scheduling techniques. Please +do use the full path and `--exec-path` techniques from the default +schedule to ensure you are executing the correct binaries in your +schedule. + GIT --- From patchwork Tue Nov 24 04:16:44 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Derrick Stolee X-Patchwork-Id: 11927043 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 32C97C64E7A for ; Tue, 24 Nov 2020 04:16:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B0E2A2085B for ; Tue, 24 Nov 2020 04:16:54 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="u5+gIq9B" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728847AbgKXEQx (ORCPT ); Mon, 23 Nov 2020 23:16:53 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:32908 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727988AbgKXEQx (ORCPT ); Mon, 23 Nov 2020 23:16:53 -0500 Received: from mail-wr1-x444.google.com (mail-wr1-x444.google.com [IPv6:2a00:1450:4864:20::444]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D089AC0613CF for ; Mon, 23 Nov 2020 20:16:52 -0800 (PST) Received: by mail-wr1-x444.google.com with SMTP id r3so2597670wrt.2 for ; Mon, 23 Nov 2020 20:16:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=Xu62scyEh52jlJAm5s5hOS4+gy0J4w4Af61fRGj2v/Q=; b=u5+gIq9B6b/x64O0+JwO2KtIvOSHmrGszI5jbCfnFAljRaBUxAe6d3QOSQ5AkrbIOn iuL/TsXFR7wwqE85vWjgY033Ix2XHKD5B7fPdW9zzYz255y1i1yJHDwTwlQV6gx//c9A vvx35Fyhh9a7Zmyt0AJx/2WYVPJtxHkj3pIvyg18NBP+FXhFIE7gk+MZzfWPKnKub6sD lXzYcre2qFEaiR9ricdS1olkahyR3HEP/bqiMFzDV6FQhbvLO3wIaScOvNQZfLuoX0dy e9/BTgpZKgFKIu2d0yPVz2MxSywuSoZf9r5xaKmHpNZNv8J5VUll4zP0Vbz5YcwjIh7P V3tQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=Xu62scyEh52jlJAm5s5hOS4+gy0J4w4Af61fRGj2v/Q=; b=NNkZlcpvO+vh/2EVJLoi7XcAR6NWlClGI9yhWGDajvq3rWT+25TbZmrpDeh1er5gI4 RJORpYpr3WPNJHJy7hgOveonXiD10vLl7kiP7OePcpUVGc7TqQVCAna8lP/3gxX+6036 9eGwS9GrArJc49Mf+e5qCXlR6G5uCdoKGjFKgxvbCtQ5zbgNSmIeRsG5b4Mm/IwbhvjG Z9Quu6rtj3OJYIHQiJbAiQ4jxmlTeBEAcc76lxoo/Bv/E4dbIe4B05lp+Bp4gaNGN6Iu qYrAmjw+q7QB2lyI0oDrYkmsZk0M0jWdcNITkbrrEeJcbbfA8VmFVyXs80tR7bwyMwF9 2S5w== X-Gm-Message-State: AOAM53215CpFoUtz4doRVhk0QD+PiEVlztopmPAGLsixHXXQk5ROMr/y aoXPNQ/RYjYBek3Br9GjyFDy46nwVSo= X-Google-Smtp-Source: ABdhPJzaAOVzXSTSR0lw81G6Dm6NmtUaBltqJGV/HkXrnF8g75nJFWxpBcVXpWQRbMtry/M9sRVP7w== X-Received: by 2002:a5d:51c2:: with SMTP id n2mr2962695wrv.326.1606191411019; Mon, 23 Nov 2020 20:16:51 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id j127sm2750561wma.31.2020.11.23.20.16.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 23 Nov 2020 20:16:50 -0800 (PST) Message-Id: In-Reply-To: References: Date: Tue, 24 Nov 2020 04:16:44 +0000 Subject: [PATCH v5 3/4] maintenance: use launchctl on macOS Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: jrnieder@gmail.com, jonathantanmy@google.com, sluongng@gmail.com, =?utf-8?b?xJBvw6BuIFRy4bqnbiBDw7RuZw==?= Danh , Martin =?utf-8?b?w4VncmVu?= , Eric Sunshine , Derrick Stolee , Derrick Stolee , Derrick Stolee Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Derrick Stolee From: Derrick Stolee The existing mechanism for scheduling background maintenance is done through cron. The 'crontab -e' command allows updating the schedule while cron itself runs those commands. While this is technically supported by macOS, it has some significant deficiencies: 1. Every run of 'crontab -e' must request elevated privileges through the user interface. When running 'git maintenance start' from the Terminal app, it presents a dialog box saying "Terminal.app would like to administer your computer. Administration can include modifying passwords, networking, and system settings." This is more alarming than what we are hoping to achieve. If this alert had some information about how "git" is trying to run "crontab" then we would have some reason to believe that this dialog might be fine. However, it also doesn't help that some scenarios just leave Git waiting for a response without presenting anything to the user. I experienced this when executing the command from a Bash terminal view inside Visual Studio Code. 2. While cron initializes a user environment enough for "git config --global --show-origin" to show the correct config file information, it does not set up the environment enough for Git Credential Manager Core to load credentials during a 'prefetch' task. My prefetches against private repositories required re-authenticating through UI pop-ups in a way that should not be required. The solution is to switch from cron to the Apple-recommended [1] 'launchd' tool. [1] https://developer.apple.com/library/archive/documentation/MacOSX/Conceptual/BPSystemStartup/Chapters/ScheduledJobs.html The basics of this tool is that we need to create XML-formatted "plist" files inside "~/Library/LaunchAgents/" and then use the 'launchctl' tool to make launchd aware of them. The plist files include all of the scheduling information, along with the command-line arguments split across an array of tags. For example, here is my plist file for the weekly scheduled tasks: Labelorg.git-scm.git.weekly ProgramArguments /usr/local/libexec/git-core/git --exec-path=/usr/local/libexec/git-core for-each-repo --config=maintenance.repo maintenance run --schedule=weekly StartCalendarInterval Day0 Hour0 Minute0 The schedules for the daily and hourly tasks are more complicated since we need to use an array for the StartCalendarInterval with an entry for each of the six days other than the 0th day (to avoid colliding with the weekly task), and each of the 23 hours other than the 0th hour (to avoid colliding with the daily task). The "Label" value is currently filled with "org.git-scm.git.X" where X is the frequency. We need a different plist file for each frequency. The launchctl command needs to be aligned with a user id in order to initialize the command environment. This must be done using the 'launchctl bootstrap' subcommand. This subcommand is new as of macOS 10.11, which was released in September 2015. Before that release the 'launchctl load' subcommand was recommended. The best source of information on this transition I have seen is available at [2]. The current design does not preclude a future version that detects the available fatures of 'launchctl' to use the older commands. However, it is best to rely on the newest version since Apple might completely remove the deprecated version on short notice. [2] https://babodee.wordpress.com/2016/04/09/launchctl-2-0-syntax/ To remove a schedule, we must run 'launchctl bootout' with a valid plist file. We also need to 'bootout' a task before the 'bootstrap' subcommand will succeed, if such a task already exists. The need for a user id requires us to run 'id -u' which works on POSIX systems but not Windows. Further, the need for fully-qualitifed path names including $HOME behaves differently in the Git internals and the external test suite. The $HOME variable starts with "C:\..." instead of the "/c/..." that is provided by Git in these subcommands. The test therefore has a prerequisite that we are not on Windows. The cross- platform logic still allows us to test the macOS logic on a Linux machine. We can verify the commands that were run by 'git maintenance start' and 'git maintenance stop' by injecting a script that writes the command-line arguments into GIT_TEST_MAINT_SCHEDULER. An earlier version of this patch accidentally had an opening "" tag when it should have had a closing "" tag. This was caught during manual testing with actual 'launchctl' commands, but we do not want to update developers' tasks when running tests. It appears that macOS includes the "xmllint" tool which can verify the XML format. This is useful for any system that might contain the tool, so use it whenever it is available. Co-authored-by: Eric Sunshine Signed-off-by: Eric Sunshine Signed-off-by: Derrick Stolee --- Documentation/git-maintenance.txt | 40 +++++++ builtin/gc.c | 188 +++++++++++++++++++++++++++++- t/t7900-maintenance.sh | 58 +++++++++ 3 files changed, 285 insertions(+), 1 deletion(-) diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt index 1aa1112418..5f8f63f098 100644 --- a/Documentation/git-maintenance.txt +++ b/Documentation/git-maintenance.txt @@ -273,6 +273,46 @@ schedule to ensure you are executing the correct binaries in your schedule. +BACKGROUND MAINTENANCE ON MACOS SYSTEMS +--------------------------------------- + +While macOS technically supports `cron`, using `crontab -e` requires +elevated privileges and the executed process does not have a full user +context. Without a full user context, Git and its credential helpers +cannot access stored credentials, so some maintenance tasks are not +functional. + +Instead, `git maintenance start` interacts with the `launchctl` tool, +which is the recommended way to schedule timed jobs in macOS. Scheduling +maintenance through `git maintenance (start|stop)` requires some +`launchctl` features available only in macOS 10.11 or later. + +Your user-specific scheduled tasks are stored as XML-formatted `.plist` +files in `~/Library/LaunchAgents/`. You can see the currently-registered +tasks using the following command: + +----------------------------------------------------------------------- +$ ls ~/Library/LaunchAgents/org.git-scm.git* +org.git-scm.git.daily.plist +org.git-scm.git.hourly.plist +org.git-scm.git.weekly.plist +----------------------------------------------------------------------- + +One task is registered for each `--schedule=` option. To +inspect how the XML format describes each schedule, open one of these +`.plist` files in an editor and inspect the `` element following +the `StartCalendarInterval` element. + +`git maintenance start` will overwrite these files and register the +tasks again with `launchctl`, so any customizations should be done by +creating your own `.plist` files with distinct names. Similarly, the +`git maintenance stop` command will unregister the tasks with `launchctl` +and delete the `.plist` files. + +To create more advanced customizations to your background tasks, see +launchctl.plist(5) for more information. + + GIT --- Part of the linkgit:git[1] suite diff --git a/builtin/gc.c b/builtin/gc.c index 18ae7f7138..782769f243 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1491,6 +1491,186 @@ static int maintenance_unregister(void) return run_command(&config_unset); } +static const char *get_frequency(enum schedule_priority schedule) +{ + switch (schedule) { + case SCHEDULE_HOURLY: + return "hourly"; + case SCHEDULE_DAILY: + return "daily"; + case SCHEDULE_WEEKLY: + return "weekly"; + default: + BUG("invalid schedule %d", schedule); + } +} + +static char *launchctl_service_name(const char *frequency) +{ + struct strbuf label = STRBUF_INIT; + strbuf_addf(&label, "org.git-scm.git.%s", frequency); + return strbuf_detach(&label, NULL); +} + +static char *launchctl_service_filename(const char *name) +{ + char *expanded; + struct strbuf filename = STRBUF_INIT; + strbuf_addf(&filename, "~/Library/LaunchAgents/%s.plist", name); + + expanded = expand_user_path(filename.buf, 1); + if (!expanded) + die(_("failed to expand path '%s'"), filename.buf); + + strbuf_release(&filename); + return expanded; +} + +static char *launchctl_get_uid(void) +{ + return xstrfmt("gui/%d", getuid()); +} + +static int launchctl_boot_plist(int enable, const char *filename, const char *cmd) +{ + int result; + struct child_process child = CHILD_PROCESS_INIT; + char *uid = launchctl_get_uid(); + + strvec_split(&child.args, cmd); + if (enable) + strvec_push(&child.args, "bootstrap"); + else + strvec_push(&child.args, "bootout"); + strvec_push(&child.args, uid); + strvec_push(&child.args, filename); + + child.no_stderr = 1; + child.no_stdout = 1; + + if (start_command(&child)) + die(_("failed to start launchctl")); + + result = finish_command(&child); + + free(uid); + return result; +} + +static int launchctl_remove_plist(enum schedule_priority schedule, const char *cmd) +{ + const char *frequency = get_frequency(schedule); + char *name = launchctl_service_name(frequency); + char *filename = launchctl_service_filename(name); + int result = launchctl_boot_plist(0, filename, cmd); + unlink(filename); + free(filename); + free(name); + return result; +} + +static int launchctl_remove_plists(const char *cmd) +{ + return launchctl_remove_plist(SCHEDULE_HOURLY, cmd) || + launchctl_remove_plist(SCHEDULE_DAILY, cmd) || + launchctl_remove_plist(SCHEDULE_WEEKLY, cmd); +} + +static int launchctl_schedule_plist(const char *exec_path, enum schedule_priority schedule, const char *cmd) +{ + FILE *plist; + int i; + const char *preamble, *repeat; + const char *frequency = get_frequency(schedule); + char *name = launchctl_service_name(frequency); + char *filename = launchctl_service_filename(name); + + if (safe_create_leading_directories(filename)) + die(_("failed to create directories for '%s'"), filename); + plist = xfopen(filename, "w"); + + preamble = "\n" + "\n" + "" + "\n" + "Label%s\n" + "ProgramArguments\n" + "\n" + "%s/git\n" + "--exec-path=%s\n" + "for-each-repo\n" + "--config=maintenance.repo\n" + "maintenance\n" + "run\n" + "--schedule=%s\n" + "\n" + "StartCalendarInterval\n" + "\n"; + fprintf(plist, preamble, name, exec_path, exec_path, frequency); + + switch (schedule) { + case SCHEDULE_HOURLY: + repeat = "\n" + "Hour%d\n" + "Minute0\n" + "\n"; + for (i = 1; i <= 23; i++) + fprintf(plist, repeat, i); + break; + + case SCHEDULE_DAILY: + repeat = "\n" + "Day%d\n" + "Hour0\n" + "Minute0\n" + "\n"; + for (i = 1; i <= 6; i++) + fprintf(plist, repeat, i); + break; + + case SCHEDULE_WEEKLY: + fprintf(plist, + "\n" + "Day0\n" + "Hour0\n" + "Minute0\n" + "\n"); + break; + + default: + /* unreachable */ + break; + } + fprintf(plist, "\n\n\n"); + fclose(plist); + + /* bootout might fail if not already running, so ignore */ + launchctl_boot_plist(0, filename, cmd); + if (launchctl_boot_plist(1, filename, cmd)) + die(_("failed to bootstrap service %s"), filename); + + free(filename); + free(name); + return 0; +} + +static int launchctl_add_plists(const char *cmd) +{ + const char *exec_path = git_exec_path(); + + return launchctl_schedule_plist(exec_path, SCHEDULE_HOURLY, cmd) || + launchctl_schedule_plist(exec_path, SCHEDULE_DAILY, cmd) || + launchctl_schedule_plist(exec_path, SCHEDULE_WEEKLY, cmd); +} + +static int launchctl_update_schedule(int run_maintenance, int fd, const char *cmd) +{ + if (run_maintenance) + return launchctl_add_plists(cmd); + else + return launchctl_remove_plists(cmd); +} + #define BEGIN_LINE "# BEGIN GIT MAINTENANCE SCHEDULE" #define END_LINE "# END GIT MAINTENANCE SCHEDULE" @@ -1579,7 +1759,11 @@ static int crontab_update_schedule(int run_maintenance, int fd, const char *cmd) return result; } +#if defined(__APPLE__) +static const char platform_scheduler[] = "launchctl"; +#else static const char platform_scheduler[] = "crontab"; +#endif static int update_background_schedule(int enable) { @@ -1603,7 +1787,9 @@ static int update_background_schedule(int enable) if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF) < 0) return error(_("another process is scheduling background maintenance")); - if (!strcmp(scheduler, "crontab")) + if (!strcmp(scheduler, "launchctl")) + result = launchctl_update_schedule(enable, lk.tempfile->fd, cmd); + else if (!strcmp(scheduler, "crontab")) result = crontab_update_schedule(enable, lk.tempfile->fd, cmd); else die("unknown background scheduler: %s", scheduler); diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index eeb939168d..6d37312901 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -7,6 +7,19 @@ test_description='git maintenance builtin' GIT_TEST_COMMIT_GRAPH=0 GIT_TEST_MULTI_PACK_INDEX=0 +test_lazy_prereq XMLLINT ' + xmllint --version +' + +test_xmllint () { + if test_have_prereq XMLLINT + then + xmllint --noout "$@" + else + true + fi +} + test_expect_success 'help text' ' test_expect_code 129 git maintenance -h 2>err && test_i18ngrep "usage: git maintenance " err && @@ -395,6 +408,51 @@ test_expect_success 'start preserves existing schedule' ' grep "Important information!" cron.txt ' +test_expect_success !MINGW 'start and stop macOS maintenance' ' + uid=$(id -u) && + + write_script print-args <<-\EOF && + echo $* >>args + EOF + + rm -f args && + GIT_TEST_MAINT_SCHEDULER=launchctl:./print-args git maintenance start && + + # start registers the repo + git config --get --global maintenance.repo "$(pwd)" && + + ls "$HOME/Library/LaunchAgents" >actual && + cat >expect <<-\EOF && + org.git-scm.git.daily.plist + org.git-scm.git.hourly.plist + org.git-scm.git.weekly.plist + EOF + test_cmp expect actual && + + rm expect && + for frequency in hourly daily weekly + do + PLIST="$HOME/Library/LaunchAgents/org.git-scm.git.$frequency.plist" && + test_xmllint "$PLIST" && + grep schedule=$frequency "$PLIST" && + echo "bootout gui/$uid $PLIST" >>expect && + echo "bootstrap gui/$uid $PLIST" >>expect || return 1 + done && + test_cmp expect args && + + rm -f args && + GIT_TEST_MAINT_SCHEDULER=launchctl:./print-args git maintenance stop && + + # stop does not unregister the repo + git config --get --global maintenance.repo "$(pwd)" && + + printf "bootout gui/$uid $HOME/Library/LaunchAgents/org.git-scm.git.%s.plist\n" \ + hourly daily weekly >expect && + test_cmp expect args && + ls "$HOME/Library/LaunchAgents" >actual && + test_line_count = 0 actual +' + test_expect_success 'register preserves existing strategy' ' git config maintenance.strategy none && git maintenance register && From patchwork Tue Nov 24 04:16:45 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Derrick Stolee X-Patchwork-Id: 11927039 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D5658C64E7B for ; Tue, 24 Nov 2020 04:16:55 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7E91E2086A for ; Tue, 24 Nov 2020 04:16:55 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="JegBhTKo" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728858AbgKXEQy (ORCPT ); Mon, 23 Nov 2020 23:16:54 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:32914 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727988AbgKXEQy (ORCPT ); Mon, 23 Nov 2020 23:16:54 -0500 Received: from mail-wm1-x341.google.com (mail-wm1-x341.google.com [IPv6:2a00:1450:4864:20::341]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F0F01C0613CF for ; Mon, 23 Nov 2020 20:16:53 -0800 (PST) Received: by mail-wm1-x341.google.com with SMTP id w24so1512657wmi.0 for ; Mon, 23 Nov 2020 20:16:53 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=I5yBdDTXqI57h59MTpcOKh4XdaaCl6u/q65+pPPF/5A=; b=JegBhTKokBC+Kuk5KUMvF2SlJbiC1qbovDoISSD++vLXkTrcTy56S5SEcvGx1NUard oBsQjBuiUm52lfiReuWY57JsCUONyLP9OB1zFMWGNESLllgH8r8lJ0zy/GykrqXVeC0M BfhywAjMZOQteAYDTeCeNE04KKp5ky93hKL0d0z33dEZOj0eHWy/b8DidtVykZsXdoWI GW6QMRGVS/yiDb3SgjnuF43BgRjPZ5I4TKNsLhAe7qicSfA0IkZaHZRBD2qNOSlly9Mu nsKPJ9TqcFG1i0k64lY3SsrlBgUW0Qm6ujmTj2OwJhXby5SuY9BUiLAZCz96aPiNtlgo Wlig== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=I5yBdDTXqI57h59MTpcOKh4XdaaCl6u/q65+pPPF/5A=; b=m2JMnvwB5InK5xKyMeFqgWY7//Xc2pxeKkZJRAZYTbAJ3PEv1i39iixa1hcsX77fgv 1neT4cENuGYMtCOaGC623ZoiFss2sfEpfly308aWJtwmJF3HiyigoQW1hBDXd94FaAOy 46zAn3agkthDE/mSJHsw32foT2RUVPQQP0bJIoYX7pdtPvRK67tYw8DaE5cde50R+X71 lMVYwIk5YO3ojsj3oi+mPRBzWmy6qGWf/aIZp5eq8ym74yxC4MGao9xqhQ0JYVLtHqth o5IHzcultg3kd5quQoAXaWs16rAm1WBa6HJB2QtSrfx4wgcBrMdoWGYRTyRdm8lAhb3e SZDA== X-Gm-Message-State: AOAM532/MGq5eHELizPPe+m0fOYOWQPOJHJ03C6EA069zjlPVh20eah7 6gQ/cc//m8fi12Es5vREJICKwv2peXU= X-Google-Smtp-Source: ABdhPJwYjUm6ZcFdRaNVN63FDjGETXnAamm/ERubMfpa6OIsRcmB4uuozvL9ozGvQQYNeUHK6xq3mA== X-Received: by 2002:a1c:309:: with SMTP id 9mr2018070wmd.80.1606191412366; Mon, 23 Nov 2020 20:16:52 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id f23sm2475743wmb.43.2020.11.23.20.16.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 23 Nov 2020 20:16:51 -0800 (PST) Message-Id: In-Reply-To: References: Date: Tue, 24 Nov 2020 04:16:45 +0000 Subject: [PATCH v5 4/4] maintenance: use Windows scheduled tasks Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: jrnieder@gmail.com, jonathantanmy@google.com, sluongng@gmail.com, =?utf-8?b?xJBvw6BuIFRy4bqnbiBDw7RuZw==?= Danh , Martin =?utf-8?b?w4VncmVu?= , Eric Sunshine , Derrick Stolee , Derrick Stolee , Derrick Stolee Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Derrick Stolee From: Derrick Stolee Git's background maintenance uses cron by default, but this is not available on Windows. Instead, integrate with Task Scheduler. Tasks can be scheduled using the 'schtasks' command. There are several command-line options that can allow for some advanced scheduling, but unfortunately these seem to all require authenticating using a password. Instead, use the "/xml" option to pass an XML file that contains the configuration for the necessary schedule. These XML files are based on some that I exported after constructing a schedule in the Task Scheduler GUI. These options only run background maintenance when the user is logged in, and more fields are populated with the current username and SID at run-time by 'schtasks'. Since the GIT_TEST_MAINT_SCHEDULER environment variable allows us to specify 'schtasks' as the scheduler, we can test the Windows-specific logic on other platforms. Thus, add a check that the XML file written by Git is valid when xmllint exists on the system. Since we use a temporary file for the XML files sent to 'schtasks', we must copy the file to a predictable filename. Use the number of lines in the 'args' file to provide a filename for xmllint. Instead of an exact match on the 'args' file, we 'grep' for the arguments other than the filename. There is a deficiency in the current design. Windows has two kinds of applications: GUI applications that start by "winmain()" and console applications that start by "main()". Console applications are attached to a new Console window if they are not already associated with a GUI application. This means that every hour the scheudled task launches a command window for the scheduled tasks. Not only is this visually obtrusive, but it also takes focus from whatever else the user is doing! A simple fix would be to insert a GUI application that acts as a shim between the scheduled task and Git. This is currently possible in Git for Windows by setting the tag equal to C:\Program Files\Git\git-bash.exe with options "--hide --no-needs-console --command=cmd\git.exe" followed by the arguments currently used. Since git-bash.exe is not included in Windows builds of core Git, I chose to leave out this feature. My plan is to submit a small patch to Git for Windows that converts the use of git.exe with this use of git-bash.exe in the short term. In the long term, we can consider creating this GUI shim application within core Git, perhaps in contrib/. Co-authored-by: Eric Sunshine Signed-off-by: Eric Sunshine Signed-off-by: Derrick Stolee --- Documentation/git-maintenance.txt | 22 ++++ builtin/gc.c | 160 ++++++++++++++++++++++++++++++ t/t7900-maintenance.sh | 44 ++++++++ 3 files changed, 226 insertions(+) diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt index 5f8f63f098..6970f2b898 100644 --- a/Documentation/git-maintenance.txt +++ b/Documentation/git-maintenance.txt @@ -313,6 +313,28 @@ To create more advanced customizations to your background tasks, see launchctl.plist(5) for more information. +BACKGROUND MAINTENANCE ON WINDOWS SYSTEMS +----------------------------------------- + +Windows does not support `cron` and instead has its own system for +scheduling background tasks. The `git maintenance start` command uses +the `schtasks` command to submit tasks to this system. You can inspect +all background tasks using the Task Scheduler application. The tasks +added by Git have names of the form `Git Maintenance ()`. +The Task Scheduler GUI has ways to inspect these tasks, but you can also +export the tasks to XML files and view the details there. + +Note that since Git is a console application, these background tasks +create a console window visible to the current user. This can be changed +manually by selecting the "Run whether user is logged in or not" option +in Task Scheduler. This change requires a password input, which is why +`git maintenance start` does not select it by default. + +If you want to customize the background tasks, please rename the tasks +so future calls to `git maintenance (start|stop)` do not overwrite your +custom tasks. + + GIT --- Part of the linkgit:git[1] suite diff --git a/builtin/gc.c b/builtin/gc.c index 782769f243..43224e0dec 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1671,6 +1671,162 @@ static int launchctl_update_schedule(int run_maintenance, int fd, const char *cm return launchctl_remove_plists(cmd); } +static char *schtasks_task_name(const char *frequency) +{ + struct strbuf label = STRBUF_INIT; + strbuf_addf(&label, "Git Maintenance (%s)", frequency); + return strbuf_detach(&label, NULL); +} + +static int schtasks_remove_task(enum schedule_priority schedule, const char *cmd) +{ + int result; + struct strvec args = STRVEC_INIT; + const char *frequency = get_frequency(schedule); + char *name = schtasks_task_name(frequency); + + strvec_split(&args, cmd); + strvec_pushl(&args, "/delete", "/tn", name, "/f", NULL); + + result = run_command_v_opt(args.v, 0); + + strvec_clear(&args); + free(name); + return result; +} + +static int schtasks_remove_tasks(const char *cmd) +{ + return schtasks_remove_task(SCHEDULE_HOURLY, cmd) || + schtasks_remove_task(SCHEDULE_DAILY, cmd) || + schtasks_remove_task(SCHEDULE_WEEKLY, cmd); +} + +static int schtasks_schedule_task(const char *exec_path, enum schedule_priority schedule, const char *cmd) +{ + int result; + struct child_process child = CHILD_PROCESS_INIT; + const char *xml; + struct tempfile *tfile; + const char *frequency = get_frequency(schedule); + char *name = schtasks_task_name(frequency); + + tfile = xmks_tempfile("schedule_XXXXXX"); + if (!tfile || !fdopen_tempfile(tfile, "w")) + die(_("failed to create temp xml file")); + + xml = "\n" + "\n" + "\n" + "\n"; + fputs(xml, tfile->fp); + + switch (schedule) { + case SCHEDULE_HOURLY: + fprintf(tfile->fp, + "2020-01-01T01:00:00\n" + "true\n" + "\n" + "1\n" + "\n" + "\n" + "PT1H\n" + "PT23H\n" + "false\n" + "\n"); + break; + + case SCHEDULE_DAILY: + fprintf(tfile->fp, + "2020-01-01T00:00:00\n" + "true\n" + "\n" + "\n" + "\n" + "\n" + "\n" + "\n" + "\n" + "\n" + "\n" + "1\n" + "\n"); + break; + + case SCHEDULE_WEEKLY: + fprintf(tfile->fp, + "2020-01-01T00:00:00\n" + "true\n" + "\n" + "\n" + "\n" + "\n" + "1\n" + "\n"); + break; + + default: + break; + } + + xml = "\n" + "\n" + "\n" + "\n" + "InteractiveToken\n" + "LeastPrivilege\n" + "\n" + "\n" + "\n" + "IgnoreNew\n" + "true\n" + "true\n" + "true\n" + "false\n" + "PT72H\n" + "7\n" + "\n" + "\n" + "\n" + "\"%s\\git.exe\"\n" + "--exec-path=\"%s\" for-each-repo --config=maintenance.repo maintenance run --schedule=%s\n" + "\n" + "\n" + "\n"; + fprintf(tfile->fp, xml, exec_path, exec_path, frequency); + strvec_split(&child.args, cmd); + strvec_pushl(&child.args, "/create", "/tn", name, "/f", "/xml", tfile->filename.buf, NULL); + close_tempfile_gently(tfile); + + child.no_stdout = 1; + child.no_stderr = 1; + + if (start_command(&child)) + die(_("failed to start schtasks")); + result = finish_command(&child); + + delete_tempfile(&tfile); + free(name); + return result; +} + +static int schtasks_schedule_tasks(const char *cmd) +{ + const char *exec_path = git_exec_path(); + + return schtasks_schedule_task(exec_path, SCHEDULE_HOURLY, cmd) || + schtasks_schedule_task(exec_path, SCHEDULE_DAILY, cmd) || + schtasks_schedule_task(exec_path, SCHEDULE_WEEKLY, cmd); +} + +static int schtasks_update_schedule(int run_maintenance, int fd, const char *cmd) +{ + if (run_maintenance) + return schtasks_schedule_tasks(cmd); + else + return schtasks_remove_tasks(cmd); +} + #define BEGIN_LINE "# BEGIN GIT MAINTENANCE SCHEDULE" #define END_LINE "# END GIT MAINTENANCE SCHEDULE" @@ -1761,6 +1917,8 @@ static int crontab_update_schedule(int run_maintenance, int fd, const char *cmd) #if defined(__APPLE__) static const char platform_scheduler[] = "launchctl"; +#elif defined(GIT_WINDOWS_NATIVE) +static const char platform_scheduler[] = "schtasks"; #else static const char platform_scheduler[] = "crontab"; #endif @@ -1789,6 +1947,8 @@ static int update_background_schedule(int enable) if (!strcmp(scheduler, "launchctl")) result = launchctl_update_schedule(enable, lk.tempfile->fd, cmd); + else if (!strcmp(scheduler, "schtasks")) + result = schtasks_update_schedule(enable, lk.tempfile->fd, cmd); else if (!strcmp(scheduler, "crontab")) result = crontab_update_schedule(enable, lk.tempfile->fd, cmd); else diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index 6d37312901..0246e4ce30 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -453,6 +453,50 @@ test_expect_success !MINGW 'start and stop macOS maintenance' ' test_line_count = 0 actual ' +test_expect_success 'start and stop Windows maintenance' ' + write_script print-args <<-\EOF && + echo $* >>args + while test $# -gt 0 + do + case "$1" in + /xml) shift; xmlfile=$1; break ;; + *) shift ;; + esac + done + lines=$(wc -l args | awk "{print \$1;}") + test -z "$xmlfile" || cp "$xmlfile" "schedule-$lines.xml" + EOF + + rm -f args && + GIT_TEST_MAINT_SCHEDULER="schtasks:./print-args" git maintenance start && + + # start registers the repo + git config --get --global maintenance.repo "$(pwd)" && + + for frequency in hourly daily weekly + do + grep "/create /tn Git Maintenance ($frequency) /f /xml" args \ + || return 1 + done && + + for i in 1 2 3 + do + test_xmllint "schedule-$i.xml" && + grep "encoding=.US-ASCII." "schedule-$i.xml" || return 1 + done && + + rm -f args && + GIT_TEST_MAINT_SCHEDULER="schtasks:./print-args" git maintenance stop && + + # stop does not unregister the repo + git config --get --global maintenance.repo "$(pwd)" && + + rm expect && + printf "/delete /tn Git Maintenance (%s) /f\n" \ + hourly daily weekly >expect && + test_cmp expect args +' + test_expect_success 'register preserves existing strategy' ' git config maintenance.strategy none && git maintenance register &&