bug#30785: Man pages truncated, repeated

[G. Branden Robinson]

Hello Guix folks,

I submitted a change addressing this bug to groff Git back in
January[1].  Despite repeated requests I couldn't scare up any Windows
users to test it, and that's only environment except for Guix (until you
patched around it) that ever exhibited it.

If my understanding of the problem is correct, then something (possibly
preconv) using libgroff should throw an assertion failure.

My thanks if you can help.

Regards,
Branden

[1] https://savannah.gnu.org/bugs/?55475


signature.asc
Description: PGP signature

bug#30785: Man pages truncated, repeated

[Marius Bakke]

Pierre Neidhardt  writes:

> Keeping this bug open till we patch groff on core-updates.

This was done in 5466e82a1e61349e5a3f9726a03874e4f9817226.


signature.asc
Description: PGP signature

bug#30785: Man pages truncated, repeated

[Danny Milosavljevic]

Hi Pierre,

I've applied the groff-minimal patch to master now.


pgpuxua7Q9TAF.pgp
Description: OpenPGP digital signature

bug#30785: Man pages truncated, repeated

[Ludovic Courtès]

Pierre Neidhardt  skribis:

>> It alone does not fix the bug.  Only combined with Pierre's part does it fix 
>> it,
>> the part which makes man-db find groff's preconv in the first place (I don't
>> believe a patch about that was submitted yet).
>
> I can only only add my patch (well, Markus patch, really) after yours, 
> otherwise
> it cause `man` to segfault all the time.
>
> Let me know when you've applied it on core-updates and I'll follow you.

On ‘master’ you mean?  The patch that touches ‘groff-minimal’ (not
‘groff’) can go to ‘master’.

Thanks,
Ludo’.

bug#30785: Man pages truncated, repeated

[Ludovic Courtès]

Hello,

Pierre Neidhardt  skribis:

> Danny, Efraim just reverted your commit because it rebuilds 3600+ packages :p

BTW, we must pay a great deal of attention to issues like this one:
someone who pulled right after that commit must have been wondering
what’s going on.  That makes for a very bad user experience.

I wonder if we could come up with a pre-push hook that runs ‘guix
refresh -l’ or something.

Thoughts?

Ludo’.

bug#30785: Man pages truncated, repeated

[Pierre Neidhardt]

> It alone does not fix the bug.  Only combined with Pierre's part does it fix 
> it,
> the part which makes man-db find groff's preconv in the first place (I don't
> believe a patch about that was submitted yet).

I can only only add my patch (well, Markus patch, really) after yours, otherwise
it cause `man` to segfault all the time.

Let me know when you've applied it on core-updates and I'll follow you.

-- 
Pierre Neidhardt
https://ambrevar.xyz/


signature.asc
Description: PGP signature

bug#30785: Man pages truncated, repeated

[Danny Milosavljevic]

> Please add the “Fixes” line in the log.

It alone does not fix the bug.  Only combined with Pierre's part does it fix it,
the part which makes man-db find groff's preconv in the first place (I don't
believe a patch about that was submitted yet).

Should I still add a "Fixes" line here?

>(I like the description of the bug ;-)).

:-)


pgpeRacLNcxHS.pgp
Description: OpenPGP digital signature

bug#30785: Man pages truncated, repeated

[Ludovic Courtès]

Hello!

Danny Milosavljevic  skribis:

> Apparently, man-db has groff-minimal as a regular input, so I guess we are 
> lucky.
> So we could adapt groff-minimal only if we wanted to.
>
> But that would mean that even after that, the "groff" package would still
> contain a memory corruption bug in preconv - which is arguably a security
> problem.  Should we use grafts instead?

Though in practice we don’t do much with ‘groff’ itself, unless users
explicitly install it and use it, right?  I’m leaning towards fixing it
in ‘core-updates’.

> From adc9e7940b54e467732ec923c6a3fcec810dce48 Mon Sep 17 00:00:00 2001
> From: Danny Milosavljevic 
> Date: Wed, 16 Jan 2019 15:00:04 +0100
> Subject: [PATCH] gnu: groff-minimal: Disable relocatability.
> Tags: patch
>
> * gnu/packages/groff.scm (groff-minimal)[arguments]<#:phases>
> [disable-relocatability]: New phase.

Please add the “Fixes” line in the log.  Otherwise LGTM (I like the
description of the bug ;-)).

Thanks!

Ludo’.

bug#30785: Man pages truncated, repeated

[Danny Milosavljevic]

Apparently, man-db has groff-minimal as a regular input, so I guess we are 
lucky.
So we could adapt groff-minimal only if we wanted to.

But that would mean that even after that, the "groff" package would still
contain a memory corruption bug in preconv - which is arguably a security
problem.  Should we use grafts instead?

Patch for the former is attached.
From adc9e7940b54e467732ec923c6a3fcec810dce48 Mon Sep 17 00:00:00 2001
From: Danny Milosavljevic 
Date: Wed, 16 Jan 2019 15:00:04 +0100
Subject: [PATCH] gnu: groff-minimal: Disable relocatability.
Tags: patch

