Menu

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

Platform Specific
closed-fixed
nobody
None
7
2016-08-15
2016-05-03
Bjorn L
No

Due to commit 3bdfa9 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

Related

Bugs: #534

Discussion

  • Sebastian Pipping

    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

     
    • Bjorn L

      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?

       
  • Sebastian Pipping

    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?

     
  • EBERSOLD

    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)

     
  • Sebastian Pipping

    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?

     
  • EBERSOLD

    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)

     
    • Karl Waclawek

      Karl Waclawek - 2016-07-07

      The included Visual Studio solution should build fine under vs 2013 and vs
      2015. 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)

      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

  • EBERSOLD

    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.

     
    • Karl Waclawek

      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
  • Sergey Nikulov

    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:

    1. 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.
    2. 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)
    
     
    • Sebastian Pipping

      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.

       
  • beaglejoe

    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).

     
    • Sebastian Pipping

      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.

       
  • Sebastian Pipping

    • status: open --> closed-fixed
     
  • Sebastian Pipping

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

     

Log in to post a comment.