Re: [PATCH v4 3/4] maintenance: use launchctl on macOS

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Nov 18, 2020, 06:45 UTC
Message-ID: <CAPig+cRQJ9VpRk_0oz4BP3GF7dtsHtE3Qcd=nGsS26=JZvps7g@mail.gmail.com>
In-Reply-To: <ed0a0011fb0b06780bc6ce2920f17cad46eccdf9.1605647598.git.gitgitgadget@gmail.com>

On Tue, Nov 17, 2020 at 4:13 PM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 6 quoted lines

> +static int launchctl_boot_plist(int enable, const char *filename, const char *cmd)
> +{
> +       child.no_stderr = 1;
> +       child.no_stdout = 1;
> +       if (start_command(&child))
> +               die(_("failed to start launchctl"));

Did you have any thoughts on the observation I made in a followup response[1] during review of v3 in which I suggested that we might be able to avoid suppressing stderr (and stdout) here? In particular, the idea was that if, in launchctl_schedule_plist(), we do a simple existence check for the .plist file and only call launchctl_boot_plist(0,...) to `bootout` the .plist file if it exists, then we shouldn't need to muck with stderr/stdout suppression. The benefit is that if `bootout` fails for some reason, then the user would see the (hopefully) meaningful error message emitted `launchctl bootout`.

The same .plist existence check could be done in launchctl_remove_plist() before trying to `bootout` the file.

Anyhow, such refinement can be done later is desired, so not worth a re-roll, but I was curious about your thoughts on the issue.

[1]: https://lore.kernel.org/git/CAPig+cTRJb-fn2R6rJO1hkeCc_ehVhkNufO4=LhtPQudVeonnA@mail.gmail.com/