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 #6856 Make --lib warning louder and clearer #6857

Merged
merged 1 commit into from
Jun 4, 2020
Merged

Fix #6856 Make --lib warning louder and clearer #6857

merged 1 commit into from
Jun 4, 2020

Conversation

TomMD
Copy link
Contributor

@TomMD TomMD commented May 28, 2020

In so far as I didn't even add a newline, just some output characters.

  • Any changes that could be relevant to users have been recorded in the changelog (add file to changelog.d directory).

  • The documentation has been updated, if necessary.

I tested it by running the binary and observing the warning is now red on my screen. I googled and found claims the color works on windows terminals as well.

changelog.d/issue-6856 Outdated Show resolved Hide resolved
cabal-install/Distribution/Client/CmdInstall.hs Outdated Show resolved Hide resolved
@phadej
Copy link
Collaborator

phadej commented May 29, 2020

But generally. I'm not sure if this is a joke. Shouldn't we try to find and work (i.e. discuss) on proper solutions like #6481.

@TomMD
Copy link
Contributor Author

TomMD commented May 29, 2020

It's not a joke. People consistently aren't reading the messaging and going to the internet for help. An ASCII banner would be more portable and might have enough impact on behavior to be worth considering.

@yaxu
Copy link

yaxu commented Jun 3, 2020

"You asked to install executables" - doesn't this come up when you just do e.g. cabal install tidal? tidal is a library, without any executables, in which case the warning would be incorrect. Making it red or magenta wouldn't make it any less confusing.

@TomMD
Copy link
Contributor Author

TomMD commented Jun 3, 2020

@yaxu Good point, the wording itself might be more of the problem. It seems useful to re-iterate that there really is a problem and users get stuck regularly. Hopefully everyone is open to both fast and slow solutions to this issue.

Proposal: A bit of word-smithing and ascii-banner to call out the message to get users on the right track. This should be faster and less contentious than colors while not impeding the alternative linked by @phadej .

We currently say:

Warning: You asked to install executables, but there are no executables in
target: entropy. Perhaps you want to use --lib to install libraries instead.

How about instead we borrow from SSH and say:

@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@ WARNING: Installation might not be completed as desired! @
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
Without flags, the command "cabal install" only installs executables.
You might have wanted to run "cabal install --lib ${TARGET}"

It's a fast change, can make the next release, and no one will feel like it was wasted work if the whole issue is better fixed before the release.

@yaxu
Copy link

yaxu commented Jun 4, 2020

Agreed that's better. Howabout rather than WARNING: Installation might not be completed as desired! it gets to the point and says WARNING: Nothing has been installed!.

(I'm still unclear why it doesn't just install what's listed in the .cabal file like the old days, but understand there's some technical reason..)

@phadej
Copy link
Collaborator

phadej commented Jun 4, 2020

WARNING: Nothing has been installed!. is incorrect. Something is installed (namely library in the store).

I don't like this, but I'll accept a patch which

  • Doesn't use ansi colors
  • Isn't oversimplifying the message

We want to respect users and not assume that they are completely dumb and cannot read at all.


It would be nicer to cut the installation early, similarly to #6832
Then if cabal-install doesn't actually build anything but just output some (moderate amount of) stuff with a warning or error at the end, it will be visible.

But as I don't expect #6832 to be fixed for 3.4, I'll merge (the better version of) this PR to 3.4 branch when it's cut.

@yaxu
Copy link

yaxu commented Jun 4, 2020

If the library isn't usable, I'd say it absolutely hasn't been installed.

I'd also say it's not the end-user that's stupid, but the software, for not installing a library when asked. Especially when this is undocumented, changed behaviour.. cabal install used to install libraries fine.

Well this is worse than undocumented - it seems the old (working) behaviour is documented, and the current (non-working) behaviour is not. Here's some results from a google search result for "cabal install". Some results look authoritative and up-to-date. None explain that "cabal install" doesn't install libraries, or give details of '--lib'.

Actually, I can't find any reference material that says how to install libraries with cabal.

@phadej
Copy link
Collaborator

phadej commented Jun 4, 2020

@yaxu, It's not like we (or myself) not trying to improve situation like I pointed in previous comments referring to other issues in this space.

If it is how you think, then I'll merge this PR, but don't assume that we (well, me) will have any motivation to fix the root cause. This warning should be then enough for unforeseeable future.

@yaxu
Copy link

yaxu commented Jun 4, 2020

Apologies for my tone there @phadej, in my defence I hadn't yet eaten lunch.

I'm super grateful for this and everything, just a little frustrated sometimes with the teething problems, but thanks a lot!

@TomMD
Copy link
Contributor Author

TomMD commented Jun 4, 2020

Doesn't use ansi colors

Done. Using the banner instead, which is very SSH in appearance.

Isn't oversimplifying the message

@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@ WARNING: Installation might not be completed as desired! @
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
Without flags, the command "cabal install" doesn't expose libraries in a
usable manner. You might have wanted to run "cabal install --lib tidal".

This appears to be saying really only as much as we know

  • We're talking about installing something
  • It might or might not have been done how the user wants (and perhaps they do want a copy to the store without link to package env, it's just the rare case)
  • Any installed libraries are not directly "usable", by typical developers using GHCi, without "--lib". The word "usable" masks a world of nuance with which we are walking a fine line of respecting peoples time and existing knowledge, not forcing them to read a long winded explanation.

Well this is worse than undocumented - it seems the old (working) behaviour is documented, and the current (non-working) behaviour is not.

Interesting and terrifying statement, @yaxu. While it's not related to this PR it is related to the issue I'm trying to solve (public education and cold-start ability re: Haskell tooling). Looking deeper:

  • @phadej was the one who made the best documentation in exactly the right spot in the official readthedocs, 2 months ago (3752c3e)
  • The intro you link should probably mention --lib. Care to make a PR? The file is Cabal/doc/intro.rst.
  • The wiki links is flagged as out of date right at the top. Should probably be deleted if you care to take a moment I'd encourage it.
  • The hackage page at least isn't incorrect as far as I looked but could probably be revamped to link to readthedocs, mention ghcup etc.

Care to make an issue to discuss and push through a PR?

@TomMD TomMD changed the title Fix #6856 Make --lib warning red so people read Fix #6856 Make --lib warning louder and clearer Jun 4, 2020
@yaxu
Copy link

yaxu commented Jun 4, 2020

I think intro probably needs the attention of someone more expert than me, it's currently documenting v1- instructions so probably needs more updates than just adding --lib. For example it gives commands for installing packages from tarballs and URLs which as far as I know isn't supported by v2-install.

@phadej
Copy link
Collaborator

phadej commented Jun 4, 2020

Installing packages from tarballs is supported in master: #6393

@phadej phadej merged commit 07c1a43 into haskell:master Jun 4, 2020
@phadej phadej mentioned this pull request Jul 10, 2020
@phadej phadej added this to the 3.4.0.0-rc1 milestone Jul 13, 2020
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.

4 participants