bug#20231: 25.0.50; package.el mangles non-ascii content in downloaded archive-contents data

bug#20231: 25.0.50; package.el mangles non-ascii content in downloaded archive-contents data

[Steve Purcell]

Some packages in MELPA have Chinese characters in their
descriptions. These characters are displayed as numeric backslash
escapes in the *packages* buffer. (See
https://github.com/milkypostman/melpa/issues/2313.)

A specific example is the helm-zhihu-daily package.

The archive-contents MELPA provides is served with a utf-8 Content-Type header
which is consistent with the encoding of the data within it:

  http://melpa.org/packages/archive-contents

I see that `package--read-archive-file' uses `insert-file-literally':
changing this to `insert-file' fixes the problem for me, but this is
probably only because I have my defaut local encoding set to utf-8.

What seems to be needed, then, is one or more of the following, in
roughly decreasing order of preference:

- package.el should note the Content-Type received with archive-contents
  data so that it can be used when applied the locally-cached version,
  or store the cached version on disk in a uniform encoding

- archive maintainers should be able to insert a "-*- coding: ... -*-"
  local variable or similar marker in their archive-contents in order to
  declare its encoding

- package.el should dictate a standard encoding with which it will
  `read' cached archive-contents data

The second option doesn't appear to work currently.

I had a cursory look for package descriptions containing non-ascii chars
in ELPA and Marmalade, but didn't find any. However, I don't see that
this issue would be specific to any single archive.

Happy to help get this fixed if we can agree on an approach. And
obviously if I've missed an approach that would reliably work with
existing package.el versions, that would be even better.

-Steve





In GNU Emacs 25.0.50.2 (x86_64-apple-darwin14.1.0, NS appkit-1344.72 Version 10.10.2 (Build 14C109))
 of 2015-02-19 on Mandala.local
Repository revision: d4ed798d2598a914b1313e58acaff5b66c487318
Windowing system distributor `Apple', version 10.3.1344
Configured using:
 `configure --with-ns --without-dbus --without-x CFLAGS=-g'

Configured features:
IMAGEMAGICK ACL LIBXML2 ZLIB

Important settings:
  value of $LC_CTYPE: en_US.UTF-8
  value of $LANG: en_US
  locale-coding-system: utf-8

Major mode: Emacs-Lisp

Minor modes in effect:
  checkdoc-minor-mode: t
  global-magit-wip-save-mode: t
  magit-wip-save-mode: t
  magit-auto-revert-mode: t
  global-flycheck-mode: t
  flycheck-mode: t
  redshank-mode: t
  aggressive-indent-mode: t
  rainbow-delimiters-mode: t
  elisp-slime-nav-mode: t
  highlight-quoted-mode: t
  highlight-symbol-nav-mode: t
  highlight-symbol-mode: t
  diff-hl-mode: t
  diff-auto-refine-mode: t
  bug-reference-prog-mode: t
  paredit-mode: t
  goto-address-prog-mode: t
  auto-insert-mode: t
  auto-compile-on-load-mode: t
  auto-compile-on-save-mode: t
  auto-compile-mode: t
  ipretty-mode: t
  global-rinari-mode: t
  shell-dirtrack-mode: t
  guide-key-mode: t
  hes-mode: t
  global-page-break-lines-mode: t
  page-break-lines-mode: t
  show-paren-mode: t
  global-undo-tree-mode: t
  undo-tree-mode: t
  global-whitespace-cleanup-mode: t
  whitespace-cleanup-mode: t
  global-auto-revert-mode: t
  electric-pair-mode: t
  savehist-mode: t
  desktop-save-mode: t
  winner-mode: t
  global-auto-complete-mode: t
  auto-complete-mode: t
  ido-ubiquitous-mode: t
  ido-everywhere: t
  recentf-mode: t
  global-anzu-mode: t
  anzu-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  global-prettify-symbols-mode: t
  prettify-symbols-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t

Recent messages:
Mark set
package--read-archive-file
Evaluating...
Mark set
See buffer *Pp Eval Output*.
Mark set
Saving file /Users/steve/.emacs.d/elpa/archives/melpa/archive-contents...
Wrote /Users/steve/.emacs.d/elpa/archives/melpa/archive-contents
Evaluating...
See buffer *Pp Eval Output*.

Features:
(shadow sort mail-extr emacsbug sendmail sanityinc-tomorrow-night-theme
cal-move cal-iso diary-lib diary-loaddefs calc-aent calc-yank calc-misc
calc-bin calc-alg calc-ext calc-menu calc calc-loaddefs calc-macs
default-text-scale ac-inf-ruby parse-time sql-indent sql yaml-mode
eieio-opt speedbar sb-image ezimage dframe checkdoc org-capture tramp-sh
cider-debug cider-repl apropos sh-script let-alist ert helm-imenu
semantic/util-modes semantic/util semantic semantic/tag semantic/lex
semantic/fw mode-local cedet async-bytecomp async helm-aliases
helm-elisp helm-eval helm-mode helm-files ffap helm-buffers helm-tags
helm-bookmark helm-locate helm-help helm-org helm-grep helm-regexp
helm-elscreen helm-adaptive helm-info helm-match-plugin helm-plugin
bookmark helm-external helm-net browse-url helm-utils helm helm-source
eieio-compat tar-mode names edebug autoload mm-archive network-stream
nsm starttls url-cache url-handlers smerge-mode magit-svn magit-wip
magit-key-mode magit grep epa epg git-rebase-mode git-commit-mode
log-edit message rfc822 mml mml-sec mailabbrev mail-utils gmm-utils
mailheader pcvs-util add-log rainbow-mode color view debug mc-mark-more
multiple-cursors-core rect hippie-exp align move-dup misearch
multi-isearch executable org-agenda haskell-doc inf-haskell
haskell-decl-scan sanityinc-tomorrow-bright-theme smex haml-mode js
textile-mode css-mode js2-imenu-extras js2-mode markdown-mode robe
url-http tls url-auth url-gw url url-proxy url-privacy url-expand
url-methods url-history url-cookie url-domsuf mailcap cc-mode cc-fonts
cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs
face-remap tidy mmm-erb mmm-region mmm-utils tagedit sgml-mode
nxml-uchnm rng-xsd xsd-regexp rng-cmpct rng-nxml rng-valid rng-loc
rng-uri rng-parse nxml-parse rng-match rng-dt rng-util rng-pttrn nxml-ns
nxml-mode nxml-outln nxml-rap nxml-util nxml-glyph nxml-enc xmltok
cider-mode cider-eldoc cider-interaction arc-mode archive-mode cider-doc
org-table cider-test cider-stacktrace package-build mm-decode mm-bodies
mm-encode mail-parse rfc2231 rfc2047 rfc2045 ietf-drums vc-mtn vc-hg
vc-bzr vc-src vc-sccs vc-svn vc-cvs vc-rcs diff-hl-dired ruler-mode
tabify hexl cap-words superword subword ac-haskell-process haskell
haskell-load haskell-commands highlight-uses-mode haskell-modules
haskell-sandbox haskell-repl haskell-debug haskell-interactive-mode
haskell-presentation-mode haskell-font-lock haskell-show
haskell-collapse haskell-navigate-imports haskell-compile
haskell-process haskell-session url-util haskell-indentation hl-line
flycheck-haskell haskell-cabal haskell-utils haskell-mode haskell-string
haskell-sort-imports haskell-align-imports haskell-compat
haskell-complete-module dabbrev haskell-customize org-element org-rmail
org-mhe org-irc org-info org-gnus org-docview doc-view image-mode
dired-sort dired+ image-dired image-file dired-x dired-aux dired
org-bibtex bibtex org-bbdb org-w3m disp-table vc-git vc-darcs xml
flycheck-ledger flycheck-clojure url-parse url-vars json cider-client
nrepl-client tramp tramp-compat auth-source gnus-util mm-util mail-prsvr
password-cache tramp-loaddefs trampver queue cider-util clojure-mode
flycheck-package lisp-mnt flycheck-hdevtools flycheck subr-x flymake
redshank skeleton aggressive-indent rainbow-delimiters elisp-slime-nav
etags xref highlight-quoted highlight-symbol diff-hl vc-dir ewoc vc
vc-dispatcher diff-mode bug-reference paredit-everywhere paredit
goto-addr init init-locales init-local session
sanityinc-tomorrow-eighties-theme color-theme-sanityinc-tomorrow
autoinsert cus-start cus-load server jka-compr init-ledger init-twitter
init-dash init-misc init-common-lisp init-clojure-cider init-clojure
init-slime init-lisp cl-lib-highlight auto-compile packed ipretty pp
init-paredit init-sql init-rails rinari jump inflections findr
ruby-compilation which-func imenu inf-ruby compile ruby-mode smie
init-ruby-mode derived init-haskell init-python-mode init-haml init-css
init-html init-nxml init-org ob-sqlite ob-sh shell ob-ruby ob-python
ob-octave ob-ledger ob-latex ob-gnuplot ob-dot ob-ditaa ob-R org-clock
org org-macro org-footnote org-pcomplete pcomplete org-list org-faces
org-entities noutline outline org-version ob-emacs-lisp ob ob-tangle
ob-ref ob-lob ob-table ob-exp org-src ob-keys ob-comint comint
ansi-color ob-core ob-eval org-compat org-macs org-loaddefs format-spec
find-func cal-menu calendar cal-loaddefs init-php init-javascript
easy-mmode init-erlang erlang-start init-csv init-markdown init-textile
init-crontab init-compile init-github init-git init-darcs init-vc
init-editing-utils guide-key s ucs-normalize popwin dash
highlight-escape-sequences whole-line-or-region page-break-lines paren
undo-tree diff whitespace-cleanup-mode whitespace autorevert filenotify
elec-pair init-mmm mmm-auto mmm-vars mmm-compat init-fonts init-sessions
savehist desktop frameset init-windows windmove switch-window quail
help-mode winner ring init-auto-complete auto-complete-config
auto-complete popup init-hippie-expand init-ido ido-ubiquitous warnings
ido init-recentf recentf tree-widget wid-edit init-flycheck init-ibuffer
ibuf-macs init-uniquify init-grep init-isearch diminish anzu mule-util
thingatpt init-dired init-proxies init-gui-frames init-osx-keys edmacro
kmacro init-themes init-xterm init-frame-hooks init-exec-path
exec-path-from-shell init-elpa fullframe finder-inf eieio byte-opt
bytecomp byte-compile cl-extra seq cconv eieio-core cl-generic pcase rx
cl-macs slime-autoloads info easymenu package epg-config init-site-lisp
cl gv cl-loaddefs cl-lib init-utils init-compat init-benchmarking advice
help-fns time-date tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel ns-win tool-bar dnd fontset image regexp-opt
fringe tabulated-list newcomment elisp-mode lisp-mode prog-mode register
page menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock
font-lock syntax facemenu font-core frame cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese hebrew
greek romanian slovak czech european ethiopic indian cyrillic chinese
case-table epa-hook jka-cmpr-hook help simple abbrev minibuffer
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote make-network-process
cocoa ns multi-tty emacs)

Memory information:
((conses 16 2066523 297407)
 (symbols 48 78106 0)
 (miscs 40 11519 16113)
 (strings 32 449119 39379)
 (string-bytes 1 7884651)
 (vectors 16 107393)
 (vector-slots 8 2411374 20255)
 (floats 8 1294 3276)
 (intervals 56 115973 379)
 (buffers 976 176))

bug#20231: 25.0.50; package.el mangles non-ascii content in downloaded archive-contents data

[Stefan Monnier]

> - package.el should dictate a standard encoding with which it will
>   `read' cached archive-contents data

archive-contents should always use utf-8, yes.


        Stefan

bug#20231: 25.0.50; package.el mangles non-ascii content in downloaded archive-contents data

[Steve Purcell]

[-- Attachment #1: Type: text/plain, Size: 109 bytes --]

The attached patch seems reasonable and works nicely for me. Happy to modify it if necessary.

-Steve



[-- Attachment #2: archive-contents-encoding.patch --]
[-- Type: application/octet-stream, Size: 1794 bytes --]

4dc1966abf44aff8baab2bf20a395260ea640428 HEAD master
Author: Steve Purcell <steve@sanityinc.com>
Date:   Sat Apr 11 18:01:06 2015 +0100

    Assume package archive-contents are UTF8-encoded
    
    * lisp/emacs-lisp/package.el (package--read-archive-file):
    Set `coding-system-for-read' explicitly to 'utf-8 when reading the
    downloaded and cached archive-contents files, so that non-ASCII
    characters in package descriptions are displayed correctly in the
    `list-packages' menu. (Bug #20231)

1 file changed, 7 insertions(+), 6 deletions(-)
 lisp/emacs-lisp/package.el | 13 +++++++------

	Modified   lisp/emacs-lisp/package.el
diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 40d02f8..dadaf27 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -1279,12 +1279,13 @@ Will throw an error if the archive version is too new."
   (let ((filename (expand-file-name file package-user-dir)))
     (when (file-exists-p filename)
       (with-temp-buffer
-        (insert-file-contents-literally filename)
-        (let ((contents (read (current-buffer))))
-          (if (> (car contents) package-archive-version)
-              (error "Package archive version %d is higher than %d"
-                (car contents) package-archive-version))
-          (cdr contents))))))
+        (let ((coding-system-for-read 'utf-8))
+          (insert-file-contents filename)
+          (let ((contents (read (current-buffer))))
+            (if (> (car contents) package-archive-version)
+                (error "Package archive version %d is higher than %d"
+                       (car contents) package-archive-version))
+            (cdr contents)))))))
 
 (defun package-read-archive-contents (archive)
   "Re-read archive contents for ARCHIVE.

bug#20231: 25.0.50; package.el mangles non-ascii content in downloaded archive-contents data

[Eli Zaretskii]

> From: Steve Purcell <steve@sanityinc.com>
> Date: Sat, 11 Apr 2015 18:08:16 +0100
> 
> +        (let ((coding-system-for-read 'utf-8))

'utf-8 or 'utf-8-unix?  Or is that unimportant in this case?

bug#20231: 25.0.50; package.el mangles non-ascii content in downloaded archive-contents data

[Ivan Shmakov]

>>>>> Steve Purcell <steve@sanityinc.com> writes:

 > --- a/lisp/emacs-lisp/package.el
 > +++ b/lisp/emacs-lisp/package.el
 > @@ -1279,12 +1279,13 @@ defun package--read-archive-file (file)
 >    (let ((filename (expand-file-name file package-user-dir)))
 >      (when (file-exists-p filename)
 >        (with-temp-buffer
 > -        (insert-file-contents-literally filename)
 > -        (let ((contents (read (current-buffer))))
 > -          (if (> (car contents) package-archive-version)
 > -              (error "Package archive version %d is higher than %d"
 > -                (car contents) package-archive-version))
 > -          (cdr contents))))))
 > +        (let ((coding-system-for-read 'utf-8))
 > +          (insert-file-contents filename)

	As insert-file-contents may imply a whole lot of additional
	processing beyond decoding UTF-8, I’d rather use an explicit
	decode-coding-region here, like (untested):

       (with-temp-buffer
         (insert-file-contents-literally filename)
+        (decode-coding-region (point-min) (point-max) 'utf-8)
         (let ((contents (read (current-buffer))))
           …))

 > +          (let ((contents (read (current-buffer))))
 > +            (if (> (car contents) package-archive-version)
 > +                (error "Package archive version %d is higher than %d"
 > +                       (car contents) package-archive-version))
 > +            (cdr contents)))))))
 > 
 >  (defun package-read-archive-contents (archive)
 >    "Re-read archive contents for ARCHIVE.

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

bug#20231: 25.0.50; package.el mangles non-ascii content in downloaded archive-contents data

[Eli Zaretskii]

> From: Ivan Shmakov <ivan@siamics.net>
> Date: Sat, 11 Apr 2015 18:07:13 +0000
> 
> >>>>> Steve Purcell <steve@sanityinc.com> writes:
> 
>  > --- a/lisp/emacs-lisp/package.el
>  > +++ b/lisp/emacs-lisp/package.el
>  > @@ -1279,12 +1279,13 @@ defun package--read-archive-file (file)
>  >    (let ((filename (expand-file-name file package-user-dir)))
>  >      (when (file-exists-p filename)
>  >        (with-temp-buffer
>  > -        (insert-file-contents-literally filename)
>  > -        (let ((contents (read (current-buffer))))
>  > -          (if (> (car contents) package-archive-version)
>  > -              (error "Package archive version %d is higher than %d"
>  > -                (car contents) package-archive-version))
>  > -          (cdr contents))))))
>  > +        (let ((coding-system-for-read 'utf-8))
>  > +          (insert-file-contents filename)
> 
> 	As insert-file-contents may imply a whole lot of additional
> 	processing beyond decoding UTF-8, I’d rather use an explicit
> 	decode-coding-region here

Can you explain why?  What additional processing can get in the way?

bug#20231: 25.0.50; package.el mangles non-ascii content in downloaded archive-contents data

[Steve Purcell]

On 11 Apr 2015, at 18:41, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> 'utf-8 or 'utf-8-unix?  Or is that unimportant in this case?


It’s not important in this case, but I suppose the latter would make it even more explicit.

bug#20231: 25.0.50; package.el mangles non-ascii content in downloaded archive-contents data

[Eli Zaretskii]

> From: Steve Purcell <steve@sanityinc.com>
> Date: Sat, 11 Apr 2015 19:48:48 +0100
> Cc: monnier@IRO.UMontreal.CA,
>  20231@debbugs.gnu.org
> 
> On 11 Apr 2015, at 18:41, Eli Zaretskii <eliz@gnu.org> wrote:
> > 
> > 'utf-8 or 'utf-8-unix?  Or is that unimportant in this case?
> 
> 
> It’s not important in this case, but I suppose the latter would make it even more explicit.

-unix is also closer to the original, so I agree.

bug#20231: 25.0.50; package.el mangles non-ascii content in downloaded archive-contents data

[Ivan Shmakov]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:

[…]

 >> As insert-file-contents may imply a whole lot of additional
 >> processing beyond decoding UTF-8, I’d rather use an explicit
 >> decode-coding-region here

 > Can you explain why?  What additional processing can get in the way?

	One of the examples I can readily think of is enriched-decode,
	which will automatically be called should there be a
	‘Content-Type: text/enriched’ line near the top of the file
	being read (per format-alist.)

	I have no example at hand of how such processing may be
	triggered in practice, but avoiding it altogether seems to me
	like a saner approach.

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

bug#20231: 25.0.50; package.el mangles non-ascii content in downloaded archive-contents data

[Eli Zaretskii]

> From: Ivan Shmakov <ivan@siamics.net>
> Date: Sat, 11 Apr 2015 19:30:21 +0000
> 
> >>>>> Eli Zaretskii <eliz@gnu.org> writes:
> 
> […]
> 
>  >> As insert-file-contents may imply a whole lot of additional
>  >> processing beyond decoding UTF-8, I’d rather use an explicit
>  >> decode-coding-region here
> 
>  > Can you explain why?  What additional processing can get in the way?
> 
> 	One of the examples I can readily think of is enriched-decode,
> 	which will automatically be called should there be a
> 	‘Content-Type: text/enriched’ line near the top of the file
> 	being read (per format-alist.)

Yes, but is that kind of processing harmful for the case at point?

Also, are we sure we will never get a compressed file, for example, or
some other binary file?  In those cases, decoding is not TRT.

> 	I have no example at hand of how such processing may be
> 	triggered in practice, but avoiding it altogether seems to me
> 	like a saner approach.

It is also slower.  Again, I have no idea if speed matters in this
case.

bug#20231: 25.0.50; package.el mangles non-ascii content in downloaded archive-contents data

[Artur Malabarba]

>>       I have no example at hand of how such processing may be
>>       triggered in practice, but avoiding it altogether seems to me
>>       like a saner approach.
>
> It is also slower.  Again, I have no idea if speed matters in this
> case.

I vote for the fastest approach. Now that package-refresh can be done
in the background, any post-processing done in the foreground is all
the more noticeable by the user.

bug#20231: 25.0.50; package.el mangles non-ascii content in downloaded archive-contents data

[Stefan Monnier]

>> It is also slower.  Again, I have no idea if speed matters in this
>> case.
> I vote for the fastest approach. Now that package-refresh can be done
> in the background, any post-processing done in the foreground is all
> the more noticeable by the user.

Either way is fine by me.  Just one detail, tho: if you let-bind
coding-system-for-read, please only do it around the call to
insert-file-contents and not around the subsequent code (even if it's
short and seems not to matter).


        Stefan

bug#20231: 25.0.50; package.el mangles non-ascii content in downloaded archive-contents data

[Steve Purcell]

On 13 Apr 2015, at 15:02, Stefan Monnier <monnier@IRO.UMontreal.CA> wrote:
> 
>>> It is also slower.  Again, I have no idea if speed matters in this
>>> case.
>> I vote for the fastest approach. Now that package-refresh can be done
>> in the background, any post-processing done in the foreground is all
>> the more noticeable by the user.
> 
> Either way is fine by me.  Just one detail, tho: if you let-bind
> coding-system-for-read, please only do it around the call to
> insert-file-contents and not around the subsequent code (even if it's
> short and seems not to matter).


Not sure at this stage who’s going to do this, and what the consensus is for how it should be done. It’s probably quickest if someone who has an opinion about the best fix just goes ahead and commits whatever variation of my patch is considered optimal.

bug#20231: 25.0.50; package.el mangles non-ascii content in downloaded archive-contents data

[Artur Malabarba]

I can do it.
Is the copyright cleared or do I mark this as a tiny change?

2015-04-13 21:45 GMT+01:00 Steve Purcell <steve@sanityinc.com>:
> On 13 Apr 2015, at 15:02, Stefan Monnier <monnier@IRO.UMontreal.CA> wrote:
>>
>>>> It is also slower.  Again, I have no idea if speed matters in this
>>>> case.
>>> I vote for the fastest approach. Now that package-refresh can be done
>>> in the background, any post-processing done in the foreground is all
>>> the more noticeable by the user.
>>
>> Either way is fine by me.  Just one detail, tho: if you let-bind
>> coding-system-for-read, please only do it around the call to
>> insert-file-contents and not around the subsequent code (even if it's
>> short and seems not to matter).
>
>
> Not sure at this stage who’s going to do this, and what the consensus is for how it should be done. It’s probably quickest if someone who has an opinion about the best fix just goes ahead and commits whatever variation of my patch is considered optimal.

bug#20231: 25.0.50; package.el mangles non-ascii content in downloaded archive-contents data

[Steve Purcell]

I have a confirmed copyright assignment on file for Emacs.

bug#20231: 25.0.50; package.el mangles non-ascii content in downloaded archive-contents data

[Eli Zaretskii]

> From: Steve Purcell <steve@sanityinc.com>
> Date: Sun, 19 Apr 2015 16:23:15 +0100
> 
> I have a confirmed copyright assignment on file for Emacs.

Indeed, I see it on file.

bug#20231: 25.0.50; package.el mangles non-ascii content in downloaded archive-contents data

[Artur Malabarba]

Done.

bug#20231: closed (Re: bug#20231: 25.0.50; package.el mangles non-ascii content in downloaded archive-contents data)

[Steve Purcell]

Thanks Artur & everyone else who chipped in.