Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmake: add new macOS default location for QT discovery #8321

Merged
merged 1 commit into from
Sep 12, 2022
Merged

cmake: add new macOS default location for QT discovery #8321

merged 1 commit into from
Sep 12, 2022

Conversation

Okeanos
Copy link
Contributor

@Okeanos Okeanos commented Jul 31, 2022

The build & install instructions for macOS say that using Homebrew to install the dependencies is enough and the build process will then work without additional setup.

However, QT5 as offered by Homebrew is versioned since March 2021 and the default, non-suffixed path points to QT6 (if installed). New installations of qt5/qt@5 have a suffixed path that should be used instead.

There's an explanation for using -DCMAKE_PREFIX_PATH=/usr/local/opt/qt/lib/cmake but I'd argue this only applies to non-default setups and shouldn't be required for newly setup development environments.

Technically I would think that removing the non-suffixed paths is feasible (and a good idea for better reproducibility) as well but I don't want to break existing setups. I suppose that's something to be done during #7774 ?

Screenshots

n/a

Testing strategy

Following the build & install instructions from a clean state.

Type of change

  • ✅ Bug fix (non-breaking change that fixes an issue)

@codecov-commenter
Copy link

codecov-commenter commented Jul 31, 2022

Codecov Report

Merging #8321 (51fd7bf) into develop (33d8b6d) will decrease coverage by 0.04%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #8321      +/-   ##
===========================================
- Coverage    64.45%   64.41%   -0.04%     
===========================================
  Files          339      339              
  Lines        43963    43963              
===========================================
- Hits         28333    28317      -16     
- Misses       15630    15646      +16     
Impacted Files Coverage Δ
src/gui/Clipboard.cpp 78.95% <0.00%> (-7.02%) ⬇️
...rc/fdosecrets/widgets/SettingsWidgetFdoSecrets.cpp 56.06% <0.00%> (-3.03%) ⬇️
src/fdosecrets/dbus/DBusMgr.cpp 52.20% <0.00%> (-1.47%) ⬇️
src/core/Entry.cpp 82.55% <0.00%> (-0.20%) ⬇️
src/core/FileWatcher.cpp 86.75% <0.00%> (+1.20%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

QT5 as offered by Homebrew is versioned since March 2021 and the default
path points to QT6 (if installed). New installations of qt5 have a
suffixed path (qt@5) that should be used instead.
@droidmonkey
Copy link
Member

Removed the old entries since they are obsolete. I also added include_directories to force the use of the qt@5 include files. If you install Qt6 it will overwrite the system include files with Qt6 includes.

@droidmonkey droidmonkey merged commit d181f80 into keepassxreboot:develop Sep 12, 2022
@Okeanos Okeanos deleted the macos-qt branch September 12, 2022 05:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants