Qmake to cmake migration

[zjeffer]

Fixes actions/runner-images#408.

TODO:

• Migrate from qmake to cmake
• Successful build and run
• Fix Github Action workflows
  • Linux
  • Windows
  • macOS

[zjeffer]

Hi @guihkx, you seem to be familiar with macOS building, do you have time to fix the macOS builds? I already fixed the Windows and Linux builds, but I don't have a mac so that makes it very hard to debug.

And don't mind the above commits, it took me a while to fix them lol

[guihkx]

It looks like it's failing because the macOS image provided by GitHub Actions comes with Python installed by default now? Nevertheless, if I read correctly, I think we can prevent homebrew from updating the dependencies by setting HOMEBREW_NO_INSTALLED_DEPENDENTS_CHECK=1.

And don't mind the above commits, it took me a while to fix them lol

No worries, lol. Just squash them afterward.

[zjeffer]

@guihkx Thanks. There's also a problem with the non-homebrew builds. Something about lambdas not supporting auto, maybe an old/wrong compiler version?

[guihkx]

Upstream bug: actions/setup-python#577

People have provided some workarounds there. You might wanna try some of them in case HOMEBREW_NO_INSTALLED_DEPENDENTS_CHECK=1 doesn't work.

If you do add a workaround though, please also add a TODO comment pointing to the issue, so we can later remove the workaround once it's fixed by GitHub.

[guihkx]

@guihkx Thanks. There's also a problem with the non-homebrew builds. Something about lambdas not supporting auto, maybe an old/wrong compiler version?

I'll take a look at that.

[zjeffer]

Ok, thanks for your help!

[guihkx]

Regarding the compilation errors, I believe we must require a compiler that supports at least the C++14 standard.

Add this to CMakeLists.txt:

set(CMAKE_CXX_STANDARD 14)
set(CMAKE_CXX_STANDARD_REQUIRED ON)

Though I'd wait for other people to confirm this later, it's not exactly my area of expertise.

[guihkx]

Heads up: You may want to sync your branch with dev to avoid merge conflicts.

I have some questions/suggestions about the changes I see in the Linux workflow, but I'll wait until you're happy with your changes and the PR is unmarked as draft.

[zjeffer]

Still failing on the brew update/upgrade, while this shouldn't be an issue anymore: actions/runner-images#2322 ?

[guihkx]

Still failing on the brew update/upgrade, while this shouldn't be an issue anymore: actions/runner-images#2322 ?

That's from 2020, though... Check the new bug report I linked a few posts ago.

HOMEBREW_NO_INSTALLED_DEPENDENTS_CHECK=1 should work around it.

[zjeffer]

That's from 2020, though... Check the new bug report I linked a few posts ago.

Oops, I completely forgot about that post, sorry. I just tested positive for Covid, I'll blame it on that :)

[guihkx]

From the Qt 6 docs:

For a consistent developer experience on all platforms, use the Ninja or Ninja Multi-Config generator.

You can select the CMake generator either by setting the CMAKE_GENERATOR environment variable or using the -G argument:

cmake -G Ninja ...

Didn't know that, but if the docs recommend then we should probably do it. Hopefully it just works for Qt 5 builds too...

[guihkx]

On Ubuntu the package is actually called ninja-build... 😅

Quite odd that it doesn't come pre-installed on Linux, but it does on Windows...

Anyway, you'll have to install ninja on macOS too... For the build-homebrew job on macOS, you can try adding ninja to the homebrew list of packages, on the Install Qt ${{ matrix.qt-version-major }} (homebrew) step.

Now, for the dmg-aqtinstall job, you'll have to create a new step either before or after Install Qt ${{ matrix.qt-version }} (aqtinstall) and install ninja with homebrew too...

resolved

[guihkx]

Sorry in advance about my silly nitpicks about sorting things alphabetically... 😅

[guihkx]

Awesome! Thanks a lot for the fixes! 🏆

LGTM CI-wise, although I haven't actually tested any of the artifacts generated by it (just yet).

Anyway, I noticed that we had a couple of Linux-specific subcommands in src/Notes.pro, like make deb and make appimage.

I personally never used them, but I wonder if it's easy enough to re-add them here? 🤔

[zjeffer]

Awesome! Thanks a lot for the fixes!

Thank you very much for your help!

Anyway, I noticed that we had a couple of Linux-specific subcommands in src/Notes.pro, like make deb and make appimage.

I saw we already have make_deb.sh and make_snap.sh scripts in packaging, maybe we can just add a make_appimage.sh script as well, in another PR?

[guihkx]

Sure. Sounds good to me.

[guihkx]

Some minor things I noticed.

Btw, are you aware of something like clang-format for CMakeLists.txt? Just to help us keep the formatting consistent.

[zjeffer]

Btw, are you aware of something like clang-format for CMakeLists.txt? Just to help us keep the formatting consistent.

I found this: https://github.com/cheshirekow/cmake_format. I'm adding a .cmake-format file to the repo, and I'm currently trying to get the vscode extension to work.

[nuttyartist]

Thanks for this!

Btw, I just tested this PR using this binary (https://github.com/nuttyartist/notes/actions/runs/3809547762) on Windows, and the app icon doesn't show up (not on the desktop, task manager, install/uninstall). Is it related to this PR? Because it worked well on a previous PR (of the Qt6 Migration).

[guihkx]

I also noticed that the version is getting set incorrectly... Will open a PR shortly to see if it fixes both of our problems.

[zjeffer]

@guihkx Thanks! Can you explain what you mean by the version getting set incorrecty?

Ah, I see, the verion inside the app is empty, and therefore it thinks there is an update.

[guihkx]

My guess is that $CMAKE_PROJECT_VERSION is undefined here:

https://github.com/nuttyartist/notes/blob/11028d0cd44e3db63a835e232269fd57fe0c7e17/CMakeLists.txt#LL126C11-L126C11

For that to work, I believe we should've set the version in project().

Nevertheless this might also not be optimal, because $CMAKE_PROJECT_VERSION was only introduced in CMake 3.12, but we still have to support 3.10 because of Ubuntu 18.04...

No worries, though. I'm preparing a fix.