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 portable build #7232

Open
wants to merge 49 commits into
base: main
Choose a base branch
from

Conversation

mmomtchev
Copy link
Contributor

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the existing coding patterns and practices as demonstrated in the repository.

This is the CMake portable build that was started by @MarcoMartins86 and then maintained by @Cyriuz.

This build is mainly targeted at portable software projects that include ImageMagick as a library and are distributed as source.

It is still not capable of fully replacing the existing autotools build on UNIX for application users - though it might be one day. It will never replace the existing application build on Windows which also includes GUI elements - though it might become one day one of its components.

It is currently in a much more advanced state and comes with:

  • unified build system that supports (most) of the original delegates in an identical way across all platforms
  • optional conan integration on all platforms allowing for a fully self-contained and fully reproducible build on all platforms
  • supports pkg-config packages without conan
  • supports cross-compilation and comes with a WASM build
  • unit-tested on all supported platforms and configrations (CMake portable build)
  • fully integrated debug builds which include all dependencies
  • generates native build files on each platform but can also use non-standard compilers and runners (clang on Linux and Windows, or ninja on all platforms)

You can see a brief overview of the build process here: https://github.com/mmomtchev/ImageMagick/blob/cmake-portable-build/README.CMake.md

@urban-warrior
Copy link
Member

Thank you for submitting your pull request. However, after reviewing it, we have decided not to merge it as it is incomplete and would disrupt the current build environment. Our recommendation is to package the build so a user can download and unarchive it into the ImageMagick source folder for a CMake build. In the mean-time, after successfully testing the build, we can reference it on the ImageMagick build page as an alternative build. We will continue to test and fix any issues with the CMake build and may revisit the possibility of replacing the ImageMagick autoconf/automake build in the future with a CMake build.

@mmomtchev
Copy link
Contributor Author

Can you provide more details on what is incomplete and what would disrupt the environment, I am willing to fix it and to maintain it.
Everyone can check this out from my branch.

@Cyriuz
Copy link

Cyriuz commented Apr 8, 2024

We've been very close getting this into conan which would be a great first step I think. @valgur did a huge job trying to get it to work (Cyriuz#2 & conan-io/conan-center-index#21699) but I think both of us didn't have more time to spend on it yet unfortunately.

@urban-warrior
Copy link
Member

We need to ensure the robustness and stability of the CMake build before we can contemplate merging it, and we're not keen on maintaining both builds within the same source distribution. Instead, we propose offering users the option to download the CMake files and integrate them into the ImageMagick source distribution. We'll then rely on user feedback to identify any potential build issues, missing delegate support, or other concerns. Once we confirm that the build is not only complete but also particularly robust, we'll consider replacing the current autoconf/automake build. It's important to note that the Azure pipelines currently rely on the autoconf/automake build, so we'll need time to test the pipeline's dependencies with CMake.

Could you post instructions for the CMake build in the discussion section and mark it as an announcement? The build should be hosted on GitHub so that users can submit pull requests to encourage collaboration. The ImageMagick development team will initiate local builds and assess the Azure build pipelines. Once we are assured that CMake effectively replaces and enhances all the functionalities of the autoconf/automake builds, we will reconsider merging.

@valgur
Copy link
Contributor

valgur commented Apr 9, 2024

My version of the CMake-based build system is still missing some major parts for general-purpose use:

  1. Dependency handling without Conan. I.e. creating Find*.cmake modules for dependencies that don't install a CMake config file for find_package(). This is not going to be an easy task for a project with this magnitude of dependencies.
  2. Generating and installing a CMake config for ImageMagick itself. Should not be too difficult.

Another aspect is whether CMake is the build system of choice that you want to go with. Meson has been popular a popular choice in many OSS projects recently, especially ones that have previously relied on Autotools, such as the whole GNOME ecosystem. I'm thinking of creating an alternative Meson-based implementation in addition to the CMake one to see how it compares.

@mmomtchev
Copy link
Contributor Author

mmomtchev commented Apr 9, 2024

Ok, I am not very sure that I understand how exactly robustness and stability apply to a build, and I will continue to maintain this build. This PR will always stay current.

If someone else thinks that meson should be used, he is obviously free to make a meson build.

I also intend to officially question and doubt the actual motives for those decisions.

MarcoMartins86 and others added 23 commits May 6, 2024 10:07
Compile coders, filters, Magick++, MagickCore and MagickWand as static libraries using CMake
-Added magick project from utilities
-Added some options to cmake-gui where static or shared build option being the most relevant
- Added a macro for finding delegates.
- Separated environment tests to its own cmake file.
- Use lower case cmake functions since it is recommended nowadays and it was already mixed.
- Added missing JXL delegate.
check_function_exists fails to find them so we have to use check_symbol_exists
Cyriuz and others added 22 commits May 6, 2024 10:07
It seems to be missing from its exported dependencies in v3+
* Finish and polish the CMake build
* Add conan integration
* Add build testing in different configurations
* fix many builtin paths

* add a zeroconf build

* move the quotes

* use macOS 13 for now

* do not share caches between jobs

* no need for shared zeroconf builds

* run a single workflow at a time

* build all conan artifacts only once

* checkout the conanfile

* go back to cairo 1.17.8

* schedule a regular Sunday Mass

* extra builds are now much cheaper

* reduce the number of debug builds

* drop the windows debug build with conan

* windows / pkgconf is not a supported build

* add the conan pkg-config name of openjpeg

* zstd should be expected

* add zstd pkgconfig name
* include this unusual solution for meson compatibility

* compatibility with the VStudio CMake generator

---------

Co-authored-by: Momtchil Momtchev <momtchil@motmchev.com>
* prefer static pkg-config settings when building a static lib

* show the required dynamic libraries

* add a kludge for macOS frameworks in pkgconfig files

* slightly cleaner solution
mmomtchev and others added 4 commits May 6, 2024 11:23
* ability to disable select delegates

* fix the conan logic

* fix fallback to pkgconfig when cmake module is found but does not have the target

* handle X11 on Windows
)

* disable jemalloc by default and test disabling/enabling

* add the CUSTOM_DELEGATES var to the CMake command

* replace the expected delegates
* use POSIX file structure on Windows

* restore line ident

* restore formatting

* add missing config defines

---------

Co-authored-by: Momtchil Momtchev <momtchil@motmchev.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants