Expat XML Parser / Bugs / #534 Not building on win due to doc/xmlwf.1 not being present or generated

Sebastian Pipping - 2016-05-05

Hello Bjorn,

on the related commit, there should be 2.1.2 containing that soon.

The source tarball should contain xmlwf.1 (just confirmed) but building from Git should be troubled, agreed. I think we should have CMake call out to "make -C doc xmlwf.1" to not duplicate more than needed and skip that step plus installation on Windows. I'm unsure about Cygwin (and if they have docbook2x-man and sgml2xml-isoent or substitutes) right now. My CMake skills are not the best. If you can help out with a patch, that would be very welcome.

Best, Sebastian

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
- Bjorn L - 2016-05-15
  
  I'm not sure what you mean by "I think we should have CMake call out to "make -C doc xmlwf.1" to not duplicate more than needed..."
  On Windows (not using Cygwin) anything with xmlwf.1 should be ignored you mean?
  
  On Cygwin docbook2x-man and sgml2xml-isoent seem to be available. Looking at the docbook2x-man package, sgml2xml-isoent seem to be included.
  
  So I guess only the install step should be excluded on windows, right?
  
  If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Sebastian Pipping - 2016-05-16

Good point, maybe having CMake (or its generated makefile invoke "make -C doc xmlwf.1" should be done with Linux/Unix/Cygwin but not plain Windows. Maybe omitting that single file for plain Windows is not a bad idea. So there is two parts to a fix. Sounds right?

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

EBERSOLD - 2016-07-07

The lines below should solve the issue.
IF(NOT WIN32)
add_custom_command(TARGET expat PRE_BUILD COMMAND $(MAKE) -C doc xmlwf.1)
ENDIF(NOT WIN32)

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Sebastian Pipping - 2016-07-07

Hello ebersold,
I actually added just that (without the windows guard) in commit 5cfcdc3f40189799ac13ba1406989cc711da38ae on June 5th. I didn't think of commenting here back then, sorry!
I'm unsure if the Windows guard is needed as I am unsure about all the different ways CMake could be used on Windows. Maybe with MinGW but without Cygwin the command should still be run?

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

EBERSOLD - 2016-07-07

Hi,

I do understand your concerne and I must confess that my only concerne was to be able to build the library under windows with Visual Studio.
If we want to take into account the MinGW and Cygwin platform I would suggest to move the add_custom_command under the if (BUILD_tools AND NOT WINCE) condition. This takes into account MinGW and Cygwin. It's consistent with the install(FILES doc/xmlwf.1 ...) that is under the same condition.

What do you think about that ?

add_custom_command(TARGET expat PRE_BUILD COMMAND $(MAKE) -C doc xmlwf.1)

if(BUILD_tools AND NOT WINCE)
set(xmlwf_SRCS
xmlwf/xmlwf.c
xmlwf/xmlfile.c
xmlwf/codepage.c
xmlwf/readfilemap.c
)

add_executable(xmlwf ${xmlwf_SRCS}) set_property(TARGET xmlwf PROPERTY RUNTIME_OUTPUT_DIRECTORY xmlwf) target_link_libraries(xmlwf expat) install(TARGETS xmlwf DESTINATION bin) install(FILES doc/xmlwf.1 DESTINATION share/man/man1)

endif(BUILD_tools AND NOT WINCE)
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
- Karl Waclawek - 2016-07-07
  
  The included Visual Studio solution should build fine under vs 2013 and vs
  
  Are you using that solution?
  On Jul 7, 2016 12:20, "EBERSOLD" andre67@users.sf.net wrote:
  
  Hi,
  
  I do understand your concerne and I must confess that my only concerne was
  to be able to build the library under windows with Visual Studio.
  If we want to take into account the MinGW and Cygwin platform I would
  suggest to move the add_custom_command under the if (BUILD_tools AND NOT
  WINCE) condition. This takes into account MinGW and Cygwin. It's consistent
  with the install(FILES doc/xmlwf.1 ...) that is under the same condition.
  
  What do you think about that ?
  
  add_custom_command(TARGET expat PRE_BUILD COMMAND $(MAKE) -C doc xmlwf.1)
  
  if(BUILD_tools AND NOT WINCE)
  set(xmlwf_SRCS
  xmlwf/xmlwf.c
  xmlwf/xmlfile.c
  xmlwf/codepage.c
  xmlwf/readfilemap.c
  )
  
  add_executable(xmlwf ${xmlwf_SRCS})
  set_property(TARGET xmlwf PROPERTY RUNTIME_OUTPUT_DIRECTORY xmlwf)
  target_link_libraries(xmlwf expat)
  install(TARGETS xmlwf DESTINATION bin)
  install(FILES doc/xmlwf.1 DESTINATION share/man/man1)
  
  endif(BUILD_tools AND NOT WINCE)
  
  [bugs:#534] https://sourceforge.net/p/expat/bugs/534/ Not building on
  win due to doc/xmlwf.1 not being present or generated*
  
  Status: open
  Group: Platform Specific
  Created: Tue May 03, 2016 09:00 AM UTC by Bjorn L
  Last Updated: Thu Jul 07, 2016 01:08 PM UTC
  Owner: nobody
  
  Due to commit 3bdfa9
  https://sourceforge.net/p/expat/code_git/ci/3bdfa930a959bbbc3ce0ea9f20010daa0c1a33d8
  where doc/xmlwf.1 was removed, build on Windows (using VS generated with
  CMake) is not working since file isn't created. Install step in CMake
  complains about this file not existing. Not sure if install step:
  install(FILES doc/xmlwf.1 DESTINATION share/man/man1)
  should be removed (on Windows) or what's the appropriate solution.
  
  I'm running expat-code_git-379213ca196f82d19ae72195c4a9bec0553d0bb2
  snapshot of master branch since expat v2.1.1 was not building on Windows
  using Visual Studio (generated with CMake) since it lack commit 247cc3
  https://sourceforge.net/p/expat/code_git/ci/247cc3af30028a0ba9e5f5567aa917d38e226cf8/tree/expat/lib/xmlparse.c?diff=a124f43dadadb6cc776b5309d136937ec48f2a20
  
  Sent from sourceforge.net because you indicated interest in
  https://sourceforge.net/p/expat/bugs/534/
  
  To unsubscribe from further messages, please visit
  https://sourceforge.net/auth/subscriptions/
  
  Related
  
  Bugs: ~~#534~~
  
  alternate
  
  If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

EBERSOLD - 2016-07-07

I'm using the following command :
cmake -G "NMake Makefiles" ../

Meaning I don't use the Visual Studio Solution but NMake makefiles.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
- Karl Waclawek - 2016-07-07
  
  If your goal is a successful Windows build then use the Visual Studio
  solution. Nothing else has really been tested on Windows.
  
  On Thu, Jul 7, 2016 at 1:16 PM, EBERSOLD andre67@users.sf.net wrote:
  
  I'm using the following command :
  cmake -G "NMake Makefiles" ../
  
  Meaning I don't use the Visual Studio Solution but NMake makefiles.
  
  Last edit: Karl Waclawek 2016-07-07
  
  If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Sergey Nikulov - 2016-07-08

Hello Karl,

Anyway

add_custom_command(TARGET expat PRE_BUILD COMMAND $(MAKE) -C doc xmlwf.1)`

incorrect in CMakeLists.txt for Windows, because $(MAKE) not defined for most Windows generators and even on Linux for ninja build tool.

Also it assumes that build started in-source and doc accessible right in current folder.
So better is

add_custom_command(TARGET expat PRE_BUILD COMMAND $(MAKE) -C ${PROJECT_SOURCE_DIR}/doc xmlwf.1)

Here is some options:

Provide option BUILD_doc and use this as ability to avoid MAN generation explicitly. Personnaly, I think it is better one. I've updated attached patch for this.

As @ebersold says guard this with WIN32, Cygwin will be fine because camke v2.8.4+ not define WIN32 for Cygwin, for example

if(NOT WIN32 OR MINGW) add_custom_command(TARGET expat PRE_BUILD COMMAND $(MAKE) -C ${PROJECT_SOURCE_DIR}/doc xmlwf.1) endif()

And finally third option (not completely correct one, but shorter and will fix issue described by disabling BUILD_tools=OFF):
It should be only generated when xmlwf tool produced. Which should be the case only if BUILD_tools equals ON.

So third idea, at least put this line right before, which is conditionally covered with BUILD_tools variable

add_custom_command(TARGET expat PRE_BUILD COMMAND $(MAKE) -C ${PROJECT_SOURCE_DIR}/doc xmlwf.1) install(FILES doc/xmlwf.1 DESTINATION share/man/man1)

0001-fix-for-534.patch
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
- Sebastian Pipping - 2016-08-15
  
  That patch is now in Git, thank you!
  
  I also used the ${PROJECT_SOURCE_DIR} idea to symptom-fix out-of-source compilation for now.
  
  If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

beaglejoe - 2016-08-14

Hi all,
I build expat via CMake's ExternalProject_Add() function and as such I cannot use the bundled solution. I like Sergey's BUILD_doc option best, but putting a guard around the 'add_custom_command' will also work. I would suggest IF (NOT MSVC) instead of IF (NOT WIN32).

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
- Sebastian Pipping - 2016-08-15
  
  I have added an extra "AND NOT MSVC" guard now. If that turns out too much, I'm happy to revert. Commit 2074c16a9963e67848af1a9c2c5ad1768b2b0b98 it is.
  
  If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Sebastian Pipping - 2016-08-15

status: open --> closed-fixed
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Sebastian Pipping - 2016-08-15

Looks fixed to me but feel free to re-open.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Not building on win due to doc/xmlwf.1 not being present or generated

Fast XML parser library in C

Group

Searches

Help

#534 Not building on win due to doc/xmlwf.1 not being present or generated

Related

Discussion

endif(BUILD_tools AND NOT WINCE)

Related