From patchwork Tue Nov 3 14:03:14 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Derrick Stolee X-Patchwork-Id: 11877615 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=-9.6 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, 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 DA4E5C388F7 for ; Tue, 3 Nov 2020 14:04:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7D53522280 for ; Tue, 3 Nov 2020 14:04:52 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="aaS5ClxI" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729465AbgKCOEv (ORCPT ); Tue, 3 Nov 2020 09:04:51 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43076 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729375AbgKCODW (ORCPT ); Tue, 3 Nov 2020 09:03:22 -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 A1D29C0613D1 for ; Tue, 3 Nov 2020 06:03:20 -0800 (PST) Received: by mail-wr1-x444.google.com with SMTP id b3so12777205wrx.11 for ; Tue, 03 Nov 2020 06:03:20 -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=Nly5aoUwoSfFzapqnHpVG8ijbTA7fs42NqNdOyOuJhA=; b=aaS5ClxIVHLVAzWR2ymQXcxAOgJ4vdaQ9UN5DxY3kALiPJ7t55cW7+5DlMFDojjeup laULMwLyZENCUHBAHyPLKgfPtfbru4yc+aMZKQpXvgFUU6tv97Mv7lCktECeKY0/fiPf Jyig9h4CvoMvksutFxHxpW4Jqz/uSPmJ/33DWNBHO/XlJPG4DYMLtDPSa3/bZFEMWAPT yVv5qdUWujuYzhKRpzkdQ22UpAhrwfbAV2XzEGLlrzRdhI2Z4B/RN0gnMmZkTEOvJCc2 SV16QosFUdKPNJ5qgnMdcbyCRJ8vslWnfZT8c8pJJbY790gQ8PSzp6L9QGsD3wBBB1Bs Tb2g== 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=Nly5aoUwoSfFzapqnHpVG8ijbTA7fs42NqNdOyOuJhA=; b=PEkJss/9CpihtPOJLy0+So8uD9H9YxKTOl3+giXkgOIYrytQxK6AzNSD8e0TD41DAH SRNZ2p9sAcX8FTUBEIVa3Pw77KRCCLq7oxaPYjQ5ZohiolwSMnkr7HyaLlGpk/81ROAF MuwECaR6t9F2EG4s5yCfPbuPFa7NmvKjoCsrbpsl32mekCp2FhWjDCeqCczoK3uFdEIX /a4UpWIw0sxgjEg8CX0pEChOQaD47f9HAq4yo8P0tqUVWpJ+fx25ot6DoMS5OrrwS9nv 78EFEDBi3g94b21FN13+o6HP2gK7yn4uuFtL/gA5zgjPXxg7HqZ20K2pKJgvh78iqJVG ezOg== X-Gm-Message-State: AOAM530nV0Nyac5OuqSxsO8h4HJYQxoBZkotaTHLXF9M0RK4QHlqgS+F 6ohBw8E/v3ab/D5RlZ31ynXQz79sWvM= X-Google-Smtp-Source: ABdhPJxRm3uYkQWEkVvOnFdz4CQNT6nienWTPUWlyqsdCI0ZTSMatvDruHA4wWLhaoVw8/aY92t1ww== X-Received: by 2002:a05:6000:1046:: with SMTP id c6mr2034573wrx.315.1604412199159; Tue, 03 Nov 2020 06:03:19 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id e3sm26491521wrn.32.2020.11.03.06.03.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Nov 2020 06:03:18 -0800 (PST) Message-Id: In-Reply-To: References: Date: Tue, 03 Nov 2020 14:03:14 +0000 Subject: [PATCH 1/3] 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, Derrick Stolee , =?utf-8?b?xJBvw6BuIFRy4bqnbiBDw7RuZw==?= Danh , Martin =?utf-8?b?w4VncmVu?= , 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. Signed-off-by: Derrick Stolee --- builtin/gc.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index e3098ef6a1..c1f7d9bdc2 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1494,7 +1494,7 @@ 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 platform_update_schedule(int run_maintenance, int fd) { int result = 0; int in_old_region = 0; @@ -1503,11 +1503,6 @@ static int update_background_schedule(int run_maintenance) 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) @@ -1516,12 +1511,11 @@ static int update_background_schedule(int run_maintenance) strvec_split(&crontab_list.args, crontab_name); 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; + return error(_("failed to run 'crontab -l'; your system might not support 'cron'")); } /* Ignore exit code, as an empty crontab will return error. */ @@ -1531,7 +1525,7 @@ 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); @@ -1539,8 +1533,7 @@ static int update_background_schedule(int run_maintenance) crontab_edit.git_cmd = 0; if (start_command(&crontab_edit)) { - result = error(_("failed to run 'crontab'; your system might not support 'cron'")); - goto cleanup; + return error(_("failed to run 'crontab'; your system might not support 'cron'")); } cron_in = fdopen(crontab_edit.in, "w"); @@ -1586,13 +1579,24 @@ 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; - } - fclose(cron_list); + else + fclose(cron_list); + return result; +} + +static int update_background_schedule(int run_maintenance) +{ + int result; + 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")); + + result = platform_update_schedule(run_maintenance, lk.tempfile->fd); -cleanup: rollback_lock_file(&lk); return result; } From patchwork Tue Nov 3 14:03:15 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Derrick Stolee X-Patchwork-Id: 11877619 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=-9.6 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, 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 1BFD1C55179 for ; Tue, 3 Nov 2020 14:05:04 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id AA5A122243 for ; Tue, 3 Nov 2020 14:05:03 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Fz08/kdw" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729495AbgKCOFC (ORCPT ); Tue, 3 Nov 2020 09:05:02 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43082 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729368AbgKCODW (ORCPT ); Tue, 3 Nov 2020 09:03:22 -0500 Received: from mail-wr1-x42c.google.com (mail-wr1-x42c.google.com [IPv6:2a00:1450:4864:20::42c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C99C2C0617A6 for ; Tue, 3 Nov 2020 06:03:21 -0800 (PST) Received: by mail-wr1-x42c.google.com with SMTP id e6so1443763wro.1 for ; Tue, 03 Nov 2020 06:03:21 -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=m8gW5bCgQFFAeBqgmq1s4ELpH4Xy61wnkyZoneclv/g=; b=Fz08/kdwA6jNgrvlbSyp125bg530Pq1Qf+/y6Mv25ZwvsVbMHm1iUccD+h6BHs6e2G Fx2RmzJImI5PDvhBZ7vNIcnmJbqM6Jju0mfGomvRyJh/P6/PLqHzg+FuMLfGD8KY8UGb Nq9y9BRr/K6ebDbMFKjljHcRk1HXUajG+gQINGQ2FH19yIS+quqQ/Gv0zR/hzVqId8ui /P6ox2betp7PTythM8G44/ra2mb+pwv5+oGPfkw4NAfHbfrBfMESPa9SeHrouI9M8GNH rQYZXe/UIq5OpNx/GmxFI1ZX+rWJ6tyc0u1NKOfZUyQDftxFgWaRnRVlgZdAy8/g3W7K 98aQ== 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=m8gW5bCgQFFAeBqgmq1s4ELpH4Xy61wnkyZoneclv/g=; b=hXXfRD1Idxzhrb9Dzw0mw83Arnx98y7MlLlpCEGgQ/hX71grEfl48HM2ZIktMZPfqr aDDvHLMqPTxFYyEdO1YXn7ly6rrvsxSCGHXz5C0/qAwW+Gz37pXGmA/B8Pq4XHKwCtEj 0OFN7Z2Q7/ilQTA5Nwz7skTC6D1tRkU69KdsI7DDsByjjEX/mKx2mS96B8Mkw+Jw58qx 4w7QnORMkH18Tn0hGeI8U9wVjQJFe+pXiGQFOC+/u9G782tlctYORnbslpUCR1RRgt9T o78eFAC4yJ0mK8DmtsDNdJfgm4fblPFm5JqEcfsKSZ5mc7RTVsJsfOtoyJsZsCPRkQZE YBag== X-Gm-Message-State: AOAM532+bVCfmjhro9AzjH16CMu4xlZIpnoNdtUraWCYOu0sUVv+/G0i 7bsfLWWfXmVwKs8yq/T2R8yF/JCaGIY= X-Google-Smtp-Source: ABdhPJysT26RwBmU1YtL3htUr0wI5brV/+2waGISnN+i4p3eMSW3V7Ylv53jpkQSCZjjlwoDJV9m4A== X-Received: by 2002:adf:b1d6:: with SMTP id r22mr25299659wra.136.1604412200088; Tue, 03 Nov 2020 06:03:20 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id f7sm27432107wrx.64.2020.11.03.06.03.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Nov 2020 06:03:19 -0800 (PST) Message-Id: <832fdf16872cbfee4a5e15b559b2b40dabd545f4.1604412197.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Tue, 03 Nov 2020 14:03:15 +0000 Subject: [PATCH 2/3] 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, Derrick Stolee , =?utf-8?b?xJBvw6BuIFRy4bqnbiBDw7RuZw==?= Danh , Martin =?utf-8?b?w4VncmVu?= , 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]. [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. 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_CRONTAB. Signed-off-by: Derrick Stolee --- builtin/gc.c | 209 +++++++++++++++++++++++++++++++++++++++++ t/t7900-maintenance.sh | 52 +++++++++- t/test-lib.sh | 4 + 3 files changed, 262 insertions(+), 3 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index c1f7d9bdc2..fa0ae63a80 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1491,6 +1491,214 @@ static int maintenance_unregister(void) return run_command(&config_unset); } +#if defined(__APPLE__) + +static char *get_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 *get_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 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 *get_uid(void) +{ + struct strbuf output = STRBUF_INIT; + struct child_process id = CHILD_PROCESS_INIT; + + strvec_pushl(&id.args, "/usr/bin/id", "-u", NULL); + if (capture_command(&id, &output, 0)) + die(_("failed to discover user id")); + + strbuf_trim_trailing_newline(&output); + return strbuf_detach(&output, NULL); +} + +static int bootout(const char *filename) +{ + int result; + struct strvec args = STRVEC_INIT; + char *uid = get_uid(); + const char *launchctl = getenv("GIT_TEST_CRONTAB"); + if (!launchctl) + launchctl = "/bin/launchctl"; + + strvec_split(&args, launchctl); + strvec_push(&args, "bootout"); + strvec_pushf(&args, "gui/%s", uid); + strvec_push(&args, filename); + + result = run_command_v_opt(args.v, 0); + + strvec_clear(&args); + free(uid); + return result; +} + +static int bootstrap(const char *filename) +{ + int result; + struct strvec args = STRVEC_INIT; + char *uid = get_uid(); + const char *launchctl = getenv("GIT_TEST_CRONTAB"); + if (!launchctl) + launchctl = "/bin/launchctl"; + + strvec_split(&args, launchctl); + strvec_push(&args, "bootstrap"); + strvec_pushf(&args, "gui/%s", uid); + strvec_push(&args, filename); + + result = run_command_v_opt(args.v, 0); + + strvec_clear(&args); + free(uid); + return result; +} + +static int remove_plist(enum schedule_priority schedule) +{ + const char *frequency = get_frequency(schedule); + char *name = get_service_name(frequency); + char *filename = get_service_filename(name); + int result = bootout(filename); + free(filename); + free(name); + return result; +} + +static int remove_plists(void) +{ + return remove_plist(SCHEDULE_HOURLY) || + remove_plist(SCHEDULE_DAILY) || + remove_plist(SCHEDULE_WEEKLY); +} + +static int schedule_plist(const char *exec_path, enum schedule_priority schedule) +{ + FILE *plist; + int i; + const char *preamble, *repeat; + const char *frequency = get_frequency(schedule); + char *name = get_service_name(frequency); + char *filename = get_service_filename(name); + + if (safe_create_leading_directories(filename)) + die(_("failed to create directories for '%s'"), filename); + plist = fopen(filename, "w"); + + if (!plist) + die(_("failed to open '%s'"), filename); + + 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"); + + /* bootout might fail if not already running, so ignore */ + bootout(filename); + if (bootstrap(filename)) + die(_("failed to bootstrap service %s"), filename); + + fclose(plist); + free(filename); + free(name); + return 0; +} + +static int add_plists(void) +{ + const char *exec_path = git_exec_path(); + + return schedule_plist(exec_path, SCHEDULE_HOURLY) || + schedule_plist(exec_path, SCHEDULE_DAILY) || + schedule_plist(exec_path, SCHEDULE_WEEKLY); +} + +static int platform_update_schedule(int run_maintenance, int fd) +{ + if (run_maintenance) + return add_plists(); + else + return remove_plists(); +} +#else #define BEGIN_LINE "# BEGIN GIT MAINTENANCE SCHEDULE" #define END_LINE "# END GIT MAINTENANCE SCHEDULE" @@ -1585,6 +1793,7 @@ static int platform_update_schedule(int run_maintenance, int fd) fclose(cron_list); return result; } +#endif static int update_background_schedule(int run_maintenance) { diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index 20184e96e1..f0210aa206 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -367,7 +367,7 @@ test_expect_success 'register and unregister' ' test_cmp before actual ' -test_expect_success 'start from empty cron table' ' +test_expect_success !MACOS_MAINTENANCE 'start from empty cron table' ' GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance start && # start registers the repo @@ -378,7 +378,7 @@ test_expect_success 'start from empty cron table' ' grep "for-each-repo --config=maintenance.repo maintenance run --schedule=weekly" cron.txt ' -test_expect_success 'stop from existing schedule' ' +test_expect_success !MACOS_MAINTENANCE 'stop from existing schedule' ' GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance stop && # stop does not unregister the repo @@ -389,12 +389,58 @@ test_expect_success 'stop from existing schedule' ' test_must_be_empty cron.txt ' -test_expect_success 'start preserves existing schedule' ' +test_expect_success !MACOS_MAINTENANCE 'start preserves existing schedule' ' echo "Important information!" >cron.txt && GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance start && grep "Important information!" cron.txt ' +test_expect_success MACOS_MAINTENANCE 'start and stop macOS maintenance' ' + echo "#!/bin/sh\necho \$@ >>args" >print-args && + chmod a+x print-args && + + rm -f args && + GIT_TEST_CRONTAB="./print-args" git maintenance start && + + # start registers the repo + git config --get --global maintenance.repo "$(pwd)" && + + # ~/Library/LaunchAgents + 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" && + 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_CRONTAB="./print-args" git maintenance stop && + + # stop does not unregister the repo + git config --get --global maintenance.repo "$(pwd)" && + + # stop does not remove plist files, but boots them out + rm expect && + for frequency in hourly daily weekly + do + PLIST="$HOME/Library/LaunchAgents/org.git-scm.git.$frequency.plist" && + grep schedule=$frequency "$PLIST" && + echo "bootout gui/$UID $PLIST" >>expect || return 1 + done && + test_cmp expect args +' + test_expect_success 'register preserves existing strategy' ' git config maintenance.strategy none && git maintenance register && diff --git a/t/test-lib.sh b/t/test-lib.sh index 4a60d1ed76..620ffbf3af 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -1703,6 +1703,10 @@ test_lazy_prereq REBASE_P ' test -z "$GIT_TEST_SKIP_REBASE_P" ' +test_lazy_prereq MACOS_MAINTENANCE ' + launchctl list +' + # 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 From patchwork Tue Nov 3 14:03:16 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Derrick Stolee X-Patchwork-Id: 11877617 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=-9.6 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, 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 008B0C2D0A3 for ; Tue, 3 Nov 2020 14:04:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 95FCA2236F for ; Tue, 3 Nov 2020 14:04:53 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="OH/MwMwm" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729469AbgKCOEw (ORCPT ); Tue, 3 Nov 2020 09:04:52 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43084 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729378AbgKCODW (ORCPT ); Tue, 3 Nov 2020 09:03:22 -0500 Received: from mail-wm1-x342.google.com (mail-wm1-x342.google.com [IPv6:2a00:1450:4864:20::342]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6CDE4C0617A6 for ; Tue, 3 Nov 2020 06:03:22 -0800 (PST) Received: by mail-wm1-x342.google.com with SMTP id p19so3467089wmg.0 for ; Tue, 03 Nov 2020 06:03:22 -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=0dWYwY1UMHclsAWSGhds6Et6Ts3N5pObvdJeHdFKCN8=; b=OH/MwMwmlvoVK5rzNJxhmzI/vzArxqnKGwAhHulpZw1z8Ei4iNy9vmfngL8mFmAZO+ fEhXBgCUwGA7c1jHZ8unT0/8uIf72V8DioDzDQwtcRfz0qTomonJgVgEGdKIo3n3dDof B6bermDlnrNx5snhPrwJF/WRd5szrQp/qvSx3q7ATxnu5EJA03ikOXmGir3M9MBYQBCQ KhXvDDNG4LhLqQ/5wmBHSohfOv9RKgj1UpGvndpA/hcnwQn2v5lS2TDBSbxYT6NLZs+d npIBuCfBJFlXsBWsHa1S7kK4q0pCgV8XJ1Yr6D0rP+6W0I0YpM067F6nZf0+qN577SQz hRJA== 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=0dWYwY1UMHclsAWSGhds6Et6Ts3N5pObvdJeHdFKCN8=; b=VSuZP270oSGDw94uWH3CnS3iWusd8gxXxNxMmannRGpP5sCsYp5VfJJ5KX49xrCkeg /Rer6BPcThgpVjQoXZZ5kZ93VWG4EDQShcRXZHHEWiEmOWp1b4yU02dNiN9ARZb5WzHc YVkMt+OPHX//W8BWBwPgDgYxQiITvnC+DfJz2RhV7XHdU+EVzy0/vGZBjBX8VZAzaRUZ C1WlX32Zsa7VLT4/56Dq1wmfTZu2jzKd+7+tLMxm0B23WNMuhV26rVBWGe/yNWU8bYYA WcWYjiO1omu3w6RU7ovuVf0n++O7uX6NKp8KyBBcQS2k4BKd5PD487giATMLo/y9V0as 0dbA== X-Gm-Message-State: AOAM531XpPhq2uvyBD8FPJxL6IFdNXeZZuUV9tmtFcFZXdlB3y/P/9Lh WsoxAQioxs80vcIQ0HJdlW3bAxdnP98= X-Google-Smtp-Source: ABdhPJyIg8XiSmmUVa3cL82UwH/Ux3LE5h17HimstO8rdh2wBkngLFM1C+GKZ/mS8vFwRxP2pT1VGg== X-Received: by 2002:a1c:1d92:: with SMTP id d140mr3799771wmd.48.1604412200801; Tue, 03 Nov 2020 06:03:20 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id u3sm26655169wrq.19.2020.11.03.06.03.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Nov 2020 06:03:20 -0800 (PST) Message-Id: In-Reply-To: References: Date: Tue, 03 Nov 2020 14:03:16 +0000 Subject: [PATCH 3/3] 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, Derrick Stolee , =?utf-8?b?xJBvw6BuIFRy4bqnbiBDw7RuZw==?= Danh , Martin =?utf-8?b?w4VncmVu?= , 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'. 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/. Signed-off-by: Derrick Stolee --- builtin/gc.c | 181 +++++++++++++++++++++++++++++++++++++++++ t/t7900-maintenance.sh | 40 ++++++++- 2 files changed, 218 insertions(+), 3 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index fa0ae63a80..24511fec2e 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1698,6 +1698,187 @@ static int platform_update_schedule(int run_maintenance, int fd) else return remove_plists(); } + +#elif defined(GIT_WINDOWS_NATIVE) + +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 *get_task_name(const char *frequency) +{ + struct strbuf label = STRBUF_INIT; + strbuf_addf(&label, "Git Maintenance (%s)", frequency); + return strbuf_detach(&label, NULL); +} + +static int remove_task(enum schedule_priority schedule) +{ + int result; + struct strvec args = STRVEC_INIT; + const char *frequency = get_frequency(schedule); + char *name = get_task_name(frequency); + const char *schtasks = getenv("GIT_TEST_CRONTAB"); + if (!schtasks) + schtasks = "schtasks"; + + strvec_split(&args, schtasks); + 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 remove_scheduled_tasks(void) +{ + return remove_task(SCHEDULE_HOURLY) || + remove_task(SCHEDULE_DAILY) || + remove_task(SCHEDULE_WEEKLY); +} + +static int schedule_task(const char *exec_path, enum schedule_priority schedule) +{ + int result; + struct strvec args = STRVEC_INIT; + const char *xml, *schtasks; + char *xmlpath; + FILE *xmlfp; + const char *frequency = get_frequency(schedule); + char *name = get_task_name(frequency); + + xmlpath = xstrfmt("%s/schedule-%s.xml", + the_repository->objects->odb->path, + frequency); + xmlfp = fopen(xmlpath, "w"); + if (!xmlfp) + die(_("failed to open '%s'"), xmlpath); + + xml = "\n" + "\n" + "\n" + "\n"; + fprintf(xmlfp, xml); + + switch (schedule) { + case SCHEDULE_HOURLY: + fprintf(xmlfp, + "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(xmlfp, + "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(xmlfp, + "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(xmlfp, xml, exec_path, exec_path, frequency); + fclose(xmlfp); + + schtasks = getenv("GIT_TEST_CRONTAB"); + if (!schtasks) + schtasks = "schtasks"; + strvec_split(&args, schtasks); + strvec_pushl(&args, "/create", "/tn", name, "/f", "/xml", xmlpath, NULL); + + result = run_command_v_opt(args.v, 0); + + strvec_clear(&args); + unlink(xmlpath); + free(xmlpath); + free(name); + return result; +} + +static int add_scheduled_tasks(void) +{ + const char *exec_path = git_exec_path(); + + return schedule_task(exec_path, SCHEDULE_HOURLY) || + schedule_task(exec_path, SCHEDULE_DAILY) || + schedule_task(exec_path, SCHEDULE_WEEKLY); +} + +static int platform_update_schedule(int run_maintenance, int fd) +{ + if (run_maintenance) + return add_scheduled_tasks(); + else + return remove_scheduled_tasks(); +} + #else #define BEGIN_LINE "# BEGIN GIT MAINTENANCE SCHEDULE" #define END_LINE "# END GIT MAINTENANCE SCHEDULE" diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index f0210aa206..73dc0078da 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -367,7 +367,7 @@ test_expect_success 'register and unregister' ' test_cmp before actual ' -test_expect_success !MACOS_MAINTENANCE 'start from empty cron table' ' +test_expect_success !MACOS_MAINTENANCE,!MINGW 'start from empty cron table' ' GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance start && # start registers the repo @@ -378,7 +378,7 @@ test_expect_success !MACOS_MAINTENANCE 'start from empty cron table' ' grep "for-each-repo --config=maintenance.repo maintenance run --schedule=weekly" cron.txt ' -test_expect_success !MACOS_MAINTENANCE 'stop from existing schedule' ' +test_expect_success !MACOS_MAINTENANCE,!MINGW 'stop from existing schedule' ' GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance stop && # stop does not unregister the repo @@ -389,7 +389,7 @@ test_expect_success !MACOS_MAINTENANCE 'stop from existing schedule' ' test_must_be_empty cron.txt ' -test_expect_success !MACOS_MAINTENANCE 'start preserves existing schedule' ' +test_expect_success !MACOS_MAINTENANCE,!MINGW 'start preserves existing schedule' ' echo "Important information!" >cron.txt && GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance start && grep "Important information!" cron.txt @@ -441,6 +441,40 @@ test_expect_success MACOS_MAINTENANCE 'start and stop macOS maintenance' ' test_cmp expect args ' +test_expect_success MINGW 'start and stop Windows maintenance' ' + echo "echo \$@ >>args" >print-args && + chmod a+x print-args && + + rm -f args && + GIT_TEST_CRONTAB="/bin/sh print-args" git maintenance start && + cat args && + + # start registers the repo + git config --get --global maintenance.repo "$(pwd)" && + + rm expect && + for frequency in hourly daily weekly + do + echo "/create /tn Git Maintenance ($frequency) /f /xml .git/objects/schedule-$frequency.xml" >>expect \ + || return 1 + done && + test_cmp expect args && + + rm -f args && + GIT_TEST_CRONTAB="/bin/sh print-args" git maintenance stop && + + # stop does not unregister the repo + git config --get --global maintenance.repo "$(pwd)" && + + rm expect && + for frequency in hourly daily weekly + do + echo "/delete /tn Git Maintenance ($frequency) /f" >>expect \ + || return 1 + done && + test_cmp expect args +' + test_expect_success 'register preserves existing strategy' ' git config maintenance.strategy none && git maintenance register && From patchwork Wed Nov 4 20:06:08 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Derrick Stolee X-Patchwork-Id: 11882039 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=-9.6 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, 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 7B8A4C55179 for ; Wed, 4 Nov 2020 20:06:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 17F9620759 for ; Wed, 4 Nov 2020 20:06:19 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="SJl6YJnI" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731457AbgKDUGR (ORCPT ); Wed, 4 Nov 2020 15:06:17 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42200 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731451AbgKDUGQ (ORCPT ); Wed, 4 Nov 2020 15:06:16 -0500 Received: from mail-wr1-x442.google.com (mail-wr1-x442.google.com [IPv6:2a00:1450:4864:20::442]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9F1FFC0613D3 for ; Wed, 4 Nov 2020 12:06:14 -0800 (PST) Received: by mail-wr1-x442.google.com with SMTP id b8so23420108wrn.0 for ; Wed, 04 Nov 2020 12:06:14 -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=E1oTRbYlN4h2w+qTySOblOHiEG3aB0pPuDaEFN0maQQ=; b=SJl6YJnIcEhOXqhMMU27OIiHsKwrO14ru8PjV+r8Urhsq4O6Us27/WRx9j4YRKK9Pn EgA6NW58/GzwVXKcAkd79G0XMFlQxmdRnl3OX/Kn0MXpaAROiXiPcHmlkJMt9CopapHq SoNHZP1QIEqdjscWRn9JrjAq3zQQFuOzHbMY4Lz3YqCEa3fPb//MX/8gxjIdnasuR4Tk 8tzk2OVYuDpQDNte2t+XetOIJ5IvNMpIvzapt436ulSfz+3bKt3NlrJBVqldCSvrdex9 8bPY9e4NMv4v/fP0Tlt2YwPGpTZp5W3WWYukQtwz4jjuLfXpO6o4n50Y3z0ZmVkjf4nV 87AA== 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=E1oTRbYlN4h2w+qTySOblOHiEG3aB0pPuDaEFN0maQQ=; b=YhsRL5CghnX5GBPSyiEc59hI0bRll/rZgtvhM7RF0Yyec9MuCtM9aM5l7VipJf9q38 ONH3uLvAhT0Ift1XXUJzdzbnUAHt1PeG91247FdnQGMamOYFoCZHu+eAqmybtppnxxDm E+0HimBILTPgiAYHWsOsRU90oA5Bu8u+1H2gTwPldghEMdT8pxNNxp0k4Lof3fl1qk2v /PV4q82UwtwMxQIdCna1se2gsaEaEhggKbU5qIyHFO1xCo5Ii9b2N4bMjTo8+Z29ek2O UdSfQnzAWlntDvw6aWsXuKpHjt2GJTsyqdB5O6lNQTUMxlYy208xJ65BlYiMHwJ17YBY NWLA== X-Gm-Message-State: AOAM530JLNC0K3Tx1aUH1pYqMl1iezpD60pQOpJKww7fcBUAn7dGnqVs sYYXoK4S05PbksppYhhsykcJYJ2cdRM= X-Google-Smtp-Source: ABdhPJw2mKWY/9jqxQ757FBItLG8UtMNe9jLXBS+Wv90bCZlOqx2vmP/PBNBxFwF5Ds9CtxVKmObZA== X-Received: by 2002:adf:eaca:: with SMTP id o10mr33393294wrn.9.1604520373045; Wed, 04 Nov 2020 12:06:13 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id d2sm4329484wrq.34.2020.11.04.12.06.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 04 Nov 2020 12:06:12 -0800 (PST) Message-Id: <84eb44de31f04b2a94f57ee11d70be81f5bbeee2.1604520368.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Wed, 04 Nov 2020 20:06:08 +0000 Subject: [PATCH v2 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, Derrick Stolee , =?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'. 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/. Helped-by: Eric Sunshine Signed-off-by: Derrick Stolee --- Documentation/git-maintenance.txt | 22 ++++ builtin/gc.c | 181 ++++++++++++++++++++++++++++++ t/t7900-maintenance.sh | 36 +++++- 3 files changed, 236 insertions(+), 3 deletions(-) diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt index 451ebac131..f4f6a4091b 100644 --- a/Documentation/git-maintenance.txt +++ b/Documentation/git-maintenance.txt @@ -316,6 +316,28 @@ https://developer.apple.com/library/archive/documentation/MacOSX/Conceptual/BPSy 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 7604064a8d..80f43a59ce 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1698,6 +1698,187 @@ static int platform_update_schedule(int run_maintenance, int fd) else return remove_plists(); } + +#elif defined(GIT_WINDOWS_NATIVE) + +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 *get_task_name(const char *frequency) +{ + struct strbuf label = STRBUF_INIT; + strbuf_addf(&label, "Git Maintenance (%s)", frequency); + return strbuf_detach(&label, NULL); +} + +static int remove_task(enum schedule_priority schedule) +{ + int result; + struct strvec args = STRVEC_INIT; + const char *frequency = get_frequency(schedule); + char *name = get_task_name(frequency); + const char *schtasks = getenv("GIT_TEST_CRONTAB"); + if (!schtasks) + schtasks = "schtasks"; + + strvec_split(&args, schtasks); + 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 remove_scheduled_tasks(void) +{ + return remove_task(SCHEDULE_HOURLY) || + remove_task(SCHEDULE_DAILY) || + remove_task(SCHEDULE_WEEKLY); +} + +static int schedule_task(const char *exec_path, enum schedule_priority schedule) +{ + int result; + struct strvec args = STRVEC_INIT; + const char *xml, *schtasks; + char *xmlpath; + FILE *xmlfp; + const char *frequency = get_frequency(schedule); + char *name = get_task_name(frequency); + + xmlpath = xstrfmt("%s/schedule-%s.xml", + the_repository->objects->odb->path, + frequency); + xmlfp = fopen(xmlpath, "w"); + if (!xmlfp) + die(_("failed to open '%s'"), xmlpath); + + xml = "\n" + "\n" + "\n" + "\n"; + fprintf(xmlfp, xml); + + switch (schedule) { + case SCHEDULE_HOURLY: + fprintf(xmlfp, + "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(xmlfp, + "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(xmlfp, + "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(xmlfp, xml, exec_path, exec_path, frequency); + fclose(xmlfp); + + schtasks = getenv("GIT_TEST_CRONTAB"); + if (!schtasks) + schtasks = "schtasks"; + strvec_split(&args, schtasks); + strvec_pushl(&args, "/create", "/tn", name, "/f", "/xml", xmlpath, NULL); + + result = run_command_v_opt(args.v, 0); + + strvec_clear(&args); + unlink(xmlpath); + free(xmlpath); + free(name); + return result; +} + +static int add_scheduled_tasks(void) +{ + const char *exec_path = git_exec_path(); + + return schedule_task(exec_path, SCHEDULE_HOURLY) || + schedule_task(exec_path, SCHEDULE_DAILY) || + schedule_task(exec_path, SCHEDULE_WEEKLY); +} + +static int platform_update_schedule(int run_maintenance, int fd) +{ + if (run_maintenance) + return add_scheduled_tasks(); + else + return remove_scheduled_tasks(); +} + #else #define BEGIN_LINE "# BEGIN GIT MAINTENANCE SCHEDULE" #define END_LINE "# END GIT MAINTENANCE SCHEDULE" diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index 1c43b34a93..e7ad130cbc 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -367,7 +367,7 @@ test_expect_success 'register and unregister' ' test_cmp before actual ' -test_expect_success !MACOS_MAINTENANCE 'start from empty cron table' ' +test_expect_success !MACOS_MAINTENANCE,!MINGW 'start from empty cron table' ' GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance start && # start registers the repo @@ -378,7 +378,7 @@ test_expect_success !MACOS_MAINTENANCE 'start from empty cron table' ' grep "for-each-repo --config=maintenance.repo maintenance run --schedule=weekly" cron.txt ' -test_expect_success !MACOS_MAINTENANCE 'stop from existing schedule' ' +test_expect_success !MACOS_MAINTENANCE,!MINGW 'stop from existing schedule' ' GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance stop && # stop does not unregister the repo @@ -389,7 +389,7 @@ test_expect_success !MACOS_MAINTENANCE 'stop from existing schedule' ' test_must_be_empty cron.txt ' -test_expect_success !MACOS_MAINTENANCE 'start preserves existing schedule' ' +test_expect_success !MACOS_MAINTENANCE,!MINGW 'start preserves existing schedule' ' echo "Important information!" >cron.txt && GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance start && grep "Important information!" cron.txt @@ -442,6 +442,36 @@ test_expect_success MACOS_MAINTENANCE 'start and stop macOS maintenance' ' test_cmp expect args ' +test_expect_success MINGW 'start and stop Windows maintenance' ' + write_script print-args <<-\EOF && + echo $* >>args + EOF + + rm -f args && + GIT_TEST_CRONTAB="/bin/sh print-args" git maintenance start && + + # start registers the repo + git config --get --global maintenance.repo "$(pwd)" && + + for frequency in hourly daily weekly + do + printf "/create /tn Git Maintenance (%s) /f /xml .git/objects/schedule-%s.xml\n" \ + $frequency $frequency + done >expect && + test_cmp expect args && + + rm -f args && + GIT_TEST_CRONTAB="/bin/sh 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 &&