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

new groff incompatibilities? #99

Closed
sten0 opened this issue Aug 15, 2023 · 11 comments · Fixed by #105 or #111
Closed

new groff incompatibilities? #99

sten0 opened this issue Aug 15, 2023 · 11 comments · Fixed by #105 or #111

Comments

@sten0
Copy link

sten0 commented Aug 15, 2023

Hi,

I'm a user of the Debian package (2.0.2+ds1-1), and lintian recently started forwarding these groff warnings:

groff-message an.tmac::508: warning: tbl preprocessor failed, or it or soelim was not run; table(s) likely not rendered (TE macro called with TW register undefined)

groff-message troff::113: warning: cannot select font 'C'

I'm guessing there was some sort of change in groff that go-md2man now needs to adapt to.

'hope it's an easy fix!

@paravoid
Copy link

paravoid commented Aug 17, 2023

I believe these are artifacts of the groff 1.23 release, so it'll affect all distributions soon. I don't have any insight myself, but several packages of mine have been affected, and I've been following along the bug reports and mailing list threads.

groff-message an.tmac::508: warning: tbl preprocessor failed, or it or soelim was not run; table(s) likely not rendered (TE macro called with TW register undefined)

This actually results in malformed output, tables not rendered. Thankfully the fix is easy, as described here: add '\" t as the very first line of the output:
See https://lists.debian.org/debian-devel/2023/08/msg00220.html

I tested this with the crun manpage.

groff-message troff::113: warning: cannot select font 'C'

Debian bugs #1041809 and #1043256 provide some context. Basically, go-md2man generates things like \fB\fCstart\fR, and \fC is not valid. There is an alternative described there, to use .EX/.EE, but notably this breaks the groff -Thtml output (which is reported upstream). I don't know if there is an alternative that is able to produce the exact same output in HTML, but without the new warnings.

@paravoid
Copy link

I reported this in the Debian specifically for the go-md2man package, as bug #1049968, and Cc'ed G. Branden Robinson (groff upstream, among other affiliations), who gave a very long and elaborate response, including specific recommendations for the go-md2man code. You may want to look into it and perhaps talk to him before implementing any fixes here.

@g-branden-robinson
Copy link

Can I be of any assistance?

@thaJeztah
Copy link
Collaborator

Contributions are welcome for sure 😅 (looks like I missed the original report, and just now saw this ticket). Also /cc @cpuguy83

