package-vc-install fails to build docs

package-vc-install fails to build docs

[Joseph Turner]

Hello!

With emacs -Q, package-vc-install fails to install the manual for
org-transclusion as described here:

https://github.com/nobiot/org-transclusion/issues/182

I attempted to fix the issue by renaming the manual file from
docs/org-transclusion-manual.org -> docs/org-transclusion.org
but the problem persists.

Can anyone else reproduce this issue?

Best,

Joseph Turner

Re: package-vc-install fails to build docs

[Ruijie Yu via Users list for the GNU Emacs text editor]

Joseph Turner <joseph@breatheoutbreathe.in> writes:

> Hello!
>
> With emacs -Q, package-vc-install fails to install the manual for
> org-transclusion as described here:
>
> https://github.com/nobiot/org-transclusion/issues/182
>
> I attempted to fix the issue by renaming the manual file from
> docs/org-transclusion-manual.org -> docs/org-transclusion.org
> but the problem persists.
>
> Can anyone else reproduce this issue?

I'm looking at the package-vc source code, and it doesn't seem to try to
autodetect any doc files at the moment.  Try adding `:doc` in your
`package-vc-install' call and see if anything changes.

I'm on recent master, what is your Emacs version?

In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.37, cairo version 1.17.8) of 2023-03-25 built on ruijie
Repository revision: db7e95531ac36ae842787b6c5f2859d0642c78cc
Repository branch: makepkg
System Description: Arch Linux

-- 
Best,


RY

[Please note that this mail might go to spam due to some
misconfiguration in my mail server -- still investigating.]

Re: package-vc-install fails to build docs

[Philip Kaludercic]

Ruijie Yu via Users list for the GNU Emacs text editor
<help-gnu-emacs@gnu.org> writes:

> Joseph Turner <joseph@breatheoutbreathe.in> writes:
>
>> Hello!
>>
>> With emacs -Q, package-vc-install fails to install the manual for
>> org-transclusion as described here:
>>
>> https://github.com/nobiot/org-transclusion/issues/182
>>
>> I attempted to fix the issue by renaming the manual file from
>> docs/org-transclusion-manual.org -> docs/org-transclusion.org
>> but the problem persists.
>>
>> Can anyone else reproduce this issue?
>
> I'm looking at the package-vc source code, and it doesn't seem to try to
> autodetect any doc files at the moment.  Try adding `:doc` in your
> `package-vc-install' call and see if anything changes.

Right, if you just pass a URL, then package-vc has no metadata.  But
since org-transclusion is on ELPA, you can invoke it like this:

  (package-vc-install 'org-transclusion)

then it should re-use the package specification that GNU ELPA provides.
If you invoke package-vc-install interactively, it would also prompt you
with this package name.

That being said, I just tried it out and it seems that it still doesn't
work because of a discrepancy between the package specification on ELPA
that mentions docs/org-transclusion.org to be the documentation file

 (org-transclusion	:url "https://github.com/nobiot/org-transclusion"
  :make "org-transclusion.org"
  :doc "docs/org-transclusion.org"
  :readme "README.org")

and as you say the fact that the actual manual lies in
docs/org-transclusion-manual.org.  But this is an issue that should
either be fixed in ELPA if the maintainer wishes this file to be used,
or upstream if this is a mistake.

> I'm on recent master, what is your Emacs version?
>
> In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
>  3.24.37, cairo version 1.17.8) of 2023-03-25 built on ruijie
> Repository revision: db7e95531ac36ae842787b6c5f2859d0642c78cc
> Repository branch: makepkg
> System Description: Arch Linux

-- 
Philip Kaludercic

Re: package-vc-install fails to build docs

[Joseph Turner]

Thank you Ruijie Yu and Philip Kaludercic for your help!

Philip Kaludercic <philipk@posteo.net> writes:

