Opened 2 years ago

Closed 2 years ago

#10449 closed bug (fixed)

Out-of-tree tests broken on MinGW + native Python due to quoting of config.compiler

Reported by: Rufflewind Owned by:
Priority: normal Milestone: 7.10.2
Component: Test Suite Version: 7.10.1
Keywords: Cc:
Operating System: Windows Architecture: Unknown/Multiple
Type of failure: Other Test Case:
Blocked By: Blocking:
Related Tickets: #10441 Differential Rev(s): Phab:D911
Wiki Page:

Description (last modified by Rufflewind)

Ever since commit 5258566ee5c89aa757b0cf1433169346319c018f it seems that out-of-tree tests will not run on MinGW anymore. Note that I am using MinGW tools with a native Windows build of Python, which is the main cause of the problem.

The apparent error is something along the lines of "cannot eval an empty string":

Traceback (most recent call last):
  File "testsuite/driver/runtests.py", line 196, in <module>
    get_compiler_info()
  File "<string>", line 166, in get_compiler_info
  File "<string>", line 0
    ^
SyntaxError: unexpected EOF while parsing

But this error is quite misleading. What really happened was that it tried to run GHC with the --info argument but failed silently and returned an empty string. If you apply D908, it becomes a lot more obvious: Python is failing to run /c/programs/ghc/bin/ghc --info because /c/programs/ghc/bin/ghc "does not exist" as Python does not understand MinGW-style paths.

The funny thing is that it worked just fine before 525856, so you have to wonder why quoting the paths caused this. It turns out that MinGW does some magic behind the scenes, automagically converting MinGW-style paths into ordinary Python paths when a native Windows program is invoked. In particular, it means running this in MinGW shell

python run-tests.py -e 'config.compiler=/c/programs/ghc/ghc'

will actually invoke Python with config.compiler=C:/programs/ghc/ghc. However, if it's explicitly quoted (edit: and also escaped with backslashes) like this

python run-tests.py -e 'config.compiler="\"/c/programs/ghc/ghc\""'

the magic doesn't work anymore.

Change History (13)

comment:1 Changed 2 years ago by thomie

I guess this works as well?

python run-tests.py -e "config.compiler='/c/programs/ghc/ghc'"

Can we try to not rely on this magic? See also #10441.

comment:2 Changed 2 years ago by Rufflewind

Oops, what I said wasn't quite correct. The problem is actually that the quotes are escaped:

python run-tests.py -e 'config.compiler="\"/c/programs/ghc/ghc\""'

MinGW has no problem with single or double quotes. It's the escaped double quotes that cause problems.

comment:3 Changed 2 years ago by Rufflewind

Description: modified (diff)

comment:4 Changed 2 years ago by thomie

But escaped single quotes are still ok? That's what was used before 525856.

comment:5 in reply to:  4 Changed 2 years ago by Rufflewind

Replying to thomie:

But escaped single quotes are still ok? That's what was used before 525856.

Before 525856, it was invoked like this (contains a double-quote only):

… -e 'config.compiler="$(TEST_HC)"'

Now it's invoked like this (with a double-quote 'and an escaped double-quote'):

… -e 'config.compiler="\"$(TEST_HC)\""'

The escaped double-quote is what's tripping up MinGW (an escaped single-quote would cause the same problem).

comment:6 Changed 2 years ago by thomie

Actually, I don't see what is wrong with 525856. These are the 2 relevant changes.

In mk/test.mk:

-       -e 'config.compiler="$(TEST_HC)"' \
+       -e 'config.compiler="\"$(TEST_HC)\""' \

In config/ghc:

-    h = os.popen('"' + config.compiler + '" --info', 'r')
+    h = os.popen(config.compiler + ' --info', 'r')

That should give the same result, shouldn't it?

In mk/test.mk there are 3 pairs of quotes:

  • outer ' : this is a shell string
  • outer " : this is a python string
  • inner " : this is a path

comment:7 Changed 2 years ago by thomie

Ok, now I understand, sorry for taking so long. The MINGW magic should happen *before* the string reaches Python, as you said. And now it doesn't, because the string is too heavily quoted.

comment:8 Changed 2 years ago by thomie

Differential Rev(s): Phab:D911
Milestone: 7.12.1
Status: newpatch

Can you test if the patch in Phab:D911 works (after it validates)? You should be to able apply it by running 'arc patch --nobranch D911' and ignoring the warning about the parent commit not existing.

Last edited 2 years ago by thomie (previous) (diff)

comment:9 Changed 2 years ago by Rufflewind

Unfortunately, the wrap call still breaks the magic. The conversion seems to be documented here: http://www.mingw.org/wiki/Posix_path_conversion

What is the reason for quoting compiler.config? A quick grep shows the following code use compiler.config:

./testsuite/config/ghc:config.compiler              = 'ghc'
./testsuite/config/ghc:    h = os.popen(config.compiler + ' --info', 'r')
./testsuite/config/ghc:    h = os.popen(config.compiler + ' +RTS --info', 'r')
./testsuite/mk/test.mk:	-e 'config.compiler=$(call quote_path,$(TEST_HC))' \
./testsuite/driver/testglobals.py:        self.compiler = ''
./testsuite/driver/runtests.py:    # testframe -e 'config.compiler=ghc-5.04'

There might be others I've missed, but I think in many situations the quotes will either get stripped anyway, or can be rewritten using subprocess so as to avoid the need for them. If quotes are necessary, they could be just added at those specific places.

comment:10 Changed 2 years ago by Thomas Miedema <thomasmiedema@…>

In ce166a3aaab28a9b08c60da6d0cfdab998b6e8ca/ghc:

Testdriver: do not interfer with MinGW path magic (#10449)

This should fix the testsuite driver on Windows using the MinGW tools
with a native build of Python.

MinGW automagically converts MinGW-style paths (e.g.
'/c/programs/ghc/bin/ghc') into ordinary Windows paths (e.g.
'C:/programs/ghc/bin/ghc') when a native Windows program is invoked. But
it doesn't do so when those paths are wrapped with a pair of escaped
double quotes.

The fix is to not call `eval` on the paths in Python, which let's us use
one less pair of quotes, and makes MinGW happy.

Reviewers: Rufflewind, austin

Differential Revision: https://phabricator.haskell.org/D911

comment:11 in reply to:  9 Changed 2 years ago by thomie

Replying to Rufflewind:

What is the reason for quoting compiler.config?

Copy pasting from the comments:

# Wrap non-empty program paths in quotes, because they may contain spaces. Do
# it here, so we don't have to (and don't forget to do it) in the .T test
# scripts (search for '{compiler}' or '{hpc}'). This may or may not be a good
# idea.

comment:12 Changed 2 years ago by thomie

Milestone: 7.12.17.10.2
Status: patchmerge

Please merge the commit above to ghc-7.10. It depends on 6694ccf9444baf565eb0f38f7808767616f23825, so that one will have to go in as well.

comment:13 Changed 2 years ago by thoughtpolice

Resolution: fixed
Status: mergeclosed

Merged to ghc-7.10.

Note: See TracTickets for help on using tickets.