GHCup set command error improvement #861

July541 · 2023-07-19T14:54:36Z

This is a demo for #180.

The pr has no effect for command like ghcup set ghc xxx, ghcup set cabal xxx.

For old-style commands likes ghcup set xxx, it has two results. One can set ghc properly, like ghcup set latest and ghcup set recommended, and others are not, likes ghcup set ghc-9.2, it outputs looks like the following even if I've ghc-9.2.8 installed.

 ~ ghcup set ghc-9.2
[ Warn  ] This is an old-style command for setting GHC. Use 'ghcup set ghc' instead.
[ Warn  ] Assuming you meant version 9.2.8
[ Error ] [GHCup-00130] The version ghc-9.2.8 of the tool ghc is not installed.

So my solution is:

For scenario one, gives a warning and continue the set operation since they might be valid and otherwise gives an error and stop the follow-up steps.
For compatibility considerations, the optparse section is extracted to test every potential command that has the same effect during this pr.

Now it looks like:

BEFORE:

~ ghcup set ghc-9.2
[ Warn  ] This is an old-style command for setting GHC. Use 'ghcup set ghc' instead.
[ Warn  ] Assuming you meant version 9.2.8
[ Error ] [GHCup-00130] The version ghc-9.2.8 of the tool ghc is not installed.

AFTER:

~ cabal run ghcup -- set ghc-9.2
[ Error ] Did you mean `ghcup set ghc 9.2`?
[ ...   ]
[ ...   ]   View all candidates with `ghcup set --help`
[ ...   ]

BEFORE:

~ ghcup set
[ Warn  ] This is an old-style command for setting GHC. Use 'ghcup set ghc' instead.
[ Info  ] GHC 9.2.8 successfully set as default version

AFTER:

~ cabal run ghcup -- set 
[ Warn  ] This is an old-style command for setting GHC. Use `ghcup set ghc recommended` instead.
[ Info  ] GHC 9.2.8 successfully set as default version

BEFORE:

~ ghcup set latest
[ Warn  ] This is an old-style command for setting GHC. Use 'ghcup set ghc' instead.
[ Info  ] GHC 9.6.2 successfully set as default version

AFTER:

~ cabal run ghcup -- set latest
[ Warn  ] This is an old-style command for setting GHC. Use `ghcup set ghc latest` instead.
[ Info  ] GHC 9.6.2 successfully set as default version

BEFORE:

~ ghcup set ghc-javascript-unknown-ghcjs-9.6
[ Warn  ] This is an old-style command for setting GHC. Use 'ghcup set ghc' instead.
[ Warn  ] Assuming you meant version 9.6.2
[ Error ] [GHCup-00130] The version ghc-javascript-unknown-ghcjs-9.6.2 of the tool ghc is not installed.

AFTER:

~ cabal run ghcup -- set ghc-javascript-unknown-ghcjs-9.6
[ Error ] Did you mean `ghcup set ghc javascript-unknown-ghcjs-9.6`?
[ ...   ]
[ ...   ]   View all candidates with `ghcup set --help`
[ ...   ]

@hasufell I'm not sure if it is worth it just for a more readable prompt. It's more appreciable if you could take a look at this before I continue improvement so I can cut losses early maybe:)

hasufell · 2023-07-19T15:42:06Z

That looks interesting. I may be able to do proper review during weekend.

hasufell · 2023-07-20T10:14:42Z

So, I think we might not need this.

If you look at the current branch ghc-compile from this PR: #854

Then you will see the behavior is already a bit different:

$ ghcup install ghc-9.2.5
[ Warn  ] This is an old-style command for installing GHC. Use 'ghcup install ghc' instead.
[ Info  ] downloading: https://raw.githubusercontent.com/haskell/ghcup-metadata/master/ghcup-0.0.7.yaml as file /home/hasufell/.ghcup/cache/ghcup-0.0.7.yaml
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
[ Error ] [GHCup-00010] Unable to find a download for the requested version/distro.
[ Error ] Also check the logs in /home/hasufell/.ghcup/logs

This makes sense, because now we have proper cross support. So what happens here is that ghc-9.2.5 gets parsed as:

GHCTargetVersion { _tvTarget = Just "ghc", _tvVersion = 9.2.5 }

Previously, it would throw away the ghc part. Now it doesn't anymore, so it becomes a fatal error.

