Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix pattern errors in windows #93

Merged
merged 6 commits into from
Feb 10, 2020
Merged

Conversation

jneira
Copy link
Collaborator

@jneira jneira commented Jan 31, 2020

@jneira
Copy link
Collaborator Author

jneira commented Jan 31, 2020

The errors calling stack path had been:

Failures:

  src\CabalHelper\Compiletime\Program\Stack.hs:88:19: 
  1) CabalHelper, cabal-helper spec, cradle discovery, dummy filepath, finds none-cradle, stack repo, dummy filepath
       uncaught exception: PatternMatchFail
       src\CabalHelper\Compiletime\Program\Stack.hs:88:19-59: Irrefutable pattern failed for pattern (key,
                                                              ' ' : val)

This one (you can see it here) is fixed by the patch.
The other one i've seen with this patch applied is:

1) CabalHelper, cabal-helper spec, cradle discovery and loading, dummy filepath, finds none-cradle, stack repo
       uncaught exception: IOException of type OtherError
       readCreateProcess: stack "path" "--stack-yaml=D:\\a\\1\\s\\test\\testdata\\cabal-helper\\simple-stack\\stack.yaml" (exit 1): failed

Unfortunately the error is not very specific

@jneira jneira requested a review from DanielG January 31, 2020 08:55
@DanielG
Copy link
Owner

DanielG commented Jan 31, 2020

1) CabalHelper, cabal-helper spec, cradle discovery and loading, dummy filepath, finds none-cradle, stack repo
       uncaught exception: IOException of type OtherError
       readCreateProcess: stack "path" "--stack-yaml=D:\\a\\1\\s\\test\\testdata\\cabal-helper\\simple-stack\\stack.yaml" (exit 1): failed

Unfortunately the error is not very specific

Seems pretty specific to me: "(exit 1): failed", so the stack path call itself failed.

I think you guys really should be capturing stderr for the tests, otherwise that sort of thing is going to remain a mistery. But honestly it's probably quite difficult to do with hspec because of it's parallel test support. I'd use something else for integration tests like this TBH. (If you find a good alternative let me know, I just roll my own ATM :)

@jneira jneira force-pushed the win-pattern-errors branch from 3b77386 to a54febb Compare February 8, 2020 22:38
@jneira
Copy link
Collaborator Author

jneira commented Feb 9, 2020

After replacing the missing pattern case with an error i can see the following in azure ci:

src\CabalHelper\Compiletime\CompPrograms.hs:98:18: 
  1) CabalHelper, cabal-helper spec, cradle discovery and loading, dummy filepath, finds none-cradle, stack repo
       uncaught exception: ErrorCall
       Error trying to create symlink to haddock: haddock executable not found.
       CallStack (from HasCallStack):
         error, called at src\CabalHelper\Compiletime\CompPrograms.hs:98:18 in cabal-helper-1.0.0.0-8rncFQ8Hkql8OdWNk7I7Bw-c-h-internal:CabalHelper.Compiletime.CompPrograms

However it seems being haddock the program name it should match

patchBuildToolProgs SStack progs
-- optimization; if none of the program paths are non-default we don't
-- even have to add anything to PATH.
| ghcProgram progs == "ghc"
, ghcPkgProgram progs == "ghc-pkg"
, haddockProgram progs == "haddock"
= return progs

If all the three programs follow that scheme...

So it seems not all follow it but haddock does. I wonder if we can simply ignore the programs that has the simple name.

@jneira
Copy link
Collaborator Author

jneira commented Feb 10, 2020

With the last commit (87f41cf) the build with stack and ghc-8.6.4 return to green:
https://dev.azure.com/jneira/haskell-ide-engine/_build/results?buildId=612&view=results

@jneira jneira requested a review from DanielG February 10, 2020 07:08
@jneira jneira force-pushed the win-pattern-errors branch from 87fc71f to 2812d43 Compare February 10, 2020 09:11
Copy link
Owner

@DanielG DanielG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This smells pretty hacky to me overall, I think you're trying to paper over a symptom not the root cause here.

From some more staring at the existing code I notice two problems:

  • I'm using withSystemTempDirectory which will remove the tempdir after patchBuildToolProgs returns, that can't be right :)
  • GHC.configure doesn't actually require haddock to be present, that together with your Error trying to create symlink to haddock: haddock executable not found error from above makes me think that's probably the real problem we have here.

So instead of deciding whether to create the symlink based on the respective setting being non-default I think you should check if the program was found and skip creating the symlink if not, but only for haddock, since ghc and ghc-pkg are legitimately required (though GHC.configure will already have errored out about that).

when (ghcPkgProgram progs /= "ghc-pkg") $
createProgSymlink bindir $ ghcPkgProgram progs
when (haddockProgram progs /= "haddock") $
createProgSymlink bindir $ haddockProgram progs
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure this isn't going to work either. IIRC Stack will search for the ghc related tools in the directory it found ghc.

So if ghc is unqualified here but the other tools aren't we create the symlink farm but stack will search for ghc, find it on PATH, say in /usr/bin or something and pick up ghc-pkg and haddock from there too instead of the ones the user specified.

Also you're not handling the case where the tools are unqualified but have a version postfix.

@jneira
Copy link
Collaborator Author

jneira commented Feb 10, 2020

ok, with 257273c i think i've adressed:

I think you should check if the program was found and skip creating the symlink if not, but only for haddock, since ghc and ghc-pkg are legitimately required (though GHC.configure will already have errored out about that).

About:

I'm using withSystemTempDirectory which will remove the tempdir after patchBuildToolProgs returns, that can't be right :)

and your alternative proposed in irc:

I think instead of having the stackEnv field in Programs if we just stick a bool in there that says whether or not to do the symlink farm we can create it when we actually run stack

do you think we could merge the pr as is (the code will be reused in the alternative)?

@jneira jneira force-pushed the win-pattern-errors branch from 257273c to 83f1af4 Compare February 10, 2020 11:51
@DanielG DanielG merged commit 4ca6570 into DanielG:master Feb 10, 2020
@jneira jneira deleted the win-pattern-errors branch February 10, 2020 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Runtime errors using stack and ghc-8.6.4/ghc-8.4.4 on windows
3 participants