> IMHO current cmake in master is not ready for real usage:

So?

> 1. It uses autotools files:
>
>   builds/unix/configure.raw
>   builds/unix/freetype2.in

`configure.raw` gets accessed to extract the current libtool DLL version; it is the only place where this is stored.  I consider this solution much better than specifying it again in another place.

The current `CMakeLists.txt` file also creates a `freetype2.pc` file (based on `freetype2.in`), needed for many other projects.  It's not clear to me why your patch adds a `freetype2.cmake` file that is completely equivalent to `freetype2.pc`.  Again, it's a much better solution IMHO to have a single template file storing all the necessary information, probably making `CMakeLists.txt` slightly more complicated than necessary.

> 3. It check UNIX not HAVE_MMAP to use mmap() on platform.

OK.

> 4. It requires two pass execution to build static and shared lib.

Well, this isn't a show-stopper at all.  However, I don't object if this gets simpler.

> 5. It does not install: freetype-config, freetype-config.1.gz, freetype2.m4
> (at least for me in short test)

You are the first that notices this in all the years.  It's OK with me if this gets added.

> 6. All config flags only set REQUIRED to depend libs,
>    it is weird behavior, as user I want ON/OFF,
>    then on - REQUIRED, off - do not use, even if lin available.

Again, this is not a show-stopper.  It's rather YOUR preferred behaviour :-)
Since I'm not a CMake user I won't argue further whether this is 'common practice' or not.

> I add BROTLI option.

Brotli support is already present in the current `CMakeLists.txt` file.


To summarize:

If you want to contribute changes and improvements to the CMake support, for which I'm grateful, please submit small, incremental patches that address single problems.  Right now, you are delivering a single, big diff file that does everything.  I won't accept that.

Pushed the change for compiling builds/unix/ftsystem.c on UNIX instead of the generic one. Is there anything here left to incorporate?

I started a branch at http://git.savannah.gnu.org/cgit/freetype/freetype2.git/log/?h=cmakelists-updates to update the CMakeLists.txt file.

I found that the default build system builds produces static and shared libs in one go, by compiling twice (or thrice?). You can get a similar effect by building twice with cmake: ``cmake -B build-shared -D BUILD_SHARED_LIBS=ON && cmake -B build-static``, installing the result then places shared and static object next to each other. The freetype-config*.cmake files will be overwritten however. There are probably ways around that, but I am honestly unsure if packagers should instead just use the default build system if they want everything in one go.

Thanks to all of you for taking care of this issue!

> "Is this a CMake bug?" - I do not remember, but I remember that BZip2 does not work like all other staff, and this is the best solution that I found.

But it does work in the setup in git master on my Linux system? I am strongly opposed to adding platform-specific hacks. If it turns out to be a CMake bug, the bug must be fixed, not the CMakeLists.txt.

> "Does check_c_compiler_flag by itself not suffice?" - yes, it not enough.

I mean check with check_c_compiler_flag and then add it to the target via target_compile_options.

> "Why is this necessary?" - this supress some cmake warnings at least on freebsd, that adds extra spaces to CFLAGS.

I have not seen these warnings on my system, please check on your system where those spaces are coming from. The CMakeLists.txt file is not the place to scrub input.

> "This looks like it's duplicating what is possible with" - as I remember they in different format and, probably, different content.

Please use the "standard" CMake way to do things. I know that CMake has been anarchy for years, but there are efforts to make this less hare-brained. See e.g. https://gist.github.com/mbinna/c61dbb39bca0e4fb7d1f73b0d66a4fd1#modules. If we need a newer CMake to do this cleanly, then I'm all for it. Also, see further guides at https://gist.github.com/mbinna/c61dbb39bca0e4fb7d1f73b0d66a4fd1#sources.

Somewhere, there is an explanation for why you should not do freetype_static and freetype_shared (one reason is that it makes handling easier for projects that include FreeType; CMake will do the right thing with the library depending on what the parent project needs). More info: https://cgold.readthedocs.io/en/latest/tutorials/libraries/static-shared.html. This says that autotools compiles twice under the hood anyway, so...

> Good and proper way is add ftoption.h.cmake, ftconfig.in.cmake, freetype2.cmake, freetype-config.cmake and do not use hacks for pathing autotools files.

The problem is this: CMake is an unofficial build system for FreeType, so Werner won't do much with it and someone has to maintain it to keep it useful. Your patch already increases the maintenance burden, splitting off duplicate files does so even more.

