Winetricks / winetricks

Winetricks is an easy way to work around problems in Wine
GNU Lesser General Public License v2.1
2.79k stars 403 forks source link

tmp.ahk gets corrupted by \r characters (OSX/BSD sed) #697

Closed kenorb closed 7 years ago

kenorb commented 7 years ago

Reproducible steps (tested on macOS Sierra):

sh -xs mt4 < <(wget -qO- https://raw.githubusercontent.com/Winetricks/winetricks/master/src/winetricks)

This also happens for me when running in the standard way:

winetricks mt4  

Error:

screen shot 2016-11-17 at 15 14 45
$ cat ./drive_c/windows/temp/_mt4/tmp.ahk
w_opt_unattended = 0r
r
        Run, mt4setup.exer
        WinWait, MetaTrader 4 Setup, license agreementr
        ControlClick, Button1r
        Sleep 100r
        ControlClick, Button3r
        WinWait, MetaTrader 4 Setup, Installation successfully completedr
        ControlClick, Button4r
        Process, Wait, terminal.exer
        Process, Close, terminal.exer
    r

Relevant trace log:

Executing cd /Users/username/.cache/winetricks/mt4
+ test -f /Users/username/.cache/winetricks/ahk/AutoHotkey.exe
++ printf '\\r'
+ _W_CR='\r'
+ cat
+ sed 's/$/\r/'
+ w_try wine 'y:\ahk\AutoHotkey.exe' 'C:\windows\Temp\_mt4\tmp.ahk'
+ export WINEDLLOVERRIDES
+ printf '%s\n' 'Executing wine y:\ahk\AutoHotkey.exe C:\windows\Temp\_mt4\tmp.ahk'
Executing wine y:\ahk\AutoHotkey.exe C:\windows\Temp\_mt4\tmp.ahk
+ case "$1" in
+ wine 'y:\ahk\AutoHotkey.exe' 'C:\windows\Temp\_mt4\tmp.ahk'

Somehow created tmp.ahk file consist weird r characters at the end of each line.

Most like it's because of BSD sed.

$ strings $(which sed) | head
$FreeBSD: src/usr.bin/sed/compile.c,v 1.28 2005/08/04 10:05:11 dds Exp $
$FreeBSD: src/usr.bin/sed/main.c,v 1.36 2005/05/10 13:40:50 glebius Exp $
$FreeBSD: src/usr.bin/sed/misc.c,v 1.10 2004/08/09 15:29:41 dds Exp $
$FreeBSD: src/usr.bin/sed/process.c,v 1.39 2005/04/09 14:31:41 stefanf Exp $

The problem is how the BSD sed handles the substitution of $. Here is the difference between GNU sed and BSD sed:

$ _W_CR=$(printf \\\\r)
$ /usr/bin/sed "s/\$/$_W_CR/" winetricks | tail
            execute_command "$verb"r
        doner
        ;;r
r
    esacr
r
    winetricks_stats_reportr
fir
r
# vim: tabstop=8 expandtab shiftwidth=4 softtabstop=4r

$ gsed "s/\$/$_W_CR/" winetricks | tail
            execute_command "$verb"
        done
        ;;

    esac

    winetricks_stats_report
fi

# vim: tabstop=8 expandtab shiftwidth=4 softtabstop=4
austin987 commented 7 years ago

Can you see if this works please? https://github.com/Winetricks/winetricks/commit/d3205e91f2ab7e532c2b786cb98ad54d0555db81

austin987 commented 7 years ago

I was able to test it in Travis CI, though it's very slow.

Merged it https://github.com/Winetricks/winetricks/commit/d3205e91f2ab7e532c2b786cb98ad54d0555db81

kenorb commented 7 years ago

It seems it works, thanks. Ex could the substitution as well (e.g. ex +"s/$/\r/" -scwq tmp.ahk).

austin987 commented 7 years ago

ex is vim specific, which isn't portable. sed is way more common :)

kenorb commented 7 years ago

When Vim is not installed, there is still POSIX version of Ex installed on every Unix/Linux machine by default. On the other hand Sed parameters can be incompatible between Linux/Unix (e.g. -i which is a custom BSD extension). So sed is common as a StrEm eDitor, Ex is common as a in-place file editor. In this case it's a stream, so sed or awk is fine.

Chiitoo commented 7 years ago

When Vim is not installed, there is still POSIX version of Ex installed on every Unix/Linux machine by default.

As far as I can tell, without Vim, there will be no Ex on Gentoo installations, where '/usr/bin/ex' is a symlink to 'vim' (unless the user did something outside the package manager).

Gentoo might be a bit different from 'the default' in many other ways as well, but still. :]

austin987 commented 7 years ago

@kenorb yeah, I'm aware that BSD/GNU sed are different (and there's another case that's probably broken that I'll fix soon). I don't regularly test on osx, but will fix any issues reported there (I have access to a mac mini).

Regarding ex, thanks for the tip, I wasn't aware of it. I'd rather use sed/awk, as they are much more well known.