Skip to content

cabal check fails (exitcode 1), but produces warnings only, no error #8880

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

Closed
andreasabel opened this issue Mar 28, 2023 · 4 comments · Fixed by #8908
Closed

cabal check fails (exitcode 1), but produces warnings only, no error #8880

andreasabel opened this issue Mar 28, 2023 · 4 comments · Fixed by #8908
Labels
cabal-install: cmd/check re: error-message Concerning error messages delivered to the user re: user experience User experience (UX) issue

Comments

@andreasabel
Copy link
Member

I am getting a failure from cabal check (3.10.1.0) with the following warnings:
https://github.com/diagrams/active/actions/runs/4546584124/jobs/8015396504?pr=41#step:21:21

Warning: These warnings may cause trouble when distributing the package:
Warning: Please consider moving the file 'CHANGES' from the
'extra-source-files' section of the .cabal file to the section
'extra-doc-files'.
Warning: These packages miss upper bounds:
- doctest
Please add them, using `cabal gen-bounds` for suggestions. For more
information see: https://pvp.haskell.org/
Warning: The following errors will cause portability problems on other
environments:
Warning: The dependency 'setup-depends: 'Cabal' does not specify an upper
bound on the version number. Each major release of the 'Cabal' package changes
the API in various ways and most packages will need some changes to compile
with it. If you are not sure what upper bound to use then use the next major
version.
Warning: Hackage would reject this package.
Error: Process completed with exit code 1.

I wish for a clearer communication of the problems:

  • Which of these is just a recommendation (warning), and which leads to the exit code 1 (error)?
  • Which of these prompts Hackage to reject the package?
@andreasabel andreasabel added cabal-install: cmd/check re: error-message Concerning error messages delivered to the user labels Mar 28, 2023
@ffaf1 ffaf1 added the re: user experience User experience (UX) issue label Mar 28, 2023
@ffaf1
Copy link
Collaborator

ffaf1 commented Mar 28, 2023

Having a different prompt for warnings and errors is indeed reasonable request. The information is there, but as now it gets drowned in the output. Specifically:

Warning: These warnings may cause trouble when distributing the package:
Warning: The following errors will cause portability problems on other
environments:

are the "headers" for different error severities, fixing setup-depends: would get the exit code back to 0. Relevant code:

unless (null buildImpossible) $ do
warn verbosity "The package will not build sanely due to these errors:"
printCheckMessages buildImpossible
unless (null buildWarning) $ do
warn verbosity "The following warnings are likely to affect your build negatively:"
printCheckMessages buildWarning
unless (null distSuspicious) $ do
warn verbosity "These warnings may cause trouble when distributing the package:"
printCheckMessages distSuspicious
unless (null distInexusable) $ do
warn verbosity "The following errors will cause portability problems on other environments:"
printCheckMessages distInexusable
let isDistError (PackageDistSuspicious {}) = False
isDistError (PackageDistSuspiciousWarn {}) = False
isDistError _ = True
isCheckError (PackageDistSuspiciousWarn {}) = False
isCheckError _ = True
errors = filter isDistError packageChecks
unless (null errors) $
warn verbosity "Hackage would reject this package."
when (null packageChecks) $
notice verbosity "No errors or warnings could be found in the package."

I will sketch (and welcome suggestions on) some ideas for a better UX. I see the file was modified in #8427, so — like Sisyphus — that would have to be rebased.

@ffaf1
Copy link
Collaborator

ffaf1 commented Mar 30, 2023

Notes to my future self. This is where exitFailure is called:

checkAction :: Flag Verbosity -> [String] -> Action
checkAction verbosityFlag extraArgs _globalFlags = do
let verbosity = fromFlag verbosityFlag
unless (null extraArgs) $
die' verbosity $ "'check' doesn't take any extra arguments: " ++ unwords extraArgs
allOk <- Check.check (fromFlag verbosityFlag)
unless allOk exitFailure

and this is where the warning message is formed:

-- | Non fatal conditions that may be indicative of an error or problem.
--
-- We display these at the 'normal' verbosity level.
--
warn :: Verbosity -> String -> IO ()
warn verbosity msg = withFrozenCallStack $ do
when ((verbosity >= normal) && not (isVerboseNoWarn verbosity)) $ do
ts <- getPOSIXTime
hFlush stdout
hPutStr stderr . withMetadata ts NormalMark FlagTrace verbosity
. wrapTextVerbosity verbosity
$ "Warning: " ++ msg

Warning: is baked in. I wonder how sensible it would be to add a waringPrompt :: String -> String -> IO () where we control what is prepended (Maybe some IDE use that to color-code output?). Sample output:

Warning: These warnings may cause trouble when distributing the package:
Warning: Please consider moving the file 'CHANGES' from the
'extra-source-files' section of the .cabal file to the section
'extra-doc-files'.
Warning: These packages miss upper bounds:
- doctest
Please add them, using `cabal gen-bounds` for suggestions. For more
information see: https://pvp.haskell.org/
Error: The following errors will cause portability problems on other
environments:
Error: The dependency 'setup-depends: 'Cabal' does not specify an upper
bound on the version number. Each major release of the 'Cabal' package changes
the API in various ways and most packages will need some changes to compile
with it. If you are not sure what upper bound to use then use the next major
version.
Error: Hackage would reject this package.
Error: Process completed with exit code 1.

A less invasive option is to just style header messages differently (add *** or similar? ☞☞☞ ✝✝✝ ✚✚✚ ⭍⭍⭍?) and word them in a stronger manner. Care needed not to annoy blind users who rely on TTS or braille displays. Sample output:

Warning: *** These warnings may cause trouble when distributing the package:
Warning: Please consider moving the file 'CHANGES' from the
'extra-source-files' section of the .cabal file to the section
'extra-doc-files'.
Warning: These packages miss upper bounds:
- doctest
Please add them, using `cabal gen-bounds` for suggestions. For more
information see: https://pvp.haskell.org/
Warning: *** The following errors will cause portability problems on other
environments and will be REFUSED by Hackage:
Warning: The dependency 'setup-depends: 'Cabal' does not specify an upper
bound on the version number. Each major release of the 'Cabal' package changes
the API in various ways and most packages will need some changes to compile
with it. If you are not sure what upper bound to use then use the next major
version.
Warning: Hackage would reject this package.
Error: Process completed with exit code 1.

@ulysses4ever
Copy link
Collaborator

@ffaf1 for the record, i think Cabal's messaging is severely under-developed in terms of decorations. Please, give me more emojis and alike! One of the ways to figure how to do it is to steal from someone, i think. E.g. cargo would be my first choice to look for an example. I guess, I could look into what Julia and its package manager do in such cases. I just need to figure out what would be the closest scenario…

@ulysses4ever
Copy link
Collaborator

Also, I think, the presentation here would benefit greatly from simply having blank lines after every message... Sorry I didn't mention it before the merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cabal-install: cmd/check re: error-message Concerning error messages delivered to the user re: user experience User experience (UX) issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants