Doctest ghc pkg fix
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:
- [x] Patches conform to the coding conventions.
- [ ] Any changes that could be relevant to users have been recorded in the changelog.
- [ ] The documentation has been updated, if necessary.
- [ ] Manual QA notes have been included.
- [x] Tests have been added. (Ask for help if you don’t know how to write them! Ask for an exemption if tests are too complex for too little coverage!)
Revival of #8718
Fixes sol/doctest#396.
@sol
I wonder if the key in the augmented ghc --info (currently just ghc) would better follow some format, such as ghc:path or path:ghc. This would allow to extend the concept to other keys/tools.
https://github.com/haskell/cabal/actions/runs/9250116181/job/25443085129?pr=10057#step:19:342
ScopedTypeVariables is included in the GHC2021 extension/language, but must be explicitly enabled for earlier versions.
@geekosaur good catch, thanks! I went with TypeApplications which is more elegant IMHO.
This patch seems to rely on some custom behaviour of doctest when acting as a GHC. The "ghc" variable is not (at the moment) part of ghc --info.
Perhaps a more principled way is to look at the LibDir variable and look in ../bin for ghc-pkg?
@mpickering I am merely reviving #8718, so I lack the knowledge for the best approach. The use of this key ghc is explained at this comment. And:
AFAIK, e.g.
hie-bios/hlsuse the same approach to get the correct flags for a source file + other tools could benefit from it too.
but:
I'm not aware that they would do anything in that regard right now.
I do not know the internals of these tools.
Perhaps a more principled way is to look at the "LibDir
variable and look in../bin` for ghc-pkg?
@mpickering Not expert, so I will wait that experts settle down on the best approach.
@mpickering I am merely reviving #8718, so I lack the knowledge for the best approach. The use of this key
ghcis explained at this comment. And:AFAIK, e.g.
hie-bios/hlsuse the same approach to get the correct flags for a source file + other tools could benefit from it too.but:
I'm not aware that they would do anything in that regard right now.
I do not know the internals of these tools.
Perhaps a more principled way is to look at the "LibDir
variable and look in../bin` for ghc-pkg?@mpickering Not expert, so I will wait that experts settle down on the best approach.
To clarify, what I was saying is that hie-bios / hls use the same cabal repl --with-ghc=... trick to get the correct command-line options for GHC, AFAIK. my second point was that they could then also augment the --info output with the ghc key to benefit from this patch as well.
I think the cleanest solution would be to add a command-line option --with-repl so that you can invoke doctest with
$ cabal repl --with-repl=doctest
instead of
$ cabal repl --with-ghc=doctest
cabal then should also allow for both options to be specified at the same time, e.g.:
$ cabal repl --with-ghc=ghc-9.8.2 --with-repl=doctest
Another solution to consider is using external commands to provide a cabal doctest command which hides with --with-ghc stuff from users. So you can just pass --with-ghc=... --with-ghc-pkg=....
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 doctest and cabal-doctest when the user writes cabal install doctest.
I can't immediately imagine how augmenting --info like this will go wrong but it is definitely something ad-hoc which will cause head scratching in 10 years time when doctest doesn't exist in the same form. If this is the decided implementation strategy then please document how this works in the user manual and explain why the implementation is like this.
cabalthen should also allow for both options to be specified at the same time, e.g.:$ cabal repl --with-ghc=ghc-9.8.2 --with-repl=doctest
@sol I think this is too complicated. doctest is currently compiled for a specific version of GHC, so providing this new option would still require that the user provide the correct pair (GHC, doctest), instead of cabal deducing it. I mean, it’s not really better than the current workaround you proposed:
$ 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.
Perhaps a more principled way is to look at the
LibDirvariable and look in../binfor ghc-pkg?
@mpickering looks a better option 👍
- Is this entry inconsistent across the GHC version currently supported by cabal? i.e. missing entry, other format or semantics?
- Are there settings where this would not work?
If one of the answer is yes, we should add a fallback with the current behavior. So the lookup would be:
- Try the path
<LibDir>/../bin, where<LibDir>comes fromghc --info. - If no target executable is found, try with the path of the compiler, either from
--with-{ghc,compiler}or found inPATH.
This would solve the issue for doctest and provide a new feature for creative uses of cabal.
Perhaps a more principled way is to look at the
LibDirvariable and look in../binfor ghc-pkg?@mpickering looks a better option 👍
I'm definitely not opposed, will probably be more robust than what Cabal currently does.
- Is this entry inconsistent across the GHC version currently supported by cabal? i.e. missing entry, other format or semantics?
Prior to GHC 9.4, apparently that would need to be ../../bin instead of ../bin.
If one of the answer is yes, we should add a fallback with the current behavior. So the lookup would be:
- Try the path
<LibDir>/../bin, where<LibDir>comes fromghc --info.- If no target executable is found, try with the path of the compiler, either from
--with-{ghc,compiler}or found inPATH.
This sounds good to me 👍
cabalthen should also allow for both options to be specified at the same time, e.g.:$ cabal repl --with-ghc=ghc-9.8.2 --with-repl=doctest@sol I think this is too complicated.
doctestis currently compiled for a specific version of GHC, so providing this new option would still require that the user provide the correct pair (GHC, doctest), instead of cabal deducing it. I mean, it’s not really better than the current workaround you proposed:$ cabal repl --with-ghc=doctest --with-hc-pkg=ghc-pkg-9.8.2The key it that we want cabal to deduce the correct path for other executables.
There's still the issue that when doctest acts as a proxy for GHC, then theghc-paths package is not complied correctly (not sure if any other package are affected). For that reason a user should always do a clean build first, before invoking cabal repl --with-ghc=doctest.
--with-repl would make this more robust, with the disadvantage that now the user will be responsible to make sure that versions of doctest and ghc fit.
Prior to GHC 9.4, apparently that would need to be
../../bininstead of../bin.
- 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 ghc entry seems easier now.
@wismill I imagine in 9.2 the path is $libdir/bin and in 9.4 the path is $libdir/../bin.
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 doctest to also install a suitable wrapper script which would point to the right ghc-pkg.
Are you both against my proposed solution of using an external cabal command? That seems to solve many issues to me.
@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 ghc-pkg is in PATH? How would you select a different GHC version, apart by using say ghcup set ghc XXX then running you script?
Or do you mean we somehow reverse the roles: e.g. a bash script cabal-doctest that:
- Has as first argument the doctest executable, the rest will be passed to cabal as is;
- Runs e.g.
doctest --infoto query the various executables it needs to pass to--with-xxx, in order to not depend on cabal’s resolution for these; say it stores them in$DOCTEST_xxxvariables; - Finally runs e.g.:
Note that there might be other arguments needed to pass tocabal repl --with-compiler="$1" --with-hc-pkg="$DOCTEST_PKG" "${@:2}"cabalto avoid incorrect path resolution.
This is simply automatizing the workaround and leave the responsibility to doctest to configure cabal args. It sounds like a proper solution:
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.
@wismill I imagine in 9.2 the path is
$libdir/binand in 9.4 the path is$libdir/../bin.
Depending on whether you want the wrapper script or the binary it is:
- GHC 9.2 and earlier
$libdir/bin(binary) and$libdir/../../bin(wrapper script) - GHC 9.4 and later
$libdir/../bin(binary) and$libdir/../../../bin(wrapper script)
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 libdir instead of executable path seems more robust. At least I can't come up with any immediate downsides. That's why as of now I would be in favor of that approach.
Are you both against my proposed solution of using an external cabal command? That seems to solve many issues to me.
I think an external cabal subcommand could be neat. If I can choose, then I want all three:
- A more reliable way for Cabal to determine
ghc-pkgby looking atLibDir - A
--with-reploption so thatdoctestdoes not have to proxy calls toghc - An external
cabalsubcommand as sugarcoating for the end user
@wismill
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 setup_doctest). (So you don't have to manually install doctest for many different ghc versions)
Then the second part automates the --with-repl passing. It should also probably query ghc-pkg in setup-doctest and then pass --with-ghc-pkg here as well.
With the external command support in the new version of cabal you can run this as cabal doctest.
Another idea would be that you keep the installation story for doctest the same as it is now (ie you manually install doctest for the version you want), but it also installs a cabal-doctest wrapper which hard-codes in the paths to the right ghc-pkg version in the wrapper.
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-doctest with some of the ideas developped previously.
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 cabal doctest, but not when invoked as CABAL=cabal doctest.
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.