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 support #743

Closed
rcdailey opened this issue Sep 8, 2017 · 33 comments
Closed

CMake support #743

rcdailey opened this issue Sep 8, 2017 · 33 comments

Comments

@rcdailey
Copy link

rcdailey commented Sep 8, 2017

I find that ImageMagick is very difficult to build, and steps to do so vary per platform. This is especially difficult when cross compiling (e.g. cross compiling using the Android NDK toolchains on Windows or Linux as the host OS). Cross compiling is very simple in CMake, especially for Android since the NDK provides a CMake toolchain file. How do the developers feel about adding CMake support for building ImageMagick?

I did an ad hoc CMake script for building MagickWand and MagickCore and I've had success with this, although I had difficulty with it mostly due to my lack of understanding of the complexities of the build requirements. For example, I have to maintain a different magick-baseconfig.h per platform (linux vs windows). We only have a basic usage of ImageMagick, so this minimal build works out.

I'd be willing to help contribute CMake support, if the developers are open to using it. Thoughts?

@mikayla-grace
Copy link

mikayla-grace commented Sep 8, 2017

I find that ImageMagick is very difficult to build, and steps to do so vary per platform.

The steps are the same on all systems:

  ./configure
  make
  make install

Of course, non-standard builds require intimate knowledge of automake / autoconf. Most non-standard builds are accomplished simply by setting a few environment variables but getting these right, for example, for cross-compiling, can be daunting.

We are interested in a CMake script for building ImageMagick and we would be willing to replace the current automake / autoconf build-- if and only if the CMake script can perform all the functionality of the existing build functionality. That means the CMake script needs to support all the delegate libraries and programs that configure.ac currently supports and also updates various files in the ImageMagick distribution that require build information such as common.sh, config/delegates.xml, MagickCore/MagickCore-config, PerlMagick/quantum/quantum.xs, utilities/ImageMagick.1, MagickWand-config.in, and many others. We will try to help where possible but the initial CMake scripts would need to come from you or other contributors. Once proven to work, the ImageMagick team would then include them in future ImageMagick distributions and we would maintain the scripts.

@rcdailey
Copy link
Author

rcdailey commented Sep 8, 2017

Awesome, I'm glad to hear you are open to it. Is it fair to start with just MagickWand and MagicCore as a first pass? CMake offers a few facilities that can be used for general tasks you outline:

  • find_package() to grab installed dependencies on the host system, such as libpng (you would not build these yourself; user is responsible for providing installed dependencies prior to building ImageMagick)
  • configure_file() for converting foo.in files to foo, which allows you to procedurally generate certain files required for the build. Any shell scripts, configure scripts, etc that are doing this would get translated to CMake scripts that do that work.

I'm more than happy to start the first pass of this. I'll probably need your help as I go, since I'm not familiar with ImageMagick nor am I very well-versed with linux-specific build methods like autoconf and makefile. I'll try to deduce as much information as I can from existing build scripts, however I hope you won't mind me pestering you with questions in the interest of making my work easier/quicker. Let me know if this issue is an acceptable location to ask those questions.

@mikayla-grace
Copy link

We suspect it will take several iterations to get a proper CMake script. Starting with a minimal script and building upon that work makes perfect sense. We will of course help you as needed.

@rcdailey
Copy link
Author