* gnu/packages/groff.scm (groff-minimal)[arguments]<#:phases>
[disable-relocatability]: New phase.
---
 gnu/packages/groff.scm | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/gnu/packages/groff.scm b/gnu/packages/groff.scm
index 77728a5d7..cf392f546 100644
--- a/gnu/packages/groff.scm
+++ b/gnu/packages/groff.scm
@@ -96,6 +96,36 @@ is usually the formatter of \"man\" documentation pages.")
,@(substitute-keyword-arguments (package-arguments groff)
((#:phases phases)
 `(modify-phases ,phases
+   (add-after 'unpack 'disable-relocatability
+ (lambda _
+   ;; Groff contains a Rube Goldberg-esque relocator for the
+   ;; file "charset.alias".
+   ;; It tries to find the current executable using realpath,
+   ;; a do-it-yourself search in $PATH and so on.
+   ;; Furthermore, the routine that does the search is buggy
+   ;; in that it doesn't handle error cases when they arise.
+   ;; This causes preconv to segfault when trying to look up
+   ;; the file "charset.alias" in the NULL location.
+   ;; The "charset.alias" parser is a copy of gnulib's, and a
+   ;; non-broken version of gnulib's "charset.alias" parser
+   ;; is part of glibc's libcharset.
+   ;; However, groff unconditionally uses their own
+   ;; "charset.alias" parser, but then DOES NOT INSTALL the
+   ;; file "charset.alias" when glibc is too new.
+   ;; In Guix, our file "charset.alias" only contains an
+   ;; obscure alias for ASCII and nothing else.
+   ;; So just disable relocation and make the entire
+   ;; "charset.alias" lookup fail.
+   ;; See 
+   ;; for details.
+   (substitute* "src/libs/libgroff/Makefile.sub"
+(("-DENABLE_RELOCATABLE=1") ""))
+   ;; That file contains a crash bug--so make sure that
+   ;; its contents are not there.
+   (call-with-output-file "src/libs/libgroff/relocate.cpp"
+ (lambda (port)
+   #t))
+   #t))
(add-after 'install 'remove-non-essential-programs
  (lambda* (#:key outputs #:allow-other-keys)
;; Keep only the programs that man-db needs at run time,


pgpgCHNh7_8SO.pgp
Description: OpenPGP digital signature

bug#30785: Man pages truncated, repeated

[Pierre Neidhardt]

My guess is that groff-minimal would work.
Danny?

-- 
Pierre Neidhardt
https://ambrevar.xyz/


signature.asc
Description: PGP signature

bug#30785: Man pages truncated, repeated

[Efraim Flashner]

On Wed, Jan 16, 2019 at 06:39:19PM +0100, Pierre Neidhardt wrote:
> Danny, Efraim just reverted your commit because it rebuilds 3600+ packages :p
> 
> How shall we proceed then?
> 

groff-minimal only has ~20 dependents, does that work? Otherwise
groff-for-manpages is an option


-- 
Efraim Flashner  אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted


signature.asc
Description: PGP signature

bug#30785: Man pages truncated, repeated

[Pierre Neidhardt]

Danny, Efraim just reverted your commit because it rebuilds 3600+ packages :p

How shall we proceed then?

-- 
Pierre Neidhardt
https://ambrevar.xyz/


signature.asc
Description: PGP signature

bug#30785: Man pages truncated, repeated

[Danny Milosavljevic]

Hi Pierre,

On Wed, 16 Jan 2019 14:02:10 +0100
Pierre Neidhardt  wrote:

> OK, now it works.
> 
> Who would like to merge? :)

I've pushed the disable-relocatability patch, amended by a huge comment that's
larger than the actual code, as f57693e17c7905d2f11e46d04cb558fe0b0fc39b.

Please push the part that does the preconv path substitution and mention that
it fixes this bug.




pgpcyrvUJOB1W.pgp
Description: OpenPGP digital signature

bug#30785: Man pages truncated, repeated

[Danny Milosavljevic]

Hi Pierre,

On Wed, 16 Jan 2019 11:01:44 +0100
Pierre Neidhardt  wrote:

> Nope, the patch does not work for me.
> 
> I applied the patch in the Guix checkout, 
> then
> 
> --8<---cut here---start->8---
> $ ./pre-inst-env guix build man-db
> ...
> /gnu/store/1w83i51wvap67kl7gdz51ly3pbbx29dv-man-db-2.8.3
> $ /gnu/store/1w83i51wvap67kl7gdz51ly3pbbx29dv-man-db-2.8.3/bin/man 
> nmcli-examples
> --8<---cut here---end--->8---
> 
> Still truncated.
> 

Does this include your groff preconv path substitution in man?
Does preconv still segfault?
How can I reproduce this situation?  Is it a guix branch other than master?


pgpTAQ_kGNtkv.pgp
Description: OpenPGP digital signature

bug#30785: Man pages truncated, repeated

[Pierre Neidhardt]

Nope, the patch does not work for me.

I applied the patch in the Guix checkout, 
then

--8<---cut here---start->8---
$ ./pre-inst-env guix build man-db
...
/gnu/store/1w83i51wvap67kl7gdz51ly3pbbx29dv-man-db-2.8.3
$ /gnu/store/1w83i51wvap67kl7gdz51ly3pbbx29dv-man-db-2.8.3/bin/man 
nmcli-examples
--8<---cut here---end--->8---

Still truncated.

-- 
Pierre Neidhardt
https://ambrevar.xyz/


signature.asc
Description: PGP signature

bug#30785: Man pages truncated, repeated

[Ludovic Courtès]

Danny Milosavljevic  skribis:

> Correction, the file "charset.alias" is only installed for groff for glibc < 
> 2.1 .
> The code that reads charset.alias is always created.
> What could possibly go wrong?
>
> Anyway, I suggest the following minimal patch:
>
> diff --git a/gnu/packages/groff.scm b/gnu/packages/groff.scm
> index 77728a5d7..ee2896367 100644
> --- a/gnu/packages/groff.scm
> +++ b/gnu/packages/groff.scm
> @@ -62,6 +62,16 @@
>  `(#:parallel-build? #f   ; parallel build fails
>#:phases
>(modify-phases %standard-phases
> +(add-after 'unpack 'disable-relocatability
> +  (lambda _
> +(substitute* "src/libs/libgroff/Makefile.sub"
> + (("-DENABLE_RELOCATABLE=1") ""))
> +;; That file contains a crash bug--so make sure that
> +;; its contents are not there.
> +(call-with-output-file "src/libs/libgroff/relocate.cpp"
> +  (lambda (port)
> +#t))
> +#t))
>  (add-after 'unpack 'setenv
>(lambda _
>  (setenv "GS_GENERATE_UUIDS" "0")
>
> I filed an upstream bug report:
>
> https://savannah.gnu.org/bugs/index.php?55475

Neat!  Pierre, can you confirm it fixes the issue?

If it does, I agree with Pierre that you should add a comment
referencing this bug and upstream’s, but apart from that, please go
ahead!

Thanks,
Ludo’.

bug#30785: Man pages truncated, repeated

[Pierre Neidhardt]

I suggest your patch explain that crash involves man-db + add a link to this 
discussion.

-- 
Pierre Neidhardt
https://ambrevar.xyz/


signature.asc
Description: PGP signature

bug#30785: Man pages truncated, repeated

[Danny Milosavljevic]

Correction, the file "charset.alias" is only installed for groff for glibc < 
2.1 .
The code that reads charset.alias is always created.
What could possibly go wrong?

Anyway, I suggest the following minimal patch:

diff --git a/gnu/packages/groff.scm b/gnu/packages/groff.scm
index 77728a5d7..ee2896367 100644
--- a/gnu/packages/groff.scm
+++ b/gnu/packages/groff.scm
@@ -62,6 +62,16 @@
 `(#:parallel-build? #f   ; parallel build fails
   #:phases
   (modify-phases %standard-phases
+(add-after 'unpack 'disable-relocatability
+  (lambda _
+(substitute* "src/libs/libgroff/Makefile.sub"
+ (("-DENABLE_RELOCATABLE=1") ""))
+;; That file contains a crash bug--so make sure that
+;; its contents are not there.
+(call-with-output-file "src/libs/libgroff/relocate.cpp"
+  (lambda (port)
+#t))
+#t))
 (add-after 'unpack 'setenv
   (lambda _
 (setenv "GS_GENERATE_UUIDS" "0")

I filed an upstream bug report:

https://savannah.gnu.org/bugs/index.php?55475


pgp7eMd2yFlgL.pgp
Description: OpenPGP digital signature

bug#30785: Man pages truncated, repeated

[Pierre Neidhardt]

Maybe I'm judging a bit too fast, but it seems to me that this part of the code
is rusty to the point that I wonder if getting down to understanding the hows
and the whys will ever reveal useful or enlightening at all.  It's rather safe
to bet that nothing will ever depend on man-db's rendering ;)

I feel that it's just easier to apply one of the 3-4 fixes we've already come up
with :p

My two cents :)

-- 
Pierre Neidhardt
https://ambrevar.xyz/


signature.asc
Description: PGP signature

bug#30785: Man pages truncated, repeated

[Danny Milosavljevic]

Hi Pierre,
Hi Ludo,

On Mon, 14 Jan 2019 21:27:35 +0100
Ludovic Courtès  wrote:

> The ‘set_current_prefix’ logic is extremely fragile; it should readlink
> from /proc/self/exe on GNU/Linux.
> 
> But in our case, no relocation happens, so we can just patch it to do:
> 
>   void set_current_prefix (void)
>   {
> curr_prefix = "/gnu/store/…";
>   }

I agree.

However, in this case the question is why is it picking up all that stuff 
anyway.

The relocation is only used in the case of the groff-local localcharset.c being
used.
It's only a fallback that is used if glibc < 2.1.  I've checked the guix build
logs, the glibc version is newer and also found by the 'configure phase of 
groff.

I can't tell about groff-minimal because the build log as given by
"guix build --log-file groff-minimal"
(https://ci.guix.info/log/mpjn4jfx5i8jxblfj538n22hv56rdnhw-groff-minimal-1.22.3)
has 0 Bytes.

Anyway, a simple way to disable this weird relocation business would be to 
remove:

./src/libs/libgroff/Makefile.sub: -DENABLE_RELOCATABLE=1 \

However, it would be preferrable to find out why 
./src/libs/gnulib/lib/localcharset.c
(the only user of it) is used at all.  It really shouldn't be.


pgpT3noXfq2nj.pgp
Description: OpenPGP digital signature

bug#30785: Man pages truncated, repeated

[Ludovic Courtès]

Danny Milosavljevic  skribis:

> On Mon, 14 Jan 2019 18:48:53 +0100
> Danny Milosavljevic  wrote:
>
>> set_current_prefix() searches for the current program executable file in 
>> $PATH,
>> and it can fail and return 0. 
>> 
>> In that case, relocatep will probably segfault at the location marked below:
>> 
>> >   char *relocated_path = new char[curr_prefix_len + relative_path_len + 1];
>> >   strcpy(relocated_path, curr_prefix);  
>>^^^
>
> This probably happens when argv[0] is not an absolute path, but still the 
> program
> somehow got started, but it is not found in $PATH either.

The ‘set_current_prefix’ logic is extremely fragile; it should readlink
from /proc/self/exe on GNU/Linux.

But in our case, no relocation happens, so we can just patch it to do:

  void set_current_prefix (void)
  {
curr_prefix = "/gnu/store/…";
  }

Ludo’.

bug#30785: Man pages truncated, repeated

[Pierre Neidhardt]

You are right:

CHARSETALIASDIR=/does_not_exist 
/gnu/store/fm7jlkf93bh7bw03m8hbha2b5qxjz0kz-man-db-2.8.3/bin/man nmcli-examples

also works.

-- 
Pierre Neidhardt
https://ambrevar.xyz/


signature.asc
Description: PGP signature

bug#30785: Man pages truncated, repeated

[Danny Milosavljevic]

On Mon, 14 Jan 2019 18:48:53 +0100
Danny Milosavljevic  wrote:

> set_current_prefix() searches for the current program executable file in 
> $PATH,
> and it can fail and return 0. 
> 
> In that case, relocatep will probably segfault at the location marked below:
> 
> >   char *relocated_path = new char[curr_prefix_len + relative_path_len + 1];
> >   strcpy(relocated_path, curr_prefix);  
>^^^

This probably happens when argv[0] is not an absolute path, but still the 
program
somehow got started, but it is not found in $PATH either.


pgpB1W1EhQ4Ho.pgp
Description: OpenPGP digital signature

bug#30785: Man pages truncated, repeated

[Danny Milosavljevic]

set_current_prefix() searches for the current program executable file in $PATH,
and it can fail and return 0. 

In that case, relocatep will probably segfault at the location marked below:

>   char *relocated_path = new char[curr_prefix_len + relative_path_len + 1];
>   strcpy(relocated_path, curr_prefix);
   ^^^


pgpwWZyigP2Is.pgp
Description: OpenPGP digital signature

bug#30785: Man pages truncated, repeated

[Danny Milosavljevic]

Hi Pierre,

On Mon, 14 Jan 2019 17:57:12 +0100
Pierre Neidhardt  wrote:

> > (libs/libgroff/localcharset.c's locale_charset looks overly complicated, 
> > WTF?)  
> 
> Oh dear... relocatep.cpp is not much better, here is the offending function:
> 
> --8<---cut here---start->8---
> // Strip the installation prefix and replace it
> // with the current installation prefix; return the relocated path.
> char *relocatep(const char *path)
> {
> #if DEBUG
>   fprintf(stderr, "relocatep: path = %s\n", path);
>   fprintf(stderr, "relocatep: INSTALLPATH = %s\n", INSTALLPATH);
>   fprintf(stderr, "relocatep: INSTALLPATHLEN = %d\n", INSTALLPATHLEN);
> #endif
>   if (!curr_prefix)
> set_current_prefix();
>   if (strncmp(INSTALLPATH, path, INSTALLPATHLEN))
> return strsave(path);
>   char *relative_path = (char *)path + INSTALLPATHLEN;
>   size_t relative_path_len = strlen(relative_path);
>   char *relocated_path = new char[curr_prefix_len + relative_path_len + 1];
>   strcpy(relocated_path, curr_prefix);
>   strcat(relocated_path, relative_path);
> #if DEBUG
>   fprintf(stderr, "relocated_path: %s\n", relocated_path);
> #endif /* DEBUG */
>   return relocated_path;
> }
> --8<---cut here---end--->8---
> 
> I don't even get the use case of the function...  From locale_charset, it 
> seems
> that the argument is LIBDIR.

It's used inside get_charset_aliases, which reads the file "charset.alias".
We have only one alias in our file "charset.alias" (in GuixSD), so let's try
ignoring it completely for now.

What happens if you don't mess with LC_ALL but set the environment variable
CHARSETALIASDIR to "/does_not_exist" ?

> Can you see a fix then?

If the avove also works, we can just patch get_charset_aliases to always return
the empty string (or a string literal).

The one alias we do have in "charset.alias" is:

ISO_646.IRV:1983 ASCII

Alternatively, we could patch get_charset_aliases so it always takes our
charset.alias from our libiconv (without get_charset_aliases calling
"relocate").

Still it would be interesting to find the Segmentation fault (that's
not supposed to ever happen).


pgpvMPudWKgnI.pgp
Description: OpenPGP digital signature

bug#30785: Man pages truncated, repeated

[Danny Milosavljevic]

Hi Pierre,

locale_charset, implemented in groff, uses nl_langinfo(CODESET)--and just a few
days ago we had a bug report that CP437 doesn't work there.  Maybe it's related.

(libs/libgroff/localcharset.c's locale_charset looks overly complicated, WTF?)

Maybe argv[0] is still the cause, but try setting the environment variable
LC_ALL to "C" to disable locales before invoking "man".  Does this make it not
crash?


pgpeDKNp6rWTi.pgp
Description: OpenPGP digital signature

bug#30785: Man pages truncated, repeated

[Ludovic Courtès]

Pierre Neidhardt  skribis:

>> https://sourceware.org/gdb/onlinedocs/gdb/Forks.html
>> 
>> this might help here.
>
> Thanks Gábor.
> So I tried the following in gdb:
>
>> set follow-for-mode child
>> set detach-on-fork off
>> run emacs
> [inferior 2 completes]
>> inferior 1
> [inferior 3 completes]
>> inferior 1
> [inferior 4 completes]
>> inferior 1
> [inferior 5 hangs]
>
> GDB seems to be stuck on the 5th inferior.  Maybe this is not so reliable.

Maybe it’s just that the inferior is still running, no?

>> Can we trigger the segfault without libpipeline?
>
> "preconv" by itself seems to work perfectly.  It even works in man if it's in
> the PATH.  It only breaks from libpipeline.

What about reproducing the segfault and getting a core dump?  Roughly
you can do:

  ulimit -c unlimited
  man whatever that causes the crash
  gdb /path/to/preconv core

HTH!

Ludo’.

bug#30785: Man pages truncated, repeated

[Gábor Boskovits]

Hello Pierre,

Pierre Neidhardt  ezt írta (időpont: 2019. jan.
14., H, 11:42):
>
> Hmmm... Not sure how to trace that with GDB.
> I'm not sure how to handle child processes.
>

https://sourceware.org/gdb/onlinedocs/gdb/Forks.html

this might help here.

> The offending libpipeline call is "pipeline_pump" in src/man.c:2329 in the
> "display" function.
>
> I'd need more time to investigate this, time which I don't have at the moment,
> sorry :(
>
> --
> Pierre Neidhardt
> https://ambrevar.xyz/

Can we trigger the segfault without libpipeline?

If it is so, then IMO preconv is at fault...

Best regards,
g_bor

bug#30785: Man pages truncated, repeated

[Pierre Neidhardt]

Hmmm... Not sure how to trace that with GDB.
I'm not sure how to handle child processes.

The offending libpipeline call is "pipeline_pump" in src/man.c:2329 in the
"display" function.

I'd need more time to investigate this, time which I don't have at the moment,
sorry :(

-- 
Pierre Neidhardt
https://ambrevar.xyz/


signature.asc
Description: PGP signature

bug#30785: Man pages truncated, repeated

[Ludovic Courtès]

Hi,

Pierre Neidhardt  skribis:

>> It would have the same effect as Marius’ commit
>> 296551a2e9310d4a030ee49530e9367e73aaeecf, wouldn’t it?
>
> I tested and it works! :)
>
>> My understanding, at this point, is that we need to find out why
>> ‘preconv’ segfaults, no?
>
> I'm not completely sure but it seems that libpipeline breaks preconv for one 
> of
> those two reasons:

Did you get a backtrace of ‘preconv’ showing that the segfault is in
libpipeline?

Thanks,
Ludo’.

bug#30785: Man pages truncated, repeated

[Ludovic Courtès]

Pierre Neidhardt  skribis:

> My proposed patch:
>
> (add-after 'install 'wrap-program
>(lambda* (#:key inputs outputs #:allow-other-keys)
> (let* ((out  (assoc-ref outputs "out"))
>(groff (assoc-ref inputs "groff-minimal")))
>   (wrap-program (string-append out "/bin/man")
> `("PATH" ":" prefix (,(string-append groff "/bin")
> #t))

It would have the same effect as Marius’ commit
296551a2e9310d4a030ee49530e9367e73aaeecf, wouldn’t it?

My understanding, at this point, is that we need to find out why
‘preconv’ segfaults, no?

Thanks,
Ludo’.

bug#30785: Man pages truncated, repeated

[Ludovic Courtès]

Pierre Neidhardt  skribis:

> And... from the log I just noticed that Marius had arrived to the same
> conclusion 8 months ago :(
>
> That'll teach me to look at logs before getting started...

Heheh.  :-)

Another useful debugging tip is setting PIPELINE_DEBUG=1.

Anyway I hope we’ll soon squash that bug!

Ludo’.

bug#30785: Man pages truncated, repeated

[Pierre Neidhardt]

The 'man' program from the 'man-db' Guix package has an issue with UTF-8
man pages: the page is truncated and sometimes the truncation is
repeated a couple of times.

The bug does not occur when non-ASCII characters are removed.
I haven't tried with other non-ASCII encodings.

There must be something wrong with the way we package man-db.

See https://lists.gnu.org/archive/html/help-guix/2018-05/msg00084.html
for a discussion.

-- 
Pierre Neidhardt
https://ambrevar.xyz/


signature.asc
Description: PGP signature

bug#30785: Man pages truncated, repeated

[Marius Bakke]

reopen 30785

Marius Bakke  writes:

> Marius Bakke  writes:
>
>> Marius Bakke  writes:
>>
>>> Tobias Geerinckx-Rice  writes:
>>>
 Guix,

 Perhaps he's just getting old, but our man has a tendency to forget 
 where he was, start over from the beginning, and repeat himself several 
 times:

$ guix package -i knot rofi
$ man 5 knot.conf | grep -E '^(NAME|DESCRIPTION)'
NAME
DESCRIPTION
NAME
DESCRIPTION
NAME
DESCRIPTION
NAME
DESCRIPTION
NAME
$
>>>
>>> One thing rofi.1 and knot.conf.5 have in common is that they are both
>>> unicode.  Whereas working pages are ASCII.
>>
>> This ultimately turned out to be due to man-db not finding "preconv".
>>
>> The attached patches fixes it.
>
> I pushed the fix as 296551a2e9310d4a030ee49530e9367e73aaeecf.

This patch caused `man` to segfault when invoked outside `guix
environment`(!) so I reverted it for now.

To be continued...

bug#30785: Man pages truncated, repeated

[Marius Bakke]

Marius Bakke  writes:

> Marius Bakke  writes:
>
>> Tobias Geerinckx-Rice  writes:
>>
>>> Guix,
>>>
>>> Perhaps he's just getting old, but our man has a tendency to forget 
>>> where he was, start over from the beginning, and repeat himself several 
>>> times:
>>>
>>>$ guix package -i knot rofi
>>>$ man 5 knot.conf | grep -E '^(NAME|DESCRIPTION)'
>>>NAME
>>>DESCRIPTION
>>>NAME
>>>DESCRIPTION
>>>NAME
>>>DESCRIPTION
>>>NAME
>>>DESCRIPTION
>>>NAME
>>>$
>>
>> One thing rofi.1 and knot.conf.5 have in common is that they are both
>> unicode.  Whereas working pages are ASCII.
>
> This ultimately turned out to be due to man-db not finding "preconv".
>
> The attached patches fixes it.

I pushed the fix as 296551a2e9310d4a030ee49530e9367e73aaeecf.


signature.asc
Description: PGP signature

bug#30785: Man pages truncated, repeated

[Marius Bakke]

Marius Bakke  writes:

> Tobias Geerinckx-Rice  writes:
>
>> Guix,
>>
>> Perhaps he's just getting old, but our man has a tendency to forget 
>> where he was, start over from the beginning, and repeat himself several 
>> times:
>>
>>$ guix package -i knot rofi
>>$ man 5 knot.conf | grep -E '^(NAME|DESCRIPTION)'
>>NAME
>>DESCRIPTION
>>NAME
>>DESCRIPTION
>>NAME
>>DESCRIPTION
>>NAME
>>DESCRIPTION
>>NAME
>>$
>
> One thing rofi.1 and knot.conf.5 have in common is that they are both
> unicode.  Whereas working pages are ASCII.

This ultimately turned out to be due to man-db not finding "preconv".

The attached patches fixes it.

Adding preconv adds ~0.1 MiB to the groff-minimal closure size.

WDYT?

From 42b43fca11ab1f9417499aea252ae7a871a53b1b Mon Sep 17 00:00:00 2001
From: Marius Bakke 
Date: Sat, 5 May 2018 23:13:16 +0200
Subject: [PATCH 1/2] gnu: groff-minimal: Preserve 'preconv' executable.

* gnu/packages/groff.scm (groff-minimal)[arguments]: Add 'preconv' to the list
of kept programs.
---
 gnu/packages/groff.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnu/packages/groff.scm b/gnu/packages/groff.scm
index 5f1b8d0ed..77728a5d7 100644
--- a/gnu/packages/groff.scm
+++ b/gnu/packages/groff.scm
@@ -101,7 +101,7 @@ is usually the formatter of \"man\" documentation pages.")
;; Keep only the programs that man-db needs at run time,
;; and make sure we don't pull in Perl.
(let ((out  (assoc-ref outputs "out"))
- (kept '("eqn" "neqn" "pic" "tbl" "refer"
+ (kept '("eqn" "neqn" "pic" "tbl" "refer" "preconv"
  "nroff" "groff" "troff" "grotty")))
  (for-each (lambda (file)
  (unless (member (basename file) kept)
-- 
2.17.0


From 095d0db7db77261f59e6582af3a9b18b61dcb630 Mon Sep 17 00:00:00 2001
From: Marius Bakke 
Date: Sat, 5 May 2018 23:30:04 +0200
Subject: [PATCH 2/2] gnu: man-db: Embed absolute reference to 'preconv'.

Fixes .
Reported by Tobias Geerinckx-Rice .

* gnu/packages/man.scm (man-db)[arguments]: Rename 'patch-iconv-path' to
'patch-absolute-paths'.  Add substitution for "preconv".
---
 gnu/packages/man.scm | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/man.scm b/gnu/packages/man.scm
index ca6fd351d..f0bb2b66e 100644
--- a/gnu/packages/man.scm
+++ b/gnu/packages/man.scm
@@ -80,11 +80,19 @@ a flexible and convenient way.")
   (string-append "#!" (which "sh")
  (remove file-is-directory?
  (find-files "src/tests" ".*"))
- (add-after 'unpack 'patch-iconv-path
+ (add-after 'unpack 'patch-absolute-paths
(lambda* (#:key inputs #:allow-other-keys)
  (substitute* "src/man.c"
(("\"iconv\"")
 (string-append "\"" (which "iconv") "\"")))
+ ;; Embed an absolute reference to "preconv", otherwise it
+ ;; falls back to searching in PATH and ultimately fails
+ ;; to render unicode data (see ).
+ (substitute* "lib/encodings.c"
+   (("groff_preconv = NULL")
+(string-append "groff_preconv = \""
+   (assoc-ref inputs "groff-minimal")
+   "/bin/preconv\"")))
  #t)))
#:configure-flags
(let ((groff (assoc-ref %build-inputs "groff"))
-- 
2.17.0



signature.asc
Description: PGP signature

bug#30785: Man pages truncated, repeated

[Vagrant Cascadian]

On 2018-03-12, Tobias Geerinckx-Rice wrote:
> Perhaps he's just getting old, but our man has a tendency to forget 
> where he was, start over from the beginning, and repeat himself several 
> times:
>
>$ guix package -i knot rofi
>$ man 5 knot.conf | grep -E '^(NAME|DESCRIPTION)'
>NAME
>DESCRIPTION
>NAME

I'm seeing this same behavior with the gpg-agent manpage as well, for
what it's worth.

  $ man gpg-agent | grep ^NAME
  :172: warning [p 2, 10.5i]: cannot
  adjust line
  :172: warning [p 6, 4.8i]: cannot
  adjust line
  NAME
  :172: warning [p 9, 10.0i]: cannot
  adjust line
  NAME
  NAME
  NAME

Grepping the file directly:

  $ zgrep NAME $(dirname $(readlink $(which 
gpg)))/../share/man/man1/gpg-agent.1.gz
  .SH NAME
  identified in the environment variable
  \fBLISTEN_FDNAMES\fR (see
  

Interestingly enough, gpg manpage from the same package doesn't seem to
have the issue:

  $ man gpg | grep ^NAME
  NAME
  :1481: warning [p 19, 7.2i]: cannot adjust line

I've also copied the gpg-agent.1.gz file to a Debian system running the
same upstream version of man 2.8.3, and that has no issue. So it's
something specific to the guix version of the "man" command.


live well,
  vagrant


signature.asc
Description: PGP signature

bug#30785: Man pages truncated, repeated

[Marius Bakke]

Tobias Geerinckx-Rice  writes:

> Guix,
>
> Perhaps he's just getting old, but our man has a tendency to forget 
> where he was, start over from the beginning, and repeat himself several 
> times:
>
>$ guix package -i knot rofi
>$ man 5 knot.conf | grep -E '^(NAME|DESCRIPTION)'
>NAME
>DESCRIPTION
>NAME
>DESCRIPTION
>NAME
>DESCRIPTION
>NAME
>DESCRIPTION
>NAME
>$

One thing rofi.1 and knot.conf.5 have in common is that they are both
unicode.  Whereas working pages are ASCII.

I've tried running groff and nroff manually on rofi.1, but haven't been
able to reproduce the duplicate sections.


signature.asc
Description: PGP signature

bug#30785: Man pages truncated, repeated

[Andreas Enge]

Hello,

On Mon, Mar 12, 2018 at 10:24:46PM +0100, Tobias Geerinckx-Rice wrote:
>   $ man 5 knot.conf | grep -E '^(NAME|DESCRIPTION)'
>   NAME
>   DESCRIPTION
>   NAME
>   DESCRIPTION
>   NAME
>   DESCRIPTION
>   NAME
>   DESCRIPTION
>   NAME
>   $
> 
> There's also some stderr...
>   :25: error: end of file while defining macro `UNINDENT'

with Guix on Debian (git commit 464f5447396fcec9b43f7eab71d5d42b522a157f),
I am not seeing this problem:
$ man 5 knot.conf | grep -E '^(NAME|DESCRIPTION)'
NAME
DESCRIPTION

and no stderr. This uses the "man" binary from Debian (/usr/bin/man) and
/gnu/store/a9w7mm0mrgrakl9khmi91ybp34ly4iqc-knot-2.6.5/share/man/man5/knot.conf.5.gz

What is your distribution? Could it be that the problem is in the "man" binary 
and
not in the man page?

Actually, also this does work:
$ /gnu/store/kh2av28w5rd3fbidqssp0fzv661hb9l5-man-db-2.7.6.1/bin/man 5 
knot.conf | grep -E '^(NAME|DESCRIPTION)'
NAME
DESCRIPTION

Maybe your store is corrupted?

Or did I misunderstand the problem?

Andreas

bug#30785: Man pages truncated, repeated

[Ludovic Courtès]

Hi,

Tobias Geerinckx-Rice  skribis:

> On 2018-03-13 22:34, l...@gnu.org wrote:
>>> However, even longer man pages such as bash(1) render without fail, so
>>> there might be something special about the two examples above that
>>> triggers this behaviour.
>>
>> I suspect something wrong with ‘knot.conf.5.gz’, but I don’t have
>> tangible evidence.
>
> Yup, that's about as far as I got before giving up and submitting to
> the wisdom of the crowd. We need someone who knows something —
> anything — about man pages, or someone who can reproduce this on
> another distro. I had no luck searching for similar bug reports.
>
> ...or do you mean with the knot.conf page *specifically*, as opposed
> to the rofi one? Is your suspicion based on something you saw in
> there?

I didn’t check the rofi one.  The knot.conf one is generated by some
“docutils”, a reStructuredText thing, which is where my unfounded
suspicion comes from.  :-)

We’d need to analyze the nroff “code” in there to understand what’s
going on.

> For the record: apparently this doesn't happen on Debian, according to
> some fellow on IRC named ‘civodul’. There goes my brief hope that this
> was an (exclusively) upstream problem after all.

I think I know that person.  ;-)  Perhaps we could check how Debian
generates the page.  Do they have a patch for “docutils”?  Should we try
with a different version to see if that helps?

Cheers,
Ludo’.

bug#30785: Man pages truncated, repeated

[Tobias Geerinckx-Rice]

Ludo',

On 2018-03-13 22:34, l...@gnu.org wrote:

However, even longer man pages such as bash(1) render without fail, so
there might be something special about the two examples above that
triggers this behaviour.


I suspect something wrong with ‘knot.conf.5.gz’, but I don’t have
tangible evidence.


Yup, that's about as far as I got before giving up and submitting to the 
wisdom of the crowd. We need someone who knows something — anything — 
about man pages, or someone who can reproduce this on another distro. I 
had no luck searching for similar bug reports.


...or do you mean with the knot.conf page *specifically*, as opposed to 
the rofi one? Is your suspicion based on something you saw in there?


It's not the .gz part: opening the uncompressed page with man directly 
has the same result.


--

For the record: apparently this doesn't happen on Debian, according to 
some fellow on IRC named ‘civodul’. There goes my brief hope that this 
was an (exclusively) upstream problem after all.


Kind regards,

T G-R

Sent from a Web browser. Excuse or enjoy my brevity.

bug#30785: Man pages truncated, repeated

[Tobias Geerinckx-Rice]

Guix,

Perhaps he's just getting old, but our man has a tendency to forget 
where he was, start over from the beginning, and repeat himself several 
times:


  $ guix package -i knot rofi
  $ man 5 knot.conf | grep -E '^(NAME|DESCRIPTION)'
  NAME
  DESCRIPTION
  NAME
  DESCRIPTION
  NAME
  DESCRIPTION
  NAME
  DESCRIPTION
  NAME
  $

There's also some stderr...

  :25: error: end of file while defining macro 
`UNINDENT'


...but I think that's just a symptom of the input being cut short; man 
rofi(1) prints no such error yet suffers the same fate.


The affected man pages themselves are not truncated, nor do they repeat:

  $ zgrep '\.SH' `man -w knot.conf`
  .SH NAME
  .SH DESCRIPTION
  .SH COMMENTS
  .SH INCLUDES
  .SH MODULE SECTION
  .SH SERVER SECTION
  .SH KEY SECTION
  .SH ACL SECTION
  .SH CONTROL SECTION
  .SH STATISTICS SECTION
  .SH KEYSTORE SECTION
  .SH SUBMISSION SECTION
  .SH POLICY SECTION
  .SH REMOTE SECTION
  .SH TEMPLATE SECTION
  .SH ZONE SECTION
  .SH LOGGING SECTION
  .SH AUTHOR
  .SH COPYRIGHT
  $

However, even longer man pages such as bash(1) render without fail, so 
there might be something special about the two examples above that 
triggers this behaviour.


Kind regards,

T G-R

Sent from a Web browser. Excuse or enjoy my brevity.