Opened 3 years ago

Closed 3 years ago

#1727 closed defect (fixed)

chicken-install srfi-18 fails on Windows

Reported by: Josh Helzer Owned by:
Priority: major Milestone: 5.3
Component: unknown Version: 5.2.0
Keywords: Cc:
Estimated difficulty:

Description

The culprit appears to be improper path escaping:

C:\>chicken-install srfi-18
building srfi-18
   C:\Users\j\AppData\Local\chicken-install\srfi-18\build-srfi-18.bat -host -D compiling-extension -J -s -regenerate-import-libraries -setup-mode -I C:\Users\j\AppData\Local\chicken-install\srfi-18 -C -IC:\Users\j\AppData\Local\chicken-install\srfi-18 -O2 -d1 srfi-18.scm -o C:\Users\j\AppData\Local\chicken-install\srfi-18\srfi-18.so
'""C:' is not recognized as an internal or external command,
operable program or batch file.
        1 file(s) copied.
'""C:' is not recognized as an internal or external command,
operable program or batch file.

Error: shell command terminated with nonzero exit code
1
"C:\\Users\\j\\AppData\\Local\\chicken-install\\srfi-18\\srfi-18.build.bat"

C:\>

Attachments (1)

0001-Correctly-quote-set-calls-in-Windows-scripts.patch (1.5 KB) - added by Vasilij Schneidermann 3 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 3 years ago by sjamaan

Hi there! I think this isn't a SRFI-18 specific problem, but rather a chicken-install problem.

It looks like you might have missed the CHICKEN build instructions from the README file:

          - When installing under mingw, with a windows shell
            ("cmd.exe"), pass an absolute pathname (including the
            drive letter) as PREFIX and use forward slashes. If you
            are building the sources from git, use backslashes to
            specify the path to `chicken' (the "CHICKEN" variable).

Note that this requires you to use forward slashes, probably because of this issue. Of course, if you can fix CHICKEN and chicken-install specifically to make it work with backslashes that would be great too.

comment:2 Changed 3 years ago by Tim Rooney

Hey, I've also had this problem with some eggs in the past on Windows. I've been able to get around it when it pops up by removing some extra quotes in the build-*.bat files.

The problem seems to be that %CHICKEN_CSC% and %CHICKEN_CSI% are already quoted, but some of the batch files themselves quote the variable, causing doubled quotes that Windows cannot handle.

I believe that this should be all of the affected eggs:

; In
$ grep --include=*.bat -rw . -e '"%CHICKEN_CSC%"' -e '"%CHICKEN_CSI%"' | awk -F "/" '{ print $2 }'

; Out
breadline
imlib2
openssl
socket
srfi-18
taglib

comment:3 Changed 3 years ago by Vasilij Schneidermann

I've looked into this today and discovered that we might need to do a breaking change. Windows handles quoting differently when applied to environment variables:

C:\Users\me> set JAVA="C:\path\to\java"
C:\Users\me> echo %JAVA%
"C:\path\to\java"
C:\Users\me> echo "%JAVA%"
""C:\path\to\java""
C:\Users\me> %JAVA% -version
openjdk version "11.0.8" 2020-07-14
[...]
C:\Users\me> "%JAVA%" -version
'""C:' is not recognized as an internal or external command,
operable program or batch file.

In fact, quotes aren't permitted in paths. If I inspect the environment variables on my Windows machine, it may look like this:

PSModulePath=C:\Program Files\WindowsPowerShell\Modules;[...]

Which is not what egg-compile.scm does:

(printf #<<EOF
@echo off~%
set PATH=~a;%PATH%
set CHICKEN_CC=~a
set CHICKEN_CXX=~a
set CHICKEN_CSC=~a
set CHICKEN_CSI=~a

EOF
             (qs* default-bindir platform) (qs* default-cc platform)
	     (qs* default-cxx platform) (qs* default-csc platform)
	     (qs* default-csi platform))

default-bindir should not be quoted at all. According to https://ss64.com/nt/set.html, the set command should instead quote its entire argument, including the name. The above session again, but with correct quoting:

C:\Users\me> set "JAVA=C:\path\to\java"
C:\Users\me> echo %JAVA%
C:\path\to\java
C:\Users\me> echo "%JAVA%"
"C:\path\to\java"
C:\Users\me> %JAVA% -version
'""C:' is not recognized as an internal or external command,
operable program or batch file.
C:\Users\me> "%JAVA%" -version
openjdk version "11.0.8" 2020-07-14
[...]

I believe that to be the correct behavior and am willing to write a patch to implement it. However it will require every single egg installable on Windows (I've found nine with a .bat file) to use quoting for the CHICKEN_CSC, CHICKEN_CSI, CHICKEN_CC and CHICKEN_CXX variables, otherwise the scripts will break on every installation that contains spaces in its installation path. On the upside, this would relieve me from fixing the above six scripts, five of which I'm directly responsible for :)

Last edited 3 years ago by Vasilij Schneidermann (previous) (diff)

Changed 3 years ago by Vasilij Schneidermann

comment:4 Changed 3 years ago by Vasilij Schneidermann

Here's an untested patch that should resolve the reported problem. If it's accepted, I'll look into adjusting the three remaining eggs to quote CHICKEN_CSC and CHICKEN_CSI.

comment:5 Changed 3 years ago by Vasilij Schneidermann

I've tested the patch successfully using "PREFIX=C:/chicken 5.2.0" for my new installation. Both the srfi-18 and taglib egg (socket egg flaked out due to WINAPI nonsense) have been installed successfully using it. As mentioned earlier, I'd submit patches to the sendfile, kiwi and bind eggs.

Last edited 3 years ago by Vasilij Schneidermann (previous) (diff)

comment:6 Changed 3 years ago by sjamaan

Milestone: someday5.4

comment:7 Changed 3 years ago by sjamaan

Milestone: 5.45.3

comment:8 Changed 3 years ago by Vasilij Schneidermann

I've tested it now with Cygwin in addition to my previous test with mingw64-msys2, seems like Cygwin isn't affected by the patch at all. So that's one less thing to worry about.

comment:9 Changed 3 years ago by evhan

Resolution: fixed
Status: newclosed

This should be fixed by 278c2477.

Note: See TracTickets for help on using tickets.