[bug#48443,1/2] gnu: Add texlive-libkpathsea.

* gnu/packages/tex.scm (texlive-libkpathsea): New variable.
---
 gnu/packages/tex.scm | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

Hello Leo,

I have very little experience creating Guix packages, but I spent
some time working on the TeX Live ones recently so hopefully my comments 
and suggestions below are helpful.

I like adding a separate package for libkpathsea, and it’s what other 
distros such as Debian and Ubuntu do. Thank you for implementing it. I just 
have a few comments below:

Em sábado, 15 de maio de 2021, às 11:42:29 -03, Leo Prikler escreveu:
> * gnu/packages/tex.scm (texlive-libkpathsea): New variable.
> ---
>  gnu/packages/tex.scm | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/gnu/packages/tex.scm b/gnu/packages/tex.scm
> index b9eeb0e792..3e8384eaad 100644
> --- a/gnu/packages/tex.scm
> +++ b/gnu/packages/tex.scm
> @@ -457,6 +457,38 @@ This package contains the binaries.")
>     (license (license:fsf-free
> "https://www.tug.org/texlive/copying.html")) (home-page
> "https://www.tug.org/texlive/")))
> 
> +(define-public texlive-libkpathsea
> +  (package/inherit texlive-bin

According to a recent message from Ludo¹, ‘package/inherit’ is meant to be 
used in specific situations, and IIUC it doesn’t apply here:

> It should also be (package (inherit …) …) rather than (package/inherit
> …).  The latter is only useful when defining variants of a package (same
> version, same code) where the same security updates would apply.

I also wonder whether inheriting from texlive-bin is the best option. One 
disadvantage is that it makes this package too sensitive to changes in 
texlive-bin. As an example, it doesn’t work anymore with the version in 
core-updates because in the branch, the ‘postint’ phase has been renamed to 
‘post-install’. Also, I assume many texlive-bin inputs aren’t needed for 
texlive-kpathsea, causing unnecessary work when building
texlive-libkpathsea and packages depending on it such as evince.

In addition, if it were a separate package then texlive-bin could be made 
to use it, rather than shipping its own copy.

> +    (name "texlive-libkpathsea")
> +    (source
> +     (origin
> +       (inherit (package-source texlive-bin))

Perhaps a ‘texlive-source-src’ variable analogous to ‘texlive-extra-src’ 
and ‘texlive-texmf-src’ would be useful?

> +       (snippet
> +        `(begin
> +           ,(origin-snippet (package-source texlive-bin))
> +           (with-directory-excursion "texk"
> +             (let ((preserved-directories '("." ".." "kpathsea")))
> +               (for-each
> +                delete-file-recursively
> +                (scandir "."
> +                         (lambda (file)
> +                           (and (not (member file
> preserved-directories)) +                                (eq? 'directory
> (stat:type (stat file))))))))))))) +    (arguments
> +     (substitute-keyword-arguments (package-arguments texlive-bin)
> +       ((#:configure-flags flags)
> +        `(cons* "--disable-all-pkgs" "--enable-kpathsea"
> +                "--enable-shared" ,flags))
> +       ((#:phases phases)
> +        `(modify-phases ,phases
> +           (delete 'configure-ghostscript-executable)
> +           (delete 'use-code-for-new-poppler)
> +           (delete 'patch-dvisvgm-build-files)
> +           (delete 'disable-failing-test)
> +           (replace 'postint
> +             (lambda* (#:key inputs outputs #:allow-other-keys)
> +               (with-directory-excursion "texk/kpathsea"
> +                 (invoke "make" "install"))))))))))

If you decide to continue inheriting from texlive-bin, you’d also need to 
change the synopsis and description.

Hello Thiago,

Am Montag, den 12.07.2021, 21:32 -0300 schrieb Thiago Jung Bauermann:
> [...]
> > +(define-public texlive-libkpathsea
> > +  (package/inherit texlive-bin
> 
> According to a recent message from Ludo¹, ‘package/inherit’ is meant
> to be 
> used in specific situations, and IIUC it doesn’t apply here:
> 
> > It should also be (package (inherit …) …) rather than
> > (package/inherit …).  The latter is only useful when defining
> > variants of a package (same version, same code) where the same
> > security updates would apply.
I'm a little confused here, as that is exactly the rationale I'm
applying.  When texlive-bin gets grafted due to kpathsea, the graft
also applies to texlive-libkpathsea.  Granted, there is a large room
for false positives, that would result in gratuitous grafts for
texlive-libkpathsea, but I prefer erring on the side of security rather
than graftlessness here.

> I also wonder whether inheriting from texlive-bin is the best option.
> One disadvantage is that it makes this package too sensitive to
> changes in texlive-bin. As an example, it doesn’t work anymore with
> the version in core-updates because in the branch, the ‘postint’
> phase has been renamed to ‘post-install’. Also, I assume many
> texlive-bin inputs aren’t needed for texlive-kpathsea, causing
> unnecessary work when building texlive-libkpathsea and packages
> depending on it such as evince.
The postinst thing was my mistake – instead of inheriting from
%standard-phases as I should, I naïvely inherited texlive-bin's phases
instead.  It turns out, I actually don't need any of those (and if I
did they'd be trivially copyable).

On the part of inputs, sure, we could make libkpathsea smaller, but I
have little experience with TeX Live and its build system, so I decided
not to change its inputs for now.  If you have suggestions on how a
better closure could be achieved, please do bring them forth.

> In addition, if it were a separate package then texlive-bin could be
> made to use it, rather than shipping its own copy.

Perhaps that's an idea worth entertaining, but given the TeX Live build
system I fear it's not an overwhelmingly practical one.

> > +    (name "texlive-libkpathsea")
> > +    (source
> > +     (origin
> > +       (inherit (package-source texlive-bin))
> 
> Perhaps a ‘texlive-source-src’ variable analogous to ‘texlive-extra-
> src’ and ‘texlive-texmf-src’ would be useful?

I'm… not too sure on this one.  What would texlive-source-src capture? 
Just the upstream source?  Then we'd have to carefully apply all the
fitting patches.  The same as (package-source texlive-bin)?  What's the
point then?

> > +       (snippet
> > +        `(begin
> > +           ,(origin-snippet (package-source texlive-bin))
> > +           (with-directory-excursion "texk"
> > +             (let ((preserved-directories '("." ".." "kpathsea")))
> > +               (for-each
> > +                delete-file-recursively
> > +                (scandir "."
> > +                         (lambda (file)
> > +                           (and (not (member file
> > preserved-directories)) +                                (eq?
> > 'directory
> > (stat:type (stat file))))))))))))) +    (arguments
> > +     (substitute-keyword-arguments (package-arguments texlive-bin)
> > +       ((#:configure-flags flags)
> > +        `(cons* "--disable-all-pkgs" "--enable-kpathsea"
> > +                "--enable-shared" ,flags))
> > +       ((#:phases phases)
> > +        `(modify-phases ,phases
> > +           (delete 'configure-ghostscript-executable)
> > +           (delete 'use-code-for-new-poppler)
> > +           (delete 'patch-dvisvgm-build-files)
> > +           (delete 'disable-failing-test)
> > +           (replace 'postint
> > +             (lambda* (#:key inputs outputs #:allow-other-keys)
> > +               (with-directory-excursion "texk/kpathsea"
> > +                 (invoke "make" "install"))))))))))
> 
> If you decide to continue inheriting from texlive-bin, you’d also
> need to change the synopsis and description.
Fair enough, that's on me.  I've sent a v2 applying some of your
suggestions.  Please feel free to point out anything I've missed or you
noticed in addition to what's already discussed.

Regards,
Leo

Hi Leo,

Thank you for your response and your new version of the patches.

Em terça-feira, 13 de julho de 2021, às 04:58:22 -03, Leo Prikler escreveu:
> Hello Thiago,
> 
> Am Montag, den 12.07.2021, 21:32 -0300 schrieb Thiago Jung Bauermann:
> > [...]
> > 
> > > +(define-public texlive-libkpathsea
> > > +  (package/inherit texlive-bin
> > 
> > According to a recent message from Ludo¹, ‘package/inherit’ is meant
> > to be
> > 
> > used in specific situations, and IIUC it doesn’t apply here:
> > > It should also be (package (inherit …) …) rather than
> > > (package/inherit …).  The latter is only useful when defining
> > > variants of a package (same version, same code) where the same
> > > security updates would apply.
> 
> I'm a little confused here, as that is exactly the rationale I'm
> applying.  When texlive-bin gets grafted due to kpathsea, the graft
> also applies to texlive-libkpathsea.  Granted, there is a large room
> for false positives, that would result in gratuitous grafts for
> texlive-libkpathsea, but I prefer erring on the side of security rather
> than graftlessness here.

My reasoning was that libkpathsea is just a small part of texlive-bin, so 
in principle a minority of texlive-bin security updates would apply to it. 
But you are right, there may well be some which would apply.

> > I also wonder whether inheriting from texlive-bin is the best option.
> > One disadvantage is that it makes this package too sensitive to
> > changes in texlive-bin. As an example, it doesn’t work anymore with
> > the version in core-updates because in the branch, the ‘postint’
> > phase has been renamed to ‘post-install’. Also, I assume many
> > texlive-bin inputs aren’t needed for texlive-kpathsea, causing
> > unnecessary work when building texlive-libkpathsea and packages
> > depending on it such as evince.
> 
> The postinst thing was my mistake – instead of inheriting from
> %standard-phases as I should, I naïvely inherited texlive-bin's phases
> instead.  It turns out, I actually don't need any of those (and if I
> did they'd be trivially copyable).

That is nice solution.

> On the part of inputs, sure, we could make libkpathsea smaller, but I
> have little experience with TeX Live and its build system, so I decided
> not to change its inputs for now.  If you have suggestions on how a
> better closure could be achieved, please do bring them forth.

I was able to build the package with an empty input list. I compared a 
texlive-libkpathsea built with your unchanged patches and one with the 
empty input list and they are identical, except for the hash of the
/gnu/store directory. Even the binary files, which I compared using 
hexdump. So my suggestion is to use an empty input list. :-)

> > In addition, if it were a separate package then texlive-bin could be
> > made to use it, rather than shipping its own copy.
> 
> Perhaps that's an idea worth entertaining, but given the TeX Live build
> system I fear it's not an overwhelmingly practical one.

I can look into that separately, after your patches go in.

> > > +    (name "texlive-libkpathsea")
> > > +    (source
> > > +     (origin
> > > +       (inherit (package-source texlive-bin))
> > 
> > Perhaps a ‘texlive-source-src’ variable analogous to ‘texlive-extra-
> > src’ and ‘texlive-texmf-src’ would be useful?
> 
> I'm… not too sure on this one.  What would texlive-source-src capture?
> Just the upstream source?  Then we'd have to carefully apply all the
> fitting patches.  The same as (package-source texlive-bin)?  What's the
> point then?

Yes, the point would be just to not duplicate the origin information. There 
would indeed be more work sorting out which security updates apply.

> > > +       (snippet
> > > +        `(begin
> > > +           ,(origin-snippet (package-source texlive-bin))
> > > +           (with-directory-excursion "texk"
> > > +             (let ((preserved-directories '("." ".." "kpathsea")))
> > > +               (for-each
> > > +                delete-file-recursively
> > > +                (scandir "."
> > > +                         (lambda (file)
> > > +                           (and (not (member file
> > > preserved-directories)) +                                (eq?
> > > 'directory
> > > (stat:type (stat file))))))))))))) +    (arguments
> > > +     (substitute-keyword-arguments (package-arguments texlive-bin)
> > > +       ((#:configure-flags flags)
> > > +        `(cons* "--disable-all-pkgs" "--enable-kpathsea"
> > > +                "--enable-shared" ,flags))
> > > +       ((#:phases phases)
> > > +        `(modify-phases ,phases
> > > +           (delete 'configure-ghostscript-executable)
> > > +           (delete 'use-code-for-new-poppler)
> > > +           (delete 'patch-dvisvgm-build-files)
> > > +           (delete 'disable-failing-test)
> > > +           (replace 'postint
> > > +             (lambda* (#:key inputs outputs #:allow-other-keys)
> > > +               (with-directory-excursion "texk/kpathsea"
> > > +                 (invoke "make" "install"))))))))))
> > 
> > If you decide to continue inheriting from texlive-bin, you’d also
> > need to change the synopsis and description.
> 
> Fair enough, that's on me.  I've sent a v2 applying some of your
> suggestions.  Please feel free to point out anything I've missed or you
> noticed in addition to what's already discussed.

Thanks!

Hi Thiago,

Am Dienstag, den 13.07.2021, 22:48 -0300 schrieb Thiago Jung Bauermann:
> [...]
> 
> > On the part of inputs, sure, we could make libkpathsea smaller, but
> > I
> > have little experience with TeX Live and its build system, so I
> > decided
> > not to change its inputs for now.  If you have suggestions on how a
> > better closure could be achieved, please do bring them forth.
> 
> I was able to build the package with an empty input list. I compared
> a texlive-libkpathsea built with your unchanged patches and one with
> the empty input list and they are identical, except for the hash of
> the /gnu/store directory. Even the binary files, which I compared
> using hexdump. So my suggestion is to use an empty input list. :-)
Thanks for checking, v3 now uses an empty input list.

> > > In addition, if it were a separate package then texlive-bin could
> > > be
> > > made to use it, rather than shipping its own copy.
> > 
> > Perhaps that's an idea worth entertaining, but given the TeX Live
> > build
> > system I fear it's not an overwhelmingly practical one.
> 
> I can look into that separately, after your patches go in.
Fair enough.

> > > > +    (name "texlive-libkpathsea")
> > > > +    (source
> > > > +     (origin
> > > > +       (inherit (package-source texlive-bin))
> > > 
> > > Perhaps a ‘texlive-source-src’ variable analogous to ‘texlive-
> > > extra-
> > > src’ and ‘texlive-texmf-src’ would be useful?
> > 
> > I'm… not too sure on this one.  What would texlive-source-src
> > capture?
> > Just the upstream source?  Then we'd have to carefully apply all
> > the
> > fitting patches.  The same as (package-source texlive-bin)?  What's 
> > the
> > point then?
> 
> Yes, the point would be just to not duplicate the origin information.
> There would indeed be more work sorting out which security updates
> apply.
I'm not really convince that would help us.  texlive-libkpathsea still
needs to inherit from the origin so as to strip away all the others
sources.  So would every other part of texlive if built on its own. 
Perhaps one could instead do computed origins, but that increases
complexity rather than reducing it.  Therefore I'm not convinced
extracting this origin into its own variable is beneficial.

Regards,
Leo

Hi Leo,

Em quarta-feira, 14 de julho de 2021, às 05:55:55 -03, Leo Prikler 
escreveu:
> Am Dienstag, den 13.07.2021, 22:48 -0300 schrieb Thiago Jung Bauermann:
> > [...]
> > 
> > > On the part of inputs, sure, we could make libkpathsea smaller, but
> > > I
> > > have little experience with TeX Live and its build system, so I
> > > decided
> > > not to change its inputs for now.  If you have suggestions on how a
> > > better closure could be achieved, please do bring them forth.
> > 
> > I was able to build the package with an empty input list. I compared
> > a texlive-libkpathsea built with your unchanged patches and one with
> > the empty input list and they are identical, except for the hash of
> > the /gnu/store directory. Even the binary files, which I compared
> > using hexdump. So my suggestion is to use an empty input list. :-)
> 
> Thanks for checking, v3 now uses an empty input list.

Thanks! v3 looks great to me.
 
> > > > > +    (name "texlive-libkpathsea")
> > > > > +    (source
> > > > > +     (origin
> > > > > +       (inherit (package-source texlive-bin))
> > > > 
> > > > Perhaps a ‘texlive-source-src’ variable analogous to ‘texlive-
> > > > extra-
> > > > src’ and ‘texlive-texmf-src’ would be useful?
> > > 
> > > I'm… not too sure on this one.  What would texlive-source-src
> > > capture?
> > > Just the upstream source?  Then we'd have to carefully apply all
> > > the
> > > fitting patches.  The same as (package-source texlive-bin)?  What's
> > > the
> > > point then?
> > 
> > Yes, the point would be just to not duplicate the origin information.
> > There would indeed be more work sorting out which security updates
> > apply.
> 
> I'm not really convince that would help us.  texlive-libkpathsea still
> needs to inherit from the origin so as to strip away all the others
> sources.  So would every other part of texlive if built on its own.
> Perhaps one could instead do computed origins, but that increases
> complexity rather than reducing it.  Therefore I'm not convinced
> extracting this origin into its own variable is beneficial.

Yes, that is true. In my previous message I was agreeing with you that the 
origin idea didn’t bring much benefit. Sorry for not being clear.

Am Mittwoch, den 14.07.2021, 13:23 -0300 schrieb Thiago Jung Bauermann:
> [...]
> Yes, that is true. In my previous message I was agreeing with you
> that the origin idea didn’t bring much benefit. Sorry for not being
> clear.
No problem.  I've pushed this now, thanks for your review!