> Ruijie Yu via Users list for the GNU Emacs text editor
> <help-gnu-emacs@gnu.org> writes:
>> I'm looking at the package-vc source code, and it doesn't seem to try to
>> autodetect any doc files at the moment.  Try adding `:doc` in your
>> `package-vc-install' call and see if anything changes.

Actually, it looks like extra plist elements like :doc are stripped away
in package-vc--unpack. package-vc--unpack doesn't pass its pkg-spec arg
along to package-vc--unpack-1. package-vc--unpack-1 then calls
package-vc--desc->spec, which gets the spec anew from ELPA IIUC.

Regardless, I believe the issue lies elsewhere, see below.

> Right, if you just pass a URL, then package-vc has no metadata.  But
> since org-transclusion is on ELPA, you can invoke it like this:
>
>   (package-vc-install 'org-transclusion)
>
> then it should re-use the package specification that GNU ELPA provides.
> If you invoke package-vc-install interactively, it would also prompt you
> with this package name.
>
> That being said, I just tried it out and it seems that it still doesn't
> work because of a discrepancy between the package specification on ELPA
> that mentions docs/org-transclusion.org to be the documentation file
>
>  (org-transclusion	:url "https://github.com/nobiot/org-transclusion"
>   :make "org-transclusion.org"
>   :doc "docs/org-transclusion.org"
>   :readme "README.org")
>
> and as you say the fact that the actual manual lies in
> docs/org-transclusion-manual.org.  But this is an issue that should
> either be fixed in ELPA if the maintainer wishes this file to be used,
> or upstream if this is a mistake.

nobiot (org-transclusion author) points out here
https://github.com/nobiot/org-transclusion/issues/182#issuecomment-1518676316
that docs/org-transclusion.org needs to be generated from
docs/org-transclusion-manual.org according to

:make "org-transclusion.org"

(search for ":make" in
https://git.savannah.gnu.org/cgit/emacs/elpa.git/plain/README )

It appears that package-vc hasn't been taught to make use of the :make
package spec key. A quick search inside of
https://git.savannah.gnu.org/cgit/emacs/elpa.git/tree/elpa-packages#n491
reveals only two other packages besides org-transclusion that use it.

>> I'm on recent master, what is your Emacs version?

GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.30,
cairo version 1.16.0)

In conclusion, I think the issue is that package-vc doesn't yet support
the package spec :make key.

Cheers,

Joseph

Re: package-vc-install fails to build docs

[Philip Kaludercic]

Joseph Turner <joseph@breatheoutbreathe.in> writes:

> Thank you Ruijie Yu and Philip Kaludercic for your help!
>
> Philip Kaludercic <philipk@posteo.net> writes:
>
>> Ruijie Yu via Users list for the GNU Emacs text editor
>> <help-gnu-emacs@gnu.org> writes:
>>> I'm looking at the package-vc source code, and it doesn't seem to try to
>>> autodetect any doc files at the moment.  Try adding `:doc` in your
>>> `package-vc-install' call and see if anything changes.
>
> Actually, it looks like extra plist elements like :doc are stripped away
> in package-vc--unpack. package-vc--unpack doesn't pass its pkg-spec arg
> along to package-vc--unpack-1. package-vc--unpack-1 then calls
> package-vc--desc->spec, which gets the spec anew from ELPA IIUC.
>
> Regardless, I believe the issue lies elsewhere, see below.
>
>> Right, if you just pass a URL, then package-vc has no metadata.  But
>> since org-transclusion is on ELPA, you can invoke it like this:
>>
>>   (package-vc-install 'org-transclusion)
>>
>> then it should re-use the package specification that GNU ELPA provides.
>> If you invoke package-vc-install interactively, it would also prompt you
>> with this package name.
>>
>> That being said, I just tried it out and it seems that it still doesn't
>> work because of a discrepancy between the package specification on ELPA
>> that mentions docs/org-transclusion.org to be the documentation file
>>
>>  (org-transclusion	:url "https://github.com/nobiot/org-transclusion"
>>   :make "org-transclusion.org"
>>   :doc "docs/org-transclusion.org"
>>   :readme "README.org")
>>
>> and as you say the fact that the actual manual lies in
>> docs/org-transclusion-manual.org.  But this is an issue that should
>> either be fixed in ELPA if the maintainer wishes this file to be used,
>> or upstream if this is a mistake.
>
> nobiot (org-transclusion author) points out here
> https://github.com/nobiot/org-transclusion/issues/182#issuecomment-1518676316
> that docs/org-transclusion.org needs to be generated from
> docs/org-transclusion-manual.org according to
>
> :make "org-transclusion.org"
>
> (search for ":make" in
> https://git.savannah.gnu.org/cgit/emacs/elpa.git/plain/README )
>
> It appears that package-vc hasn't been taught to make use of the :make
> package spec key. A quick search inside of
> https://git.savannah.gnu.org/cgit/emacs/elpa.git/tree/elpa-packages#n491
> reveals only two other packages besides org-transclusion that use it.

You are right (and I totally missed the :make entry in the specification
above).  Package-vc did intentionally NOT add :make or :shell-command
support, for the sake of user safety.  It wouldn't be difficult to
implement this, if we could find a way to make it reasonably safe
(e.g. if the user has to opt-in or we trust package specifications if
they were delivered by a trusted package archive).

>>> I'm on recent master, what is your Emacs version?
>
> GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.30,
> cairo version 1.16.0)
>
> In conclusion, I think the issue is that package-vc doesn't yet support
> the package spec :make key.
>
> Cheers,
>
> Joseph

Re: package-vc-install fails to build docs

[Joseph Turner]

What would be the first steps to support :make in package-vc?

Philip Kaludercic <philipk@posteo.net> writes:

> Joseph Turner <joseph@breatheoutbreathe.in> writes:
>
>> Thank you Ruijie Yu and Philip Kaludercic for your help!
>>
>> Philip Kaludercic <philipk@posteo.net> writes:
>>
>>> Ruijie Yu via Users list for the GNU Emacs text editor
>>> <help-gnu-emacs@gnu.org> writes:
>>>> I'm looking at the package-vc source code, and it doesn't seem to try to
>>>> autodetect any doc files at the moment.  Try adding `:doc` in your
>>>> `package-vc-install' call and see if anything changes.
>>
>> Actually, it looks like extra plist elements like :doc are stripped away
>> in package-vc--unpack. package-vc--unpack doesn't pass its pkg-spec arg
>> along to package-vc--unpack-1. package-vc--unpack-1 then calls
>> package-vc--desc->spec, which gets the spec anew from ELPA IIUC.
>>
>> Regardless, I believe the issue lies elsewhere, see below.

How shall we pass :doc and :make values from package-vc-install down to
package-vc--build-documentation?

Do we want to refactor package-vc-unpack-1 so that it accepts a pkg-spec
argument? Or maybe add :doc an :make keys to the pkg-desc plist?

> You are right (and I totally missed the :make entry in the specification
> above).  Package-vc did intentionally NOT add :make or :shell-command
> support, for the sake of user safety.  It wouldn't be difficult to
> implement this, if we could find a way to make it reasonably safe
> (e.g. if the user has to opt-in or we trust package specifications if
> they were delivered by a trusted package archive).

How about yes-or-no-p by default, with an customization option to
automatically trust specs from a trusted package archive?

Re: package-vc-install fails to build docs

[Philip Kaludercic]

Joseph Turner <joseph@breatheoutbreathe.in> writes:

> What would be the first steps to support :make in package-vc?

If you are interested in working on this, I would take a look at
`package-vc--unpack-1' which would probably be the best place for that.

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> Joseph Turner <joseph@breatheoutbreathe.in> writes:
>>
>>> Thank you Ruijie Yu and Philip Kaludercic for your help!
>>>
>>> Philip Kaludercic <philipk@posteo.net> writes:
>>>
>>>> Ruijie Yu via Users list for the GNU Emacs text editor
>>>> <help-gnu-emacs@gnu.org> writes:
>>>>> I'm looking at the package-vc source code, and it doesn't seem to try to
>>>>> autodetect any doc files at the moment.  Try adding `:doc` in your
>>>>> `package-vc-install' call and see if anything changes.
>>>
>>> Actually, it looks like extra plist elements like :doc are stripped away
>>> in package-vc--unpack. package-vc--unpack doesn't pass its pkg-spec arg
>>> along to package-vc--unpack-1. package-vc--unpack-1 then calls
>>> package-vc--desc->spec, which gets the spec anew from ELPA IIUC.
>>>
>>> Regardless, I believe the issue lies elsewhere, see below.
>
> How shall we pass :doc and :make values from package-vc-install down to
> package-vc--build-documentation?
>
> Do we want to refactor package-vc-unpack-1 so that it accepts a pkg-spec
> argument? Or maybe add :doc an :make keys to the pkg-desc plist?

:doc is passed via an argument, and :make would have to be handled
before.

>> You are right (and I totally missed the :make entry in the specification
>> above).  Package-vc did intentionally NOT add :make or :shell-command
>> support, for the sake of user safety.  It wouldn't be difficult to
>> implement this, if we could find a way to make it reasonably safe
>> (e.g. if the user has to opt-in or we trust package specifications if
>> they were delivered by a trusted package archive).
>
> How about yes-or-no-p by default, with an customization option to
> automatically trust specs from a trusted package archive?

The latter should work fine, but the first would only be possible in
interactive usage.

-- 
Philip Kaludercic

Re: package-vc-install fails to build docs

[Joseph Turner]

Philip Kaludercic <philipk@posteo.net> writes:
> Joseph Turner <joseph@breatheoutbreathe.in> writes:
>
>> What would be the first steps to support :make in package-vc?
>
> If you are interested in working on this, I would take a look at
> `package-vc--unpack-1' which would probably be the best place for that.

Thank you. Yes, I am interested in working on this.

>> How shall we pass :doc and :make values from package-vc-install down to
>> package-vc--build-documentation?
>>
>> Do we want to refactor package-vc-unpack-1 so that it accepts a pkg-spec
>> argument? Or maybe add :doc an :make keys to the pkg-desc plist?
>
> :doc is passed via an argument, and :make would have to be handled
> before.

Okay!

>> How about yes-or-no-p by default, with an customization option to
>> automatically trust specs from a trusted package archive?
>
> The latter should work fine, but the first would only be possible in
> interactive usage.

Got it.

I'd like to look at the ELPA code for handling :make. I tried grepping
for ":make" in the Emacs and ELPA repos, but to no avail.

Mind pointing me in the right direction?

Best,

Joseph

Re: package-vc-install fails to build docs

[Philip Kaludercic]

Joseph Turner <joseph@breatheoutbreathe.in> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>> Joseph Turner <joseph@breatheoutbreathe.in> writes:
>>
>>> What would be the first steps to support :make in package-vc?
>>
>> If you are interested in working on this, I would take a look at
>> `package-vc--unpack-1' which would probably be the best place for that.
>
> Thank you. Yes, I am interested in working on this.

Great!

>>> How shall we pass :doc and :make values from package-vc-install down to
>>> package-vc--build-documentation?
>>>
>>> Do we want to refactor package-vc-unpack-1 so that it accepts a pkg-spec
>>> argument? Or maybe add :doc an :make keys to the pkg-desc plist?
>>
>> :doc is passed via an argument, and :make would have to be handled
>> before.
>
> Okay!
>
>>> How about yes-or-no-p by default, with an customization option to
>>> automatically trust specs from a trusted package archive?
>>
>> The latter should work fine, but the first would only be possible in
>> interactive usage.
>
> Got it.
>
> I'd like to look at the ELPA code for handling :make. I tried grepping
> for ":make" in the Emacs and ELPA repos, but to no avail.

You have to look at elpa-admin.el, the "reference interpretation" for
how package specifications are supposed to work.  It is located on a
separate branch in elpa.git (so that it can be shared between elpa.git
and nongnu.git).

    https://git.savannah.gnu.org/cgit/emacs/elpa.git/tree/elpa-admin.el?h=elpa-admin

> Mind pointing me in the right direction?

Ping me if you need anything else, and submit a bug report with a patch
as soon as you have something workable.  Add a

   X-Debbugs-CC: Philip Kaludercic <philipk@posteo.net>

header so that I get notified as well.

> Best,
>
> Joseph

Re: package-vc-install fails to build docs

[Joseph Turner]

Philip Kaludercic <philipk@posteo.net> writes:

> Joseph Turner <joseph@breatheoutbreathe.in> writes:
>> Philip Kaludercic <philipk@posteo.net> writes:
>>> Joseph Turner <joseph@breatheoutbreathe.in> writes:
> You have to look at elpa-admin.el, the "reference interpretation" for
> how package specifications are supposed to work.  It is located on a
> separate branch in elpa.git (so that it can be shared between elpa.git
> and nongnu.git).
>
>     https://git.savannah.gnu.org/cgit/emacs/elpa.git/tree/elpa-admin.el?h=elpa-admin
>
>> Mind pointing me in the right direction?
>
> Ping me if you need anything else, and submit a bug report with a patch
> as soon as you have something workable.  Add a
>
>    X-Debbugs-CC: Philip Kaludercic <philipk@posteo.net>
>
> header so that I get notified as well.

In elpa-admin.el, :make and :shell-command are handled by calling bwrap
to sandbox the external processes. Should we do the same thing?

Do you think we should also sandbox makeinfo like elpaa does?
That could be a separate patch.

Best,

Joseph

Re: package-vc-install fails to build docs

[Philip Kaludercic]

Joseph Turner <joseph@breatheoutbreathe.in> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> Joseph Turner <joseph@breatheoutbreathe.in> writes:
>>> Philip Kaludercic <philipk@posteo.net> writes:
>>>> Joseph Turner <joseph@breatheoutbreathe.in> writes:
>> You have to look at elpa-admin.el, the "reference interpretation" for
>> how package specifications are supposed to work.  It is located on a
>> separate branch in elpa.git (so that it can be shared between elpa.git
>> and nongnu.git).
>>
>>     https://git.savannah.gnu.org/cgit/emacs/elpa.git/tree/elpa-admin.el?h=elpa-admin
>>
>>> Mind pointing me in the right direction?
>>
>> Ping me if you need anything else, and submit a bug report with a patch
>> as soon as you have something workable.  Add a
>>
>>    X-Debbugs-CC: Philip Kaludercic <philipk@posteo.net>
>>
>> header so that I get notified as well.
>
> In elpa-admin.el, :make and :shell-command are handled by calling bwrap
> to sandbox the external processes. Should we do the same thing?

The issue is that bwrap is not available on all systems, and even if it
is it is not necessarily installed.  I would take the approach of
implementing this as an opt-in system first, then improving the security
perhaps even by writing a general process isolation library that could
be added to Emacs 30, using what systems are available or what the user
prefers (Guix and Nix could be useful here, there was a discussion on
this topic in the past for elpa-admin.el).

> Do you think we should also sandbox makeinfo like elpaa does?
> That could be a separate patch.
>
> Best,
>
> Joseph