Subject: bug#58367: `package-install-file' rejects some .tar files (tentative patch)

From: Richard Hopkins <[email-masked]>
Date: Fri, 07 Oct 2022 22:15:56 +0100

`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

Teach `package-install-file' to install previously rejected tarballs

Whilst Emacs can open the varieties of tarballs with no problem, the
package management functions depended on a particular table layout in
the tarball to calculate the directory for the package.

For example, with BSD tar: `tar cf ...` would generate a rejected
tarball, and `tar cof ...` would generate an accepted one.

* lisp/emacs-lisp/package.el (package-untar-buffer): Compare against
the header name from the tarball rather than the filesystem
expanded form.
(package-tar-file-info): Also allow the first entry to be a plain
directory name without trailing path separator.
---
 lisp/emacs-lisp/package.el | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 4268f7d27a..cc371fbc50 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -926,11 +926,12 @@ package-untar-buffer
   (let ((regexp (concat "\\`" (regexp-quote (expand-file-name dir)) "/"))
         (case-fold-search (file-name-case-insensitive-p dir)))
     (dolist (tar-data tar-parse-info)
-      (let ((name (expand-file-name (tar-header-name tar-data))))
+      (let* ((header-name (tar-header-name tar-data))
+             (name (expand-file-name header-name)))
         (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 dir header-name)
                  (eq (tar-header-link-type tar-data) 5))
             (error "Package does not untar cleanly into directory %s/" dir)))))
   (tar-untar-buffer))
@@ -1192,8 +1193,10 @@ 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* ((header-name (tar-header-name (car tar-parse-info)))
+	 (dir-name (or (file-name-directory header-name)
+                       ;; Some tar utilities generate a plain directory name first
+                       (file-name-as-directory header-name)))
          (desc-file (package--description-file dir-name))
          (tar-desc (tar-get-file-descriptor (concat dir-name desc-file))))
     (unless tar-desc
2.38.0