GNU bug report logs - #58367
`package-install-file' rejects some .tar files (tentative patch)

Previous Next

To reply to this bug, email your comments to 58367 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Richard Hopkins <emacs <at> unbit.co.uk>
To: bug-gnu-emacs <at> gnu.org
Subject: `package-install-file' rejects some .tar files (tentative patch)
Date: Fri, 07 Oct 2022 22:15:56 +0100

[Message part 1 (text/plain, inline)]

`package-install-file' signals an error for some .tar packages based
on their structure - both file layout and tar format.

This is specific to the package routines for .tar files as Emacs can
understand them fine, and can also install the packages if they have
already been extracted.

There's 2 different issues that cause this:

1) An old style .tar file (autoconf tar-v7, `tar cof ...`) vs a new
style file (autoconf tar-ustar, `tar cf ...`)

2) A package where all files are in a single directory vs having
child directories.  A package with child directories currently only
installs successfully if it's already extracted; the manual also says
they are allowed.

Combining these issues the current state of package installation using
.tar files is:

* ustar-nosub - error
* ustar-withsub - error
* v7-nosub - success
* v7-withsub - error

The main issue is how the package functions try to calculate the root
of the package directory and from there it can locate the "-pkg.el"
file etc.  From this we get two different errors:

1) Cannot handle a plain directory being first and missing a path
separator.  e.g. This happens for ustar files with or without a sub
directory.

$ tar tf ustar-nosub-0.1.tar
ustar-nosub-0.1
ustar-nosub-0.1/ustar-nosub.el
ustar-nosub-0.1/ustar-nosub-pkg.el

M-: (package-install-file "ustar-nosub-0.1.tar")
Debugger entered--Lisp error: (wrong-type-argument stringp nil)
  directory-file-name(nil)
  package--description-file(nil)
  package-tar-file-info()
  package-install-from-buffer()
  package-install-file("~/tmp/pkgtest/tars/ustar-nosub-0.1.tar")
  funcall-interactively(package-install-file 
"~/tmp/pkgtest/tars/ustar-nosub-0.1.tar")
  call-interactively(package-install-file record nil)
  command-execute(package-install-file record)
  execute-extended-command(nil "package-install-file" 
"package-install-file")
  funcall-interactively(execute-extended-command nil 
"package-install-file" "package-install-file")
  call-interactively(execute-extended-command nil nil)
  command-execute(execute-extended-command)

2) Looks for "-pkg.el" in a sub directory when it won't be there.
This happens for a v7 file but may also happen for a ustar if it
wasn't for error (1).

$ tar tf v7-withsub-0.1.tar
v7-withsub-0.1/v7-sub/foo.txt
v7-withsub-0.1/v7-withsub.el
v7-withsub-0.1/v7-withsub-pkg.el

M-: (package-install-file "tar tf v7-withsub-0.1.tar")
Debugger entered--Lisp error: (wrong-type-argument arrayp nil)
  tar--describe-as-link(nil)
  tar--check-descriptor(nil)
  tar-get-file-descriptor("v7-withsub-0.1/v7-sub/v7-sub-pkg.el")
  package-tar-file-info()
  package-install-from-buffer()
  package-install-file("~/tmp/pkgtest/tars/v7-withsub-0.1.tar")
  funcall-interactively(package-install-file 
"~/tmp/pkgtest/tars/v7-withsub-0.1.tar")
  call-interactively(package-install-file record nil)
  command-execute(package-install-file record)
  execute-extended-command(nil "package-install-file" 
"package-install-file")
  funcall-interactively(execute-extended-command nil 
"package-install-file" "package-install-file")
  call-interactively(execute-extended-command nil nil)
  command-execute(execute-extended-command)

For comparison the only .tar file that works is an old style v7 file
with all files in a single directory

$ tar tf v7-nosub-0.1.tar
v7-nosub-0.1/v7-nosub.el
v7-nosub-0.1/v7-nosub-pkg.el

M-: (package-install-file "v7-nosub-0.1.tar")
success

I've attached the example files to help you investigate.

There is also a proof of concept patch as it fixes ustar files with or
without sub directories, but it still fails on v7-withsub making the
new summary:

* ustar-nosub - success
* ustar-withsub - success
* v7-nosub - success
* v7-withsub - error

The modified functions were `package-untar-buffer' and
`package-tar-file-info'.

p.s. These results are from OpenBSD 7.1 with their `tar` and
Emacs 28.2 and master.

A related issue #13136 - 
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=13136

[0001-tar-package-install-file.patch (text/x-diff, attachment)]

[tar-package-install-file.tar.gz (application/gzip, attachment)]

Message #10 received at 58367 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Richard Hopkins <emacs <at> unbit.co.uk>
Cc: 58367 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#58367: `package-install-file' rejects some .tar files
 (tentative patch)
Date: Sat, 08 Oct 2022 15:28:44 +0200

Richard Hopkins <emacs <at> unbit.co.uk> writes:

> `package-install-file' signals an error for some .tar packages based
> on their structure - both file layout and tar format.
>
> This is specific to the package routines for .tar files as Emacs can
> understand them fine, and can also install the packages if they have
> already been extracted.

Perhaps Stefan has some comments here; added to the CCs.

Message #13 received at 58367 <at> debbugs.gnu.org (full text, mbox):

From: Richard Hopkins <emacs <at> unbit.co.uk>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 58367 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#58367: `package-install-file' rejects some .tar files
 (tentative patch)
