NotepadNext icon indicating copy to clipboard operation
NotepadNext copied to clipboard

CMake support for Qt5 and Qt6

Open elcuco opened this issue 2 years ago • 32 comments

This PR brings CMake support for the Qt5 build of NotepadNext.

To build use:

cmake -S . -B cbuild -G Ninja
cmake --build cbuild

Benefits:

  1. QMake is basically EOL
  2. CMake is supported on VS out of the box.
  3. CMake has ninja generators - which on my machine speed up compilation by almost a factor of 2.
  4. All upstreams are now handled using CPM
  5. This makes the build system more modern, and hopefully - easier to manage with the rest of the C++ community.

TODO:

  • [x] Fix clang build on Linux (fails on my setup)
  • [x] Fix libraries to have proper SO name
  • [x] Add CI/CD setup to CMake on Windows/Linux/OSX
  • [x] Fix Qt5/6 building on Windows
  • [x] Add Qt6 support
  • [x] Setup install CMake targets (needed for package building)
  • [x] AppImage generation (is broken ... should be fixed again)
  • [ ] Windows installer generation (see error bellow)
  • [ ] App bundle generation
  • [x] Convert the build.yml back to ninja
  • [ ] Add support for translations
  • [ ] Hooks to upload to Windows store(s?)
  • [ ] Hooks to upload to linux stores (flatpack and what not)
  • [ ] Multi arch support for OSX

