search

doxygen / doxygen

Official doxygen git repository
https://www.doxygen.org
GNU General Public License v2.0
5.62k stars 1.27k forks source link

Doxygen 1.8.15 CMake 3.13 incompatibility #6725

Closed thomas-t1 closed 5 years ago

albert-github commented 5 years ago

And the question is ?

thomas-t1 commented 5 years ago

System: MacOs; FindDoxygen.cmake generates invalid cmake code with doxygen 1.8.15, 1.8.14 works. I'm sure it is debatable where the bug is, but it changed from 1.8.14 to 1.8.15

Make Error at /Users/vsts/agent/2.144.0/work/1/b/CMakeDoxygenDefaults.cmake:471 (set): Syntax error in cmake code at /Users/vsts/agent/2.144.0/work/1/b/CMakeDoxygenDefaults.cmake:471 when parsing string \makeindex Invalid character escape '\m'.

albert-github commented 5 years ago

Can you give a bit more precise the steps you are taking (starting from the initial cake command). It looks a bit like it is trying to execute either a LaTeX or Windows command. Did you run doxygen 1.8.14 also with Cmake 3.13 or with an older version? Maybe you can also attach the file CMakeDoxygenDefaults.cmake.

albert-github commented 5 years ago

I just installed (on Windows though!) CMake version 3.13.2 and I didn't see any problems. Furthermore I searched for the file CMakeDoxygenDefaults.cmakebut haven't been able to find it.

thomas-t1 commented 5 years ago

I ran both doxygen 1.8.14 and 1.8.15 with CMake 3.13.1.

Getting the CMakeDoxygenDefaults.cmake is a bit tricky since the build is running in an Azure DevOps Pipeline on a Microsoft hosted Macos build agent and I'm not working on a Mac.

It is not a docker image at the moment, but installs doxygen via brew at build-time and it started failing as soon as 1.8.15 was available through brew.

Apparently others are being affected by this issue as well; https://gitlab.kitware.com/cmake/cmake/issues/18738

albert-github commented 5 years ago

This looks like the 64-bit windows version, I tested the 32-bit version 3.13.2 (shouldn't give a problem but ...)

thomas-t1 commented 5 years ago

The CMake command giving the error on MacOs is something like this;

    doxygen_add_docs(BeamformerDocs
    doc/NoveldaBeamformer_README.md
    inc/Beamformer.h inc/BasebandData.h inc/Cluster.h inc/PulseDopplerData.h
    ALL)
albert-github commented 5 years ago

Looks like that CMake tries to resolve something regarding the new LATEX_MAKEINDEX_CMD and this command is only used inside .tex files and should in my opinion not be wrapped by CMake.

thomas-t1 commented 5 years ago

I was afraid that this would end up in a blame-game :) but the reality is that doxygen 1.8.15 introduced a regression wrt cmake. The temporary solution in my case was to revert to doxygen 1.8.14.

For anyone interested in a solution for Azure DevOps hosted Macos agents; The brew formulae repo on the hosted agents is a shallow clone, so it required some trickery to downgrade to a working doxygen i.e. 1.8.14

It required a script task;

cd "$(brew --repo homebrew/core)"
# The brew repo is shallow, so the sha for version 1.8.14 isn't
# available until we fetch all the history.
git fetch --unshallow
git checkout 8eb7ac11 Formula/doxygen.rb
brew install doxygen
git reset HEAD Formula/doxygen.rb
git checkout -- Formula/doxygen.rb
albert-github commented 5 years ago

When I understand you correctly you also think that CMake is the creator of the is the problem and not doxygen (due to a wrong "translation" on their side).

thomas-t1 commented 5 years ago

Well, not really, I consider it a regression in doxygen since the upgrade from doxygen 1.8.14 to 1.8.15 (in homebrew) introduced an error with a cmake build, but it all depends on whether doxygen considers cmake an important tool to be compatible with or not. I should mention that I don't fully understand the reason for failure yet, I've had to focus on fixing the build :)

albert-github commented 5 years ago

I don't agree with you (but this was already clear), CMake does a "translation" which is incorrect and that surfaced now. CMake is important, doxygen does use it for their builds as well.

I just checked the doxygen code and see that when there is no \ in front of the makeindex it is prepended in the code. Could you please test this by setting in your Doxyfile: LATEX_MAKEINDEX_CMD=makeindex

thomas-t1 commented 5 years ago

The build pipeline now configures and builds with cmake without error - with doxygen 1.8.15 :)

Your workaround is clearly a much better solution, although hopefully a temporary one; Thank you very much for your support.

albert-github commented 5 years ago

To implement my "solution" I would like to have confirmation that it does work (as I have not been able to reproduce the problem).

thomas-t1 commented 5 years ago

I can provide evidence tomorrow (I don't have direct access to a mac at the moment) in the form of cmake cache files and doxygen config files for the two cases. Extracting temporary files from the hosted build servers is tricky.

albert-github commented 5 years ago

OK no problem.

pgajdos commented 5 years ago

Hi!

I am experiencing the same while building cmocka and libssh: https://build.opensuse.org/package/live_build_log/openSUSE:Factory:Staging:L/libssh/standard/x86_64 https://build.opensuse.org/package/live_build_log/openSUSE:Factory:Staging:L/cmocka/standard/x86_64

This fixes it:

Index: doxygen-1.8.15/src/config.xml
===================================================================
--- doxygen-1.8.15.orig/src/config.xml  2018-12-27 19:05:37.000000000 +0100
+++ doxygen-1.8.15/src/config.xml   2019-01-07 13:11:07.980751501 +0100
@@ -2594,7 +2594,7 @@ EXTRA_SEARCH_MAPPINGS = tagname1=loc1 ta
 ]]>
       </docs>
     </option>
-    <option type='string' id='LATEX_MAKEINDEX_CMD' defval='\makeindex' depends='GENERATE_LATEX'>
+    <option type='string' id='LATEX_MAKEINDEX_CMD' defval='\\makeindex' depends='GENERATE_LATEX'>
       <docs>
 <![CDATA[
  The \c LATEX_MAKEINDEX_CMD tag can be used to specify the command name to

Take it as a hint, I have no idea about correct solution.

albert-github commented 5 years ago

@pgajdos Thanks for the hint, but it won't work as the result in the Doxyfile would be: LATEX_MAKEINDEX_CMD = \\makeindex and consequently in the latex/refman.tex: \\makeindex did you try also without the \ as suggested to @thomas-t1 ?

@thomas-t1 any results of your tests?

pgajdos commented 5 years ago

Hmm.

$ grep -r '\\makeindex' .
./build/doc/Doxyfile.docs:LATEX_MAKEINDEX_CMD    = \makeindex
./build/CMakeDoxygenDefaults.cmake:    set(DOXYGEN_LATEX_MAKEINDEX_CMD \\makeindex)
$
albert-github commented 5 years ago

Did you create your ./build/doc/Doxyfile.docs from scratch or reused an old one? When you have already s file with LATEX_MAKEINDEX_CMD with one \ this will not be updated. As far as I saw in the CMake code they use again a doxygen -g so creating it from scratch.

pgajdos commented 5 years ago

Building cmocka with doxygen without the patch above (cmake fails):

$ grep -r 'makeindex' .
./build/CMakeDoxygenDefaults.cmake:    set(DOXYGEN_MAKEINDEX_CMD_NAME makeindex)
./build/CMakeDoxygenDefaults.cmake:    set(DOXYGEN_LATEX_MAKEINDEX_CMD \makeindex)
$ ls -l ./build/doc/Doxyfile.docs:
ls: cannot access './build/doc/Doxyfile.docs:': No such file or directory
$

Building cmocka with doxygen with the patch above on the same buildroot, regenerated and build directory completely deleted:

$ grep -r 'makeindex' .
./build/doc/Doxyfile.docs:MAKEINDEX_CMD_NAME     = makeindex
./build/doc/Doxyfile.docs:LATEX_MAKEINDEX_CMD    = \makeindex
./build/CMakeDoxygenDefaults.cmake:    set(DOXYGEN_MAKEINDEX_CMD_NAME makeindex)
./build/CMakeDoxygenDefaults.cmake:    set(DOXYGEN_LATEX_MAKEINDEX_CMD \\makeindex)
$
albert-github commented 5 years ago

I'm not familiar with cmocka, so some questions:

pgajdos commented 5 years ago

Yes, https://cmocka.org/#git points to git clone git://git.cryptomilk.org/projects/cmocka.git and the github repo you referenced also points there.

$ cmake -DCMAKE_INSTALL_PREFIX=/usr -DCMAKE_BUILD_TYPE=Debug ..
[..]
 /home/abuild/rpmbuild/SOURCES/cmocka-1.1.3/build/CMakeDoxygenDefaults.cmake:471

  when parsing string

    \makeindex

  Invalid character escape '\m'.
[..]
$

Relevant cmake code:

470: if(NOT DEFINED DOXYGEN_LATEX_MAKEINDEX_CMD)
471:    set(DOXYGEN_LATEX_MAKEINDEX_CMD \makeindex)
472: endif()

Note cmake thinks I want to escape something. Escaping \ by \ (in the patch) results to:

470: if(NOT DEFINED DOXYGEN_LATEX_MAKEINDEX_CMD)
471:    set(DOXYGEN_LATEX_MAKEINDEX_CMD \\makeindex)
472: endif()

seem to satisfy cmake here. Of course I can not imagine what happens later.

pgajdos commented 5 years ago

(I get also the same error with libssh, https://www.libssh.org.)

albert-github commented 5 years ago

I understand, but seen my earlier comment:

I just checked the doxygen code and see that when there is no \ in front of the makeindex it is prepended in the code.

I think it would be better to remove the \ in this case completely (so in the set command and the config.xml).

pgajdos commented 5 years ago

Apologize for missing that point. Yes, this fixes the build as well and the result is:

$ grep -r 'makeindex' .
./build/doc/Doxyfile.docs:MAKEINDEX_CMD_NAME     = makeindex
./build/doc/Doxyfile.docs:LATEX_MAKEINDEX_CMD    = makeindex
./build/CMakeDoxygenDefaults.cmake:    set(DOXYGEN_MAKEINDEX_CMD_NAME makeindex)
./build/CMakeDoxygenDefaults.cmake:    set(DOXYGEN_LATEX_MAKEINDEX_CMD makeindex)
$

Is that okay?

albert-github commented 5 years ago

I think this is OK, only tests with the generation and building of the LaTeX documentation will make it certain (but I think it will be).

pgajdos commented 5 years ago

Thanks!

thomas-t1 commented 5 years ago

We did some tests to collect evidence, and the set(DOXYGEN_LATEX_MAKEINDEX_CMD "makeindex") definetly fixes the problem; i.e. without the previous CMake set statement, the cmake configure step fails, with it the configure step succeeds.

However CMakeDoxygenDefaults.cmake is identical in the two cases, and the offending line is identical; set(DOXYGEN_LATEX_MAKEINDEX_CMD \makeindex) I guess this makes sense since it is somehow generated from doxygen output.

However in the failure case, the specific doxygen file for the project is not generated because of the error we're discussing.

In the succeeding case the doxygen file for the project contains the following line LATEX_MAKEINDEX_CMD = makeindex

Note that we tested both CMake 3.12 and 3.13 by chance this time :)

albert-github commented 5 years ago

@thomas-t1 Thanks.

I'll see if I can make a proposed patch later today (in my opinion still to overcome a problem in the CMake conversion from the generated Doxyfile to their default settings file). I don't think the problem is depending on the version of CMake as it looks to me that it is a general problem.

thomas-t1 commented 5 years ago

@albert-github Thanks! I agree now that I understand the issue better; the technically correct way to fix this is in CMake, but I think the pragmatic thing is to revert the Doxygen change and perhaps reintroduce it when CMake has had the appropriate support for while...

"You are technically correct - the best kind of correct" - Number 1.0

albert-github commented 5 years ago

@thomas-t1 Thanks. The option can stay only the default in config.xml has to be changed from \makeindex to makeindex.

albert-github commented 5 years ago

I've just pushed a proposed patch, pull request #6750

albert-github commented 5 years ago

Code has been integrated in master on github.

damian123 commented 5 years ago

I used this workaround in CMake by changing FindDoxygen.cmake ...... 705 | file(STRINGS "${_Doxygen_tpl}" _file_lines) 706 | unset(_Doxygen_tpl_params) 707 | foreach(_line IN LISTS _file_lines) 708 | if(line MATCHES "([A-Z][A-Z0-9]+)( =)(.)") 709 | set(_key "${CMAKE_MATCH_1}") 710 | set(_eql "${CMAKE_MATCH_2}") 711 | # string(REPLACE ";" "\\n" _value "${CMAKE_MATCH_3}") 712 | # list(APPEND _Doxygen_tpl_params "${_key}${_eql}${_value}") 713 | string(REPLACE "\" "\\" _value1 "${CMAKE_MATCH_3}") 714 | string(REPLACE ";" "\\n" _value2 "${_value1}") 715 | list(APPEND _Doxygen_tpl_params "${_key}${_eql}${_value2}") 716 | endif() 717 | endforeach() ... | ......

Ref: CMake Issue: Not compatible with Doxygen 1.8.15

iyanmv commented 5 years ago

This should fix the issue: https://gitlab.kitware.com/cmake/cmake/commit/f4547578aa365164f1ef48c4f546828752cf4835

eli-schwartz commented 5 years ago

CMake is conceptually broken, as it tries to reparse the Doxyfile format by generating a blank template using doxygen -s -g Doxyfile, reading it in line by line as cmake strings, then writing it back out after playing ridiculous escaping games on the results rather than simply using @INCLUDE to append its own custom settings. See https://gitlab.kitware.com/cmake/cmake/issues/18738#note_507076

As such, cmake has gone ahead and updated their fragile escaping rules and released a new version of cmake to fix it.

They broke it, they fixed it. I don't particularly see why anything else is needed. Does Doxygen document the default settings as something it is okay for third parties to try to parse using repeated regular expressions followed by direct inclusion into the thirdparty turing-complete scripting language?

doxygen commented 5 years ago

This issue was previously marked 'fixed but not released', which means it should be fixed in doxygen version 1.8.16. Please verify if this is indeed the case. Reopen the issue if you think it is not fixed and please include any additional information that you think can be relevant (preferably in the form of a self-contained example).