[CMake] Added CFLAGS from MagickCore to mod_magickpp (fixes build in some OS)

[DhairyaBahl]

@ice0

This is the PR for the issue #1599 . but main purpose of this is to build using the appveyor CI and Travis CI features so that i can debug it further.

Thanks :)

[DhairyaBahl]

@ice0 @rodolforg

All test passed, no issues found. KIndly tell me if I did it correctly ?? Thank you :)

[ice0]

You forgot about that part:

# Work around for ImageMagick flags issues:
# https://gitlab.kitware.com/cmake/cmake/issues/15007
# https://gitlab.kitware.com/cmake/cmake/issues/14568
# TODO: don't hard-code
# TODO: inspect these values
set(MAGICKCORE_QUANTUM_DEPTH 16)
set(MAGICKCORE_HDRI_ENABLE 0)

You need to check if it still required, and remove it if not.

[DhairyaBahl]

@ice0 Okay, I will remove it and then check build in apveyor, if its ok :)

Thanks :)

[DhairyaBahl]

@ice0 We can't remove those lines, build is failing in the appveyor.

Kindly tell me what step to take next ?

Thanks :)

[ice0]

You need to dig deeper, and try to understand how this flags works.

1. You need to look at flags provided by pkg-config for required package.
2. Check if you set this flags correctly. You can use message(FATAL_ERROR "FLAGS: ${YOUR_FLAGS}") to check if you get it correctly.

[DhairyaBahl]

@ice0

• I was trying to understand the meaning of all the flags and CFLAGS means all required cflags, so it should do the task (maybe)

• I tried message(FATAL_ERROR "FLAGS: ${YOUR_FLAGS}") to check if the flag is provided by cflag for the required package but it
  gave error that i put arguments in wrong way. Kindly tell me what I did wrong in the following code.

if(${MAGICKCORE_CFLAGS})
	message(STATUS "MAGICKCORE_CFLAGS WORKING")
endif()

if(${MAGICKCORE_LIBRARIES})
	message(STATUS "MAGICKCORE_LIBRARIES WORKING")
endif()

I did it like this to check if that flag exists then it should give a true value and show this message on this screen.

Thanks :)

[ice0]

• I tried message(FATAL_ERROR "FLAGS: ${YOUR_FLAGS}") to check if the flag is provided by cflag for the required package but it

Just to be clear ${YOUR_FLAGS} mean variable you want to check - ${MAGICKCORE_CFLAGS} for example, or ${ImageMagick_CFLAGS} as you did.

gave error that i put arguments in wrong way. Kindly tell me what I did wrong in the following code.

Yeah, FATAL_ERROR should stop with error. Because if you use MESSAGE for debugging, you can miss the important part.

if(${MAGICKCORE_CFLAGS})
	message(STATUS "MAGICKCORE_CFLAGS WORKING")
endif()

Please read this documentation https://cmake.org/cmake/help/latest/command/if.html to better understand how to check variable. For example if you want to check if variable is defined you can use
if (DEFINED MAGICKCORE_CFLAGS) - please, note no ${} here.

But to extract variable value you should use ${}.
For example: message(STATUS "MAGICKCORE_CFLAGS: ${MAGICKCORE_CFLAGS})

[DhairyaBahl]

@ice0 Thanks for this much of help!! :) Now i should be able to solve this isse.

[DhairyaBahl]

@ice0 I tried to understand the meaning of flags and tested it with if else statements also and i get that i was doing it wrong ImageMagick_CFLAGS. I will remove it if i am right ,also kindly tell me how to check following because there are not any CFLAGS here

set(MAGICKCORE_QUANTUM_DEPTH 16)
set(MAGICKCORE_HDRI_ENABLE 0)
configure_file(config.h.cmake.in config.h) #bcoz it is having the upper two values but just as variables 

Kindly guide me further in this issue.

Thanks :)

[ice0]

You are moving in right direction. Now try to answer to next question.
How this definition is used, and why I think if you remove this line:

target_compile_definitions(mod_magickpp PRIVATE IMAGEMAGICK_CONFIG)

you can be safe to remove this lines too:

set(MAGICKCORE_QUANTUM_DEPTH 16)
set(MAGICKCORE_HDRI_ENABLE 0)
configure_file(config.h.cmake.in config.h)
target_include_directories(mod_magickpp BEFORE PRIVATE ${CMAKE_CURRENT_BINARY_DIR})

Especially notice this line:

target_include_directories(mod_magickpp BEFORE PRIVATE ${CMAKE_CURRENT_BINARY_DIR})

I think it needs only to include generated config.h file.

[DhairyaBahl]

@ice0 I am able to understand that target compile definitions are basically used to help compiler to set definitions and target compile options to set additional info like flags and target include directories for directories which will be used during compilation.

And b'coz this target_compile_definitions(mod_magickpp PRIVATE IMAGEMAGICK_CONFIG) is using
IMAGEMAGICK_CONFIG for the compile definitions that's why it is linked to config.h and those two values are part of config.h.
I mean this should be the case. Kindly tell me and guide me further.

Thanks for help :)

[ice0]

@DhairyaBahl yes. Now you need to remove unused parts from CMake and push changes to this PR :)

[DhairyaBahl]

@ice0 I have pushed the required changes in this PR. Kindly review them, if they are good to go then I will stash the commits. :)

[ice0]

@DhairyaBahl Looks good! One note - since we no longer use config.h.cmake.in, it can be safely removed as well.

[DhairyaBahl]

@ice0 Kindly review this PR and tell me if I have to make any changes or improvements.

Thank you :)

[ice0]

since we no longer use config.h.cmake.in, it can be safely removed as well.

Looks like you forgot to remove it.

[DhairyaBahl]

@ice0 I removed its mention from the file. I forgot to remove the file. Sorry for inconvenience.

Thanks :)

[DhairyaBahl]

@ice0 I fixed it. Sorry again for inconvenience. :)

[ice0]

Merged. Thank you!