Refactor: move to simple default builds #11003

droidmonkey · 2024-06-29T15:52:50Z

Remove individual feature flags in favor of a single KPXC_MINIMAL flag that removes advanced features from the build. Basic features are no longer guarded by feature flags.
Basic features: Auto-Type, Yubikey, KeeShare
Advanced features include: Browser (and passkeys), SSH Agent, and Secret Service
Networking, Documentation, and Update Checking remain as feature flags to accommodate various distro requirements.

This change also cleans up the main CMakeLists.txt by re-arranging some content and placing macros into a dedicated include file. The minimum CMake version was bumped to 3.16.0 to conform to our minimum Ubuntu support of Focal (20.04). This also allows us to default to C++20, we fall back to C++17 for Qt versions less than 5.15.0 due to lack of support.

Lastly this change removes the KEEPASSXC_BUILD_TYPE="PreRelease" which is never used. We only support "Snapshot" and "Release" now.

Testing strategy

Tested locally and with existing CI

Type of change

✅ Breaking change (causes existing functionality to change)
✅ Refactor (significant modification to existing code)

michaelk83 · 2024-06-29T16:26:26Z

Why is KeeShare considered basic? It would argue that it's an advanced niche feature. It's not needed for most single-user use, and it requires network access.

I would also argue that Secret Service is rather basic for Linux users, as it's the main method for client applications to store and retrieve their credentials. Though I agree that it's less basic than Auto-Type and Yubikey support. And I agree that it can be grouped with SSH Agent and Browser integration, since they all add an IPC layer.

I would group these features like so:

Basic (no IPC and no networking): core functionality, Yubikey, Auto-Type.
Local IPC, no networking: above + Browser (and passkeys), SSH Agent, Secret Service.
Full: above + networking: Update Checks, Key Share, remote DB, icon download, HIBP.

