Skip to content

Fix #5488: change default for logging to +nowrap #9160

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yvan-sraka
Copy link
Collaborator

@yvan-sraka yvan-sraka commented Aug 3, 2023

This PR modifies cabal behavior by change default for logging to +nowrap and so it introduces a +wrap modifier :)

Checklist:

QA notes:

Quoting the linked issue #5488, this PR brings visual differences in the solver output, as an example, this here is a solver failure pre-wrapped via wrapText:

$ cabal new-repl -w ghc-8.6.1 --build-depends 'lens'
Resolving dependencies...
cabal: Could not resolve dependencies:
[__0] trying: fake-package-0 (user goal)
[__1] trying: lens-4.17 (dependency of fake-package)
[__2] trying: transformers-0.5.5.0/installed-0.5... (dependency of lens)
[__3] next goal: hashable (dependency of lens)
[__3] rejecting: hashable-1.2.7.0 (conflict: transformers =>
base==4.12.0.0/installed-4.1..., hashable => base>=4.4 && <4.12)
[__3] rejecting: hashable-1.2.6.1 (conflict: transformers =>
base==4.12.0.0/installed-4.1..., hashable => base>=4.4 && <4.11)
[__3] rejecting: hashable-1.2.6.0 (conflict: transformers =>
base==4.12.0.0/installed-4.1..., hashable => base>=4.4 && <4.10)
[__3] rejecting: hashable-1.2.5.0, hashable-1.2.4.0 (conflict: transformers =>
base==4.12.0.0/installed-4.1..., hashable => base>=4.0 && <4.10)
[__3] rejecting: hashable-1.2.3.3, hashable-1.2.3.2 (conflict: transformers =>
base==4.12.0.0/installed-4.1..., hashable => base>=4.0 && <4.9)
[__3] rejecting: hashable-1.2.3.1, hashable-1.2.3.0, hashable-1.2.2.0,
hashable-1.2.1.0, hashable-1.2.0.10, hashable-1.2.0.9, hashable-1.2.0.8,
hashable-1.2.0.7, hashable-1.2.0.6, hashable-1.2.0.5, hashable-1.2.0.4,
hashable-1.2.0.3, hashable-1.2.0.2, hashable-1.2.0.1, hashable-1.2.0.0,
hashable-1.1.2.5, hashable-1.1.2.4, hashable-1.1.2.3 (conflict: transformers
=> base==4.12.0.0/installed-4.1..., hashable => base>=4.0 && <4.8)
[__3] rejecting: hashable-1.1.2.2, hashable-1.1.2.1, hashable-1.1.2.0,
hashable-1.1.1.0, hashable-1.1.0.0, hashable-1.0.1.1, hashable-1.0.1.0,
hashable-1.0.0 (conflict: lens => hashable>=1.1.2.3 && <1.3)
[__3] fail (backjumping, conflict set: hashable, lens, transformers)
After searching the rest of the dependency tree exhaustively, these were the
goals I've had most trouble fulfilling: hashable, lens, base, transformers,
fake-package

...and the same with +nowrap (the new default that this PR introduce):

$ cabal new-repl -w ghc-8.6.1 --build-depends 'lens' -vnormal+nowrap
Resolving dependencies...
cabal: Could not resolve dependencies:
[__0] trying: fake-package-0 (user goal)
[__1] trying: lens-4.17 (dependency of fake-package)
[__2] trying: transformers-0.5.5.0/installed-0.5... (dependency of lens)
[__3] next goal: hashable (dependency of lens)
[__3] rejecting: hashable-1.2.7.0 (conflict: transformers => base==4.12.0.0/installed-4.1..., hashable => base>=4.4 && <4.12)
[__3] rejecting: hashable-1.2.6.1 (conflict: transformers => base==4.12.0.0/installed-4.1..., hashable => base>=4.4 && <4.11)
[__3] rejecting: hashable-1.2.6.0 (conflict: transformers => base==4.12.0.0/installed-4.1..., hashable => base>=4.4 && <4.10)
[__3] rejecting: hashable-1.2.5.0, hashable-1.2.4.0 (conflict: transformers => base==4.12.0.0/installed-4.1..., hashable => base>=4.0 && <4.10)
[__3] rejecting: hashable-1.2.3.3, hashable-1.2.3.2 (conflict: transformers => base==4.12.0.0/installed-4.1..., hashable => base>=4.0 && <4.9)
[__3] rejecting: hashable-1.2.3.1, hashable-1.2.3.0, hashable-1.2.2.0, hashable-1.2.1.0, hashable-1.2.0.10, hashable-1.2.0.9, hashable-1.2.0.8, hashable-1.2.0.7, hashable-1.2.0.6, hashable-1.2.0.5, hashable-1.2.0.4, hashable-1.2.0.3, hashable-1.2.0.2, hashable-1.2.0.1, hashable-1.2.0.0, hashable-1.1.2.5, hashable-1.1.2.4, hashable-1.1.2.3 (conflict: transformers => base==4.12.0.0/installed-4.1..., hashable => base>=4.0 && <4.8)
[__3] rejecting: hashable-1.1.2.2, hashable-1.1.2.1, hashable-1.1.2.0, hashable-1.1.1.0, hashable-1.1.0.0, hashable-1.0.1.1, hashable-1.0.1.0, hashable-1.0.0 (conflict: lens => hashable>=1.1.2.3 && <1.3)
[__3] fail (backjumping, conflict set: hashable, lens, transformers)
After searching the rest of the dependency tree exhaustively, these were the goals I've had most trouble fulfilling: hashable, lens, base, transformers, fake-package