So right now I'm starting with just building MagickCore. I'm assuming that one can be built in isolation, separate of other dependencies (I'm still learning though).

It's not immediately obvious to me how magick-baseconfig.h is generated. configure shows some hints of this file being referenced, but since I'm not very sure how to read the logic there, I think it would help to get some basic overview of how this file is generated. I will be moving this specific piece of configuration logic to CMake by hand.

Thanks in advance.

@rcdailey
Copy link
Author

Does magick-baseconfig.h come from MagickCore/config.h_vms?

@mikayla-grace
Copy link

The magick-baseconfig.h header is generated from the configure script:

AX_PREFIX_CONFIG_H([MagickCore/magick-baseconfig.h],[MagickCore])

@rcdailey
Copy link
Author

ok I'm not familiar with autotools config syntax. I need to translate a list of preprocessor definitions (the superset) to CMake, that normally autotools would generate, but CMake will do it instead. I'll do some googling on what configure.ac is doing, could take a while.

@rcdailey
Copy link
Author

Which lines in the configure.ac file trigger the #define macros that appear in magick-baseconfig.h?

@rcdailey
Copy link
Author

So maybe related, maybe not, but I'm trying to figure the configuration macros out:

There is config/config.h.in which defines stuff like:

/* Define if you have LIBRAW library */
#undef RAW_DELEGATE

The above was introduced in cb6833b. I did a git grep RAW_DELEGATE to find if it was used anywhere, and it isn't. In fact, a few other samples from that file, I can't find those macros referenced anywhere. Why are they being undef'd?

@rcdailey
Copy link
Author

rcdailey commented Sep 12, 2017

I think I'm going to wait on some educational information from you guys. I started down a really deep, deep, dark & scary rabbit hole with autoconf. I have literally never used this system. I have sufficient expertise with CMake, so hoping you guys can help spoon feed me a bit (sorry to ask for this).

I think I get the gist of what your configure scripts are doing, even though they are hundreds of thousands of lines long. If all you're doing is testing for compiler features, header files, and certain platform-specific details (like sizeof(int)), I can easily translate this to CMake logic.

Seeing as how the amount of this appears to be enormous, what is the minimum required configuration to get MagickCore building? I just want to get through a first pass. Note I'm building on Windows with MSVC 2017 for now; don't care too much about other platforms or cross compiling right now.

Thanks in advance.

@rcdailey
Copy link
Author

Sorry to keep spamming, I've made a tiny bit of progress and I want to share a few snippets with you to give you an idea of the direction I'm headed. As always, feedback is welcome.

I figured out the AC_DEFINE macro in the configure.ac is a good place to start. I'm starting from the top and working my way down, it will take a lot of time to convert the settings and checks over.

The snippets I'm sharing below are all from my branch, which is located here on my fork. You are welcome to review that any time you'd like. Just note that nothing builds yet, and it's still very very early.

I created a MagickCore/magick-baseconfig.h.in:

// exclude deprecated methods in MagickCore API
#cmakedefine MAGICKCORE_ENABLE_DEPRECATED 1

// Define if you have POSIX threads libraries and header files
#cmakedefine MAGICKCORE_THREAD_SUPPORT 1

The actual output of this file after configuring (on Windows, using defaults):

// exclude deprecated methods in MagickCore API
/* #undef MAGICKCORE_ENABLE_DEPRECATED */

// Define if you have POSIX threads libraries and header files
/* #undef MAGICKCORE_THREAD_SUPPORT */

The logic in CMake (MagickCore/configure.cmake) that generates the output:

function(_configure_magick_baseconfig_h)
    set(MAGICKCORE_EXCLUDE_DEPRECATED ${MAGICK_EXCLUDE_DEPRECATED})
    
    check_include_file(pthreads.h MAGICKCORE_THREAD_SUPPORT)
    
    configure_file(magick-baseconfig.h.in magick-baseconfig.h @ONLY)
endfunction()

All variables defined in function scope above will be inherited and used by the configure_file() command. Those variables are also referenced by magick-baseconfig.h.in. Some things will modify the baseconfig based on cache variables and options set by the user through CMake's interface (this was done before via --enable-foo flags passed to ./configure; this won't be done anymore). For example, the following option is exposed to the user (located in cmake/options.cmake):

option(MAGICK_EXCLUDE_DEPRECATED "Disable deprecated methods in MagickCore and MagickWand APIs" ON)

This appears in cmake-gui.exe on Windows like so:

image

So it's a simple tickbox that the user flips on and off. From the CLI, you can set any of these from the command line as well. Example:

$ cmake -G Ninja -D MAGICK_EXCLUDE_DEPRECATED=OFF ...

There are things the user can't control, such as the presence of pthreads.h or not, which causes MAGICKCORE_THREAD_SUPPORT to be defined or not:

check_include_file(pthreads.h MAGICKCORE_THREAD_SUPPORT)

CMake will do a mock compile of a file that tries to include pthreads.h. If it can't be included, then MAGICKCORE_THREAD_SUPPORT will be undefined, resulting in the corresponding preprocessor directive not being defined as well.

This is the general direction I'm headed for configurability of ImageMagick from CMake. Let me know your thoughts!

