Created attachment 232836 [details] keepassxc 2.7.0 Makefile 2.7.0 is released. I spent some time whacking on it. This is my first time messing with ports but wanted to share the files I assembled. Currently, I can't get keeshare to work and yubikey isn't using ykpers anymore and using libusb instead...and, I don't see anything in ports for that unless you use Linux compat, which sucks. Here's my Makefile that I have that compiles successfully with keeshare and yubikey support disabled.
Hi, Thanks for your work! I was also working on this, and have something working. I don't own a yubikey, so I could not test this. I was going to file a PR myself later, but since you filed this I'm going to attach my patch here shortly. You should really provide updates to ports as patches, and not as a whole Makefile, which also is not including the needed changes to other files in the port (plist, distinfo, patches etc.). This is all well documented in the porter's handbook: https://docs.freebsd.org/en/books/porters-handbook/ Regarding libusb, FreeBSD includes libusb in base, it's in /usr/lib/libusb.so.3, so no need for extra dependencies there. I added a patch to the upstream cmake files to tell them where they can also find libusb in FreeBSD.
Created attachment 232838 [details] keepassxc port update to 2.7.0 Here is a complete patch with the update. I could not test yubikey support, since I don't own any such hardware. It compiles fine though, so if anyone with a yubikey could also test it and confirm it works it would be great.
(In reply to Guido Falsi from comment #2) Thanks very much for the patch! I have a yubikey so I can test it. I was also working on libusb stuff so thanks again for saving my time!
Hello Guido, I acknowledge that there's the porters handbook and I intend to look at it. I had a bit of spare time tonight and thought if I had the Makefile I put together someone smarter than me could figure out the rest ;) I'll be comparing what was done here in addition to looking at the handbook for the next time and do a proper PR (I've never done one before) :D
(In reply to Guido Falsi from comment #2) If this isn't an inappropriate place to ask, how did you know to add x11extras to USE_QT and change compiler to c++17? I would like to know where I could look to learn these things and contribute. Courtney
(In reply to courtney.hicks1 from comment #5) Those are also in ports handbook, and I recommend you read the codes under ports/Mk directory. BTW, the better place to discuss those porting topics will be freebsd-ports mailing list and IRC channels.
(In reply to courtney.hicks1 from comment #5) For what "compiler:c++17-lang" and USE_QT=x11extras do you need to read through the porter's handbook and read the Makefiles as Li-Wen Hsu suggested. Understanding them requires some basic concepts and much more explanation than what I can fit in here, also it's already available there. I'll also add you will want to check out poudriere, which is invaluable for mass testing and identifying dependencies correctly. Regarding my changes, I moved x11extras to the global USE_QT from the AUTOTYPE option because when testing with all options disabled in poudriere I got an error, so figured it is now unconditionally required. Regarding the compiler:c++17-lang I simply changed it from compiler:c++11-lang it because upstream declares in the release notes that this new version uses C++17 syntax.
I got following error while building on my desktop, 13.0-R with latest packages from pkg.freebsd.org. The testing in poudreire is still on going. --- FAILED: src/keeshare/CMakeFiles/keeshare.dir/ShareExport.cpp.o /usr/bin/c++ -DQT_CORE_LIB -DQT_GUI_LIB -DQT_NO_CAST_TO_ASCII -DQT_NO_DEBUG -DQT_NO_DEPRECATED_WARNINGS -DQT_NO_EXCEPTIONS -DQT_STRICT_ITERATORS -DQT_WIDGETS_LIB -DWITH_APP_BUNDLE -I/usr/home/lwhsu/freebsd-ports/security/keepassxc/work/.build/src/keeshare/keeshare_autogen/include -I/usr/home/lwhsu/freebsd-ports/security/keepassxc/work/keepassxc-2.7.0/src -I/usr/home/lwhsu/freebsd-ports/security/keepassxc/work/.build/src -I/usr/home/lwhsu/freebsd-ports/security/keepassxc/work/keepassxc-2.7.0/src/zxcvbn -I/usr/home/lwhsu/freebsd-ports/security/keepassxc/work/keepassxc-2.7.0/src/keeshare -I/usr/home/lwhsu/freebsd-ports/security/keepassxc/work/.build/src/keeshare -isystem /usr/local/include/botan-2 -isystem /usr/local/include -isystem /usr/local/include/PCSC -isystem /usr/local/include/minizip -isystem /usr/local/include/qt5 -isystem /usr/local/include/qt5/QtCore -isystem /usr/local/lib/qt5/mkspecs/freebsd-clang -isystem /usr/local/include/qt5/QtWidgets -isystem /usr/local/include/qt5/QtGui -O2 -pipe -march=native -fstack-protector-strong -fno-strict-aliasing -fno-common -fopenmp=libomp -Wall -Wextra -Wundef -Wpointer-arith -Wno-long-long -Wformat=2 -Wmissing-format-attribute -fvisibility=hidden -fvisibility-inlines-hidden -fstack-protector-strong -Wnon-virtual-dtor -Wold-style-cast -Woverloaded-virtual -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Werror=format-security -Wcast-align -Qunused-arguments -fsized-deallocation -Wno-deprecated-declarations -O2 -pipe -march=native -fstack-protector-strong -fno-strict-aliasing -fPIC -fPIC -std=gnu++17 -MD -MT src/keeshare/CMakeFiles/keeshare.dir/ShareExport.cpp.o -MF src/keeshare/CMakeFiles/keeshare.dir/ShareExport.cpp.o.d -o src/keeshare/CMakeFiles/keeshare.dir/ShareExport.cpp.o -c /usr/home/lwhsu/freebsd-ports/security/keepassxc/work/keepassxc-2.7.0/src/keeshare/ShareExport.cpp /usr/home/lwhsu/freebsd-ports/security/keepassxc/work/keepassxc-2.7.0/src/keeshare/ShareExport.cpp:119:31: error: use of undeclared identifier 'Z_DEFLATED' Z_DEFLATED, ^ /usr/home/lwhsu/freebsd-ports/security/keepassxc/work/keepassxc-2.7.0/src/keeshare/ShareExport.cpp:120:31: error: use of undeclared identifier 'Z_BEST_COMPRESSION' Z_BEST_COMPRESSION, ^ /usr/home/lwhsu/freebsd-ports/security/keepassxc/work/keepassxc-2.7.0/src/keeshare/ShareExport.cpp:125:13: error: use of undeclared identifier 'zipWriteInFileInZip' zipWriteInFileInZip(zf, data.data() + pos, len); ^ /usr/home/lwhsu/freebsd-ports/security/keepassxc/work/keepassxc-2.7.0/src/keeshare/ShareExport.cpp:129:9: error: use of undeclared identifier 'zipCloseFileInZip' zipCloseFileInZip(zf); ^ /usr/home/lwhsu/freebsd-ports/security/keepassxc/work/keepassxc-2.7.0/src/keeshare/ShareExport.cpp:186:19: error: use of undeclared identifier 'zipOpen64' auto zf = zipOpen64(resolvedPath.toLatin1().data(), 0); ^ 5 errors generated. ---
(In reply to Li-Wen Hsu from comment #8) Interesting, it was working in poudriere here. All the missing parts seem to be present in /usr/include/zlib.h which should be included by zip.h, included by the failing file. I'm noticing on my desktop I have a file /usr/local/include/zip.h though, which would be included before the minizip one, and would cause such failure. Easiest solution could be to patch the failing file to #include <minizip/zip.h> in place of #include <zip.h> I think. Otherwise maybe more complex changes to the CMakeLists.txt files would be needed. I'll attach a revised patch including the @include change described above shortly.
Created attachment 232840 [details] keepassxc port update to 2.7.0 (revised) This patch includes the change I suggested and another one related to an "unzip.h" include which could suffer the same issue if such an include ever ends up in the include path.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=05087b0b8a3f2a6314b3230eccb85130e5570343 commit 05087b0b8a3f2a6314b3230eccb85130e5570343 Author: Guido Falsi <madpilot@FreeBSD.org> AuthorDate: 2022-03-31 17:52:17 +0000 Commit: Li-Wen Hsu <lwhsu@FreeBSD.org> CommitDate: 2022-03-31 17:54:35 +0000 security/keepassxc: Update to 2.7.0 PR: 262953 Reported by: courtney.hicks1@icloud.com security/keepassxc/Makefile | 20 +++--- security/keepassxc/distinfo | 6 +- .../files/patch-src_keeshare_CMakeLists.txt (gone) | 10 --- .../files/patch-src_keeshare_ShareExport.cpp (new) | 11 +++ .../files/patch-src_keeshare_ShareImport.cpp (new) | 11 +++ ...atch-src_thirdparty_ykcore_CMakeLists.txt (new) | 11 +++ security/keepassxc/pkg-plist | 79 +++++++++++----------- 7 files changed, 84 insertions(+), 64 deletions(-)
(In reply to Guido Falsi from comment #10) Thanks, this works for me and I have tested with my yubikey. I'm thinking to upstream our local patches. I think the usb one should be fine, might be asked to add a checker for OS. But I'm not sure about the zip.h one, I guess we cannot assume it always has minizip prefix on other platforms. Do you have any better idea about how to modify CMakeLists.txt?
Why does MASTER_SITES URL contain ?dummy=/ at the end?
(In reply to Daniel Engberg from comment #13) That's for downloading from github.com and avoiding redirection. Remove it and `make fetch` then you'll see what's going on.
(In reply to Li-Wen Hsu from comment #12) I think the libusb patch is ok as is, it should not need an OS check, but if upstream asks for it it can be added. Regarding minizip, the situation gets complicated. I'm not a cmake expert also, I only hack on it when needed by some port. Actually looking at the minizip port, it does nothing special, it is the upstream Makefile that installs it in PREFIX/include/minizip. So, while we cannot be sure, assuming it will be residing there looks reasonable to me. As far as I understand the CMakeFile in cmake/FindMinizip.cmake used pkgconf, which returns this: > pkgconf --cflags minizip -I/usr/local/include/minizip so it reports the subdirectory, and based on this it is reasonable to look for zip.h unqualified. I'm actually not sure what would be correct. My opinion is that including "zip.h" blindly is dangerous on any OS but mostly works because on linux almost nobody is building outside of CI or similar environments (this includes poudriere and similar tools). Maybe you could just #ifdef __FreeBSD__ around the include with prefix #else the one without prefix? If the authors really don't like the minizip prefix.
Forgot to mention, another option could be to try to force the -isystem /usr/local/include/minizip to appear before -isystem /usr/local/include in the command line. Maybe changing: target_link_libraries(keeshare PUBLIC Qt5::Core Qt5::Widgets ${BOTAN2_LIBRARIES} ${ZLIB_LIBRARIES} PRIVATE ${MINIZIP_LIBRARIES}) to...not sure, something like: target_link_libraries(keeshare BEFORE PUBLIC Qt5::Core Qt5::Widgets ${BOTAN2_LIBRARIES} ${ZLIB_LIBRARIES} PRIVATE ${MINIZIP_LIBRARIES}) in src/keeshare/CMakeLists.txt This command is documented here: https://cmake.org/cmake/help/latest/command/target_include_directories.html another possible combination could be removing the PRIVATE flag, but I'm not completely sure I understand what that one does.
(In reply to Li-Wen Hsu from comment #14) Maybe you forgot to define DISTNAME (as the tarball filename is -src at the end)? MASTER_SITES= https://github.com/keepassxreboot/keepassxc/releases/download/${DISTVERSION}/ DISTNAME= ${PORTNAME}-${DISTVERSION}-src Works fine for me root@freebsd-dev:/usr/ports/security/keepassxc # make makesum ===> License APACHE20 BSD3CLAUSE CC0-1.0 GPLv2 GPLv3 LGPL21 LGPL3 MIT NOKIA-LGPL-EXCEPTION accepted by the user ===> License APACHE20 BSD3CLAUSE CC0-1.0 GPLv2 GPLv3 LGPL21 LGPL3 MIT NOKIA-LGPL-EXCEPTION accepted by the user ===> keepassxc-2.7.0 depends on file: /usr/local/sbin/pkg - found => keepassxc-2.7.0-src.tar.xz doesn't seem to exist in /usr/ports/distfiles/. => Attempting to fetch https://github.com/keepassxreboot/keepassxc/releases/download/2.7.0/keepassxc-2.7.0-src.tar.xz keepassxc-2.7.0-src.tar.xz 8639 kB 11 MBps 00s ===> Fetching all distfiles required by keepassxc-2.7.0 for building
Just noticed this one was still open. Thanks for committing the update.
(In reply to Guido Falsi from comment #18) Oh this is intended to remain open as a reminder for comment #17
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=421f76c27c18017d5a8a2f8bd2fc546d8e649ba8 commit 421f76c27c18017d5a8a2f8bd2fc546d8e649ba8 Author: Guido Falsi <madpilot@FreeBSD.org> AuthorDate: 2022-04-27 07:57:15 +0000 Commit: Guido Falsi <madpilot@FreeBSD.org> CommitDate: 2022-04-27 07:57:15 +0000 security/keepassxc: Update to 2.7.1 - Convert to minizip USES - Fix MASTER_SITES [1] - Remove patches already present upstream PR: 263183, 262953 [1] Approved by: lwhsu (maintainer) security/keepassxc/Makefile | 8 +++++--- security/keepassxc/distinfo | 6 +++--- .../keepassxc/files/patch-src_keeshare_ShareExport.cpp (gone) | 11 ----------- .../keepassxc/files/patch-src_keeshare_ShareImport.cpp (gone) | 11 ----------- 4 files changed, 8 insertions(+), 28 deletions(-)