GHC_STAGE1 isn't defined, use other form.
ClosedPublic

Authored by ezyang on Mar 10 2017, 4:34 PM.

Details

Summary

I don't really know why this stopped working for me, but it
did on a recent clean. I don't know if this is right but
it solved the problem for me.

Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>

Test Plan

validate

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ezyang created this revision.Mar 10 2017, 4:34 PM
bgamari requested changes to this revision.Mar 10 2017, 5:12 PM
bgamari added inline comments.
testsuite/timeout/Makefile
54

I'm slightly confused: GHC_STAGE1 is defined by mk/config.mk,

$ make show VALUE=GHC_STAGE1
make --no-print-directory -f ghc.mk show
GHC_STAGE1="inplace/bin/ghc-stage1"

Am I missing something?

This revision now requires changes to proceed.Mar 10 2017, 5:12 PM
ezyang added inline comments.Mar 10 2017, 5:38 PM
testsuite/timeout/Makefile
54

When I run make show VALUE=GHC_STAGE1 in the tests directory, GHC_STAGE1 is not defined. In fact, I don't see how it possibly could be defined, since as far as I can tell the testsuite Makefiles don't load the top level mk/config.mk.

ezyang added inline comments.Mar 10 2017, 6:01 PM
testsuite/timeout/Makefile
54

Repro steps:

cd testsuite/timeout
rm calibrate.out
make calibrate.out
This revision was automatically updated to reflect the committed changes.
Phyx added a subscriber: Phyx.Mar 17 2017, 5:20 AM

Oops, forgot to press send.

testsuite/timeout/Makefile
54

It does, in mk/boilerplate.mk https://github.com/ghc/ghc/blob/master/testsuite/mk/boilerplate.mk#L84

GHC_STAGE1 is set to a different path depending on if you did a white-space test or not. Whitespace tests is usually carried out on a validate but not a normal make test afaik, since it's part of the tarball test.

Phyx added a comment.Mar 17 2017, 5:21 AM

This change potentially breaks tests such as the whitespace and tarball validation tests, as now STAGE1_GHC will always point to the inplace one instead of the ones extracted to the specific folders.

I reverted it. I'll look more carefully why the boilerplate isn't being applied.

ezyang added inline comments.Mar 26 2017, 4:39 PM
testsuite/timeout/Makefile
54

@Phyx When I look at boilerplate.mk, I see the following lines:

# The `wildcard` function requires spaces to be escaped. Other gnu make
# functions can't seem to handle spaces at all (e.g. `abspath`).
STAGE1_TEST_SPACES := $(TOP)/../inplace/test\ \ \ spaces/ghc-stage1
STAGE1_NORMAL := $(TOP)/../inplace/bin/ghc-stage1

ifneq "$(wildcard $(STAGE1_TEST_SPACES) $(STAGE1_NORMAL))" ""
IMPLICIT_COMPILER = NO
IN_TREE_COMPILER = YES

ifneq "$(wildcard $(STAGE1_TEST_SPACES))" ""
# See Note [Spaces in TEST_HC].
STAGE1_GHC := $(abspath $(TOP)/../)/inplace/test   spaces/ghc-stage1
STAGE2_GHC := $(abspath $(TOP)/../)/inplace/test   spaces/ghc-stage2
STAGE3_GHC := $(abspath $(TOP)/../)/inplace/test   spaces/ghc-stage3
else
# Maybe we're on Windows (no symlink support), or in a bindist or sdist, which
# don't have the 'test   spaces' symlink.
STAGE1_GHC := $(abspath $(TOP)/../)/inplace/bin/ghc-stage1
STAGE2_GHC := $(abspath $(TOP)/../)/inplace/bin/ghc-stage2
STAGE3_GHC := $(abspath $(TOP)/../)/inplace/bin/ghc-stage3
endif

That's fine, but it's not setting GHC_STAGE1, it's setting STAGE1_GHC. Now look at the rule for calibrate.out in timeout/Makefile:

calibrate.out:
        $(RM) -f TimeMe.o TimeMe.hi TimeMe TimeMe.exe
        $(PYTHON) calibrate '$(GHC_STAGE1)' > $@

This is clearly making use of GHC_STAGE1, not STAGE1_GHC. If you grep for GHC_STAGE1 there are no occurrences.

Am I massively misunderstanding something?

Phyx added inline comments.Mar 26 2017, 5:16 PM
testsuite/timeout/Makefile
54

Ugh.. no Sorry, the naming confused me.. I read STAGE1_GHC for GHC_STAGE1.

That said, STAGE1_GHC seems to just be internal to boilerplate.mk. Perhaps TEST_HC should be used instead?

@ezyang, did you ever get any insight into this? A relevant issue seems to be popping up on RedHat derivatives in Trac #13534.

ezyang added a comment.Apr 6 2017, 6:05 PM

No, but I landed the fix that worked for me, see 5db415580e0738f934e35b7012fe35a79b7e97c7

In D3318#98031, @ezyang wrote:

No, but I landed the fix that worked for me, see 5db415580e0738f934e35b7012fe35a79b7e97c7

Great, I'll go ahead and cherry-pick that to ghc-8.2 in that case. Thanks!