Remember that you are asking a small handful of FreeType contributors to maintain your changeset, and time for that is sparse to nonexistent. I too would prefer we ditch the custom build system and go full-on CMake or (better yet) Meson, but unless we get Werner on board, we can forget about it.

What would go through is a change to the default build system so that there is one set of files that works for both systems.

> options (all options from autotools)

Mh, OK.

> No auto detection for dep libs!!! All off by default.

I am curious, where is that behavior the default? The custom build system certainly does autodetection. Do you have "best practice" guides for this somewhere?

> Options for hardening

I am not convinced that these need to be in the upstream build definition. They are also GCC specific from the looks of it. Please do all of these downstream, or on your local system.

"What about  the opposite? A user  removes the comment by hand and cmake fails  to find it. Cmake should put the cheese moment back." - Build system should not care about all things that user can do after dist unpacked, it only build unmodified sources.
If users do some modifications by hands - then user should handle all build errors.

So I finally took the time to look at the proposed changes and I am unhappy with them, they increase the complexity of the build definition without a clear benefit.

if (FT_WITH_BZIP2)
  find_package(BZip2 REQUIRED)
  #list(APPEND CMAKE_REQUIRED_LIBRARIES_PRIV bzip2)
  set(BZIP2_LDFLAGS "-lbz2") # pkg_check_modules() always fail on BZip2.
endif ()

Is this a CMake bug? Please avoid verbatim linker and compiler flags and leave that to CMake if at all possible. If you find that something specific is not available in the minimum required CMake version, we can always increase the minimum required version.

macro(try_c_flag prop flag)
  # Try flag once on the C compiler.
  check_c_compiler_flag("-Werror ${flag}" C_FLAG_${prop})
  if (C_FLAG_${prop})
    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${flag}")
  endif ()
endmacro()

macro(try_linker_flag prop flag)
  # Check with the C compiler.
  set(CMAKE_REQUIRED_FLAGS ${flag})
  check_c_compiler_flag(${flag} LINKER_FLAG_${prop})
  set(CMAKE_REQUIRED_FLAGS "")
  if (LINKER_FLAG_${prop})
    set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${flag}")
    set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} ${flag}")
    set(CMAKE_MODULE_LINKER_FLAGS "${CMAKE_MODULE_LINKER_FLAGS} ${flag}")
  endif ()
endmacro()

Does check_c_compiler_flag by itself not suffice? Please do not change variables with wide scope like CMAKE_C_FLAGS. Always use target_*.

Effective Modern CMake: https://gist.github.com/mbinna/c61dbb39bca0e4fb7d1f73b0d66a4fd1

# Hardening flags
try_c_flag(FPIE "-fPIE")
try_linker_flag(PIE "-pie")

I don't see this in the default build system's output, so please don't. This is something that downstream must decide.

# Silently strip whitespace.
string(STRIP "${CMAKE_C_FLAGS}" CMAKE_C_FLAGS)
string(STRIP "${CMAKE_EXE_LINKER_FLAGS}" CMAKE_EXE_LINKER_FLAGS)
string(STRIP "${CMAKE_MODULE_LINKER_FLAGS}" CMAKE_MODULE_LINKER_FLAGS)
string(STRIP "${CMAKE_SHARED_LINKER_FLAGS}" CMAKE_SHARED_LINKER_FLAGS)

Why is this necessary?

if (HAVE_MMAP)
  list(APPEND BASE_SRCS "src/base/ftsystem.c")
else ()
  # xxx: unix, amiga, vms.
  list(APPEND BASE_SRCS "builds/unix/ftsystem.c")
endif ()

What's the logic here? The builds/unix/ftsystem.c file should have indeed been used on Unix, but Unix has mmap so this won't be compiled on Unix?

add_library(freetype_static STATIC $<TARGET_OBJECTS:freetype>)
target_link_libraries(freetype_static PRIVATE
  ${BZIP2_LIBRARIES}
  ${ZLIB_LIBRARIES}
  ${PNG_LIBRARIES}
  ${HARFBUZZ_LIBRARIES})
set_target_properties(freetype_static PROPERTIES OUTPUT_NAME "freetype")

The static/shared nature of the library target can be controlled by the BUILD_SHARED_LIBS variable. What needs a seperate definition?

