| Message ID | pull.776.v6.git.1607542142.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Maintenance IV: Platform-specific background maintenance | expand |
"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes: > This is based on ds/maintenance-part-3. > > After sitting with the background maintenance as it has been cooking, I > wanted to come back around and implement the background maintenance for > Windows. However, I noticed that there were some things bothering me with > background maintenance on my macOS machine. These are detailed in PATCH 3, > but the tl;dr is that 'cron' is not recommended by Apple and instead > 'launchd' satisfies our needs. > > This series implements the background scheduling so git maintenance > (start|stop) works on those platforms. I've been operating with these > schedules for a while now without the problems described in the patches. > > There is a particularly annoying case about console windows popping up on > Windows, but PATCH 4 describes a plan to get around that. > > > Update in V6 > ============ > > * The Windows platform uses the tempfile API a bit better, including using > the frequency in the filename to make the test simpler. Are two fix-up patches from Eric that have been queued near the top of ds/maintenance-part-4 still relevant? At least, the "when invoked individually" patch that added an "-f" option to two invocations of "rm" is still applicable, I would think (I didn't look at the other one). commit e3801c41e4d4cb1dd899942e04ab78310e781d07 Author: Eric Sunshine <sunshine@sunshineco.com> t7900: make macOS-specific test work on Windows Notes (amlog): Message-Id: <20201130044224.12298-3-sunshine@sunshineco.com> commit 1e5ddd79e2da18ee19b665a045d4187c5dc6234e Author: Eric Sunshine <sunshine@sunshineco.com> t7900: fix test failures when invoked individually via --run Notes (amlog): Message-Id: <20201130044224.12298-2-sunshine@sunshineco.com>
On Wed, Dec 9, 2020 at 7:33 PM Junio C Hamano <gitster@pobox.com> wrote: > "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes: > > Update in V6 > > ============ > > > > * The Windows platform uses the tempfile API a bit better, including using > > the frequency in the filename to make the test simpler. > > Are two fix-up patches from Eric that have been queued near the top > of ds/maintenance-part-4 still relevant? Both of the patches from Sunshine are still relevant atop Stolee's latest (v6), and they should apply cleanly (I would think) since v6 didn't change anything related to those patches.
Eric Sunshine <sunshine@sunshineco.com> writes: > On Wed, Dec 9, 2020 at 7:33 PM Junio C Hamano <gitster@pobox.com> wrote: >> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes: >> > Update in V6 >> > ============ >> > >> > * The Windows platform uses the tempfile API a bit better, including using >> > the frequency in the filename to make the test simpler. >> >> Are two fix-up patches from Eric that have been queued near the top >> of ds/maintenance-part-4 still relevant? > > Both of the patches from Sunshine are still relevant atop Stolee's > latest (v6), and they should apply cleanly (I would think) since v6 > didn't change anything related to those patches. Yup, I tried rebasing these two and they applied cleanly, so I'll include them in today's pushout (which I haven't finished yet). I probably would not notice if the updated 4-patch series already solved the issue in another way without causing the textual conflict with your two fix-up patches, though ;-) Thanks.
On 12/9/2020 8:04 PM, Junio C Hamano wrote: > Eric Sunshine <sunshine@sunshineco.com> writes: > >> On Wed, Dec 9, 2020 at 7:33 PM Junio C Hamano <gitster@pobox.com> wrote: >>> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes: >>>> Update in V6 >>>> ============ >>>> >>>> * The Windows platform uses the tempfile API a bit better, including using >>>> the frequency in the filename to make the test simpler. >>> >>> Are two fix-up patches from Eric that have been queued near the top >>> of ds/maintenance-part-4 still relevant? >> >> Both of the patches from Sunshine are still relevant atop Stolee's >> latest (v6), and they should apply cleanly (I would think) since v6 >> didn't change anything related to those patches. > > Yup, I tried rebasing these two and they applied cleanly, so I'll > include them in today's pushout (which I haven't finished yet). > > I probably would not notice if the updated 4-patch series already > solved the issue in another way without causing the textual conflict > with your two fix-up patches, though ;-) I noticed a subtle issue with the v6 series, so I _will_ reroll the series squashing in Eric's patches. He will remain a co-author and I'll add the Helped-by: Ævar along with the details for that patch. Thanks, -Stolee
This is based on ds/maintenance-part-3. After sitting with the background maintenance as it has been cooking, I wanted to come back around and implement the background maintenance for Windows. However, I noticed that there were some things bothering me with background maintenance on my macOS machine. These are detailed in PATCH 3, but the tl;dr is that 'cron' is not recommended by Apple and instead 'launchd' satisfies our needs. This series implements the background scheduling so git maintenance (start|stop) works on those platforms. I've been operating with these schedules for a while now without the problems described in the patches. There is a particularly annoying case about console windows popping up on Windows, but PATCH 4 describes a plan to get around that. Update in V6 ============ * The Windows platform uses the tempfile API a bit better, including using the frequency in the filename to make the test simpler. Thanks, -Stolee cc: jrnieder@gmail.com cc: jonathantanmy@google.com cc: sluongng@gmail.com cc: Đoàn Trần Công Danh congdanhqx@gmail.com cc: Martin Ågren martin.agren@gmail.com cc: Eric Sunshine sunshine@sunshineco.com cc: Derrick Stolee stolee@gmail.com Derrick Stolee (4): maintenance: extract platform-specific scheduling maintenance: include 'cron' details in docs maintenance: use launchctl on macOS maintenance: use Windows scheduled tasks Documentation/git-maintenance.txt | 116 ++++++++ builtin/gc.c | 421 ++++++++++++++++++++++++++++-- t/t7900-maintenance.sh | 105 +++++++- t/test-lib.sh | 7 +- 4 files changed, 615 insertions(+), 34 deletions(-) base-commit: 0016b618182f642771dc589cf0090289f9fe1b4f Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-776%2Fderrickstolee%2Fmaintenance%2FmacOS-v6 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-776/derrickstolee/maintenance/macOS-v6 Pull-Request: https://github.com/gitgitgadget/git/pull/776 Range-diff vs v5: 1: 4807342b001 = 1: 4807342b001 maintenance: extract platform-specific scheduling 2: 7cc70a8fe7b = 2: 7cc70a8fe7b maintenance: include 'cron' details in docs 3: cd015a5cbd7 = 3: cd015a5cbd7 maintenance: use launchctl on macOS 4: ac9a28bea39 ! 4: 6ad4a6b98c6 maintenance: use Windows scheduled tasks @@ Commit message 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. + prefix the random characters with the frequency so it is easier to + examine the proper file during tests. 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 @@ builtin/gc.c: static int launchctl_update_schedule(int run_maintenance, int fd, + struct tempfile *tfile; + const char *frequency = get_frequency(schedule); + char *name = schtasks_task_name(frequency); ++ struct strbuf tfilename = STRBUF_INIT; + -+ tfile = xmks_tempfile("schedule_XXXXXX"); -+ if (!tfile || !fdopen_tempfile(tfile, "w")) ++ strbuf_addf(&tfilename, "schedule_%s_XXXXXX", frequency); ++ tfile = xmks_tempfile(tfilename.buf); ++ strbuf_release(&tfilename); ++ ++ if (!fdopen_tempfile(tfile, "w")) + die(_("failed to create temp xml file")); + + xml = "<?xml version=\"1.0\" encoding=\"US-ASCII\"?>\n" @@ builtin/gc.c: static int launchctl_update_schedule(int run_maintenance, int fd, + "</Task>\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); ++ strvec_pushl(&child.args, "/create", "/tn", name, "/f", "/xml", ++ get_tempfile_path(tfile), NULL); + close_tempfile_gently(tfile); + + child.no_stdout = 1; @@ t/t7900-maintenance.sh: test_expect_success !MINGW 'start and stop macOS mainten + *) shift ;; + esac + done -+ lines=$(wc -l args | awk "{print \$1;}") -+ test -z "$xmlfile" || cp "$xmlfile" "schedule-$lines.xml" ++ test -z "$xmlfile" || cp "$xmlfile" "$xmlfile.xml" + EOF + + rm -f args && -+ GIT_TEST_MAINT_SCHEDULER="schtasks:./print-args" git maintenance start && ++ GIT_TEST_MAINT_SCHEDULER="schtasks:./print-args" GIT_TRACE2_PERF=1 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 ++ grep "/create /tn Git Maintenance ($frequency) /f /xml" args && ++ file=$(ls schedule_$frequency*.xml) && ++ test_xmllint "$file" && ++ grep "encoding=.US-ASCII." "$file" || return 1 + done && + + rm -f args &&