Re: syspatch exit state
On Mon, 7 Dec 2020 13:39:30 +0100
Antoine Jacoutot :
> 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.825 Jul 2020 15:45:44 - 1.21
> +++ syspatch.87 Dec 2020 12:39:07 -
> @@ -64,6 +64,8 @@ of installed patches.
> .El
> .Sh EXIT STATUS
> .Ex -std syspatch
> +In particular, 2 indicates that applying patches was requested but no
> +additional patch was installed.
> .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 - 1.166
> +++ syspatch.sh 7 Dec 2020 12:39:07 -
> @@ -320,6 +320,7 @@ if ((OPTIND == 1)); then
> [[ -f ${_D}/rollback.tgz ]] || rm -r ${_D}
> done
> _PATCHES=$(ls_missing) # can't use errexit in a for loop
> + [[ -n ${_PATCHES} ]] || exit 2
> for _PATCH in ${_PATCHES}; do
> apply_patch ${_OSrev}-${_PATCH}
> _PATCH_APPLIED=true
>
>
In my opinion, if committed it deserves an entry in
https://www.openbsd.org/faq/current.html
Re: syspatch exit state
Hi Antoine,
Antoine Jacoutot wrote on Mon, Dec 07, 2020 at 01:39:30PM +0100:
> On Mon, Dec 07, 2020 at 01:30:55PM +0100, Ingo Schwarze wrote:
>> Antoine Jacoutot wrote on Mon, Dec 07, 2020 at 01:01:27PM +0100:
>>> I just tested this change and it seems to work:
[...]
>> I think a slightly more explicit wording might make such a
>> misunderstanding less likely, for example:
>>
>> .Sh EXIT STATUS
>> .Ex -std syspatch
>> In particular, 2 indicates that applying patches was requested
>> but no additional patch was installed.
[...]
> Sure that's fine as well.
> I did change it to your initial proposal;
Thanks, but...
I initially documented what you originally implemented.
You changed what your implementation does, so the documentation
needed to change as well to match the second iteration of the
implementation.
> but careful, since you're a doc master
> I will put whatever you send my way ;-)
Sometimes, i am wrong, so i appreciate it when people read my
suggestions with a critical eye. Authors usually know their code
better than i do, and the documentation being correct is even
more important than being nicely worded and properly formatted. ;-)
> 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.825 Jul 2020 15:45:44 - 1.21
> +++ syspatch.87 Dec 2020 12:39:07 -
> @@ -64,6 +64,8 @@ of installed patches.
> .El
> .Sh EXIT STATUS
> .Ex -std syspatch
> +In particular, 2 indicates that applying patches was requested but no
> +additional patch was installed.
> .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 - 1.166
> +++ syspatch.sh 7 Dec 2020 12:39:07 -
> @@ -320,6 +320,7 @@ if ((OPTIND == 1)); then
> [[ -f ${_D}/rollback.tgz ]] || rm -r ${_D}
> done
> _PATCHES=$(ls_missing) # can't use errexit in a for loop
> + [[ -n ${_PATCHES} ]] || exit 2
> for _PATCH in ${_PATCHES}; do
> apply_patch ${_OSrev}-${_PATCH}
> _PATCH_APPLIED=true
Re: syspatch exit state
On Mon, Dec 07, 2020 at 01:30:55PM +0100, Ingo Schwarze wrote:
> Hello Antoine,
>
> Antoine Jacoutot wrote on Mon, Dec 07, 2020 at 01:01:27PM +0100:
>
> > I just tested this change and it seems to work:
>
> I did not repeat my testing, but here is some quick feedback purely
> from code inspection:
>
> The proposed code change makes sense to me.
>
> The proposed manual page text might allow the misconception that
> the tool will always exit 2 if no additional patch was installed,
> including when -c, -l, or -r was specified.
>
> I think a slightly more explicit wording might make such a
> misunderstanding less likely, for example:
>
> .Sh EXIT STATUS
> .Ex -std syspatch
> In particular, 2 indicates that applying patches was requested
> but no additional patch was installed.
>
> With non-standard meanings of exit codes, i think it is worthwhile
> to be as precise as possible.
Sure that's fine as well.
I did change it to your initial proposal; but careful, since you're a doc master
I will put whatever you send my way ;-)
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 - 1.21
+++ syspatch.8 7 Dec 2020 12:39:07 -
@@ -64,6 +64,8 @@ of installed patches.
.El
.Sh EXIT STATUS
.Ex -std syspatch
+In particular, 2 indicates that applying patches was requested but no
+additional patch was installed.
.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 - 1.166
+++ syspatch.sh 7 Dec 2020 12:39:07 -
@@ -320,6 +320,7 @@ if ((OPTIND == 1)); then
[[ -f ${_D}/rollback.tgz ]] || rm -r ${_D}
done
_PATCHES=$(ls_missing) # can't use errexit in a for loop
+ [[ -n ${_PATCHES} ]] || exit 2
for _PATCH in ${_PATCHES}; do
apply_patch ${_OSrev}-${_PATCH}
_PATCH_APPLIED=true
--
Antoine
Re: syspatch exit state
Hello Antoine,
Antoine Jacoutot wrote on Mon, Dec 07, 2020 at 01:01:27PM +0100:
> I just tested this change and it seems to work:
I did not repeat my testing, but here is some quick feedback purely
from code inspection:
The proposed code change makes sense to me.
The proposed manual page text might allow the misconception that
the tool will always exit 2 if no additional patch was installed,
including when -c, -l, or -r was specified.
I think a slightly more explicit wording might make such a
misunderstanding less likely, for example:
.Sh EXIT STATUS
.Ex -std syspatch
In particular, 2 indicates that applying patches was requested
but no additional patch was installed.
With non-standard meanings of exit codes, i think it is worthwhile
to be as precise as possible.
Yours,
Ingo
> 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.825 Jul 2020 15:45:44 - 1.21
> +++ syspatch.87 Dec 2020 11:58:06 -
> @@ -64,6 +64,7 @@ of installed patches.
> .El
> .Sh EXIT STATUS
> .Ex -std syspatch
> +2 indicates that no additional patch was installed.
> .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 - 1.166
> +++ syspatch.sh 7 Dec 2020 11:58:06 -
> @@ -320,6 +320,7 @@ if ((OPTIND == 1)); then
> [[ -f ${_D}/rollback.tgz ]] || rm -r ${_D}
> done
> _PATCHES=$(ls_missing) # can't use errexit in a for loop
> + [[ -n ${_PATCHES} ]] || exit 2
> for _PATCH in ${_PATCHES}; do
> apply_patch ${_OSrev}-${_PATCH}
> _PATCH_APPLIED=true
Re: syspatch exit state
On Mon, Dec 07, 2020 at 11:54:04AM +0100, Ingo Schwarze wrote:
> > 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 - 1.166
> > +++ syspatch.sh 7 Dec 2020 08:47:48 -
> > @@ -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.
<...>
> 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.
It wasn't meant for commit just yet :-)
Thanks for the feedback.
I just tested this change and it seems to work:
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 - 1.21
+++ syspatch.8 7 Dec 2020 11:58:06 -
@@ -64,6 +64,7 @@ of installed patches.
.El
.Sh EXIT STATUS
.Ex -std syspatch
+2 indicates that no additional patch was installed.
.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 - 1.166
+++ syspatch.sh 7 Dec 2020 11:58:06 -
@@ -320,6 +320,7 @@ if ((OPTIND == 1)); then
[[ -f ${_D}/rollback.tgz ]] || rm -r ${_D}
done
_PATCHES=$(ls_missing) # can't use errexit in a for loop
+ [[ -n ${_PATCHES} ]] || exit 2
for _PATCH in ${_PATCHES}; do
apply_patch ${_OSrev}-${_PATCH}
_PATCH_APPLIED=true
--
Antoine
Re: syspatch exit state
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 +, Stuart Henderson wrote:
On 2020/12/06 16:39, Otto Moerbeek wrote:
> On Sun, Dec 06, 2020 at 03:31:19PM +, SW wrote:
>> On 06/12/2020 14:32, Otto Moerbeek wrote:
>>> On Sun, Dec 06, 2020 at 02:19:05PM +, 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.825 Jul 2020 15:45:44 - 1.21
> +++ syspatch.87 Dec 2020 08:47:48 -
> @@ -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 - 1.166
> +++ syspatch.sh 7 Dec 2020 08:47:48 -
> @@ -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 KB00: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:
Re: syspatch exit state
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 +, Stuart Henderson wrote:
> >> On 2020/12/06 16:39, Otto Moerbeek wrote:
> >> > On Sun, Dec 06, 2020 at 03:31:19PM +, SW wrote:
> >> >
> >> > > On 06/12/2020 14:32, Otto Moerbeek wrote:
> >> > > > On Sun, Dec 06, 2020 at 02:19:05PM +, SW wrote:
> >> > > >
> >> > > >> Hi,
> >> > > >> 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?
> >> > > >>
> >> > > >> --- syspatch Sun Dec 6 14:11:12 2020
> >> > > >> +++ syspatch Sun Dec 6 14:10:23 2020
> >> > > >> @@ -323,3 +323,9 @@ if ((OPTIND == 1)); then
> >> > > >> _PATCH_APPLIED=true
> >> > > >> done
> >> > > >> fi
> >> > > >> +
> >> > > >> +if [ "$_PATCH_APPLIED" = "true" ]; then
> >> > > >> + exit 0
> >> > > >> +else
> >> > > >> + exit 1
> >> > > >> +fi
> >> > > >>
> >> > > >> Thanks,
> >> > > >> S
> >> > > >>
> >> > > > I don't this is correct since it maks syspatch exit 1 on "no
> >patches applied".
> >> > > >
> >> > > > -Otto
> >> > > > .
> >> > > 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.
> >> >
> >> > -Otto
> >> >
> >> >
> >> > >
> >> > > 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)?
> >> > >
> >> > > Thanks,
> >> > > S
> >> >
> >>
> >> 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?
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 - 1.21
+++ syspatch.8 7 Dec 2020 08:47:48 -
@@ -64,6 +64,7 @@ of installed patches.
.El
.Sh EXIT STATUS
.Ex -std syspatch
+2 indicates that no patch is available.
.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 - 1.166
+++ syspatch.sh 7 Dec 2020 08:47:48 -
@@ -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
}
unpriv()
--
Antoine
Re: syspatch exit state
> > Returning a non-zero exit status is generally a sign of some sort of > failure. Not applying a patch due to already having the patch applied > is not really a failure. > The joys of running Solaris 2.x patches in the late 90s, where "ERROR 02" means the above, you already have the patch. And it took ages for the OS to figure this out for each one, taking longer and longer as recommended packs grew bigger over time. -- May the most significant bit of your life be positive.
Re: syspatch exit state
On December 6, 2020 8:13:26 PM GMT+01:00, Antoine Jacoutot wrote: >On Sun, Dec 06, 2020 at 05:20:31PM +, Stuart Henderson wrote: >> On 2020/12/06 16:39, Otto Moerbeek wrote: >> > On Sun, Dec 06, 2020 at 03:31:19PM +, SW wrote: >> > >> > > On 06/12/2020 14:32, Otto Moerbeek wrote: >> > > > On Sun, Dec 06, 2020 at 02:19:05PM +, SW wrote: >> > > > >> > > >> Hi, >> > > >> 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? >> > > >> >> > > >> --- syspatch Sun Dec 6 14:11:12 2020 >> > > >> +++ syspatch Sun Dec 6 14:10:23 2020 >> > > >> @@ -323,3 +323,9 @@ if ((OPTIND == 1)); then >> > > >> _PATCH_APPLIED=true >> > > >> done >> > > >> fi >> > > >> + >> > > >> +if [ "$_PATCH_APPLIED" = "true" ]; then >> > > >> + exit 0 >> > > >> +else >> > > >> + exit 1 >> > > >> +fi >> > > >> >> > > >> Thanks, >> > > >> S >> > > >> >> > > > I don't this is correct since it maks syspatch exit 1 on "no >patches applied". >> > > > >> > > >-Otto >> > > > . >> > > 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. >> > >> >-Otto >> > >> > >> > > >> > > 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)? >> > > >> > > Thanks, >> > > S >> > >> >> 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... :-) /Alexander
Re: syspatch exit state
On Sun, Dec 06, 2020 at 05:20:31PM +, Stuart Henderson wrote: > On 2020/12/06 16:39, Otto Moerbeek wrote: > > On Sun, Dec 06, 2020 at 03:31:19PM +, SW wrote: > > > > > On 06/12/2020 14:32, Otto Moerbeek wrote: > > > > On Sun, Dec 06, 2020 at 02:19:05PM +, SW wrote: > > > > > > > >> Hi, > > > >> 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? > > > >> > > > >> --- syspatch Sun Dec 6 14:11:12 2020 > > > >> +++ syspatch Sun Dec 6 14:10:23 2020 > > > >> @@ -323,3 +323,9 @@ if ((OPTIND == 1)); then > > > >> _PATCH_APPLIED=true > > > >> done > > > >> fi > > > >> + > > > >> +if [ "$_PATCH_APPLIED" = "true" ]; then > > > >> + exit 0 > > > >> +else > > > >> + exit 1 > > > >> +fi > > > >> > > > >> Thanks, > > > >> S > > > >> > > > > I don't this is correct since it maks syspatch exit 1 on "no patches > > > > applied". > > > > > > > > -Otto > > > > . > > > 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. > > > > -Otto > > > > > > > > > > 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)? > > > > > > Thanks, > > > S > > > > 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. -- Antoine
Re: syspatch exit state
On 2020/12/06 16:39, Otto Moerbeek wrote: > On Sun, Dec 06, 2020 at 03:31:19PM +, SW wrote: > > > On 06/12/2020 14:32, Otto Moerbeek wrote: > > > On Sun, Dec 06, 2020 at 02:19:05PM +, SW wrote: > > > > > >> Hi, > > >> 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? > > >> > > >> --- syspatch Sun Dec 6 14:11:12 2020 > > >> +++ syspatch Sun Dec 6 14:10:23 2020 > > >> @@ -323,3 +323,9 @@ if ((OPTIND == 1)); then > > >> _PATCH_APPLIED=true > > >> done > > >> fi > > >> + > > >> +if [ "$_PATCH_APPLIED" = "true" ]; then > > >> + exit 0 > > >> +else > > >> + exit 1 > > >> +fi > > >> > > >> Thanks, > > >> S > > >> > > > I don't this is correct since it maks syspatch exit 1 on "no patches > > > applied". > > > > > > -Otto > > > . > > 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. > > -Otto > > > > > > 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)? > > > > Thanks, > > S > How about the same exit codes as acme-client? They seem fairly reasonable - 0=updated, 1=failure, 2=no change. I use this as a workaround, which I am not at all happy with. date >> /var/log/updates syspatch 2>&1 | tee -a /var/log/updates | grep -q Installing && log="$log syspatches" && _reboot=true pkg_add -vu 2>&1 | tee -a /var/log/updates | grep -q Adding && log="$log packages" && _reboot=true fw_update -v 2>&1 | tee -a /var/log/updates | grep -q ": ok" && log="$log firmware" && _reboot=true if $_reboot; then logger "$log, rebooting" shutdown -r +1 fi
Re: syspatch exit state
> Returning a non-zero exit status is generally a sign of some sort of > failure. Not applying a patch due to already having the patch applied > is not really a failure. Generally agreed, though at the same time, sysupgrade returns non-zero if there is no upgrade to apply so the same behaviour /could/ be valid here. That being said: > Instead, you could run syspatch -c from a regular cron job and > patch+reboot whenever that tells you there's new patches available. That sounds like a better approach in this case. I'll double check the behaviour of syspatch -c, and perhaps look into making a patch that updates the usage output of syspatch. Anyway, best to disregard the original diff now I think. Thanks, S
Re: syspatch exit state
On Sun, Dec 06, 2020 at 02:19:05PM +, SW wrote: > Hi, > 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? > > --- syspatch Sun Dec 6 14:11:12 2020 > +++ syspatch Sun Dec 6 14:10:23 2020 > @@ -323,3 +323,9 @@ if ((OPTIND == 1)); then > _PATCH_APPLIED=true > done > fi > + > +if [ "$_PATCH_APPLIED" = "true" ]; then > + exit 0 > +else > + exit 1 > +fi > > Thanks, > S $_PATCH_APPLIED will be "true" or "false", which means that you could use it as is done elsewhere in the script: $_PATCH_APPLIED If that is the last line of the script, it (true or false) will supply the exit status of the script. No need for an if statement or explicit exit. Returning a non-zero exit status is generally a sign of some sort of failure. Not applying a patch due to already having the patch applied is not really a failure. Instead, you could run syspatch -c from a regular cron job and patch+reboot whenever that tells you there's new patches available. -- Andreas (Kusalananda) Kähäri SciLifeLab, NBIS, ICM Uppsala University, Sweden .
Re: syspatch exit state
On Sun, Dec 06, 2020 at 03:31:19PM +, SW wrote: > On 06/12/2020 14:32, Otto Moerbeek wrote: > > On Sun, Dec 06, 2020 at 02:19:05PM +, SW wrote: > > > >> Hi, > >> 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? > >> > >> --- syspatch Sun Dec 6 14:11:12 2020 > >> +++ syspatch Sun Dec 6 14:10:23 2020 > >> @@ -323,3 +323,9 @@ if ((OPTIND == 1)); then > >> _PATCH_APPLIED=true > >> done > >> fi > >> + > >> +if [ "$_PATCH_APPLIED" = "true" ]; then > >> + exit 0 > >> +else > >> + exit 1 > >> +fi > >> > >> Thanks, > >> S > >> > > I don't this is correct since it maks syspatch exit 1 on "no patches > > applied". > > > > -Otto > > . > 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. -Otto > > 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)? > > Thanks, > S
Re: syspatch exit state
On 06/12/2020 14:32, Otto Moerbeek wrote: > On Sun, Dec 06, 2020 at 02:19:05PM +, SW wrote: > >> Hi, >> 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? >> >> --- syspatch Sun Dec 6 14:11:12 2020 >> +++ syspatch Sun Dec 6 14:10:23 2020 >> @@ -323,3 +323,9 @@ if ((OPTIND == 1)); then >> _PATCH_APPLIED=true >> done >> fi >> + >> +if [ "$_PATCH_APPLIED" = "true" ]; then >> + exit 0 >> +else >> + exit 1 >> +fi >> >> Thanks, >> S >> > I don't this is correct since it maks syspatch exit 1 on "no patches applied". > > -Otto > . 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. 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)? Thanks, S
Re: syspatch exit state
On Sun, Dec 06, 2020 at 02:19:05PM +, SW wrote: > Hi, > 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? > > --- syspatch Sun Dec 6 14:11:12 2020 > +++ syspatch Sun Dec 6 14:10:23 2020 > @@ -323,3 +323,9 @@ if ((OPTIND == 1)); then > _PATCH_APPLIED=true > done > fi > + > +if [ "$_PATCH_APPLIED" = "true" ]; then > + exit 0 > +else > + exit 1 > +fi > > Thanks, > S > I don't this is correct since it maks syspatch exit 1 on "no patches applied". -Otto
