Re: [PATCH 3/3] maintenance: use Windows scheduled tasks

From: Derrick Stolee <stolee@gmail.com>
To: Eric Sunshine <sunshine@sunshineco.com>,
	Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Cc: "Git List" <git@vger.kernel.org>,
	"Jonathan Nieder" <jrnieder@gmail.com>,
	"Jonathan Tan" <jonathantanmy@google.com>,
	"Son Luong Ngoc" <sluongng@gmail.com>,
	"Đoàn Trần Công Danh" <congdanhqx@gmail.com>,
	"Martin Ågren" <martin.agren@gmail.com>,
	"Derrick Stolee" <derrickstolee@github.com>,
	"Derrick Stolee" <dstolee@microsoft.com>
Subject: Re: [PATCH 3/3] maintenance: use Windows scheduled tasks
Date: Tue, 3 Nov 2020 16:23:52 -0500	[thread overview]
Message-ID: <3268cfb4-22b4-301d-abbc-bb36fa737a2a@gmail.com> (raw)
In-Reply-To: <CAPig+cRE3RNkUCrmmTvVC4mm1crT+1OEU5zSwu80pjXmDdAkfg@mail.gmail.com>

On 11/3/2020 2:06 PM, Eric Sunshine wrote:
> On Tue, Nov 3, 2020 at 9:05 AM Derrick Stolee via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>> 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!
> 
> I wonder if you could use the technique explained in [1] to prevent
> the console window from popping up.
> 
> [1]: https://pureinfotech.com/prevent-command-window-appearing-scheduled-tasks-windows-10/

The critical part of that strategy is the "Run whether the user is
logged in or not". The resulting option that triggers causes the
schtasks command to require a password prompt (or a password passed
as a command-line argument). I found that interaction to be too
disruptive.

>> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
>> ---
>> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
>> @@ -441,6 +441,40 @@ test_expect_success MACOS_MAINTENANCE 'start and stop macOS maintenance' '
>> +test_expect_success MINGW 'start and stop Windows maintenance' '
>> +       echo "echo \$@ >>args" >print-args &&
>> +       chmod a+x print-args &&
> 
> Same comments as my review of [2/3] regarding $@ and write_script().

Noted!

>> +       rm -f args &&
>> +       GIT_TEST_CRONTAB="/bin/sh print-args" git maintenance start &&
>> +       cat args &&
> 
> Is this 'cat' leftover debugging gunk?

Yes. Thanks.

>> +       # 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 &&
> 
> Rather than using >> within the loop, it's often simpler to capture
> the output of the for-loop in its entirety:
> 
>     for frequency in hourly daily weekly
>     do
>         echo "/create ..." || return 1
>     done >expect &&
> 
> However, in this case 'printf' may be even simpler:
> 
>     printf "/create /tn ... .git/objects/schedule-%s.xml\n" \
>         hourly daily weekly >expect &&

Excellent.

>> +       GIT_TEST_CRONTAB="/bin/sh print-args"  git maintenance stop &&
> 
> Too many spaces before the 'git' command.

Thanks!
-Stolee