@yvan-sraka yvan-sraka force-pushed the fix-5488 branch 2 times, most recently from b483884 to 69b65ef Compare August 3, 2023 16:08
@Kleidukos
Copy link
Member

@yvan-sraka good initiative! Would you mind showing us what it looks like before and after? :)

@yvan-sraka
Copy link
Collaborator Author

@yvan-sraka good initiative! Would you mind showing us what it looks like before and after? :)

Yes! It's highlighted in the linked issue #5488

@yvan-sraka yvan-sraka requested a review from Kleidukos August 29, 2023 07:30
@yvan-sraka yvan-sraka force-pushed the fix-5488 branch 2 times, most recently from 62df668 to 7946b4e Compare November 3, 2023 11:53
@andreabedini
Copy link
Collaborator

🤔 this does not just flip a default setting right?

If the default is -vnormal+nowrap, how does one get the old behaviour? -vnormal+wrap does not seem to work.

Also, I am not sure adding a new constructor to VerbosityFlag is the right thing to do:

ghci> normal
Verbosity {vLevel = Normal, vFlags = fromList [], vQuiet = False}
ghci> verboseWrap normal
Verbosity {vLevel = Normal, vFlags = fromList [VWrap], vQuiet = False}
ghci> verboseNoWrap $ verboseWrap normal
Verbosity {vLevel = Normal, vFlags = fromList [VNoWrap,VWrap], vQuiet = False}

What would this last verbosity mean?

This introduce a +wrap modifier that make Cabal CLI follows the ancient
behavior.
@yvan-sraka
Copy link
Collaborator Author

Thanks, @andreabedini! I updated the PR (removing the VNoWrap value from the VerbosityFlag enum), it, as expected, does not wrap the output by default now. But I can't figure out why -vnormal+wrap flag does not reproduce the current Cabal default behavior (wrapping the output) … any clue?

Also, since I couldn't test it, it will be only by guessing … But I believe the final behavior will be that, e.g., -vnormal+wrap+nowrap will make the output unwrapped while -vnormal+nowrap+wrap will wrap it, if I understood correctly the parser logic here!

@Kleidukos
Copy link
Member

I have paired with Yvan and many a couple of things were quite frustrating.

It is not clear how displayException uses the wrapping CLI options.

(typeclass methods don't help, but it seems like it is called by dieWithException, through throwIO, which itself doesn't seem to apply wrapping.

For those interested, here is the profiling log that was generated.

The labyrinthine code of cabal makes it surprisingly difficult to apply such a small change.

cabal.prof.txt

@andreabedini
Copy link
Collaborator

@Kleidukos @yvan-sraka is this of any help?

-- Exception and logging utils

@yvan-sraka
Copy link
Collaborator Author

I'll try to rephrase and better explain my bug, so I will find help in solving it:

The TLDR of #9160 (comment) is that I realized that after my changes that now the output is unwrapped by default (what I want) but I can't wrap it anymore (with a verbosity modifier) which is a bug ...

The commands I'm using to experiment are to compare the output of cabal build -vnormal+unwrap (the cabal-install version provided by Nix) vs. the one of cabal run cabal-install -- build -vnormal+wrap (the cabal-binaries I just build and that contains my current changes).

But last time I ran my tests, I got Error: cabal: invalid argument to option '-v': normal+wrap on /nix/store/fykw1mb27xz82jj0530w945kqihanrw4-cabal-install-3.10.1.0/bin/cabal which confuse me a lot ... can someone help me find the right command invocations to test my changes?

After that, to sum up what we discover with @Kleidukos, is that https://github.com/haskell/cabal/blob/master/Cabal/src/Distribution/Simple/Utils.hs#L662 seems to never be called (while https://github.com/haskell/cabal/blob/master/Cabal/src/Distribution/Verbosity.hs#L202 is called), and I don't understand why ... maybe some hint on how to use the Cabal REPL to debug that (is there a documentation somewhere on that?) could help! 🙂

Thanks in advance to anyone that could help me better understand what happens here!

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.

3 participants