Handle pkg_info(1) error message "Can't find"

[sizeofvoid]

SUMMARY

OpenBSD pkg_info(1) behavior change.

As we can see pkg_info behaves different between OpenBSD -current (>7.3) and OpenBSD <= 7.3.
It prints "Can't find inst:PKGNAME" if the package is not present. This has to be handled in the module otherwise we see

failed: [dev] (item=ranger) => {"ansible_loop_var": "item", "changed": false, "item": "ranger", "msg": "failed in get_package_state(): Can't find inst:ranger\n"}

Example pkg_info(1) call on OpenBSD 7.3 -stable

$ dmesg | head
OpenBSD 7.3 (RAMDISK_CD) #1063: Sat Mar 25 10:41:49 MDT 2023
    deraadt@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/RAMDISK_CD
$ pkg_info -Iq inst:abook
abook-0.6.1p3
triangle$ echo $?
0
$ pkg_info -Iq inst:NOTINSTALLED
$ echo $?
1

Example pkg_info(1) call on OpenBSD -current

$ dmesg | head
OpenBSD 7.3-current (GENERIC.MP) #1247: Sun Jun 18 09:42:58 MDT 2023
    deraadt@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
$ pkg_info -Iq inst:abook
abook-0.6.1p3
$ echo $?
0
$ pkg_info -Iq inst:NOTINSTALLED
Can't find inst:NOTINSTALLED
$ echo $?
1

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

openbsd_pkg

[ansibullbot]

cc @JoergFiedler @MacLemon @bcoca @dch @eest @jasperla @mekanix @opoplawski @overhacked @tuxillo
click here for bot help

[felixfontein]

Thanks for your contribution! Could you please add a changelog fragment? Thanks.

[felixfontein]

Looks good to me, as far as I can tell. Will merge by the end of this week if nobody objects.

[eest]

Hello @sizeofvoid,

Thanks for a solid bug report and patch, this looks good to me :). The only thing I am thinking about is that in most cases in the module we use fairly strict regexes for matching output as in ^this is the complete output$ to not let future changes in output mistakenly pass the test where this ("Can't find inst:%s" % name) means it would also match I still Can't find inst:NOTINSTALLED what kind of case is this?.

Do you think it would be a good idea to go for a stricter regex match for this as well? I personally have preferred to be as strict as possible and have things fail rather than mistakenly work as things evolve, if that makes sense.

[sizeofvoid]

Hello @sizeofvoid,

Do you think it would be a good idea to go for a stricter regex match for this as well? I personally have preferred to be as strict as possible and have things fail rather than mistakenly work as things evolve, if that makes sense.

Thanks @eest, I like the idea. I'll chance the current pull-request.

[eest]

This looks in line with what I had in mind, thanks! One more thing: since we are using the name variable as part of the regex we probably want to pass it through re.escape(). We have had problems in the past where package names include valid regex characters, see 56b5be0

[russoz]

LGTM

[eest]

Thanks for your work on this @sizeofvoid! Also thanks to @felixfontein for the ever present support work for things like writing changelog fragments etc :)

LGTM

[patchback]

Backport to stable-6: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-6/704a3019b74bafa9ebae6770bbf75e685a27a40d/pr-6785

Backported as #6892

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

[patchback]

Backport to stable-7: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-7/704a3019b74bafa9ebae6770bbf75e685a27a40d/pr-6785

Backported as #6893

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

[felixfontein]

@sizeofvoid thanks for your contribution!
@eest thanks for reviewing and the kind words!
@russoz thanks for reviewing as well!