Fix #7714: use nroff -man | less as backend for cabal man

[andreasabel]

Fix #7714: use nroff -man /dev/stdin | less as backend for cabal man

WIP: This does not work yet, I need some help with constructing the pipe correctly in Haskell.

I seem to follow the example at
https://stackoverflow.com/questions/41030168/haskell-using-the-handle-created-from-createprocess-and-createpipe-to-pipe-int
which works for me, but my patch does not seem to invoke less correctly.

---

Please include the following checklist in your PR:

• Patches conform to the coding conventions.
• Any changes that could be relevant to users have been recorded in the changelog.
• The documentation has been updated, if necessary.

Please also shortly describe how you tested your change. Bonus points for added tests!

[andreasabel]

WIP: This does not work yet, I need some help with constructing the pipe correctly in Haskell.

Weirdly enough, it could be because the generated text is too long (800kB). If I truncate it to 192kB (this is a precise hard bound, 1 char more is too much(!)), it works...

[phadej]

i wouldn't construct pipe, but rather read in the result of nroff and then spawn less. Modern machines can handle having the whole nroff output present!

https://hackage.haskell.org/package/Cabal-3.6.2.0/docs/Distribution-Simple-Utils.html#v:rawSystemStdInOut as a bonus, no low-level process handling (which is indeed tricky, and I rather avoid it if possible).

[phadej]