@mikayla-grace
Copy link

Our preference would be to create a minimal build with a few options so we can test. We would have better insight into usability at that point so we'll reserve comment until then.

@rcdailey
Copy link
Author

@mikayla-grace I've been saying this whole time that I want to do a minimal build. I'm asking for more details but your responses are too curt. Did you read my last response? I am taking a lot of my free time to do what I've done so far. Could you give me feedback based on what I talked about? This will help with general direction and getting you the minimal build.

I need your help to understand how to do the minimal build. I'm not sure how much configuration I have to do; If I can shortcut this, that's where I really need your help. I can't do anything further at the moment.

@mikayla-grace
Copy link

mikayla-grace commented Sep 13, 2017

We are a small development group and have our time overbooked by more than 1000% percent. We welcome your efforts but we can only offer minimal help. We are interested in a CMake build of ImageMagick and certainly would consider including it in the ImageMagick distributions once its proven to work well and is documented.

@rcdailey
Copy link
Author

How do you guys resolve ssize_t on Windows? This is not defined on that platform. I don't see anything in configure.ac to create a typedef for ssize_t.

@mikayla-grace
Copy link

Look for MagickCore/magick-baseconfig.h.in from git@github.com:ImageMagick/VisualMagick.git. We have this define:

#if !defined(ssize_t) && !defined(__MINGW32__) && !defined(__MINGW64__)
#if defined(_WIN64) 
typedef __int64 ssize_t;
#else
typedef long ssize_t;
#endif
#endif

@MarcoMartins86
Copy link

MarcoMartins86 commented Mar 25, 2019

I will leave here my CMake files for version 7.0.8-27, where I am building Image Magick as static libraries and not with all dependencies supported, but for me, this suffices, I am building for Windows (MSVC), Red Hat Linux and macOS.

If someone wants to use my files as a startup to make a more complete build, feel free to make it so. There's still a lot of work to do.

@urban-warrior
Copy link
Member

Thanks for your contribution. The ImageMagick development team is interested in supporting a cmake build. We could maintain the cmake configuration files if you can provide a complete set of files that can build ImageMagick as a baseline where we can add improvements over time. With the archive you provided, we get this exception:

-- Performing Test HAVE_VOLATILE
-- Performing Test HAVE_VOLATILE - Failed
CMake Error: File /home/cristy/ImageMagick-7.0.8-36/MagickCore/version.h.cmake does not exist.
CMake Error at CMakeLists.txt:1507 (CONFIGURE_FILE):
  CONFIGURE_FILE Problem configuring file

We added a mock version.h.cmake and the compile fails with

ImageMagick-7.0.8-36/MagickCore/animate.c: In function ‘XMagickCommand’:
ImageMagick-7.0.8-36/MagickCore/animate.c:495:18: error: ‘Magick/home/cristy/PackageName’ undeclared (first use in this function); did you mean ‘MagickLogFilename’?
         "%s: %s",MagickPackageName,basename);
                  ^~~~~~~~~~~~~~~~~
                  MagickLogFilename
/home/cristy/ImageMagick-7.0.8-36/MagickCore/animate.c:495:18:

@MarcoMartins86
Copy link

I am sorry, I didn't double check if I forgot a file. Monday I will repack the files (I don't have access now) and make sure I don't do the same mistake twice. Also, what platforms will you try to make the builds? It might need some adjustments on some of them. I will try to help with the best of my knowledge with this.

@urban-warrior
Copy link
Member

Our baseline is for the cmake distribution to configure and build on one or more of the following: Fedora, CentOS, or Ubuntu.

@MarcoMartins86
Copy link

@urban-warrior I've forked the project and created a branch with the CMake files, also added shared build in the meanwhile (use cmake-gui to alternate between build types). Tested in Windows and Ubuntu with minimal delegates support. Can you test the code in my branch or you prefer that I make a pull request (not sure what's the procedure).

@urban-warrior
Copy link
Member

We saw a fork in @ https://github.com/MarcoMartins86/ImageMagick but did not see any cmake configuration files such as CMakeLists.txt. Can you post a link where we can find the latest cmake configuration files?

@MarcoMartins86
Copy link

It's not in the master, is in another branch, maybe is not visible to you guys, not sure how GitHub works. Try the link directly https://github.com/MarcoMartins86/ImageMagick/tree/CMake-support

