Menu

#1993 [Patch] Mingw/MSYS build gets confused

Bug
closed-fixed
5
2018-05-14
2018-02-01
No

Building on MinGW/MSYS, the makefile instructs to use xcopy and DEL instead of cp and rm and use translation of path separators '/' -> '\' as Windows_NT is valid here. However MSYS environment does not like Windows style paths containing backslash as separator. I suggest using cp and rm and UNIX-style path separators as in the patch.

1 Attachments

Discussion

  • Neil Hodgson

    Neil Hodgson - 2018-02-01

    Is MSYSCON documented somewhere? A search reveals some uses but nothing that really explains just what it means. When this was discussed on the scite-interest mailing list in 2013 it was thought there was no environment variable to check for MSYS.

    The expression
    DEL = $(if $(wildcard $(dir $(SHELL))rm.exe), $(dir $(SHELL))rm.exe -f, del)
    attempts to detect MSYS and use its 'rm.exe' if available. It was discussed on the mailing list here. Why is this not working to detect an MSYS rm.exe for you?

    The patch for scintilla defines COPY but that is never used.

    The patch changes 'all' to 'ALL' for both files without explanation.

     
  • Tobias Kühne

    Tobias Kühne - 2018-02-02

    Sorry, maybe I should have had a closer look at the patch before submitting.

    I already made it when 3.6.2 was current so I have to try hard to remember what it exactly was about. Maybe the problem is the invocation of make. I guess at the time I created the patch I did not know about mingw32-make, which has to be used instead of make - which in my case promptly found the cygwin make (D'OH!!). This leads at least to xcopy to be complaining about 'invalid number of parameters' - probably because of path translation. If using the correct make DEL = $(...) works well, also xcopy does not cause any problems. So it is upon you to whether to apply the MSYSCON patch to circumvent this stupid error. I found MSYSCON mentioned in the MSYS startup files (https://github.com/Alexpux/MSYS2-packages/blob/master/filesystem/msys2_shell.cmd, https://gist.github.com/ndmitchell/8b5f018f1269faaf4686).

    I cannot recall as to why the 'ALL' rule has changed to 'all', please leave it out.

    I hope this makes at least some sense to you, if not, sorry for the noise.

     
  • Neil Hodgson

    Neil Hodgson - 2018-02-02

    If minw32-make works fine then I'm not seeing a strong reason to change.

    If there is a reason to change, then this should be simplified and documented a bit more. Possibly detangle the OS test (which is needed for grabbing Windows version to determine theming) from the shell (either CMD or bash (or similar)) which determines DEL and COPY and whether path translation is needed.

     
  • Tobias Kühne

    Tobias Kühne - 2018-02-09

    Maybe I am overlooking something, but are there any other systems this makefile is for?
    There is a different file for nmake, so there is only cygwin and MinGW to build in Windows as I understand it... So in which situations do del/xcopy have to be used at all? For the latter systems we would not need path translation at all; if rm/cp are missing there, I would consider this to be a misconfiguration. If so, the attached patch would work (with both make). Otherwise at least COPY should be resolved the same ways DEL is.

     

    Last edit: Tobias Kühne 2018-02-09
    • Neil Hodgson

      Neil Hodgson - 2018-02-09

      This makefile was developed for MinGW GCC with the native Windows shell CMD.EXE where there is no 'rm' or 'cp' available.
      Original form with bare "del /q" and "copy"
      Other people then wanted to use it with MSYS or on Linux so patches were accepted over time.

       
  • Tobias Kühne

    Tobias Kühne - 2018-02-15

    I was thinking mingw make brought the Utilities in Makefiles in, which proved wrong.
    Attached please find two alternative patches, one which determines COPY/DEL via the $(SHELL) variable (as already done for DEL in scite), and uses $(SHELL) in the comparison where copying, the other messing around with uname, which however is quite complicated, but for your information.

     
  • Neil Hodgson

    Neil Hodgson - 2018-02-15

    For 01-cp-rm-via-SHELL.patch, its not good that a complex check is repeated before every COPY invocation.

    The command shells have different rules for things like quoting and brace expansion. The following produce different results with cmd and bash:

    echo ""
    echo \\
    echo a{b,c}
    

    I don't know how safe these are on older Unix shells but something like this appears possible:

    ifeq (\,$(shell echo \\))
        BASH = 1
    endif
    
     
  • Tobias Kühne

    Tobias Kühne - 2018-02-19

    There seem to be people not using bash on MSYS2: https://superuser.com/questions/961699/change-default-shell-on-msys2. So I am not sure whether all solutions will always work -- at least #3 is not working for sh -- so the most portable solution might be echo \\ ...

     

    Last edit: Tobias Kühne 2018-02-19
    • Neil Hodgson

      Neil Hodgson - 2018-02-19

      What we want to branch on really is Unix/POSIX (or emulation) shell versus Windows shell, so maybe call the result POSIX_SHELL. Or set a SHELL_TYPE variable with values Windows_CMD and Unix_sh or POSIX_sh.

       
      • Tobias Kühne

        Tobias Kühne - 2018-02-20

        So this would be the desired solution?

         
  • Neil Hodgson

    Neil Hodgson - 2018-02-20

    The SciTE changes look good but the Scintilla change breaks 'make clean' when cross-compiling on Linux:

    Before:
    [neil@localhost win32]$ make clean
    rm -f *.exe *.o *.obj *.dll *.res *.map *.plist
    
    After:
    [neil@localhost win32]$ make clean
    del /q *.exe *.o *.obj *.dll *.res *.map *.plist
    /bin/sh: del: command not found
    make: *** [makefile:65: clean] Error 127
    

    The cross-compilation changes were in [feature-requests:#1077], with change sets [76ef0b] and [668818].

     

    Related

    Feature Requests: #1077
    Commit: [668818]
    Commit: [76ef0b]

    • Tobias Kühne

      Tobias Kühne - 2018-02-21

      Revised for cross compiling from Linux host.

       
  • Neil Hodgson

    Neil Hodgson - 2018-02-21
    • labels: --> scintilla, scite, build
    • status: open --> open-fixed
    • assigned_to: Neil Hodgson
     
  • Neil Hodgson

    Neil Hodgson - 2018-02-21

    Committed as [26e774], [ed1cf4].

     

    Related

    Commit: [ed1cf4]
    Commit: [26e774]

  • Neil Hodgson

    Neil Hodgson - 2018-05-14
    • status: open-fixed --> closed-fixed
     

Log in to post a comment.

MongoDB Logo MongoDB