cmake AddHtmlTarget: use config_file() for copying files

[CWehli]

I took up some ideas from @usamimikamen-sk's #234 and, based on @gjanssens' comments, tried to make copying files as a “custom_command”.

I start the build with the command cmake -G Ninja -S ../src -D CMAKE_INSTALL_PREFIX=..install -D WITH_HTML_INSTALL=ON, which copies all files to the build directory for each languages.

However, I could not figure out how to control the dependency between build and the copying of the files during the installation of a single component, with e.g. ninja de-html && ninja install de-html.
All files are always copied, even for the languages that are not activated.

[jralls]

All files are always copied, even for the languages that are not activated.

@CWehli That's because it's independent of the target: "If the input file is modified the build system will re-run CMake to re-configure the file and generate the build system again. The generated file is modified and its timestamp updated on subsequent cmake runs only if its content is changed.". Buildsystem (meaning ninja here) targets don't apply to cmake.

[jralls]

I took up some ideas from @usamimikamen-sk's #234 and, based on @gjanssens' comments, tried to make copying files as a “custom_command”.

@gjanssens had a lot of comments on #234, and @usamimikamen-sk ultimately abandoned the PR and deleted their branches. Can you be a bit more specific about what is the goal of this PR?

[jralls]

@CWehli ping?

[CWehli]

@jralls:
I took @gjanssens' comments as an opportunity to replace copying the files using “add_custom_command” with cmake's own “configure_files” command, thereby improving the build process.

[gjanssens]

I only noticed this PR now. I'll look at it later this weekend (which is unfortunately probably too late for the upcoming release).

[jralls]

@gjanssens Don't worry about the release, this is just about building anyway.

[gjanssens]

Ok, I had some time to think this over. In hindsight my comment on #234 did not offer the ideal solution either. At the time I missed the fact that configure_file can't be limited to one build target as you have dicovered now and @jralls explained.

So there's a choice to make here:

• configure_file gives very short and readable code, but it will clutter the build directory with files you may not need.
• a custom command is more cumbersome, but avoids needless file copying.

So if I look back at this today, I personally would keep using a custom command to copy target-specific static files such as figures.

I should also clarify that my issue with the code in #234 was really the use of a dummy output file (xyz-trigger) to determine whether a custom command had run or not. My alternative suggestion had its own issues as already mentioned. The dummy output file is no longer used for figures. It simply tracks each copied figure itself. The same could be done for the stylesheets icons to eliminate the dummy output file style_trigger as well. The output of the custom command that generates the html files itself needs to be a dummy output file, because we don't know beforehand which files will be generated.

[gjanssens]

Another thing I have changed my mind on since that closed PR is the use of file(GLOB ...).

While very convenient, it won't track adding new files in that directory. So if someone adds a new figure, that will not be noticed by the build system until the cmake cache is cleared. Cmake scripts are usually written in such a way that clearing the cmake cache is hardly ever necessary, so requiring this to add a new figure is a confusing thing to do, not only for the person adding the image, but also for all others that have existing checked out projects pulling in the change.

It's slightly more cumbersome, but better to explicitly state all files you need. That's why we have lists of figures for each document in each language.

At the time I wrote the cmake scripts for gnucash-docs I took a lazy shortcut for the stylesheet icons, figuring we never add one there anyway. That's likely the case but may not absolutely be so. It would be cleaner to also explicitly define a variable with a list of icons to copy. Considering these change that rarely, I don't consider that too important. That list by the way should probably be defined in a cmake file somewhere in the stylesheet directory rather than in the cmake_HTML target.