CMake improvements for local build (with static library)

[KKoovalsky]

Hello! Thank you, @eyalroz, for continuing the project.

I prefer the fork, since it supports CMake. Likewise, I use it with FetchContent:

function(ProvidePrintfLibrary)

    include(FetchContent)

    set(BUILD_STATIC_LIBRARY ON)
    set(SUPPORT_EXPONENTIAL_SPECIFIERS OFF)
    set(SUPPORT_LONG_LONG OFF)
    set(ALIAS_STANDARD_FUNCTION_NAMES OFF)
    set(DEFAULT_FLOAT_PRECISION "3")
    set(MAX_INTEGRAL_DIGITS_FOR_DECIMAL "3")

    FetchContent_Declare(printf_library
        GIT_REPOSITORY https://github.com/eyalroz/printf.git
        GIT_TAG v5.0.0
    )
    FetchContent_MakeAvailable(printf_library)

    FetchContent_GetProperties(printf_library SOURCE_DIR printf_source_dir)
    target_include_directories(printf PUBLIC $<BUILD_INTERFACE:${printf_source_dir}>)
    
endfunction()

I would like to address some miscellaneous issues which I had to bypass:

1. I use the function here to set the local variables to not mutate the parent scope. "Normal variables" are used instead of options. There might be a risk that the name of BUILD_STATIC_LIBRARY may clash with other libraries/dependencies - it's quite general. I would prefer to make the name more specific by adding a PRINTF_ prefix to the options, so the variables could be preset by the user also with the option command.
2. I had to propagate BUILD_INTERFACE include directory, because using #include "printf.h" causes a compile error, since no include directories are propagated by the library.
3. Some variables in the config mismatch the names used in the code. One example is PRINTF_INTEGER_BUFFER_SIZE CMake option. In the code, it is PRINT_INTEGER_BUFFER_SIZE. It means that the change to the former doesn't affect the latter.

I will create a PR that addresses the issue.

[eyalroz]

First, thank you for taking the time to file the issue and even a PR. I'll reply both here and on the PR page.

1. I am not sure I am too thrilled about the use of the project's CMakeLists.txt via add_subdirectory... but I suppose that's legitimate. I'm certainly willing to cater to such use by prefixing my option/variable names; but I'm not so sure about encouraging it in the REAMDE.md file.

2. "no include directories are propagated by the library" <- when the library itself is built, it doesn't need any include directory, since printf.c and printf.h are located in the same place. But the library does propagate an include directory - when it is installed. That is how I intended for automated use to work. Are you asking me to add the repository's root as an include directory of the printf library target?

   ... also, I should say that this discussion makes me wonder about the placement of printf.h and printf.c. See Should I move printf.c and printf.h into a subdirectory? #58.

3. Hmm, yes, that's definitely a bug, and the result of me rushing things somewhat. I wish you had filed that separately before your larger PR... anyway, it's now integer-to-string buffer size not properly set via CMakeLists.txt #59 and has been fixed.

---

PS:

Likewise

Just a phrasing nitpick: I don't think you can use "likewise" like you have in your first paragraph. I think you meant "additionally" or "also"; the second sentence does not discuss something that is like the subject of the first sentence.

[KKoovalsky]

What do you mean by not encouraging it in the README.md? Not to document the approach with FetchContent module? I come from embedded systems, writing firmware for small architectures, like ARM Cortex M, where I always cross compile, thus I never install the libraries. It means that FetchContent is the main approach for getting the dependencies to the project. Since the project is tiny, consuming it in such a way is preferable. Moreover, the library is implicitly addressed to such small, bare-metal projects, where it is preferred to get dependencies on the fly, and link them via the build interface. One more thing is, that if we support installation of the project, then supporting it to be used via FetchContent could be one more way to utilize it. Lots of libraries support such approach. I don't want to say that "let's do it, because others do that", but more "let's do it, because it became a standard".

To support that, not really much should be made:

