-
Notifications
You must be signed in to change notification settings - Fork 697
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
Doctest ghc pkg fix #10057
Doctest ghc pkg fix #10057
Conversation
I wonder if the key in the augmented |
https://github.com/haskell/cabal/actions/runs/9250116181/job/25443085129?pr=10057#step:19:342 |
1f9f133
to
c53929b
Compare
@geekosaur good catch, thanks! I went with |
This patch seems to rely on some custom behaviour of Perhaps a more principled way is to look at the |
@mpickering I am merely reviving #8718, so I lack the knowledge for the best approach. The use of this key
but:
I do not know the internals of these tools.
@mpickering Not expert, so I will wait that experts settle down on the best approach. |
To clarify, what I was saying is that |
I think the cleanest solution would be to add a command-line option
instead of
|
Another solution to consider is using external commands to provide a For example: https://github.com/mpickering/cabal-doctest-demo/blob/master/cabal-doctest You could implement this as a Haskell executable rather than a bash script and install both I can't immediately imagine how augmenting |
@sol I think this is too complicated. $ cabal repl --with-ghc=doctest --with-hc-pkg=ghc-pkg-9.8.2 The key it that we want cabal to deduce the correct path for other executables.
@mpickering looks a better option 👍
If one of the answer is yes, we should add a fallback with the current behavior. So the lookup would be:
This would solve the issue for |
I'm definitely not opposed, will probably be more robust than what Cabal currently does.
Prior to GHC 9.4, apparently that would need to be
This sounds good to me 👍
There's still the issue that when
|
- Prior to GHC 9.4, apparently that would need to be `../../bin` instead of `../bin`.
+ Starting from GHC 9.4, apparently that would need to be `../../bin` instead of `../bin`. -- ghc-9.2 --info
… ("LibDir","~/.ghcup/ghc/9.2.8/lib64/ghc-9.2.8") …
-- ghc-9.4 --info
… ("LibDir","~/.ghcup/ghc/9.4.8/lib64/ghc-9.4.8/lib") … It doesn’t look like this entry is very stable. So we would have to check yet another path. Plus I would say it is safer to check subdirs than parent dirs. The current solution with the |
@wismill I imagine in 9.2 the path is Are you both against my proposed solution of using an external cabal command? That seems to solve many issues to me. |
This issue also reminds me of something similar where when you use a cross compiler then cabal will fail to find ghc-pkg due to the prefix. Perhaps yet another solution is to try to make the detection logic account for a prefix and for |
@mpickering sorry, we answered both at the same time and I did not see your proposal. I do not think the cabal-doctest file you link would solve the issue though: isn’t cabal still picking whatever Or do you mean we somehow reverse the roles: e.g. a bash script
This is simply automatizing the workaround and leave the responsibility to cabal-doctest doctest-9.2
cabal-doctest path/to/doctest --some --extra=arguments --for-cabal The extra arguments could be filtered to avoid overriding. This script would be agnostic to doctest/ghc/cabal versions. We could even have a stack variant I guess. |
Depending on whether you want the wrapper script or the binary it is:
At least for 9.2, I think you need to use the wrapper script. For later versions the binary seems to works, not sure exactly how it finds the libdir, but it seems to work from what I have tried. Still not entirely sure whether you want to rely on that, maybe always going with the wrapper script is the safer bet. From the information that I have right now, going by
I think an external
|
The script I linked takes the approach of first installing a version of doctest which matches whichever ghc you are using for your project (in Then the second part automates the With the external command support in the new version of cabal you can run this as Another idea would be that you keep the installation story for Your idea is another possibility yes, that isn't something I thought of. It overall seems quite flexible using an external command like this. |
@mpickering @sol I now fully understood the operating and interest of cabal external command. See sol/doctest#424 for an implementation of cabal install doctest --program-prefix "-9.4" --with-compiler ghc-9.4
cabal install doctest --program-prefix "-9.6" --with-compiler ghc-9.6
# This will not depend on the ghc version in PATH
cabal doctest-9.4
cabal doctest-9.6 I think this is clean solution, although a the mechanism presented in the current MR or a “custom programs resolver” is also interesting. |
There is a minor issue: I need to hit “Enter” to finish the program when invoked with Not sure what is causing this. Could it from the cabal side? |
@wismill Perhaps something to investigate in the cabal side, make a ticket? I can look into it after next week (I am on holiday). |
@sol is working on a proper solution for doctest in sol/doctest#425. |
Please read Github PR Conventions and then fill in one of these two templates.
Template Α: This PR modifies behaviour or interface
Include the following checklist in your PR:
Revival of #8718
Fixes sol/doctest#396.
@sol