if (NOT SKIP_INSTALL_LIBRARIES AND NOT SKIP_INSTALL_ALL)
  if (UNIX)
    # Create libs LD flags list.
    set(LIBS_LDFLAGS_LIST ${BZIP2_LDFLAGS})
    foreach (__m ${CMAKE_REQUIRED_LIBRARIES_PRIV})
      string(TOUPPER ${__m} __prefix)
      pkg_check_modules(${__prefix} QUIET ${__m})
      list(APPEND LIBS_LDFLAGS_LIST ${${__prefix}_LDFLAGS})
      list(APPEND LIBS_LDFLAGS_LIST ${${__prefix}_LDFLAGS_OTHER})
    endforeach ()
    list(REMOVE_DUPLICATES LIBS_LDFLAGS_LIST)
    string(REPLACE ";" " " LIBS_LDFLAGS_LIST "${LIBS_LDFLAGS_LIST}")

This looks like it's duplicating what is possible with

target_link_libraries(freetype PRIVATE ${ZLIB_LIBRARIES})
target_include_directories(freetype PRIVATE ${ZLIB_INCLUDE_DIRS})

What keeps you from using the exported defines by FindPackage?

# Generate the freetype-config file.

Please don't introduce a second freetype-config/freetype2.cmake file, keep the hacks to modify the existing ones. I don't like it either and would prefer if the standard file would be easily filterable by configure_file, but that would require changes in the default build system and I just don't have the time for that. Maybe you can find a way to filter the file first to turn it into something that configure_file can work with.

Can you please list the changes to the build and installation behavior you need to use CMake as the sole build system to build packages, etc.? Then we can distil what changes are actually needed.

What about  the opposite? A user  removes the comment by hand and cmake fails  to find it. Cmake should put the cheese moment back.

I this is what configure does.

Not all, ~half.
There is many changes, probably it will be easy to read after you apply patch.

Add options:
FT_DEBUG_MEMORY - "Memory Debugging."
FT_DEBUG_LEVEL_ERROR - "Build in error mode."
FT_DEBUG_LEVEL_TRACE - "Build in trace mode."
PCF_CONFIG_OPTION_LONG_FAMILY_NAMES - "Prepend the foundry name (plus a space) to the family name."
AF_CONFIG_OPTION_TT_SIZE_METRICS - "Use TrueType-like size metrics for 'light' auto-hinting."

Update comments.

Leave only CFLAGS:-fPIE and LD_FLAGS:-pie

(file #47095)

It should also be able to put the comment back if the package is not found. This is what configure does.

It is imperative to stay aligned between cmake and autoconf/make procedures because FreeType requires ultra-portability. This is to say that we do not optimize for a particular system or distribution, we are stay agnostic. Each distribution can do it's own tricks and compilation flags but FreeType won't distribute them. So please stay conservative: no c11 or gcc-specific features please.

There is indeed a bug in cmake build that does not comment out (disable) missing PNG or HARFBUZZ. Note that autoconf does that. Nikolaus, please fix this.

I remove most of CFLAGS and hardening things.

Autodetect dependencies is evil because:
- user can not chose what he need
- user can buld package on system with all deps but target system may not have all deps, this is hard to control.

For example I buld FreeBSD ports and make packages on my home workstation. Some installed port may require HARFBUZZ and install it. Then if I rebuild/update freetype - it may auto add HARFBUZZ as dep, and then this package will be installed on other system - app may fail to run because there is no HARFBUZZ.
I came here because this already happen with me.

Remove -fsanitize=safe-stack die to brokes some apps
add target_link_libraries() for static and dynamic libs

(file #45783)

Thanks for your contribution.  However, there are very serious problems with your patch.

• I very much dislike indentation by tabs.  AFAIK, this is not necessary for cmake, so please stay with two spaces per indentation level.

• Similarly, you are introducing overlong lines, which I very much dislike, too.  If possible, please stay within a line length of 80 characters – yes, I'm old-fashioned and conservative :-)

• To make a comparison with your version and the current state possible, please apply your changes incrementally.  I want to have a series of patches that introduce your added functionality in steps as small as possible – and formatting should be the last, optional patch in this series.

• Documentation is completely lacking in case you are adding new features.  You must fix this.

• I dislike that you add new files `freetype-config.cmake' and `freetype2.cmake'.  Why?  We already have similar files, and maintaining two such files is error-prone.  This alone is actually a reason to reject your patch...

• I need ChangeLog entries for each step (these entries should not be part of the patch files but rather added separately in the preambles of those files).

Please retry.