1. Propagate the BUILD_INTERFACE include directory: https://github.com/eyalroz/printf/pull/57/files#diff-1e7de1ae2d059d21e1dd75d5812d5a34b0222cef273b7c3a2af62eb747f9d20aR66. The include directory may be the root directory or any. I would prefer to include it like: #include "printf/printf.h". For the installed lib: #include <printf/printf.h>. IMO, it should be supported out of the box.
2. Prefix the options. When the CMake options and Makefile defines have the same name, then generation of printf_config.h from printf_config.h.in is quite easy. Additionally, it's consistent.
3. Add PRINTF_ALIAS_STANDARD_FUNCTION_NAMES to compile definitions to support that option for the build interface.

Those changes are included in the PR.

@eyalroz Let's discuss that and later on I can create a PR, or you can add the changes on your own. Whatever you prefer.

---

P.S. Thanks for that nitpick. I stupidly used the hint from spelling add-on.

[KKoovalsky]

@eyalroz what is your opinion on that? :)

[eyalroz]

@KKoovalsky : Oh, sorry, I misread "and later on I" for "later on when I".

[eyalroz]

tl;dr: Create your PR and I'll basically accept it.

What do you mean by not encouraging it in the README.md?

I mean, I am hesitant to list that as one of the ways to use the library. There are other ways as well, e.g. using ExternalProject, which I don't list. If I list it, then I'm obligated to support my CMakeLists.txt working in two distinct and possibly slightly contradictory ways.

I come from embedded systems, writing firmware for small architectures, like ARM Cortex M, where I always cross compile, thus I never install the libraries

Installing the libraries does not necessarily mean installing them on the target machine. They can be built for the target machine on a build machine and installed on the build machine, for use withe rest of your code.

Moreover, the library is implicitly addressed to such small, bare-metal projects, where it is preferred to get dependencies on the fly, and link them via the build interface.

Well, on that end of the spectrum - you could just take the two source files: printf.c and printf.h. The point of add_subdirectory() is to employ non-trivial aspects of the library's build mechanism.

Lots of libraries support such approach. I don't want to say that "let's do it, because others do that", but more "let's do it, because it became a standard".

That's what I'm worried/annoyed about. See also the issue I opened about it.

Unfortunately, I guess I just have to live with it. And Kitware is not helping project maintainers authors to cope with the conflicting use cases. Even the most trivial assistance, like a page with basic tips, or a small feature such as allowing for scoping or prefixing the subdirectory's options with its name or project name - is not offered to us.

I would prefer to include it like: #include "printf/printf.h". For the installed lib: #include <printf/printf.h>. IMO, it should be supported out of the box.

We'll see what you put in your PR. Are you worried about conflicts with another printf.h?

Add PRINTF_ALIAS_STANDARD_FUNCTION_NAMES to compile definitions to support that option for the build interface.

It's a CMake option and a value defined in printf_config.h.in - just like all the other options. So I'm not quite sure what you're suggesting here.

[KKoovalsky]

@eyalroz I understand your point. Having to support multiple ways of incorporating the library into a project might be inconvenient. I think we should find a sweet point between maintainability and usability. The usability is created by the users. When you go to a shop, you expect from the owner to fulfil your demand. If he doesn't, you simply won't come again. Of course, if you demand something impossible, like buying an elephant for $10, no owner will provide that. This is the sweet point I'm talking about. Supporting multiple ways of incorporating the library in a project won't be hard. Thanks to it, more users could benefit from this particular library. The other question is: do we care to have more users? I think we do, because this is FOSS: the fewer users we have, the less sense there is to maintain the library.

Regarding installation and cross-compiling, I meant that I never install the libraries and use FetchContent instead, because:

• When cross-compiling various, architecture-specific compile flags are involved. To properly compile the library for a specific architecture, I have to do it from sources. The compilation flags have to be tweaked sometimes. Redoing all that tweaking separately for each library would be a pain. FetchContent is very convenient, because it reuses the same toolchain file, which is used by the main CMake project.
• I didn't mean to install the library on the target machine. There is no point in doing that, because all the executables I compile I link statically. Basically, for such "small" CPUs one uses one executable per device. You only flash the executable, because there is no space for more.
• FetchContent is convenient when one wants to incorporate the dependency to the project automatically. I don't have to ask developers, or document, that they need to build and install some dependency.

I will create a PR with fixing the path to installed printf.h after this is merged: #65