debarshiray added a commit to debarshiray/go-md2man that referenced this issue Oct 10, 2023
It's difficult to portably ask the man(7) macro package for a monospaced
font (whether it's referred to as C, CW or CR) without any extra
configuration.  That's what leads to:
  warning: cannot select font 'C'

This can be reproduced on Fedora 39, which has GNU roff 1.23, with:
  $ man crun >/dev/null
  troff:<standard input>:25: warning: cannot select font 'C'
  troff:<standard input>:134: warning: cannot select font 'C'
  ...
  $ man podman >/dev/null
  troff:<standard input>:15: warning: cannot select font 'C'
  troff:<standard input>:25: warning: cannot select font 'C'
  ...
  $ man toolbox >/dev/null
  troff:<standard input>:33: warning: cannot select font 'C'
  troff:<standard input>:43: warning: cannot select font 'C'
  ...

These may look like odd uses of man(1), but it's used by the Toolbx [1]
test suite to ensure that 'toolbox help' and 'toolbox --help' do render
the toolbox(1) manual, which, like crun(1) and podman(1), is generated
by go-md2man(1).

The warnings come from the current \\fB\\fC sequence for inline code
enclosed in single backticks (ie., `) that uses 'C' to ask for a
monospace font.  Other than the difficulties with portably asking the
man(7) macro package for a monospace font, the \\fB\\fC sequence has
some oddities.

If a monospace font does get selected by \\fC, then the request for bold
with \\fB gets masked, with the possible surprise that switching back to
the 'previous font' with \\fP will result in bold.  This is what happens
when the output is in HyperText Markup Language or PostScript.  If the
output is a terminal device, \\fC is a NOP because terminals are always
monospaced and can't change the font family, so only \\fB is in effect.

Therefore, the use of \\fC is only relevant for non-terminal outputs,
like HTML and PS.  Since HTML and PS use serif fonts by default, an
inline switch to monospace looks visually jarring.  This is different
from other common uses of monospace for inline code, such as on GitLab
and GitHub, where sans-serif fonts are used by default.

Therefore, just don't attempt to use monospace for inline code enclosed
in single backticks (ie., `) and stick to bold everywhere.

As suggested by G. Branden Robinson.

[1] https://containertoolbx.org/
    https://github.com/containers/toolbox

cpuguy83#99
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1049968
@debarshiray
Copy link
Contributor

Contributions are welcome for sure 😅 (looks like I missed the original report, and just now saw this ticket). Also /cc @cpuguy83

I put something together in #105 based on the conversation between @paravoid and @g-branden-robinson on the Debian bug. I checked the HTML and PS outputs with man -Thtml ... and man -Tps respectively.

debarshiray added a commit to debarshiray/go-md2man that referenced this issue Oct 10, 2023
It's difficult to portably ask the man(7) macro package for a monospaced
font (whether it's referred to as C, CW or CR) without any extra
configuration.  That's what leads to:
  warning: cannot select font 'C'

This can be reproduced on Fedora 39, which has GNU roff 1.23, with:
  $ man crun >/dev/null
  troff:<standard input>:25: warning: cannot select font 'C'
  troff:<standard input>:134: warning: cannot select font 'C'
  ...
  $ man podman >/dev/null
  troff:<standard input>:15: warning: cannot select font 'C'
  troff:<standard input>:25: warning: cannot select font 'C'
  ...
  $ man toolbox >/dev/null
  troff:<standard input>:33: warning: cannot select font 'C'
  troff:<standard input>:43: warning: cannot select font 'C'
  ...

These may look like odd uses of man(1), but it's used by the Toolbx [1]
test suite to ensure that 'toolbox help' and 'toolbox --help' do render
the toolbox(1) manual, which, like crun(1) and podman(1), is generated
by go-md2man(1).

The warnings come from the current \\fB\\fC sequence for inline code
enclosed in single backticks (ie., `) that uses 'C' to ask for a
monospace font.  Other than the difficulties with portably asking the
man(7) macro package for a monospace font, the \\fB\\fC sequence has
some oddities.

If a monospace font does get selected by \\fC, then the request for bold
with \\fB gets masked, with the possible surprise that switching back to
the 'previous font' with \\fP will result in bold.  This is what happens
when the manual is rendered in HyperText Markup Language or PostScript:
  $ man --troff-device html <manual> > foo.html
  $ man --troff-device ps <manual> > foo.ps

If the manual is displayed on a terminal device, \\fC is a NOP because
terminals are always monospaced and can't change the font family, so
only \\fB is in effect.

Therefore, the use of \\fC is only relevant for non-terminal outputs,
like HTML and PS.  Since HTML and PS use serif fonts by default, an
inline switch to monospace looks visually jarring.  This is different
from other common uses of monospace for inline code, such as on GitLab
and GitHub, where sans-serif fonts are used by default.

So, just don't attempt to use monospace for inline code enclosed in
single backticks (ie., `) and stick to bold everywhere.

As suggested by G. Branden Robinson.

[1] https://containertoolbx.org/
    https://github.com/containers/toolbox

cpuguy83#99
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1049968
debarshiray added a commit to debarshiray/go-md2man that referenced this issue Oct 10, 2023
It's difficult to portably ask the man(7) macro package for a monospaced
font (whether it's referred to as C, CW or CR) without any extra
configuration.  That's what leads to:
  warning: cannot select font 'C'

This can be reproduced on Fedora 39, which has GNU roff 1.23, with:
  $ man crun >/dev/null
  troff:<standard input>:25: warning: cannot select font 'C'
  troff:<standard input>:134: warning: cannot select font 'C'
  ...
  $ man podman >/dev/null
  troff:<standard input>:15: warning: cannot select font 'C'
  troff:<standard input>:25: warning: cannot select font 'C'
  ...
  $ man toolbox >/dev/null
  troff:<standard input>:33: warning: cannot select font 'C'
  troff:<standard input>:43: warning: cannot select font 'C'
  ...

These may look like odd uses of man(1), but it's used by the Toolbx [1]
test suite to ensure that 'toolbox help' and 'toolbox --help' do render
the toolbox(1) manual, which, like crun(1) and podman(1), is generated
by go-md2man(1).

The warnings come from the current \\fB\\fC sequence for inline code
enclosed in single backticks (ie., `) that uses 'C' to ask for a
monospace font.  Other than the difficulties with portably asking the
man(7) macro package for a monospace font, the \\fB\\fC sequence has
some oddities.

If a monospace font does get selected by \\fC, then the request for bold
with \\fB gets masked, with the possible surprise that switching back to
the 'previous font' with \\fP will result in bold.  This is what happens
when the manual is rendered in HyperText Markup Language or PostScript:
  $ man --troff-device html <manual> > foo.html
  $ man --troff-device ps <manual> > foo.ps

If the manual is displayed on a terminal device, \\fC is a NOP because
terminals are always monospaced and can't change the font family.  So,
only \\fB is in effect.

Therefore, the use of \\fC is only relevant for non-terminal outputs,
like HTML and PS.  Since HTML and PS use serif fonts by default, an
inline switch to monospace looks visually jarring.  This is different
from other common uses of monospace for inline code, such as on GitLab
and GitHub, where sans-serif fonts are used by default.

Hence, just don't attempt to use monospace for inline code enclosed in
single backticks (ie., `) and stick to bold everywhere.

As suggested by G. Branden Robinson.

[1] https://containertoolbx.org/
    https://github.com/containers/toolbox

cpuguy83#99
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1049968
debarshiray added a commit to debarshiray/toolbox that referenced this issue Oct 13, 2023
Ansible's built-in 'package' module doesn't show any details when
installing the RPMs.  All that can be seen is:
  TASK [Install RPM packages]
  fedora-rawhide | changed

Therefore, there's no way to know what version of the packages got
installed.

In this case, not knowing the go-md2man(1) version being used by the CI
makes it difficult to know why the tests are failing on Fedora Rawhide
and Fedora 39 with:
  not ok 3 help: Command 'help' in 177ms
  # (from function `assert_line' in file
       test/system/libs/bats-assert/src/assert.bash, line 479,
  #  in test file test/system/002-help.bats, line 48)
  #   `assert_line --index 0 --partial "toolbox(1)"' failed
  # /usr/bin/man
  #
  # -- line does not contain substring --
  # index     : 0
  # substring : toolbox(1)
  # line      : troff:<standard input>:33: warning: cannot select font
                  'C'
  # --
  #

It could be either because the CI is still using an older version of
go-md2man(1) [1,2], or that there's some other problem.

[1] Fedora golang-github-cpuguy83-md2man commit 117806d50e401c19
    https://src.fedoraproject.org/rpms/golang-github-cpuguy83-md2man/c/117806d50e401c19
    https://src.fedoraproject.org/rpms/golang-github-cpuguy83-md2man/pull-request/3

[2] go-md2man commit d85280db9b54b574
    cpuguy83/go-md2man@d85280db9b54b574
    cpuguy83/go-md2man#99

containers#1386
@paravoid
Copy link

Thanks for working on this! I just tested go-md2man 2.0.3 with crun's manpage and:

groff-message an.tmac::508: warning: tbl preprocessor failed, or it or soelim was not run; table(s) likely not rendered (TE macro called with TW register undefined)

This actually results in malformed output, tables not rendered. Thankfully the fix is easy, as described here: add '\" t as the very first line of the output: See https://lists.debian.org/debian-devel/2023/08/msg00220.html

This is not fixed.

groff-message troff::113: warning: cannot select font 'C'

This is fixed.

@debarshiray
Copy link
Contributor

groff-message an.tmac::508: warning: tbl preprocessor failed, or it or soelim was not run; table(s) likely not rendered (TE macro called with TW register undefined)

This actually results in malformed output, tables not rendered. Thankfully the fix is easy, as described here: add '\" t as the very first line of the output: See https://lists.debian.org/debian-devel/2023/08/msg00220.html

This is not fixed.

Yes, sorry, my bad. I should have been more explicit.

Part of the problem is that I am unable to reproduce the tbl preprocessor failed warning with crun(1). I tried with crun 1.9.2, which was built with GNU roff 1.23, on Fedora 39.

I could try to come up with a fix if you gave me some more explicit hints on how to reproduce.

groff-message troff::113: warning: cannot select font 'C'

This is fixed.

I couldn't reproduce this one easily, either, but it was failing my test suite, which was doing something similar to man toolbox | head or man toolbox | grep ... and running into the warnings.

@paravoid
Copy link

paravoid commented Nov 2, 2023

groff-message an.tmac::508: warning: tbl preprocessor failed, or it or soelim was not run; table(s) likely not rendered (TE macro called with TW register undefined)

This actually results in malformed output, tables not rendered. Thankfully the fix is easy, as described here: add '\" t as the very first line of the output: See https://lists.debian.org/debian-devel/2023/08/msg00220.html

This is not fixed.

Yes, sorry, my bad. I should have been more explicit.

Part of the problem is that I am unable to reproduce the tbl preprocessor failed warning with crun(1). I tried with crun 1.9.2, which was built with GNU roff 1.23, on Fedora 39.

I could try to come up with a fix if you gave me some more explicit hints on how to reproduce.

On a fresh Debian sid system, e.g. by running podman run --rm -it debian:sid:

root@1fd0128f4302:/# apt update
root@1fd0128f4302:/# apt install -y go-md2man man-db wget
root@1fd0128f4302:/# wget https://raw.githubusercontent.com/containers/crun/main/crun.1.md
root@1fd0128f4302:/# go-md2man -in crun.1.md -out crun.1
root@1fd0128f4302:/# LC_ALL=C.UTF-8 MANROFFSEQ='' MANWIDTH=80 man --warnings -E UTF-8 -l -Tutf8 -Z crun.1 >/dev/null
an.tmac:<standard input>:844: warning: tbl preprocessor failed, or it or soelim was not run; table(s) likely not rendered (TE macro called with TW register undefined)

@thaJeztah
Copy link
Collaborator

Let me reopen this issue at least if there's things left to fix (for visibility)

@paravoid
Copy link

Any news here? Thanks :)

@cpuguy83
Copy link
Owner

cpuguy83 commented Feb 4, 2024

@paravoid Something like this? #111

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants