mity / md4c

C Markdown parser. Fast. SAX-like interface. Compliant to CommonMark specification.
MIT License
759 stars 140 forks source link

md2html Build/Install Error on Windows: Cannot Find "md2html.exe" #88

Closed AntumDeluge closed 5 years ago

AntumDeluge commented 5 years ago

md4c version: 0.3.4 CMake info:

$ cmake --version
cmake version 3.14.5
CMake suite maintained and supported by Kitware (kitware.com/cmake).

I think this error is more a bug in CMake, but I thought you should be aware.

I use the MSYS2/MinGW environment for building on Win32. If I use MSYS2's CMake package, I have no problems. But, If I use the "native" CMake executable from CMake's website, I get the following error:

CMake Error at md2html/cmake_install.cmake:36 (file):
  file INSTALL cannot find
  "C:/Users/antum/Development/MSYS2/mingw-packages/mingw-w64-md4c/src/build-x86_64-shared/md2html/md2html".
Call Stack (most recent call first):
  cmake_install.cmake:38 (include)

CMake's "native" build doesn't recognize the md2html executable if the extension .exe isn't explicitly declared.

I use the following patch to get past the issue:

diff -ruN md2html.orig/CMakeLists.txt md2html/CMakeLists.txt
--- md2html.orig/CMakeLists.txt 2019-06-19 15:04:48.000000000 +0000
+++ md2html/CMakeLists.txt  2019-07-11 07:28:37.465695100 +0000
@@ -4,4 +4,4 @@
 add_executable(md2html cmdline.c cmdline.h entity.c entity.h md2html.c render_html.c render_html.h)
 target_link_libraries(md2html md4c)

-install(PROGRAMS ${CMAKE_CURRENT_BINARY_DIR}/md2html DESTINATION ${CMAKE_INSTALL_BINDIR})
+install(PROGRAMS ${CMAKE_CURRENT_BINARY_DIR}/md2html.exe DESTINATION ${CMAKE_INSTALL_BINDIR})

But obviously, that makes building only work on Windows.

Again, this is probably more of a bug with CMake. Not sure if there would be a way to test whether or not the CMake executable needs an explicit .exe declaration.

--- Edit ---

I just noticed another issue related to this. If I am using the CMake version from MSYS2's package manager, calling make install renames md2html.exe executable to md2html.

mity commented 5 years ago

I see.

The fact is the install instructions are mainly intended for Linux/POSIX systems and package manager of those systems. I don't know how to really fix it for Windows as there is no standard directory where such utilities should be placed into (actually that's also the reason why on Windows we by default build only static lib so there is no dependency on a DLL.)

We could maybe wrap the install commands in CMake by

if(not WIN32)
...  # the install commands
endif()

to not cause mess on the filesystem but I am not sure whether it would make life of e.g. MSYS2 users harder.

I just noticed another issue related to this. If I am using the CMake version from MSYS2's package manager, calling make install renames md2html.exe executable to md2html.

That's bad indeed.

However I tend to say this should be handled by CMake better. I don't think that maintaining extra branch of the install commands for that platform is a scalable solution and it could maybe break if CMake fixes it (it would be even worse to generate md2html.exe.exe if/when that happens).

Taking these arguments together, maybe the aforementioned disabling of the install command on Windows is the right step.

AntumDeluge commented 5 years ago

I wonder if you could do something like:

if(WIN32)
    option(MD2HTML_EXE "" "md2html.exe")
else()
    option(MD2HTML_EXE "" "md2html")
endif()

I'm not that familiar with CMake, but I'm trying to learn it better because I want to use it in my projects.

I might try some things out tomorrow if I have time. I'll let you know if I find anything that seems like it might be a solution.

...that's also the reason why on Windows we by default build only static lib...

That reminds me of something else I found interesting. When I was using the native Win32 CMake executable downloaded from their site, it would build the static lib by default. But using the one from MSYS2's package manager, it built the dll. So I'm wondering if WIN32 is not defined in theirs. That might be something I need to report to the MSYS2 devs.

mity commented 5 years ago

I wonder if you could do something like:

if(WIN32)
    option(MD2HTML_EXE "" "md2html.exe")
else()
    option(MD2HTML_EXE "" "md2html")
endif()

I believe CMake tries to do something pretty close to that on its own via CMAKE_EXECUTABLE_SUFFIX. But it can likely fail for the strange MSYS2 platform which is sort of a hybrid between WIN32 and POSIX.

I am not using MSYS2 (I still stick to the pretty old MSYS env. instead). But AFAIK MSYS2 is kind of a "dual" platform. With some gcc toolchain you may build native Windows binaries, and with another one you may build MSYS2 binaries which do all sorts of Windows <--> Unix path translations, supports fork() etc.

If it is so then it's questionable what CMake can do there because both defining WIN32 as well as not defining it is (partially) wrong there. CMake probably even does not have any knowledge which of the toolchains you are using. Maybe you can even mix building both kinds of binaries in a single CMake recipe file.

mity commented 5 years ago

Also see this: http://cmake.3232098.n2.nabble.com/CMake-support-for-MYS2-td7596911.html

It seems there are cmake and mingw-w64-x86_64-cmake (or i686) packages. If I understand it correctly, the former is for building for the MSYS2 system itself, while the other one should build native Windows binaries.

So I would guess the former does not define WIN32 and the other one does (or should).

Which one do you have installed in your MSYS2 environment?

AntumDeluge commented 5 years ago

Currently, I am using the MSYS2 package (cmake). The mingw-w64-x86_64-cmake appears to be broken. :-\

I also found this commit message for Cygwin: https://gitlab.kitware.com/cmake/cmake/commit/85c0a69a92e78275ea0b180482bafcdb877b0dc3

I though you told me that MSYS2 is actually a fork of Cygwin, but maybe I read it somewhere else. That would explain the missing WIN32 definition. I'll try to get the mingw-w64-x86_64-cmake package working & see if behavior is normal.

--- Edit ---

Turns out the cmake build wasn't broken, I just had a dependency version incompatibility. So, using the mingw-w64-x86_64-cmake version works the same as downloadable executable from CMake's website. I have to use the patch I mentioned about, but it doesn't strip the .exe suffix.

mity commented 5 years ago

Ok. That sounds better. So we have just one problem.

Wouldn't the following work? Could you please check it in MSYS2?

-install(PROGRAMS ${CMAKE_CURRENT_BINARY_DIR}/md2html DESTINATION ${CMAKE_INSTALL_BINDIR})
+install(PROGRAMS ${CMAKE_CURRENT_BINARY_DIR}/md2html${CMAKE_EXECUTABLE_SUFFIX} DESTINATION ${CMAKE_INSTALL_BINDIR})

Because if yes, then it should work everywhere.

AntumDeluge commented 5 years ago

Okay. I will have to wait until tomorrow to try it.

mity commented 5 years ago

Ohh. Never mind. For some reason it was using INSTALL(PROGRAMS) instead of INSTALL(TARGETS) commands. The latter should do all of the required stuff on its own.

Feel free to reopen if you find out it is still not working but I would be really quite surprised.

AntumDeluge commented 4 years ago

Sorry I haven't responded, been busy with some things. I'll let you know when I get a chance to try & rebuild again. But, looks like you already fixed it.

AntumDeluge commented 4 years ago

Just tested from commit ae5ca89. It's working. Thank you.

mity commented 4 years ago

Thanks for testing it.