(Technically, browser integration is also rather useless without network access, but as I understand the network access isn't done by KPXC itself.)

varjolintu · 2024-06-29T16:42:48Z

Browser Integration still requires including networking to the code (even if it doesn't make any connections outside, it creates local sockets), so maybe it wouldn't fit to "no networking" category.

droidmonkey · 2024-06-29T16:46:54Z

KPXC_FEATURE_NETWORK is specifically to disable TCP/IP network traffic. So if there are guards in browser using this flag then those guards need to be removed.

droidmonkey · 2024-06-29T16:48:09Z

Why is KeeShare considered basic? It would argue that it's an advanced niche feature. It's not needed for most single-user use, and it requires network access.

It absolutely does not require network access.

KeeShare is going to be rebranded as simply "Sharing" when it gets refactored.

Don't get hung up on the Basic/Advanced wording, thought of a couple different buckets but they were all too much.

codecov · 2024-06-29T16:52:20Z

Codecov Report

Attention: Patch coverage is 72.50000% with 22 lines in your changes missing coverage. Please review.

Project coverage is 62.92%. Comparing base (e48ef80) to head (76624f6).
Report is 32 commits behind head on develop.

Files with missing lines	Patch %	Lines
src/core/Tools.cpp	0.00%	5 Missing ⚠️
src/gui/reports/ReportsWidgetHibp.cpp	16.67%	5 Missing ⚠️
src/gui/DatabaseWidget.cpp	42.86%	4 Missing ⚠️
src/gui/osutils/nixutils/NixUtils.cpp	50.00%	3 Missing ⚠️
src/gui/EditWidgetIcons.cpp	60.00%	2 Missing ⚠️
src/gui/MainWindow.cpp	89.47%	2 Missing ⚠️
src/gui/reports/ReportsDialog.cpp	66.67%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #11003      +/-   ##
===========================================
- Coverage    63.05%   62.92%   -0.13%     
===========================================
  Files          362      361       -1     
  Lines        37727    37545     -182     
===========================================
- Hits         23787    23624     -163     
+ Misses       13940    13921      -19

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

c4rlo · 2024-07-08T17:05:17Z

INSTALL.md

 * make (>= 4.2) or ninja (>= 1.10)
-* g++ (>= 4.9) or clang++ (>= 6.0)
+* g++ (>= 9.3.0) or clang++ (>= 10.0)
 * asciidoctor (>= 2.0)


Could we also mention the minimum versions of required libraries (e.g. Qt, Botan) in here?

c4rlo · 2024-07-08T17:08:09Z

INSTALL.md

   mkdir build
   cd build
-   cmake -DWITH_XC_ALL=ON ..
+   cmake ..
   make


Or:

Suggested change

mkdir build

cd build

cmake -DWITH_XC_ALL=ON ..

cmake ..

make

cmake -B ./build

cmake --build ./build

This way it also works with alternative "generators", such as Ninja instead of Make.

INSTALL.md

c4rlo · 2024-07-08T17:14:46Z

INSTALL.md

   make
   ```
-      
+
 If you have `vcpkg` installed, add `-DCMAKE_TOOLCHAIN_FILE=${VCPKG_ROOT}/scripts/buildsystems/vcpkg.cmake` to the `cmake` command to automatically download and install all required build and runtime dependencies locally to your build directory before compiling KeePassXC. Using `vcpkg` is the preferred way to install dependencies on macOS and required on Windows if using the MSVC toolchain.

 For more detailed build instructions for each platform, please refer to the [GitHub wiki](https://github.com/keepassxreboot/keepassxc/wiki/Building-KeePassXC).


That wiki page has a lot of info duplicated. It would be nice to standardise on a single place.

CMakeLists.txt

src/autotype/CMakeLists.txt

c4rlo · 2024-07-08T17:23:06Z

src/gui/EntryPreviewWidget.cpp

@@ -105,10 +103,6 @@ EntryPreviewWidget::EntryPreviewWidget(QWidget* parent)
    connect(m_ui->groupTabWidget, SIGNAL(tabBarClicked(int)), SLOT(updateTabIndexes()), Qt::QueuedConnection);

    setFocusProxy(m_ui->entryTabWidget);
-
-#if !defined(WITH_XC_KEESHARE)
-    removeTab(m_ui->groupTabWidget, m_ui->groupShareTab);


The removeTab function can now be removed.

* Remove individual feature flags in favor of a single `KPXC_MINIMAL` flag that removes advanced features from the build. Basic features are no longer guarded by feature flags. * Basic features: Auto-Type, Yubikey, KeeShare * Advanced features include: Browser (and passkeys), SSH Agent, and Secret Service * Networking, Documentation, and Update Checking remain as feature flags to accommodate various distro requirements. This change also cleans up the main CMakeLists.txt by re-arranging some content and placing macros into a dedicated include file. The minimum CMake version was bumped to 3.16.0 to conform to our minimum Ubuntu support of Focal (20.04). This also allows us to default to C++20, we fall back to C++17 for Qt versions less than 5.15.0 due to lack of support. Lastly this change removes the KEEPASSXC_BUILD_TYPE="PreRelease" which is never used. We only support "Snapshot" and "Release" now.

Co-authored-by: Carlo Teubner <[email protected]>

droidmonkey added high priority 🚨 build system cleanup labels Jun 29, 2024

droidmonkey added this to the v2.8.0 milestone Jun 29, 2024

droidmonkey requested review from phoerious, louib and varjolintu June 29, 2024 15:52

droidmonkey force-pushed the refactor/simple-default-build branch from a42b85d to 304d3b7 Compare June 29, 2024 16:43

droidmonkey force-pushed the refactor/simple-default-build branch 3 times, most recently from 30d2a5d to 2569be3 Compare July 1, 2024 02:07

c4rlo reviewed Jul 8, 2024

View reviewed changes

droidmonkey and others added 2 commits July 28, 2024 18:27

Remove version minimum on X11 extras

76624f6

Co-authored-by: Carlo Teubner <[email protected]>

droidmonkey force-pushed the refactor/simple-default-build branch from e983223 to 76624f6 Compare July 28, 2024 22:27

This was referenced Oct 22, 2024

WIP: Qt6 transition #10267

Draft

Cleanup code naming conventions and CMake files #8480

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor: move to simple default builds #11003

Refactor: move to simple default builds #11003

droidmonkey commented Jun 29, 2024 •

edited

Loading

michaelk83 commented Jun 29, 2024 •

edited

Loading

varjolintu commented Jun 29, 2024

droidmonkey commented Jun 29, 2024 •

edited

Loading

droidmonkey commented Jun 29, 2024 •

edited

Loading

codecov bot commented Jun 29, 2024 •

edited

Loading

c4rlo Jul 8, 2024 •

edited

Loading

c4rlo Jul 8, 2024

c4rlo Jul 8, 2024

c4rlo Jul 8, 2024

Refactor: move to simple default builds #11003

Are you sure you want to change the base?

Refactor: move to simple default builds #11003

Conversation

droidmonkey commented Jun 29, 2024 • edited Loading

Testing strategy

Type of change

michaelk83 commented Jun 29, 2024 • edited Loading

varjolintu commented Jun 29, 2024

droidmonkey commented Jun 29, 2024 • edited Loading

droidmonkey commented Jun 29, 2024 • edited Loading

codecov bot commented Jun 29, 2024 • edited Loading

Codecov Report

c4rlo Jul 8, 2024 • edited Loading

Choose a reason for hiding this comment

c4rlo Jul 8, 2024

Choose a reason for hiding this comment

c4rlo Jul 8, 2024

Choose a reason for hiding this comment

c4rlo Jul 8, 2024

Choose a reason for hiding this comment

droidmonkey commented Jun 29, 2024 •

edited

Loading

michaelk83 commented Jun 29, 2024 •

edited

Loading

droidmonkey commented Jun 29, 2024 •

edited

Loading

droidmonkey commented Jun 29, 2024 •

edited

Loading

codecov bot commented Jun 29, 2024 •

edited

Loading

c4rlo Jul 8, 2024 •

edited

Loading