@urban-warrior
Copy link
Member

That worked. We'll review the cmake configuration files. In order for us to push your patch to the ImageMagick repo, we need you to release the cmake configuration files under the ImageMagick license. Add the license with your name as the author to at least the CMakeLists.txt source file.

@MarcoMartins86
Copy link

It should be ok with the license now.
In the meanwhile, I've added the ImageMagick.rc to Windows projects build, with some limitations.

@urban-warrior
Copy link
Member

Downloaded your repo and received link exceptions on Fedora 29:

$ cmake .
...
[100%] Building CXX object Magick++/CMakeFiles/magick++.dir/lib/TypeMetric.cpp.o
[100%] Linking CXX shared library libmagick++.so
[100%] Built target magick++
Scanning dependencies of target magick
[100%] Building C object utilities/CMakeFiles/magick.dir/magick.c.o
[100%] Linking C executable magick
/usr/bin/ld: ../MagickCore/libmagickcore.so: undefined reference to `TIFFWriteScanline'
/usr/bin/ld: ../MagickCore/libmagickcore.so: undefined reference to `gzputc'
/usr/bin/ld: ../MagickCore/libmagickcore.so: undefined reference to `gzflush'
/usr/bin/ld: ../MagickCore/libmagickcore.so: undefined reference to `png_set_compression_mem_level'
/usr/bin/ld: ../MagickCore/libmagickcore.so: undefined reference to `png_get_tRNS'
/usr/bin/ld: ../MagickCore/libmagickcore.so: undefined reference to `FT_Set_Char_Size'
/usr/bin/ld: ../MagickCore/libmagickcore.so: undefined reference to `gzeof'
...
/usr/bin/ld: ../MagickCore/libmagickcore.so: undefined reference to `png_set_sRGB'
/usr/bin/ld: ../MagickCore/libmagickcore.so: undefined reference to `BZ2_bzCompressEnd'
/usr/bin/ld: ../MagickCore/libmagickcore.so: undefined reference to `png_set_tIME'
collect2: error: ld returned 1 exit status
make[2]: *** [utilities/CMakeFiles/magick.dir/build.make:90: utilities/magick] Error 1
make[1]: *** [CMakeFiles/Makefile2:258: utilities/CMakeFiles/magick.dir/all] Error 2
make: *** [Makefile:84: all] Error 2

@urban-warrior
Copy link
Member

To support multiple versions of ImageMagick installed on the same system, can you decorate the library names? You are returning libmagickcore.so, for example, whereas we need libMagickCore-7.Q16.so. With this naming convention, both libMagickCore-7.Q16.so and libMagickCore-7.Q8.so could coexist in the same folder, for example.

@MarcoMartins86
Copy link

MarcoMartins86 commented Apr 4, 2019

OK that errors are because I have a TODO there to fix exactly that, I was building with those dependencies as static or inexistent and I need to link the libraries when they're dynamic, which is the usual case, also it would build if the machine didn't have those delegates installed. I will try to fix that later today. As for the filename, I will look into it since right now I don't have that knowledge. But my feeling is yes, as a last resort we can make a custom command to rename the filename as the last build step.

@MarcoMartins86
Copy link

Just made the changes, it should build now with those delegates. Changed also all the binaries to have the format mentioned.
Major TODOs:

  • PerlMagick is not being built (not sure if it even needs to be built, I am not familiar with the language)
  • Many delegates not being used
  • Missing compilation options for the user
  • Missing installation
  • Tests not being built

@urban-warrior
Copy link
Member

Until the community has a chance to test out your cmake configuration files and fix the TODO's, our preference is to add a link to your repo @ https://imagemagick.org/#news so the community can review your contribution. Once the configuration proves robust, we will include them in the ImageMagick master repo. If cmake build ends up being robust, we will consider removing the configure/autoconf/automake build in a future release and rely solely on the cmake build.

@MarcoMartins86
Copy link

Thank you. Although, wouldn't it be better if the branch was in the official repository?

@kmadhuraganesh
Copy link

Any updates on the official support for CMake?

@KaivnD
Copy link

KaivnD commented Sep 28, 2021

Any news about support for CMake?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

7 participants