if you really want a pipe (which i wouldn't recommend), you need to make one https://hackage.haskell.org/package/process-1.6.13.2/docs/System-Process.html#g:7.

[andreasabel]

@phadej wrote:

if you really want a pipe (which i wouldn't recommend), you need to make one hackage.haskell.org/package/process-1.6.13.2/docs/System-Process.html#g:7.

The documentation suggests that a pipe is created when the CreatePipe constructor is supplied, see https://hackage.haskell.org/package/process-1.6.13.2/docs/System-Process.html#t:StdStream. If not, clarification of the docs would be in order. @phadej, do you know more details?

[andreasabel]

i wouldn't construct pipe, but rather read in the result of nroff and then spawn less. Modern machines can handle having the whole nroff output present!

hackage.haskell.org/package/Cabal-3.6.2.0/docs/Distribution-Simple-Utils.html#v:rawSystemStdInOut as a bonus, no low-level process handling (which is indeed tricky, and I rather avoid it if possible).

Thanks, I'll try that!

[phadej]

The documentation suggests that a pipe is created when the CreatePipe constructor is supplied

Yes a pipe is created, for example for standard output: Library gives the writing end to the spawned process and gives the reading end to you. A pipe (= two FDs) is created, but you get only one end of it.

[andreasabel]

but you get only one end of it.

Which I hand as stdin to the second process. I thought that is exactly what a pipe is.

[phadej]

Which I hand as stdin to the second process. I thought that is exactly what a pipe is.

Indeed. Weird. (also process code is very hard to follow). I'd rather not need to understand why it doesn't work :)

[andreasabel]

Got the piping to work by not piping.

New commit message:

Fix #7714: use nroff -man | less as backend for cabal man

Directly piping into man -l - does not work as BSD-man does not understand option -l. More standardized are the building blocks nroff and less.

cabal man now should behave as pipeline

cabal man --raw | nroff -man /dev/stdin | less

Also fixed output of cabal man --raw so that it does not produce warnings.

• .R removed. Was warning:

  `R' is a string (producing the registered sign), not a macro.
  

• No quoted 'new-FOO' should appear at beginning of line. Was
  warning:

  warning: macro `new-FOO'' not defined (probably missing space after `ne')
  

Added to cabal-testsuite/PackageTests/Man/cabal.test.hs a check that the stderr output of nroff -man /dev/stdin is empty (no warnings).

Remaining problem:

Unfortunately, after quitting less with q the following error is displayed:

fd:NNN: commitBuffer: resource vanished (Broken pipe)

Not sure how to fix this (my attempts failed).

[andreasabel]

The presence of hClose here does not seem to make a difference.

[Mikolaj]

Just brainstorming: hFlush before close?

[phadej]

It would be best to add a new helper to D.Simple.Utils. I'm somewhat surprised there isn't such "takes input, inherits output" variant already. EDIT: and do stdin writing as elsewhere.

[phadej]

fd:NNN: commitBuffer: resource vanished (Broken pipe)

You have to ignore it. Grep for ignoreSigPipe.

[phadej]

Also an error in tests:

*** Exception: nroff: readCreateProcessWithExitCode: runInteractiveProcess: exec: does not exist (No such file or directory)

I think we should fallback to man when nroff doesn't exist.

[phadej]

For the record, e.g. git commit --help seems to simply call man git-commit as git installs the man pages. That works on BSD-derivatives too, obviously. Also doesn't need *roff to be available.

[andreasabel]

@phadej: Thanks for the review, I am working on it.

For the record, e.g. git commit --help seems to simply call man git-commit as git installs the man pages. That works on BSD-derivatives too, obviously. Also doesn't need *roff to be available.

The problem that I am trying to fix here is that BSD-man does not provide a means to simply format and display a file, like Linux man -l FILE does. At least I did not see one:

$ man --help
man, version 1.6c

usage: man [-adfhktwW] [section] [-M path] [-P pager] [-S list]
	[-m system] [-p string] name ...

  a : find all matching entries
  c : do not use cat file
  d : print gobs of debugging information
  D : as for -d, but also display the pages
  f : same as whatis(1)
  h : print this help message
  k : same as apropos(1)
  K : search for a string in all pages
  t : use troff to format pages for printing
  w : print location of man page(s) that would be displayed
      (if no name given: print directories that would be searched)
  W : as for -w, but display filenames only

  C file   : use `file' as configuration file
  M path   : set search path for manual pages to `path'
  P pager  : use program `pager' to display pages
  S list   : colon separated section list
  m system : search for alternate system's man pages
  p string : string tells which preprocessors to run
               e - [n]eqn(1)   p - pic(1)    t - tbl(1)
               g - grap(1)     r - refer(1)  v - vgrind(1)

So it is not the problem of man being installed or not, but on finding a set of tools that have constant interfaces across OSs.

[phadej]

Yes, my comment was just answering my own curiosity, i.e. how git command --help seems to work. They "cheat".

[andreasabel]

Update:

• attempt to fix CI
• ignore SIGPIPE

Not addressed (yet):

• I think we should fallback to man when nroff doesn't exist.
• It would be best to add a new helper to D.Simple.Utils. I'm somewhat surprised there isn't such "takes input, inherits output" variant already.

Didn't understand this comment:

EDIT: and do stdin writing as elsewhere.

[phadej]

that is so weird that buildpack-deps base image doesn't have man-db installed. Though it kind of make sense, it still is weird.

[phadej]

and do stdin writing as elsewhere.

refers to the wish that we rather add a new helper to Distribution.Simple.Utils to spawn processes with inherited outputs, rather then going low-level in a particular command implementation.

EDIT: I should had done that myself, my only excuse is that no-one was reviewing PRs back then and preventing me from cutting corners.

[andreasabel]

wish that we rather add a new helper to Distribution.Simple.Utils to spawn processes with inherited outputs, rather then going low-level in a particular command implementation.

The existing rawSystemStdInOut

cabal/Cabal/src/Distribution/Simple/Utils.hs

Lines 884 to 892 in 69dbf54

	rawSystemStdInOut :: KnownIODataMode mode
	=> Verbosity
	-> FilePath -- ^ Program location
	-> [String] -- ^ Arguments
	-> Maybe FilePath -- ^ New working dir or inherit
	-> Maybe [(String, String)] -- ^ New environment or inherit
	-> Maybe IOData -- ^ input text and binary mode
	-> IODataMode mode -- ^ iodata mode, acts as proxy
	-> IO (mode, String, ExitCode) -- ^ output, errors, exit

does not lend itself easily to stdin/stdout inheritance; this may explain why such helpers do not exist yet.

I am not well versed with processes and don't understand the code of rawSystemStdInOut off the top of my head.
Maybe refactoring my code in terms of a new process combinator can be left to future work by someone more experienced.

CI passes now, but I would appreciate some manual inspection of the result of cabal man on Linux (I checked it on macOS). Would someone volunteer?

[andreasabel]

Which I hand as stdin to the second process. I thought that is exactly what a pipe is.

Indeed. Weird. (also process code is very hard to follow). I'd rather not need to understand why it doesn't work :)

Btw, @phadej, I got enlightened why my original code did not work: haskell/process#218. I fell into a boobie trap with hPutStr blocking the construction of the second half of the pipe, this is why it deadlocked. I have been naive about concurrency here.

[andreasabel]

Can this PR be merged now? I may not be perfect, but it improves on the status quo, I think.

[Mikolaj]

Any further comments, anybody?

@hasufell, would you like to review?

[hasufell]

@andreasabel I added some fixes here: andreasabel#1

[andreasabel]

Mergiofy was waiting on the "Artifacts" workflows, but these seem gone. So I deleted them from the required statuses list for master in Settings/Branches. Hope that was intended.

[Mikolaj]

Mergiofy was waiting on the "Artifacts" workflows, but these seem gone. So I deleted them from the required statuses list for master in Settings/Branches. Hope that was intended.

Yes, it was, thank you!

[fgaz]

This will need a changelog entry

[andreasabel]

This will need a changelog entry

@fgaz: See #7935