Hi Antoine,
Antoine Jacoutot wrote on Mon, Dec 07, 2020 at 09:48:36AM +0100:
> On Sun, Dec 06, 2020 at 10:52:37PM +0100, Alexander Hall wrote:
>> On December 6, 2020 8:13:26 PM GMT+01:00, Antoine Jacoutot wrote:
>>> On Sun, Dec 06, 2020 at 05:20:31PM +0000, Stuart Henderson wrote:
>>>> On 2020/12/06 16:39, Otto Moerbeek wrote:
>>>>> On Sun, Dec 06, 2020 at 03:31:19PM +0000, SW wrote:
>>>>>> On 06/12/2020 14:32, Otto Moerbeek wrote:
>>>>>>> On Sun, Dec 06, 2020 at 02:19:05PM +0000, SW wrote:
>>>>>>>> I've been looking to have syspatch give me a quick indication
>>>>>>>> of whether a reboot is likely to be required. As a quick and
>>>>>>>> dirty check, I've just been treating "Were patches applied?"
>>>>>>>> as the indicator.
>>>>>>>> The following diff will cause syspatch to exit when applying
>>>>>>>> patches with status code 0 only if patches were actually applied.
>>>>>>>> My biggest concern is that it does cause a change in behaviour,
>>>>>>>> so perhaps this either needs making into an option or a different
>>>>>>>> approach entirely?
[...]
>>>>>>> I don't this is correct since it maks syspatch exit 1 on "no
>>>>>>> patches applied".
>>>>>> That's precisely the idea- from previous discussion with a couple
>>>>>> of people there didn't seem to be an easy (programmatic) way to
>>>>>> figure out whether syspatch had done anything or not.
>>>>> exit code 1 normally used for error conditions. A system being
>>>>> up-to-date is not an error condition.
>>>>>> Doing this would be a bit of a blunt way of handling things, and
>>>>>> perhaps it would be better gated behind a flag, but is there a
>>>>>> better way to make a scripted update work automatically
>>>>>> (including rebooting as necessary)?
>>>> How about the same exit codes as acme-client? They seem fairly
>>>> reasonable - 0=updated, 1=failure, 2=no change.
>>> I wouldn't object to that.
>> So that'd boil down to
>> $_PATCH_APPLIED || exit 4
>> or
>> $_PATCH_APPLIED && exit
>> exit 4
>> ...if the explicit exit feels better instead of just running to the
>> end of the script.
>> But maybe this script prefers some more verbosity... :-)
> Something like this?
I'm on the fence whether regarding an up-to-date system as an error
condition makes sense; i neither like the idea nor object to it.
But if you decide to head into that direction, i suggest to try
and make the documentation more precise. "No patch is available"
sounds as if the OpenBSD project did not yet provide a single
patch for the release in question.
> Index: syspatch.8
> ===================================================================
> RCS file: /cvs/src/usr.sbin/syspatch/syspatch.8,v
> retrieving revision 1.21
> diff -u -p -r1.21 syspatch.8
> --- syspatch.8 25 Jul 2020 15:45:44 -0000 1.21
> +++ syspatch.8 7 Dec 2020 08:47:48 -0000
> @@ -64,6 +64,7 @@ of installed patches.
> .El
> .Sh EXIT STATUS
> .Ex -std syspatch
> +2 indicates that no patch is available.
To describe your proposed implementation, i think you would have
to say something like:
2 indicates that no additional patch was installed.
But i doubt that is what you actually intend to do.
> .Sh SEE ALSO
> .Xr signify 1 ,
> .Xr installurl 5 ,
> Index: syspatch.sh
> ===================================================================
> RCS file: /cvs/src/usr.sbin/syspatch/syspatch.sh,v
> retrieving revision 1.166
> diff -u -p -r1.166 syspatch.sh
> --- syspatch.sh 27 Oct 2020 17:42:05 -0000 1.166
> +++ syspatch.sh 7 Dec 2020 08:47:48 -0000
> @@ -248,7 +248,8 @@ must be run manually to install the new
> fi
> fi
>
> - ${_PATCH_APPLIED} && echo "Errata can be reviewed under ${_PDIR}"
> + ${_PATCH_APPLIED} && echo "Errata can be reviewed under ${_PDIR}" ||
> + return 2
This doesn't appear to work unless i screwed up my testing.
Even if _PATCH_APPLIED==false, syspatch still exits 0 for me.
The return value of the trap function doesn't appear to affect
the exit status of the script.
If i change "return 2" to "exit 2", (and insert a debugging statement
echo _PATCH_APPLIED = ${_PATCH_APPLIED}
right before the "${_PATCH_APPLIED} && ..." test), then i see
the following potentially unintended behaviour:
# /usr/sbin/syspatch -r ; echo $?
Reverting patch 007_xmaplen
_PATCH_APPLIED = false
2
That means -r would never again succeed? And now:
# /usr/sbin/syspatch -c ; echo $?
007_xmaplen
_PATCH_APPLIED = false
2
# /usr/sbin/syspatch -l ; echo $?
001_bgpd
002_icmp6
003_tmux
004_wg
005_unwind
006_rpki
_PATCH_APPLIED = false
2
So listing patches is an error when new patches are available?
# /usr/sbin/syspatch ; echo $?
Get/Verify syspatch68-007_xmaplen... 100% |*************| 4547 KB 00:00
Installing patch 007_xmaplen
_PATCH_APPLIED = true
Errata can be reviewed under /var/syspatch
0
That is the only test result matching what i think you might intend.
But now, this still happens:
# /usr/sbin/syspatch -c ; echo $?
_PATCH_APPLIED = false
2
# /usr/sbin/syspatch -l ; echo $?
001_bgpd
002_icmp6
003_tmux
004_wg
005_unwind
006_rpki
007_xmaplen
_PATCH_APPLIED = false
2
Listing patches is an error no matter whether or not patches are
available, no matter whether these patches are already installed
or not?
I doubt the patch is ready for commit in its present state.
Yours,
Ingo