Nice to have:

  • [x] Start asking upstreams to accept patches, and remove my local forks. (WIP: see https://github.com/editorconfig/editorconfig-core-qt/pull/2)
  • [x] Try to fix as much of the TODOs and FIXME inside each module (lexzilla has generated files, which are a pain - I am unhappy about how this went - just an example) - this is not blocking for this PR.
  • [ ] Fix AppImages to have better naming (qt version, cmake, app version etc).
  • [x] Instead of upstreaming build systems, we can use local build files, like we did on QMake, unless some code modifications are needed. Seems like LUA is a good target for this.

Status so far: I am able to build a windows ZIP. From this - I should create the EXE using NSI. However - NSI dies, since it looks for the EXE version in order to create the version for the installer. I traced this down to the Windows RC compiler. QMake passes some arguments to it - and the versions are "preprocessor defines". I do not want to generate the RC on configure time, this seems like a hack to me, I want to do what QMake does. When this is done - IMHO, the installer for windows will be ready.

Installer for OSX? I have no idea whats borked there. I hope someone with a Mac can help me.

Linux should be ready.

elcuco avatar Apr 16 '22 15:04 elcuco

Thanks for the PR!

I am very apprehensive about switching to cmake at this time. I personally have never had any good experiences with cmake...again just my personal opinion.

I know its a widely-used, mature build system so I'm willing to keep an open mind. I know in the past couple of years QtCreator has had better cmake support (and Qt moving to cmake in general)

So I think the plan for now can be to keep this PR open and slowly work on maturing cmake support to see if it is an adequate replacement for everything qmake does today. I worry all upstreams will not be willing to support cmake (could this project implement its own cmake files outside of those streams? similar to what is done with qmake and Lua currently?) Even then I don't foresee moving to cmake really soon, just because of some of the macOS and flatpak distributions might need some tweaking. But if it matures nicely and proves it can take qmake's place then I may be willing to ditch qmake.

dail8859 avatar Apr 16 '22 20:04 dail8859

I agree. I think the best way is to maintain both build systems - until you got comfortable with CMake. For you - you just need to open this branch and open VisualStudio. The rest will be similar to you - the workflow will be the same. I can find time to setup Qt6 on Windows - but it will take time. You can test Qt5/Windows now. (and Qt5/OSX, whoever tests this). It "should work".

Regarding upstreams: I have only 4 forks (you cannot add an out of source CMake, at least not trivially, yet):

  1. QSimpleUpdater - I will contact the original upstream and try to merge.
  2. editorconfig-core-qt - I will contact the original upstream and try to merge.
  3. scintilla-code - this is a huge pain, it will take several month until upstream merged this in (I need to fix 3 other platforms, not fun). They are HG only, and in source forge... so a github mirror will be necessary (until I get conan working).
  4. lexilla - this is relatively easy. But I am unsure if upstream wants this.

The first two are not a problem. The problem is scintilla's upstream. Even if they do not accept the new build system. as I have not modified their sources - I can easily merge from them back. IMHO - its a win-win scenario.

elcuco avatar Apr 16 '22 21:04 elcuco

you just need to open this branch and open VisualStudio

I use QtCreator, but think it would be similar.

(you cannot add an out of source CMake, at least not trivially, yet)

But doesn't it technically already have out of source cmake since your cmake file builds the entire application fine?

I would definitely want to avoid having to fork every repository that doesn't support cmake. Every external Qt-based repo that I've looked at using has qmake support so its at least a bit easier but not perfect. Plus out of source qmake is fine.

Regarding upstreams

I honestly regret using submodules. They are nice if you want to use the code as-is and never want to modify it. But then its just annoying because its something you always have to be aware of.

I definitely don't want Python to be a requirement for building Notepad Next since Scintilla needs it generate the two files that don't come by default with Scintilla. So I'm not sure how cmake would handle that situation.

dail8859 avatar Apr 16 '22 22:04 dail8859

CMake is the recommended build method for Qt6+

txtsd avatar Apr 17 '22 14:04 txtsd

I started this project years ago and qmake was the build system of choice for Qt based projects. Qt Creator also has much better cmake support, plus as you mentioned Qt using cmake now. At some point Notepad Next will only support Qt6. All this is why I'm willing to see how well cmake will work for this project.

dail8859 avatar Apr 17 '22 14:04 dail8859

While the CMake stuff is nice... I could have chosen to keep the old monorepo workflow. However, I decided to use a multirepo approach. What does this give me? See the latest commits in this rebased branch, see how easy it is for me to update version.

See this for example: https://github.com/elcuco/NotepadNext/blob/2a370c2d5cdbf814f6f4c20a0c597f2e463cf9c6/CMakeLists.txt#L23

(still WIP, need to add the CI stuff)

elcuco avatar Apr 17 '22 19:04 elcuco

@elcuco Is it independent of git submodules?

EDIT: nvm I was confused for a sec. Wouldn't it be better to include the other repos as git submodules instead of using cmake as a dependency manager?

txtsd avatar Apr 17 '22 19:04 txtsd

I can easily convert this to using git submodules + add_subdirectory. Its just harder to use for developers - its easier to handle text files than git submodules.

One of the future enhancements I want to add, is migrating this to conan. Using submodules defeats this goal.

elcuco avatar Apr 17 '22 19:04 elcuco

Why is conan better?

txtsd avatar Apr 17 '22 20:04 txtsd

Why is conan better?

If you add a library by conan, you can get pre-compiled version (most of the artifacts are on VS and OSX). You can get a deep dependency chain - by asking the upper level package. For example here: https://github.com/elcuco/nana-ide/blob/master/conanfile.txt I asked for libpng, which in turn got me libz.

I can handle also dependencies which are not packaged using cmake - for example boost, which in turn depends on openssl (both have non-cmake build systems). They get installed into a sandbox - and conan does magic to include the correct path for libs and includes. I heard of it as a "docker environment experience" (since nothing on the real system is modified). You can also have several applications (sub projects, for example) each depending on a different version of the same library (each sub-project will need to have a dedicated conan file).

This is what npm/gradle/pods/pip/cargo does - but for C++. You expand the amount of libraries available to your app.

elcuco avatar Apr 18 '22 19:04 elcuco

Neat. I'll have to see it in action!

btw if this is compiling for you locally, can you also make changes to the github workflow so we know if the CI can handle it?

txtsd avatar Apr 19 '22 10:04 txtsd

Awesome!

txtsd avatar Apr 19 '22 17:04 txtsd

Awesome!

Almost. The windows build is broken (scintilla's cmake is bad). I also need to add steps to build the packages for each platform. Then - I need to port whats needed to Qt6. Feel free to send a PR to my branch, if you can help. Otherwise, it will get there eventually.

PS: I don't own a mac, so building packages for it might become a trial an error with the CI. Help would be appreciated.

elcuco avatar Apr 19 '22 21:04 elcuco

I'm not good with CMake, but I have some experience with the CI. I'll take a look tomorrow and see if there's anything I can help with!

txtsd avatar Apr 20 '22 20:04 txtsd

I fixed the building of all libraries to be static, as cmake installs into /usr/lib/lib-x86_64/ but AppImage does like this. Also CMake install would also install headers and *.a files. This will be a nice blog.

I will give a try in the next week to fix the install targets, and after that (hopefully) AppImage would work.

elcuco avatar Apr 25 '22 20:04 elcuco

@elcuco

Notes how I got this to build with VS2022:

scintilla-code-src\CMakeLists.txt I noticed that you added target_link_libraries(scintilla-qt-edit-base pthread). pthread is not native to Visual Studio and in this case not needed. Maybe you need to add a ifdef there.

scintilla-code-src\qt\ScintillaEditBase\ScintillaEditBase.h Because you changed the library to be build as a static library, having EXPORT_IMPORT_API be dllimport or dllexport is incorrect. The easiest way to fix this is to use https://cmake.org/cmake/help/latest/module/GenerateExportHeader.html . This supports the shared and static build requirement while being easy on qt moc.

I hope these notes help you resolve the windows build issues.

Update: My windows CMake setup usually makes sure that all the required dependencies are copied to the install directory when I build the install target. This simplifies things for me and for whoever wants to try to. I would advice you to do the same. This also simplifies packaging.

stevenhoving avatar Apr 28 '22 11:04 stevenhoving

Minor update:

  1. Ported all code to compile under Qt6/CMake. I will push on the next days.
  2. Been working last week on compilation on MinGW/Windows. This was a pain, since the code on Windows for lexizilla exports/imports on windows - and this does not work on static libraries. So another fix will be needed there. I have a working EXE, soon will publish code.
  3. VS will follow.
  4. Then I will work on installer for Windows. I would like to use MinGW/Qt6/CMake - instead of VS - anyone objects?

@stevenhoving - just saw your comments, window did not refersh. Yeap, VS should be trivial. Nice.

elcuco avatar May 01 '22 07:05 elcuco

Ported all code to compile under Qt6/CMake

I think Qt6 is the right choice. I haven't decided yet when to move official distribution to Qt6 but it is definitely desired.

Been working last week on compilation on MinGW/Windows.

Is this required?

I would like to use MinGW/Qt6/CMake - instead of VS - anyone objects?

The only build system I will be using on Windows is VS and I want this to be the official build system. If MinGW works then great, but there's no way I can troubleshoot MinGW issues if they would ever arise.

dail8859 avatar May 01 '22 22:05 dail8859

I managed to get the static linking issues fixed on MinGW, the CI confirmed this works also on CL/MSVC. I have enabled the CI for Linux/OS/Windows on Qt6 and Qt5. App images are being built. Soon MSI, and then AppBundles.

EDIT - fixed CI as well EDIT2 - Linux App images (Qt6, Qt5) work. I should change the app image name "soon" (so I can download both zips, and unpack).

elcuco avatar May 06 '22 16:05 elcuco

Just a small comment: the CI is currently generating EXEs built with MSVC. It worked as soon as I fixed the MinGW build on my laptop. I think about adding another win64 build when this gets merged. I like the idea of using MinGW as another build (keeping with the FS on windows agenda).

elcuco avatar May 07 '22 17:05 elcuco

In my repo, you can see https://github.com/elcuco/NotepadNext/actions/runs/2318840719 - which has an artifact for Windows (a zip). I know the scripts build zip files for qt5, and qt6, and for qt6 I should also have an installer. Trying to identify whats borked. IMHO - a mingw build can also be produced.

However, I build installer only for MSVC Qt6. Because, WTF not.

elcuco avatar May 13 '22 10:05 elcuco

Rebased against master.

elcuco avatar Jul 17 '22 20:07 elcuco

Maintaining both Qt5 and Qt6 would be nice but if it makes things easier then I'd say don't worry about Qt5 for now.

At some point Qt5 will be dropped anyways.

dail8859 avatar Jul 18 '22 20:07 dail8859

Maintaining both Qt5 and Qt6 would be nice but if it makes things easier then I'd say don't worry about Qt5 for now. Yap, saw your cleanup in EditorConfigQt - it now supports only Qt6.

Kill the Qt5 support on main, and I will clean up this branch as well.

elcuco avatar Jul 18 '22 21:07 elcuco

Note that the AppBundle is working right now. I will update it "soon" with a proper version... but I wish someone with macos experience would see what I messed up in the DMG generation. (again - proper versioning for files would be great).

I will try and update the CMake libraries soon. Hopefully remove the forks, and use local builds as I am doing with Lua right now.

elcuco avatar Sep 25 '22 21:09 elcuco

Status so far:

  1. the CI creates a linux image. Cool. However... on my local setup the same cmake command generate dynamic libraries, and on the CI it genetates static libraries - thats the reason for the lasts commits. I am also unsure why cant I define configuration from the main CMakeFile.txt and instead need to pass configuration via command line. (specially, since it seems like it works on the CI!)
  2. I don't have a Mac, so I am unsure how to fix the package building on OS. Should be "trivial".
  3. On Windows - I am working on making an installer... I have this batch script I use locally to make a release (then port this to github actions) - this round, I cannot build the Windows code at all using mingw.
  4. Windows: qsimpleupdater.cmake : sometimes (!) I cmake complains that there are several targets for ui_Downloader.h. I disabled automoc, and hacked around and the problem went away. I am unsure why on Linux/Mac/Windows-CI this works (yes, I am compiling locally using mingw, and CI uses CL... but this should not affect this right?). Again - sometimes it does configure properly here.
  5. For some reason, linking Scintilla on MinGW on my machine fails - all the symbols are not visible. I am unsure why it did work previously on MY machine.
  6. Scintilla's fork is the only one left. Upstream is using mercurial, and part of the upstream build system uses python to generate files. My fork pre-"rednders" these C++ files and are part of the git repo. 7.Upstreams scintilla's code is changing. A signal has been modified - and to workaround it I needed to comment out some code (see src/NotepadNext/ScintillaNext.cpp)

Process is slow, as each iteration I make a change, I find myself in a full recompilation. I tried using ccache but it just misscompiles too much times. I also tried using CPM cache... but this also fails for me.

Anyway, the CMake/C++ front is very frustrating. Months after this PR started, I am still unable to make a working. Documentation for CMake is unfriendly. Many upstreams have no valid CMake infra... this is not simple. I wish this were simpler.

elcuco avatar Oct 05 '22 08:10 elcuco

@elcuco

I appreciate all the hard work and effort you've put into this. I knew cmake would be no easy task...especially given all the hurdles you've described.

When I get a bit of free time I'll take a much closer look at this PR and see if I can help out.

Documentation for CMake is unfriendly.

This is probably the biggest thing that has turned me away from cmake. Anytime I try to find clear documentation for regularly used cmake features, it is a pain to try to understand them. Fixing cmake for the editorconfig-qt package once again proved this.

dail8859 avatar Oct 05 '22 11:10 dail8859

Can this pull request be made to support system dependencies for Linux here? As a packager for Fedora, I'd prefer to be able to use packaged versions of NotepadNext's dependencies and link to system copies instead.

Conan-Kudo avatar Nov 11 '23 11:11 Conan-Kudo

I very recently started poking around with this PR again to get cmake working.

I think the first step is to replace the build system as is.

I have no clue what it takes to support system dependencies on Linux. There's already a flatpack and an appimage available. If there was minimal maintenance and changes involved to support the system dependencies I'd be willing to consider it. So the effort to implement + maintain it would have to be worth the benefit (especially since I'm not a Linux user).

Is there an example you can point me towards as a reference?

dail8859 avatar Nov 11 '23 13:11 dail8859

It's fairly easy with CMake to locate things either by CMake module config (with find_package()) or by pkgconfig (with pkg_check_modules())

Here are a few examples:

  • Fedora Media Writer: https://github.com/FedoraQt/MediaWriter/blob/4a3df752449b20dcd7df7aab611da6963168660e/CMakeLists.txt#L40
  • Lugaru: https://gitlab.com/osslugaru/lugaru/-/blob/master/CMakeLists.txt?ref_type=heads#L203-241
  • RPM: https://github.com/rpm-software-management/rpm/blob/d2be4773b5e6c6358ef2f4c49b7cc727b5edf1eb/CMakeLists.txt#L187-L276

Conan-Kudo avatar Nov 11 '23 13:11 Conan-Kudo