* [bug#65745] [PATCH] gnu: yelp: Provide dependencies to display "man:" URIs.
@ 2023-09-04 20:58 Ludovic Courtès
2023-09-05 3:43 ` Maxim Cournoyer
0 siblings, 1 reply; 9+ messages in thread
From: Ludovic Courtès @ 2023-09-04 20:58 UTC (permalink / raw)
To: 65745
Cc: Ludovic Courtès, Luis Felipe, Liliana Marie Prikler,
Maxim Cournoyer, Raghav Gururajan
Previously a command like the following one would fail to display the
expected man page:
guix shell yelp coreutils man-db -C \
--expose=/tmp/.X11-unix --expose=/run/user \
--expose=/var/run/dbus --expose=/etc/machine-id \
-E ^DISPLAY -- yelp man:ls
* gnu/packages/gnome.scm (yelp)[arguments]: New field.
[inputs]: Add 'man-db' and its "groff-minimal" input.
Reported-by: Luis Felipe <sirgazil@zoho.com>
---
gnu/packages/gnome.scm | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
Hi!
Initially I wanted to fix support for URLs like “info:coreutils”,
which Luis also reported on the Fediverse as being broken. However,
after much debugging I got lost in a maze of callbacks and threads
and opaque objects and eventually gave up. Someone™ should take
another look.
For the record, this is how I went on my debugging journey:
guix shell yelp coreutils man-db info-reader \
--with-debug-info=yelp gdb --pure -E ^DISPLAY -- \
gdb --args /bin/sh yelp info:coreutils
Ludo’.
diff --git a/gnu/packages/gnome.scm b/gnu/packages/gnome.scm
index 3b80692e90..894d77d7ba 100644
--- a/gnu/packages/gnome.scm
+++ b/gnu/packages/gnome.scm
@@ -7336,6 +7336,31 @@ (define-public yelp
(base32
"0h9vf4fx056imjf8ibmn03wg1c3hniipy1nsm2jqi62lp1m19c95"))))
(build-system glib-or-gtk-build-system)
+ (arguments
+ (list #:phases
+ #~(modify-phases %standard-phases
+ (add-after 'unpack 'set-man-file-name
+ (lambda* (#:key inputs outputs #:allow-other-keys)
+ ;; Create a wrapper for 'man' that knows where to find
+ ;; 'groff' and 'gunzip'.
+ (define libexecdir
+ (string-append #$output "/libexec/yelp"))
+ (define wrapper
+ (string-append libexecdir "/man"))
+
+ (mkdir-p libexecdir)
+ (symlink (search-input-file inputs "bin/man") wrapper)
+ (wrap-program wrapper
+ `("PATH" ":" =
+ ,(map (lambda (program)
+ (dirname (search-input-file inputs program)))
+ '("bin/groff" "bin/gunzip"))))
+
+ ;; Have Yelp use that 'man' wrapper instead of searching
+ ;; for 'man' in $PATH.
+ (substitute* "libyelp/yelp-man-parser.c"
+ (("\"man\"")
+ (string-append "\"" wrapper "\""))))))))
(native-inputs
(list `(,glib "bin") ; for glib-genmarshal, etc.
intltool
@@ -7347,6 +7372,8 @@ (define-public yelp
(list gsettings-desktop-schemas
libhandy
libxslt
+ man-db ;for URIs like "man:ls"
+ (lookup-package-input man-db "groff-minimal") ;ditto
sqlite
webkitgtk
yelp-xsl))
base-commit: 1af35bbb25b122cafddfd2f69b644d46c4beff85
--
2.41.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [bug#65745] [PATCH] gnu: yelp: Provide dependencies to display "man:" URIs.
2023-09-04 20:58 [bug#65745] [PATCH] gnu: yelp: Provide dependencies to display "man:" URIs Ludovic Courtès
@ 2023-09-05 3:43 ` Maxim Cournoyer
2023-09-09 16:45 ` Ludovic Courtès
2023-09-11 9:08 ` [bug#65745] [PATCH v2] " Ludovic Courtès
0 siblings, 2 replies; 9+ messages in thread
From: Maxim Cournoyer @ 2023-09-05 3:43 UTC (permalink / raw)
To: Ludovic Courtès
Cc: Raghav Gururajan, Luis Felipe, 65745, Liliana Marie Prikler
Hello,
Ludovic Courtès <ludo@gnu.org> writes:
[...]
> diff --git a/gnu/packages/gnome.scm b/gnu/packages/gnome.scm
> index 3b80692e90..894d77d7ba 100644
> --- a/gnu/packages/gnome.scm
> +++ b/gnu/packages/gnome.scm
> @@ -7336,6 +7336,31 @@ (define-public yelp
> (base32
> "0h9vf4fx056imjf8ibmn03wg1c3hniipy1nsm2jqi62lp1m19c95"))))
> (build-system glib-or-gtk-build-system)
> + (arguments
> + (list #:phases
> + #~(modify-phases %standard-phases
> + (add-after 'unpack 'set-man-file-name
> + (lambda* (#:key inputs outputs #:allow-other-keys)
> + ;; Create a wrapper for 'man' that knows where to find
> + ;; 'groff' and 'gunzip'.
> + (define libexecdir
> + (string-append #$output "/libexec/yelp"))
> + (define wrapper
> + (string-append libexecdir "/man"))
> +
> + (mkdir-p libexecdir)
> + (symlink (search-input-file inputs "bin/man") wrapper)
> + (wrap-program wrapper
> + `("PATH" ":" =
> + ,(map (lambda (program)
> + (dirname (search-input-file inputs program)))
> + '("bin/groff" "bin/gunzip"))))
> +
> + ;; Have Yelp use that 'man' wrapper instead of searching
> + ;; for 'man' in $PATH.
> + (substitute* "libyelp/yelp-man-parser.c"
> + (("\"man\"")
> + (string-append "\"" wrapper "\""))))))))
Couldn't we more simply wrap the yelp command with the 'man', 'groff' and
'gunzip' command locations added to PATH?
--
Thanks,
Maxim
^ permalink raw reply [flat|nested] 9+ messages in thread* [bug#65745] [PATCH] gnu: yelp: Provide dependencies to display "man:" URIs.
2023-09-05 3:43 ` Maxim Cournoyer
@ 2023-09-09 16:45 ` Ludovic Courtès
2023-09-09 16:56 ` Liliana Marie Prikler
2023-09-11 9:08 ` [bug#65745] [PATCH v2] " Ludovic Courtès
1 sibling, 1 reply; 9+ messages in thread
From: Ludovic Courtès @ 2023-09-09 16:45 UTC (permalink / raw)
To: Maxim Cournoyer
Cc: Raghav Gururajan, Luis Felipe, 65745, Liliana Marie Prikler
Hi,
Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>> + ;; Create a wrapper for 'man' that knows where to find
>> + ;; 'groff' and 'gunzip'.
>> + (define libexecdir
>> + (string-append #$output "/libexec/yelp"))
>> + (define wrapper
>> + (string-append libexecdir "/man"))
>> +
>> + (mkdir-p libexecdir)
>> + (symlink (search-input-file inputs "bin/man") wrapper)
>> + (wrap-program wrapper
>> + `("PATH" ":" =
>> + ,(map (lambda (program)
>> + (dirname (search-input-file inputs program)))
>> + '("bin/groff" "bin/gunzip"))))
>> +
>> + ;; Have Yelp use that 'man' wrapper instead of searching
>> + ;; for 'man' in $PATH.
>> + (substitute* "libyelp/yelp-man-parser.c"
>> + (("\"man\"")
>> + (string-append "\"" wrapper "\""))))))))
>
> Couldn't we more simply wrap the yelp command with the 'man', 'groff' and
> 'gunzip' command locations added to PATH?
Yeah it’s convoluted; the idea was that the wrapper for ‘man’ is
something that should be fixed in ‘man-db’ proper but that we cannot fix
in ‘master’ because ‘man-db’ has too many dependents.
Maybe I should start with it though…
Ludo’.
^ permalink raw reply [flat|nested] 9+ messages in thread* [bug#65745] [PATCH] gnu: yelp: Provide dependencies to display "man:" URIs.
2023-09-09 16:45 ` Ludovic Courtès
@ 2023-09-09 16:56 ` Liliana Marie Prikler
2023-09-09 22:06 ` Maxim Cournoyer
0 siblings, 1 reply; 9+ messages in thread
From: Liliana Marie Prikler @ 2023-09-09 16:56 UTC (permalink / raw)
To: Ludovic Courtès, Maxim Cournoyer
Cc: Raghav Gururajan, Luis Felipe, 65745
Am Samstag, dem 09.09.2023 um 18:45 +0200 schrieb Ludovic Courtès:
> Hi,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
> > > + ;; Create a wrapper for 'man' that knows
> > > where to find
> > > + ;; 'groff' and 'gunzip'.
> > > + (define libexecdir
> > > + (string-append #$output "/libexec/yelp"))
> > > + (define wrapper
> > > + (string-append libexecdir "/man"))
> > > +
> > > + (mkdir-p libexecdir)
> > > + (symlink (search-input-file inputs "bin/man")
> > > wrapper)
> > > + (wrap-program wrapper
> > > + `("PATH" ":" =
> > > + ,(map (lambda (program)
> > > + (dirname (search-input-file
> > > inputs program)))
> > > + '("bin/groff" "bin/gunzip"))))
> > > +
> > > + ;; Have Yelp use that 'man' wrapper instead
> > > of searching
> > > + ;; for 'man' in $PATH.
> > > + (substitute* "libyelp/yelp-man-parser.c"
> > > + (("\"man\"")
> > > + (string-append "\"" wrapper "\""))))))))
> >
> > Couldn't we more simply wrap the yelp command with the 'man',
> > 'groff' and
> > 'gunzip' command locations added to PATH?
>
> Yeah it’s convoluted; the idea was that the wrapper for ‘man’ is
> something that should be fixed in ‘man-db’ proper but that we cannot
> fix
> in ‘master’ because ‘man-db’ has too many dependents.
>
> Maybe I should start with it though…
Can we graft our man-db to get the same effect?
^ permalink raw reply [flat|nested] 9+ messages in thread* [bug#65745] [PATCH] gnu: yelp: Provide dependencies to display "man:" URIs.
2023-09-09 16:56 ` Liliana Marie Prikler
@ 2023-09-09 22:06 ` Maxim Cournoyer
0 siblings, 0 replies; 9+ messages in thread
From: Maxim Cournoyer @ 2023-09-09 22:06 UTC (permalink / raw)
To: Liliana Marie Prikler
Cc: Raghav Gururajan, Ludovic Courtès, Luis Felipe, 65745
Hi,
Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
> Am Samstag, dem 09.09.2023 um 18:45 +0200 schrieb Ludovic Courtès:
>> Hi,
>>
>> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>>
>> > > + ;; Create a wrapper for 'man' that knows
>> > > where to find
>> > > + ;; 'groff' and 'gunzip'.
>> > > + (define libexecdir
>> > > + (string-append #$output "/libexec/yelp"))
>> > > + (define wrapper
>> > > + (string-append libexecdir "/man"))
>> > > +
>> > > + (mkdir-p libexecdir)
>> > > + (symlink (search-input-file inputs "bin/man")
>> > > wrapper)
>> > > + (wrap-program wrapper
>> > > + `("PATH" ":" =
>> > > + ,(map (lambda (program)
>> > > + (dirname (search-input-file
>> > > inputs program)))
>> > > + '("bin/groff" "bin/gunzip"))))
>> > > +
>> > > + ;; Have Yelp use that 'man' wrapper instead
>> > > of searching
>> > > + ;; for 'man' in $PATH.
>> > > + (substitute* "libyelp/yelp-man-parser.c"
>> > > + (("\"man\"")
>> > > + (string-append "\"" wrapper "\""))))))))
>> >
>> > Couldn't we more simply wrap the yelp command with the 'man',
>> > 'groff' and
>> > 'gunzip' command locations added to PATH?
>>
>> Yeah it’s convoluted; the idea was that the wrapper for ‘man’ is
>> something that should be fixed in ‘man-db’ proper but that we cannot
>> fix
>> in ‘master’ because ‘man-db’ has too many dependents.
>>
>> Maybe I should start with it though…
> Can we graft our man-db to get the same effect?
We also have a core-updates branch; it could go there and we could merge
it with the docbook stuff that's going to go there.
I'd prefer this than convoluted solutions I don't understand :-).
--
Thanks,
Maxim
^ permalink raw reply [flat|nested] 9+ messages in thread
* [bug#65745] [PATCH v2] gnu: yelp: Provide dependencies to display "man:" URIs.
2023-09-05 3:43 ` Maxim Cournoyer
2023-09-09 16:45 ` Ludovic Courtès
@ 2023-09-11 9:08 ` Ludovic Courtès
2023-09-11 14:13 ` Maxim Cournoyer
1 sibling, 1 reply; 9+ messages in thread
From: Ludovic Courtès @ 2023-09-11 9:08 UTC (permalink / raw)
To: 65745
Cc: Ludovic Courtès, Luis Felipe, Liliana Marie Prikler,
Maxim Cournoyer, Raghav Gururajan
Previously a command like the following one would fail to display the
expected man page:
guix shell yelp coreutils -C \
--expose=/tmp/.X11-unix --expose=/run/user \
--expose=/var/run/dbus --expose=/etc/machine-id -E ^DISPLAY -- \
/bin/sh -c \
'export MANPATH=$GUIX_ENVIRONMENT/share/man; echo $MANPATH; yelp man:ls'
* gnu/packages/gnome.scm (yelp)[arguments]: New field.
[inputs]: Add ‘man-db’ and ‘groff-minimal’.
Reported-by: Luis Felipe <sirgazil@zoho.com>
---
gnu/packages/gnome.scm | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
Hi!
Here's an improved and more reasonable version.
There's still an issue we should fix in 'man-db' but I ran out of time before
I could find the remaining place where 'man' searches for 'groff' in $PATH.
Ludo'.
diff --git a/gnu/packages/gnome.scm b/gnu/packages/gnome.scm
index 3b80692e90..36fe1a80a1 100644
--- a/gnu/packages/gnome.scm
+++ b/gnu/packages/gnome.scm
@@ -142,6 +142,7 @@ (define-module (gnu packages gnome)
#:use-module (gnu packages geo)
#:use-module (gnu packages gperf)
#:use-module (gnu packages graphviz)
+ #:use-module (gnu packages groff)
#:use-module (gnu packages gstreamer)
#:use-module (gnu packages gtk)
#:use-module (gnu packages guile)
@@ -7336,6 +7337,28 @@ (define-public yelp
(base32
"0h9vf4fx056imjf8ibmn03wg1c3hniipy1nsm2jqi62lp1m19c95"))))
(build-system glib-or-gtk-build-system)
+ (arguments
+ (list #:phases
+ #~(modify-phases %standard-phases
+ (add-after 'unpack 'set-man-file-name
+ (lambda* (#:key inputs #:allow-other-keys)
+ ;; Invoke 'man' directly instead of searching $PATH.
+ (substitute* '("libyelp/yelp-man-parser.c"
+ "libyelp/yelp-uri.c")
+ (("\"man\"")
+ (string-append "\""
+ (search-input-file inputs "bin/man")
+ "\""))
+ (("G_SPAWN_SEARCH_PATH")
+ "0"))))
+ (add-after 'install 'help-man-find-its-dependencies
+ (lambda* (#:key inputs outputs #:allow-other-keys)
+ ;; XXX: Currently 'man' looks for 'groff' in $PATH in one
+ ;; case. This should be fixed in 'man-db' proper.
+ (wrap-program (string-append (assoc-ref outputs "out")
+ "/bin/yelp")
+ `("PATH" ":" prefix
+ (,(dirname (search-input-file inputs "bin/groff"))))))))))
(native-inputs
(list `(,glib "bin") ; for glib-genmarshal, etc.
intltool
@@ -7347,6 +7370,8 @@ (define-public yelp
(list gsettings-desktop-schemas
libhandy
libxslt
+ man-db ;for URIs like "man:ls"
+ groff-minimal ;ditto
sqlite
webkitgtk
yelp-xsl))
base-commit: a4c35c607cfd7d6b0bad90cfcc46188d489e1754
--
2.41.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [bug#65745] [PATCH v2] gnu: yelp: Provide dependencies to display "man:" URIs.
2023-09-11 9:08 ` [bug#65745] [PATCH v2] " Ludovic Courtès
@ 2023-09-11 14:13 ` Maxim Cournoyer
2023-09-11 17:16 ` Liliana Marie Prikler
0 siblings, 1 reply; 9+ messages in thread
From: Maxim Cournoyer @ 2023-09-11 14:13 UTC (permalink / raw)
To: Ludovic Courtès
Cc: Raghav Gururajan, Luis Felipe, 65745, Liliana Marie Prikler
Hi Ludo,
Ludovic Courtès <ludo@gnu.org> writes:
[...]
> Here's an improved and more reasonable version.
>
> There's still an issue we should fix in 'man-db' but I ran out of time before
> I could find the remaining place where 'man' searches for 'groff' in $PATH.
> diff --git a/gnu/packages/gnome.scm b/gnu/packages/gnome.scm
> index 3b80692e90..36fe1a80a1 100644
> --- a/gnu/packages/gnome.scm
> +++ b/gnu/packages/gnome.scm
> @@ -142,6 +142,7 @@ (define-module (gnu packages gnome)
> #:use-module (gnu packages geo)
> #:use-module (gnu packages gperf)
> #:use-module (gnu packages graphviz)
> + #:use-module (gnu packages groff)
> #:use-module (gnu packages gstreamer)
> #:use-module (gnu packages gtk)
> #:use-module (gnu packages guile)
> @@ -7336,6 +7337,28 @@ (define-public yelp
> (base32
> "0h9vf4fx056imjf8ibmn03wg1c3hniipy1nsm2jqi62lp1m19c95"))))
> (build-system glib-or-gtk-build-system)
> + (arguments
> + (list #:phases
> + #~(modify-phases %standard-phases
> + (add-after 'unpack 'set-man-file-name
> + (lambda* (#:key inputs #:allow-other-keys)
> + ;; Invoke 'man' directly instead of searching $PATH.
> + (substitute* '("libyelp/yelp-man-parser.c"
> + "libyelp/yelp-uri.c")
> + (("\"man\"")
> + (string-append "\""
> + (search-input-file inputs "bin/man")
> + "\""))
> + (("G_SPAWN_SEARCH_PATH")
> + "0"))))
> + (add-after 'install 'help-man-find-its-dependencies
> + (lambda* (#:key inputs outputs #:allow-other-keys)
> + ;; XXX: Currently 'man' looks for 'groff' in $PATH in one
> + ;; case. This should be fixed in 'man-db' proper.
> + (wrap-program (string-append (assoc-ref outputs "out")
> + "/bin/yelp")
> + `("PATH" ":" prefix
> + (,(dirname (search-input-file inputs "bin/groff"))))))))))
> (native-inputs
> (list `(,glib "bin") ; for glib-genmarshal, etc.
> intltool
> @@ -7347,6 +7370,8 @@ (define-public yelp
> (list gsettings-desktop-schemas
> libhandy
> libxslt
> + man-db ;for URIs like "man:ls"
> + groff-minimal ;ditto
> sqlite
> webkitgtk
> yelp-xsl))
This v2 LGTM! Thanks for the fix.
--
Thanks,
Maxim
^ permalink raw reply [flat|nested] 9+ messages in thread* [bug#65745] [PATCH v2] gnu: yelp: Provide dependencies to display "man:" URIs.
2023-09-11 14:13 ` Maxim Cournoyer
@ 2023-09-11 17:16 ` Liliana Marie Prikler
2023-09-11 21:56 ` bug#65745: [PATCH] " Ludovic Courtès
0 siblings, 1 reply; 9+ messages in thread
From: Liliana Marie Prikler @ 2023-09-11 17:16 UTC (permalink / raw)
To: Maxim Cournoyer, Ludovic Courtès
Cc: Raghav Gururajan, Luis Felipe, 65745
Am Montag, dem 11.09.2023 um 10:13 -0400 schrieb Maxim Cournoyer:
> Hi Ludo,
>
> Ludovic Courtès <ludo@gnu.org> writes:
>
> [...]
>
> > Here's an improved and more reasonable version.
> >
> > There's still an issue we should fix in 'man-db' but I ran out of
> > time before
> > I could find the remaining place where 'man' searches for 'groff'
> > in $PATH.
>
>
> > diff --git a/gnu/packages/gnome.scm b/gnu/packages/gnome.scm
> > index 3b80692e90..36fe1a80a1 100644
> > --- a/gnu/packages/gnome.scm
> > +++ b/gnu/packages/gnome.scm
> > @@ -142,6 +142,7 @@ (define-module (gnu packages gnome)
> > #:use-module (gnu packages geo)
> > #:use-module (gnu packages gperf)
> > #:use-module (gnu packages graphviz)
> > + #:use-module (gnu packages groff)
> > #:use-module (gnu packages gstreamer)
> > #:use-module (gnu packages gtk)
> > #:use-module (gnu packages guile)
> > @@ -7336,6 +7337,28 @@ (define-public yelp
> > (base32
> >
> > "0h9vf4fx056imjf8ibmn03wg1c3hniipy1nsm2jqi62lp1m19c95"))))
> > (build-system glib-or-gtk-build-system)
> > + (arguments
> > + (list #:phases
> > + #~(modify-phases %standard-phases
> > + (add-after 'unpack 'set-man-file-name
> > + (lambda* (#:key inputs #:allow-other-keys)
> > + ;; Invoke 'man' directly instead of searching
> > $PATH.
> > + (substitute* '("libyelp/yelp-man-parser.c"
> > + "libyelp/yelp-uri.c")
> > + (("\"man\"")
> > + (string-append "\""
> > + (search-input-file inputs
> > "bin/man")
> > + "\""))
> > + (("G_SPAWN_SEARCH_PATH")
> > + "0"))))
> > + (add-after 'install 'help-man-find-its-dependencies
> > + (lambda* (#:key inputs outputs #:allow-other-
> > keys)
> > + ;; XXX: Currently 'man' looks for 'groff' in
> > $PATH in one
> > + ;; case. This should be fixed in 'man-db'
> > proper.
> > + (wrap-program (string-append (assoc-ref outputs
> > "out")
> > + "/bin/yelp")
> > + `("PATH" ":" prefix
> > + (,(dirname (search-input-file inputs
> > "bin/groff"))))))))))
> > (native-inputs
> > (list `(,glib "bin") ; for glib-genmarshal, etc.
> > intltool
> > @@ -7347,6 +7370,8 @@ (define-public yelp
> > (list gsettings-desktop-schemas
> > libhandy
> > libxslt
> > + man-db ;for URIs like
> > "man:ls"
> > + groff-minimal ;ditto
> > sqlite
> > webkitgtk
> > yelp-xsl))
>
> This v2 LGTM! Thanks for the fix.
Also LGTM
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-09-11 21:57 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-04 20:58 [bug#65745] [PATCH] gnu: yelp: Provide dependencies to display "man:" URIs Ludovic Courtès
2023-09-05 3:43 ` Maxim Cournoyer
2023-09-09 16:45 ` Ludovic Courtès
2023-09-09 16:56 ` Liliana Marie Prikler
2023-09-09 22:06 ` Maxim Cournoyer
2023-09-11 9:08 ` [bug#65745] [PATCH v2] " Ludovic Courtès
2023-09-11 14:13 ` Maxim Cournoyer
2023-09-11 17:16 ` Liliana Marie Prikler
2023-09-11 21:56 ` bug#65745: [PATCH] " Ludovic Courtès
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).