Created attachment 249295 [details] Patch to enable YubiKey support This adds an option to build kc with YubiKey support. I shortened the install message and updated it with information on YubiKey support. Everything that was there is in the Changelog where we are already pointing the users, and this way it doesn't spam the output while installing - i.e. other messages are obvious, too. I also changed OPTIONS_RADIO to OPTIONS_SINGLE for _CLI, as far as I understood _SINGLE makes sure that there's only one option that can be selected, and these are mutually exclusive. But if I'm being honest, I would have thought the same thing about _RADIO, am not really sure, I just went with what I read in the porter's handbook.
Created attachment 252993 [details] pet portclippy, portlint; add TEST_TARGET Made some small modifications (change order, reduce tabs). Added TEST_TARGET. There is a notice: Message from pcre-8.45_4: -- ===> NOTICE: This port is deprecated; you may wish to reconsider installing it: EOLed by upstream, use devel/pcre2 instead. An error: Error: /usr/local/bin/kc is linked to /usr/local/lib/libscrypt.so which does not have a SONAME. security/libscrypt needs to be fixed. And one test failed: regress/cmd_passwd.sh: typeset: not found Could you please check these?
Hey Zsolt! 1) pcre support can be dropped, there's POSIX regex support in kc in lieu of pcre. 2) Not sure what to do with libscrypt, or what you're expecting me to do there. 3) regression tests need a shell that supports e.g. typeset. If we can't come up with a stable solution that makes sure running the test script (run_tests.sh) with $(shell) invokes a shell that does, then I suggest you remove TEST_TARGET. Daniel
Created attachment 253022 [details] revert TEST_TARGET, remove PCRE support
(In reply to Daniel from comment #2) 1) Okay. Thanks. 2) It seems it's working. I'm not good in libraries, linking, etc., I don't know it is a (serious) problem or not. 3) Maybe bash? Change the shebang in cmd_passwd.sh and add bash to TEST_DEPENDS. I think it should work.
Well, it's not just cmd_passwd.sh, there's a bunch of other tests that make use of this. Also, when invoked as a parameter to a shell - like we're doing here, that's why I alluded to the $(shell)-style invocation in the Makefile -, the shebang doesn't matter. Plus I won't change something portable (e.g. /bin/sh) to something that may work on some OSes and definitely not on some others.
(In reply to Daniel from comment #5) Good idea!
(In reply to Daniel from comment #5) "Plus I won't change something portable (e.g. /bin/sh) to something that may work on some OSes and definitely not on some others." Ah, understood what you said. I mean you modify only in FreeBSD ports with REINPLACE_CMD - not your (github) source code.
Created attachment 253103 [details] enable tests, remove PCRE I see your point. Here's an updated diff with tests enabled in a slightly different way.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=c89a37da755fc8aa59772f6be56b695999646df2 commit c89a37da755fc8aa59772f6be56b695999646df2 Author: Daniel <leva@ecentrum.hu> AuthorDate: 2024-08-26 18:17:23 +0000 Commit: Zsolt Udvari <uzsolt@FreeBSD.org> CommitDate: 2024-08-26 18:17:23 +0000 security/kc: add YUBIKEY option Remove pcre support. Add TEST_DEPENDS and use bash instead of sh in tests. Shorten pkg-message. Switch to DISTVERSION. Pet portlint, portfmt. PR: 277801 Approved by: submitter is maintainer security/kc/Makefile | 69 +++++++++++++++++++++++++++---------------------- security/kc/pkg-message | 31 ++-------------------- 2 files changed, 40 insertions(+), 60 deletions(-)
Committed, thanks!