haskell / cabal

Official upstream development repository for Cabal and cabal-install
https://haskell.org/cabal
Other
1.62k stars 691 forks source link

Cabal mangles exceptions #823

Open bos opened 12 years ago

bos commented 12 years ago

(Imported from Trac #833, reported by @igloo on 2011-04-21)

Cabal mangles exceptions, e.g.:

$ "utils/ghc-cabal/dist/build/tmp/ghc-cabal" install "/home/ian/ghc/git/ghc/bindisttest/install dir/lib/ghc-7.1.20110421/ghc" "/home/ian/ghc/git/ghc/bindisttest/install dir/lib/ghc-7.1.20110421/ghc-pkg" ":" "/home/ian/ghc/git/ghc/bindisttest/install dir/lib/ghc-7.1.20110421" libraries/ghc-prim dist-install '' '/home/ian/ghc/git/ghc/bindisttest/install dir' '/home/ian/ghc/git/ghc/bindisttest/install dir/lib/ghc-7.1.20110421' '/home/ian/ghc/git/ghc/bindisttest/install dir/share/doc/ghc/html/libraries'
Installing library in /home/ian/ghc/git/ghc/bindisttest/install
dir/lib/ghc-7.1.20110421/ghc-prim-0.2.0.0
ghc-cabal: /home/ian/ghc/git/ghc/bindisttest/install
dir/lib/ghc-7.1.20110421/settings: openFile: does not exist (No such file or
directory)
where a space in the filename has been replaced with a '\n'. This can be very confusing! It looks like Distribution.Simple.Utils.topHandler / wrapText is the culprit.
manzyuk commented 11 years ago

I'm at OdHac at the moment and am trying to do some work on Cabal, so I've looked at this issue for warmup.

The problem is indeed in the function Distribution.Simple.Utils.wrapText, which is used to wrap messages at 79 characters. No attempt is made to detect whether a space at which a line is wrapped is part of a file name. One can reproduce the problem more easily as follows:

$ cabal install http://hackage.haskell.org/packages/archive/groups/0.2.0.1/groups-0.2.0.1.tar.gz --prefix "/home/manzyuk/foo/bar/baz quux" Downloading http://hackage.haskell.org/packages/archive/groups/0.2.0.1/groups-0.2.0.1.tar.gz Resolving dependencies... In order, the following will be installed: groups-0.2.0.1 (reinstall) Warning: Note that reinstalls are always dangerous. Continuing anyway... Configuring groups-0.2.0.1... Building groups-0.2.0.1... Preprocessing library groups-0.2.0.1... [1 of 1] Compiling Data.Group ( src/Data/Group.hs, dist/build/Data/Group.o ) In-place registering groups-0.2.0.1... Installing library in /home/manzyuk/foo/bar/baz quux/lib/groups-0.2.0.1/ghc-7.4.1 Registering groups-0.2.0.1... Installed groups-0.2.0.1

Note that the line starting with "Installing library in" is wrapped at the space that is part of a directory name.

I suppose, a proper way to fix it would be to use an algebraic data type rather than String to represent messages, something like Text.PrettyPrint.Doc, so that file names can be treated as unbreakable entities that shouldn't be wrapped. This, however, seems to require quite some re-engineering of error handling in Cabal. Should I go ahead and do it?

23Skidoo commented 11 years ago

@manzyuk

This, however, seems to require quite some re-engineering of error handling in Cabal. Should I go ahead and do it?

I think it's a minor issue and your efforts are better spent elsewhere.

23Skidoo commented 11 years ago

An alternative solution would be to enclose all paths inside single quotes and then make wrapText treat all text inside single quotes as a single word. Example:

In-place registering groups-0.2.0.1...
Installing library in 
'/home/manzyuk/foo/bar/baz quux/lib/groups-0.2.0.1/ghc-7.4.1'
Registering groups-0.2.0.1...
BardurArantsson commented 8 years ago

... or perhaps simpler: Just remove the wrapping altogether?

I really don't understand why cabal making a judgement call here on 79 being the "correct" terminal width, instead of the user's actual terminal width -- in which case wrapping happens automatically anyway.

23Skidoo commented 8 years ago

I really don't understand why cabal making a judgement call here on 79 being the "correct" terminal width, instead of the user's actual terminal width

Long messages are easier to read that way on large terminals (e.g. widescreen-sized).

ezyang commented 8 years ago

Honestly, the correct way to do this is to throw pretty-printing documents rather than strings. Then the pretty-printer has enough information to correctly wrap.

23Skidoo commented 8 years ago

Yep, that's the same solution that @manzyuk proposed above.