What we want to do here now is to just give a better error description.

The error that's thrown and that triggers this message is here:

ghcup-hs/lib/GHCup/Errors.hs

Lines 208 to 218 in 6623e4b

    
           -- | Unable to find a download for the requested version/distro. 
        
           data NoDownload = NoDownload 
        
             deriving Show 
        
           instance Pretty NoDownload where 
        
             pPrint NoDownload = 
        
               text (eDesc (Proxy :: Proxy NoDownload)) 
        
           instance HFErrorProject NoDownload where 
        
             eBase _ = 10 
        
             eDesc _ = "Unable to find a download for the requested version/distro."

We throw it in several places, e.g.:

ghcup-hs/lib/GHCup/Download.hs

Lines 319 to 326 in 6623e4b

    
           maybe 
        
             (throwE NoDownload) 
        
             pure 
        
             (case p of 
        
               -- non-musl won't work on alpine 
        
               Linux Alpine -> with_distro <|> without_distro_ver 
        
               _            -> with_distro <|> without_distro_ver <|> without_distro 
        
             )

So we might just have to add more info to the NoDownload type (the GHCTargetVersion for instance) and then check if _tvTarget matches any of the existing tools (ghc/cabal/hls/stack) and then be more descriptive in the error.

That way we have pushed down this concern out of the parser.

I quite like the optparse test suite though. I think this should be extended.

hasufell · 2023-07-20T13:33:36Z

On second thought... I think my proposal has two issues:

we lose information about the exact command that was run, so the message will look something like "Perhaps you meant: 'ghcup <command> ghc 9.2.5'"
the warning will also trigger for ghcup install ghc ghc-9.2.5, in which case it will be somewhat confusing

hasufell · 2023-07-20T13:39:34Z

Maybe it isn't too bad:

$ ghcup install ghc-9.2.5
[ Warn  ] This is an old-style command for installing GHC. Use 'ghcup install ghc' instead.
[ Error ] [GHCup-00010] Unable to find a download for ghc-9.2.5
[ ...   ] Perhaps you meant: 'ghcup <command> ghc 9.2.5'
[ Error ] Also check the logs in /home/hasufell/.ghcup/logs

$ ghcup install ghc ghc-9.2.5
[ Error ] [GHCup-00010] Unable to find a download for ghc-9.2.5
[ ...   ] Perhaps you meant: 'ghcup <command> ghc 9.2.5'
[ Error ] Also check the logs in /home/hasufell/.ghcup/logs

hasufell · 2023-07-20T13:42:54Z

b46537f on branch https://github.com/haskell/ghcup-hs/compare/ghc-compile

Can still be pumped up a little, but might be fine.

July541 · 2023-07-20T14:25:03Z

Maybe it isn't too bad:

$ ghcup install ghc-9.2.5
[ Warn  ] This is an old-style command for installing GHC. Use 'ghcup install ghc' instead.
[ Error ] [GHCup-00010] Unable to find a download for ghc-9.2.5
[ ...   ] Perhaps you meant: 'ghcup <command> ghc 9.2.5'
[ Error ] Also check the logs in /home/hasufell/.ghcup/logs

$ ghcup install ghc ghc-9.2.5
[ Error ] [GHCup-00010] Unable to find a download for ghc-9.2.5
[ ...   ] Perhaps you meant: 'ghcup <command> ghc 9.2.5'
[ Error ] Also check the logs in /home/hasufell/.ghcup/logs

Not bad indeed, turns out that while I typed ghcup set ghc-9.2 and got an error, I quickly ask ghcup set -h for help. So it may not be worth to introduce much change, especially I'm not totally familiar with this system.

The only nit is Perhaps you meant: 'ghcup <command> ghc 9.2.5', the <command> a little vague.

July541 · 2023-07-20T14:29:28Z

I quite like the optparse test suite though. I think this should be extended.

I'll try to write some test for optparse on a new thread.

But it seems hard to take coverage test since the parser and the functionality are tangled.

hasufell · 2023-07-20T14:33:25Z

the a little vague

Yeah... that's the downside with this approach. The error is deep in the callstack and we don't have explicit information anymore what command triggered it.

The only way I can see to fix that is to pass the Command (e.g. InstallCommand) down into the AppState, so we can access it from anywhere. I'm not sure it's worth it.

July541 · 2023-07-20T14:48:17Z

I'm not sure it's worth it.

I'm in favor of the current solution.

hasufell · 2023-07-22T04:20:35Z

Some more tweaking leaves us with this:

ghcup install ghc-9.2.5
[ Warn  ] This is an old-style command for installing GHC. Use 'ghcup install ghc' instead.
[ Error ] [GHCup-00010] Unable to find a download for GHC version 'ghc-9.2.5' on detected platform x86_64-linux-fedora-37
[ ...   ] Perhaps you meant: 'ghcup <command> ghc 9.2.5'
[ Error ] Also check the logs in /home/hasufell/.ghcup/logs

I think it's quite useful to see what platform GHCup came up with when resolving the downloads.

July541 · 2023-07-22T07:49:27Z

Meaningful improvement!

Think from scratch, the error message Unable to find a download for ghc-9.2.5 is not correct, it turns out the failure reason is Unable to find a download for ghc-ghc-9.2.5 right?

And furthermore, the user may think if ghc-9.2.5 doesn't support x86_64-linux-fedora-37 or ghc-9.2.5 doesn't exist at all... Do we have a manual that listed all candidate options, otherwise can we have this?

hasufell · 2023-07-22T07:53:01Z

Think from scratch, the error message Unable to find a download for ghc-9.2.5 is not correct, it turns out the failure reason is Unable to find a download for ghc-ghc-9.2.5 right?

That's what the new error message basically says:

Unable to find a download for GHC version 'ghc-9.2.5'

Do we have a manual that listed all candidate options, otherwise can we have this?

ghcup list?

July541 · 2023-07-22T08:18:13Z

ghcup list?

Well, I'm looking at ghcup set command and found there are many candidates that are not listed, likes Tag and Day

ghcup-hs/lib/GHCup/Types.hs

Lines 709 to 712 in d143dae

    
           data ToolVersion = GHCVersion GHCTargetVersion 
        
                            | ToolVersion Version 
        
                            | ToolTag Tag 
        
                            | ToolDay Day

ghcup-hs/lib/GHCup/Types.hs

Lines 156 to 165 in d143dae

    
           data Tag = Latest 
        
                    | Recommended 
        
                    | Prerelease 
        
                    | LatestPrerelease 
        
                    | Nightly 
        
                    | LatestNightly 
        
                    | Base PVP 
        
                    | Old                -- ^ old versions are hidden by default in TUI 
        
                    | UnknownTag String  -- ^ used for upwardscompat 
        
                    deriving (Ord, Eq, GHC.Generic, Show) -- FIXME: manual JSON instance

hasufell · 2023-07-22T08:31:48Z

I'm not sure I understand what you mean by candidates.

July541 added 3 commits July 16, 2023 01:05

set poc

9e205a4

Add parse test for set

4a2f0e6

More tests

2270f52

July541 closed this Jul 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GHCup set command error improvement #861

GHCup set command error improvement #861

July541 commented Jul 19, 2023

hasufell commented Jul 19, 2023

hasufell commented Jul 20, 2023 •

edited

Loading

hasufell commented Jul 20, 2023

hasufell commented Jul 20, 2023

hasufell commented Jul 20, 2023 •

edited

Loading

July541 commented Jul 20, 2023

July541 commented Jul 20, 2023

hasufell commented Jul 20, 2023

July541 commented Jul 20, 2023

hasufell commented Jul 22, 2023

July541 commented Jul 22, 2023

hasufell commented Jul 22, 2023

July541 commented Jul 22, 2023

hasufell commented Jul 22, 2023

GHCup set command error improvement #861

GHCup set command error improvement #861

Conversation

July541 commented Jul 19, 2023

hasufell commented Jul 19, 2023

hasufell commented Jul 20, 2023 • edited Loading

hasufell commented Jul 20, 2023

hasufell commented Jul 20, 2023

hasufell commented Jul 20, 2023 • edited Loading

July541 commented Jul 20, 2023

July541 commented Jul 20, 2023

hasufell commented Jul 20, 2023

July541 commented Jul 20, 2023

hasufell commented Jul 22, 2023

July541 commented Jul 22, 2023

hasufell commented Jul 22, 2023

July541 commented Jul 22, 2023

hasufell commented Jul 22, 2023

hasufell commented Jul 20, 2023 •

edited

Loading

hasufell commented Jul 20, 2023 •

edited

Loading