Date: Sat, 08 Oct 2022 14:55:35 +0100

I've found another difference regarding `package-install-file' and a
package with subdirectories that I can reproduce with the previously
attached files.  This is the difference between installing a directory
or installing a .tar file.  For example

M-: (package-install-file "ustar-withsub-0.1")
;; * installs but ignores subdirectory

M-: (package-install-file "ustar-withsub-0.1.tar")
;; * 28.2 - error
;; * with patch - installs and includes subdirectory

I believe sub directories should be allowed from this part of the docs
and the patch now allows for it as well as fixing the error.

    "...Files may also extract into subdirectories of the content
    directory. ..."
    
https://www.gnu.org/software/emacs/manual/html_node/elisp/Multi_002dfile-Packages.html

It would be to good to see if anyone can reproduce these as it looks
like more patches are needed to support sub directories?

1) for v7-withsub-0.1.tar so it looks at the top level for "-pkg.el"

2) include subdirectories when installing from a directory to match
functionality of installing from .tar.

Message #16 received at 58367 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Richard Hopkins <emacs <at> unbit.co.uk>
Cc: 58367 <at> debbugs.gnu.org
Subject: Re: bug#58367: `package-install-file' rejects some .tar files
 (tentative patch)
Date: Sat, 08 Oct 2022 12:25:33 -0400

> 2) A package where all files are in a single directory vs having
> child directories.  A package with child directories currently only
> installs successfully if it's already extracted; the manual also says
> they are allowed.

I'm pretty sure I've successfully installed the Hyperbole package
(which includes subdirectories) directly from GNU ELPA and I haven't
heard complaints about the Proof-General (M|NonGNU)ELPA package (which
relies even more heavily on subdirectories) either, so the problem is
likely linked to some other "detail".

[..time passes..]

Thanks for the tarballs for the tests, they were very helpful.
The patch also pointed in the right direction.

I installed the patch below (plus regression tests) into `master`.

[ Whether the name comparisons should be done on names that have gone
  through `expand-file-name` or not is a good question.
  Efficiency/simplicity suggests not to do that, and the code's history
  doesn't explain why it's done, but I have the vague recollection that
  it tries to handle some cases of funny file names, e.g. with .. in
  them.
  In the end I kept the `expand-file-name` version mostly so as to
  reduce the size of the change (and associated risk of regression).  ]


        Stefan


diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 4268f7d27a7..d619142d64c 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -930,7 +930,7 @@ package-untar-buffer
         (or (string-match regexp name)
             ;; Tarballs created by some utilities don't list
             ;; directories with a trailing slash (Bug#13136).
-            (and (string-equal dir name)
+            (and (string-equal (expand-file-name dir) name)
                  (eq (tar-header-link-type tar-data) 5))
             (error "Package does not untar cleanly into directory %s/" dir)))))
   (tar-untar-buffer))
@@ -1192,8 +1192,12 @@ package-tar-file-info
   "Find package information for a tar file.
 The return result is a `package-desc'."
   (cl-assert (derived-mode-p 'tar-mode))
-  (let* ((dir-name (file-name-directory
-                    (tar-header-name (car tar-parse-info))))
+  (let* ((dir-name (named-let loop
+                       ((filename (tar-header-name (car tar-parse-info))))
+                     (let ((dirname (file-name-directory filename)))
+                       ;; The first file can be in a subdir: look for the top.
+                       (if dirname (loop (directory-file-name dirname))
+                         (file-name-as-directory filename)))))
          (desc-file (package--description-file dir-name))
          (tar-desc (tar-get-file-descriptor (concat dir-name desc-file))))
     (unless tar-desc

Message #19 received at 58367 <at> debbugs.gnu.org (full text, mbox):

From: Richard Hopkins <emacs <at> unbit.co.uk>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 58367 <at> debbugs.gnu.org
Subject: Re: bug#58367: `package-install-file' rejects some .tar files
 (tentative patch)
Date: Sat, 08 Oct 2022 23:45:11 +0100

On 2022-10-08 17:25, Stefan Monnier wrote:
> Thanks for the tarballs for the tests, they were very helpful.
> The patch also pointed in the right direction.
> 
> I installed the patch below (plus regression tests) into `master`.

Great, thanks for taking a look at this and also fixing the top level
lookup for v7-withsub-0.1.tar.  I've pulled the latest changes and all
the original tar files now install.

I've managed to get the ustar-withsub-0.1 to install from a directory
and include the sub directories by using `copy-directory' instead of
only copying the .el files in the top level.  As more inspiration...

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index d619142d64..4144a12718 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -947,14 +947,10 @@ package-unpack
          (pkg-dir (expand-file-name dirname package-user-dir)))
     (pcase (package-desc-kind pkg-desc)
       ('dir
-       (make-directory pkg-dir t)
        (let ((file-list
-              (directory-files
-               default-directory 'full "\\`[^.].*\\.el\\'" 'nosort)))
-         (dolist (source-file file-list)
-           (let ((target-el-file
-                  (expand-file-name (file-name-nondirectory 
source-file) pkg-dir)))
-             (copy-file source-file target-el-file t)))
+              (directory-files-recursively
+               default-directory "." t)))
+         (copy-directory default-directory pkg-dir nil t 
'copy-contents)
          ;; Now that the files have been installed, this package is
          ;; indistinguishable from a `tar' or a `single'. Let's make
          ;; things simple by ensuring we're one of them.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.