bug#33587: [PATCH v2] Add native image scaling (bug#33587)

bug#33587: [PATCH v2] Add native image scaling (bug#33587)

[Paul Eggert]

On 1/10/19 11:42 AM, Alan Third wrote:
> I’ve pushed this to master as‐is. If nobody else picks it up I’ll see
> if I can work out an ImageMagick fall‐back system.

Thanks for doing all that. I tweaked the indentation a bit to fit in 80 
columns etc.

At some point soon I plan to install the patch in Bug#33587#5, which 
would disable ImageMagick by default when configuring Emacs (you can 
still enable it if you like). On my Fedora 29 desktop this patch 
decouples Emacs from libMagickCore, libMagickWand, libfftw3 (the FFT 
library), libgomp (OpenMP), libtldl (libtool), and libXt (X toolkit), 
which shaves 5-10 ms off the Emacs startup time in my little experiments.

bug#33587: [PROPOSED] Default to disabling ImageMagick

[Paul Eggert]

Paul Eggert wrote:

> At some point soon I plan to install the patch in Bug#33587#5

It wasn't soon, but I did install the patch just now. Closing the bug report.

bug#33587: [PROPOSED] Default to disabling ImageMagick

[Paul Eggert]

From: Paul Eggert <eggert@Penguin.CS.UCLA.EDU>

ImageMagick has continuing stability and security problems, suggesting
that 'configure' should disable it by default.  See Glenn Morris's notes
at: https://lists.gnu.org/r/emacs-devel/2018-12/msg00036.html
* INSTALL, etc/NEWS: Mention this.
* configure.ac (imagemagick): Default to off.
---
 INSTALL      | 4 +++-
 configure.ac | 2 +-
 etc/NEWS     | 4 ++++
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/INSTALL b/INSTALL
index 0c56fff6d4..9696904dce 100644
--- a/INSTALL
+++ b/INSTALL
@@ -294,7 +294,9 @@ or more of these options:
   --without-gif          for GIF image support
   --without-png          for PNG image support
   --without-rsvg         for SVG image support
-  --without-imagemagick  for Imagemagick support
+
+Although ImageMagick support is disabled by default due to security
+and stability concerns, you can enable it with --with-imagemagick.
 
 Use --without-toolkit-scroll-bars to disable Motif or Xaw3d scroll bars.
 
diff --git a/configure.ac b/configure.ac
index 8b34c3b658..b70393925a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -354,7 +354,7 @@ AC_DEFUN
 OPTION_DEFAULT_ON([libsystemd],[don't compile with libsystemd support])
 OPTION_DEFAULT_OFF([cairo],[compile with Cairo drawing (experimental)])
 OPTION_DEFAULT_ON([xml2],[don't compile with XML parsing support])
-OPTION_DEFAULT_ON([imagemagick],[don't compile with ImageMagick image support])
+OPTION_DEFAULT_OFF([imagemagick],[compile with ImageMagick image support])
 OPTION_DEFAULT_ON([json], [don't compile with native JSON support])
 
 OPTION_DEFAULT_ON([xft],[don't use XFT for anti aliased fonts])
diff --git a/etc/NEWS b/etc/NEWS
index 6297d07879..07c6f74c44 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -37,6 +37,10 @@ functions 'json-serialize', 'json-insert', 'json-parse-string', and
 'json-parse-buffer' are typically much faster than their Lisp
 counterparts from json.el.
 
+** Emacs no longer defaults to using ImageMagick to display images,
+due to security and stability concerns.  To override the default, use
+'configure --with-imagemagick'.
+
 ** The etags program now uses the C library's regular expression matcher
 when possible, and a compatible regex substitute otherwise.  This will
 let developers maintain Emacs's own regex code without having to also
-- 
2.19.2

bug#33587: [PROPOSED] Default to disabling ImageMagick

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sun,  2 Dec 2018 10:09:19 -0800
> Cc: Paul Eggert <eggert@Penguin.CS.UCLA.EDU>
> 
> From: Paul Eggert <eggert@Penguin.CS.UCLA.EDU>
> 
> ImageMagick has continuing stability and security problems, suggesting
> that 'configure' should disable it by default.  See Glenn Morris's notes
> at: https://lists.gnu.org/r/emacs-devel/2018-12/msg00036.html
> * INSTALL, etc/NEWS: Mention this.
> * configure.ac (imagemagick): Default to off.

No objections from me, but let's please wait for a week, to let people
chance to voice objections.

Thanks.

bug#33587: [PROPOSED] Default to disabling ImageMagick

[Andreas Schwab]

On Dez 02 2018, Paul Eggert <eggert@cs.ucla.edu> wrote:

> +** Emacs no longer defaults to using ImageMagick to display images,
> +due to security and stability concerns.  To override the default, use
> +'configure --with-imagemagick'.

ImageMagick is the only backend that supports scaling.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

bug#33587: [PROPOSED] Default to disabling ImageMagick

[Paul Eggert]

Andreas Schwab wrote:
> ImageMagick is the only backend that supports scaling.

Good point, and if we make the change, the scaling issue should be mentioned in 
INSTALL. Perhaps something like the following wording:

"Although ImageMagick support is disabled by default due to security
and stability concerns, you can enable it by configuring with
--with-imagemagick.  ImageMagick is the only backend that supports
image scaling."

bug#33587: [PROPOSED] Default to disabling ImageMagick

[Alan Third]

On Sun, Dec 02, 2018 at 03:51:57PM -0800, Paul Eggert wrote:
> Andreas Schwab wrote:
> > ImageMagick is the only backend that supports scaling.
> 
> Good point, and if we make the change, the scaling issue should be mentioned
> in INSTALL. Perhaps something like the following wording:
> 
> "Although ImageMagick support is disabled by default due to security
> and stability concerns, you can enable it by configuring with
> --with-imagemagick.  ImageMagick is the only backend that supports
> image scaling."

FWIW the NS port on master supports scaling through the NS toolkit,
although there is the problem that most lisp code that wants to scale
checks exclusively for imagemagick support.
-- 
Alan Third

bug#33587: [PROPOSED] Default to disabling ImageMagick

[Glenn Morris]

I'm a bit surprised by the lack of objections so far, though it's early
days yet of course. Maybe it's an experiment that needs to be tried out
for the implications to be seen.

A related alternative would be to lower the priority of the ImageMagick
backend. At the moment, visiting eg a png image uses ImageMagick rather
than libpng if both are linked in.

bug#33587: [PROPOSED] Default to disabling ImageMagick

[Paul Eggert]

On 12/3/18 11:08 AM, Glenn Morris wrote:
> A related alternative would be to lower the priority of the ImageMagick
> backend. At the moment, visiting eg a png image uses ImageMagick rather
> than libpng if both are linked in.

If this alternative is taken and the user requests scaling, presumably 
the ImageMagick library would need to be used anyway since it can scale 
and libpng can't.

bug#33587: [PROPOSED] Default to disabling ImageMagick

[Glenn Morris]

Paul Eggert wrote:

> On 12/3/18 11:08 AM, Glenn Morris wrote:
>> A related alternative would be to lower the priority of the ImageMagick
>> backend. At the moment, visiting eg a png image uses ImageMagick rather
>> than libpng if both are linked in.
>
> If this alternative is taken and the user requests scaling, presumably
> the ImageMagick library would need to be used anyway since it can
> scale and libpng can't.

Sure. I mean, make use of ImageMagick require an explicit request, for
uses that might need those features (eww?), rather than just happening
by default like it does now.

bug#33587: [PROPOSED] Default to disabling ImageMagick

[David Engster]

Glenn Morris writes:
> I'm a bit surprised by the lack of objections so far, though it's early
> days yet of course. Maybe it's an experiment that needs to be tried out
> for the implications to be seen.

Well, I do depend on image scaling, but I (like many others here, I
guess) build Emacs myself, so defaults don't matter much to me.

Question is: will disabling Imagemagick by default also have an impact
on how Emacs is shipped in distributions? I don't think so, at least as
long as they don't drop Imagemagick completely. If for instance Debian
has to take care of Imagemagick security issues anyway, why shouldn't
Emacs link to it?

But that's just my guess...

-David

bug#33587: [PROPOSED] Default to disabling ImageMagick

[Glenn Morris]

David Engster wrote:

> Question is: will disabling Imagemagick by default also have an impact
> on how Emacs is shipped in distributions?

I don't know. It depends whether they go with the default configure
options or not.

> I don't think so, at least as long as they don't drop Imagemagick
> completely.

Note that Red Hat Enterprise Linux 8 _will_ drop ImageMagick completely
(though it will probably be available from an add-on repository),
presumably because they don't feel able to keep up with the security
issues. That's what prompted me to first raise this in

http://lists.gnu.org/r/emacs-devel/2018-12/msg00036.html

> If for instance Debian has to take care of Imagemagick security issues
> anyway, why shouldn't Emacs link to it?

(For reference:
https://security-tracker.debian.org/tracker/source-package/imagemagick )

Because one can never guarantee all security issues are fixed, and if a
project has a history of having a lot of them, it may be considered
likely to be insecure. Also there are the various Emacs crash reports
due to ImageMagick.

bug#33587: [PROPOSED] Default to disabling ImageMagick

[David Engster]

Glenn Morris writes:
> Note that Red Hat Enterprise Linux 8 _will_ drop ImageMagick completely
> (though it will probably be available from an add-on repository),
> presumably because they don't feel able to keep up with the security
> issues. That's what prompted me to first raise this in
>
> http://lists.gnu.org/r/emacs-devel/2018-12/msg00036.html

RHEL can do this because they're supporting way less packages than other
distributions. As you know, enterprise customers have other priorities
than home desktop users. Debian cannot remove Imagemagick because many
other packages depend on it, at least currently.

>> If for instance Debian has to take care of Imagemagick security issues
>> anyway, why shouldn't Emacs link to it?
>
> (For reference:
> https://security-tracker.debian.org/tracker/source-package/imagemagick )
>
> Because one can never guarantee all security issues are fixed, and if a
> project has a history of having a lot of them, it may be considered
> likely to be insecure. Also there are the various Emacs crash reports
> due to ImageMagick.

I understand the reasoning. To me, image scaling is essential for what
I'm doing with Emacs, so I'm willing to take that risk. But that's just
one data point.

Don't get me wrong: I don't object to disable it by default. Let's see
what happens. Maybe distributions will then disable it as well, but they
have their own ways to see how changes like these affect users (by
having an 'unstable' tree or whatever).

-David

bug#33587: [PROPOSED] Default to disabling ImageMagick

[Glenn Morris]

PS GraphicsMagick allegedly has fewer security issues than ImageMagick,
but https://debbugs.gnu.org/14358 saw no interest.

bug#33587: [PROPOSED] Default to disabling ImageMagick

[Paul Eggert]

Elias Mårtenson wrote in 
<http://lists.gnu.org/r/emacs-devel/2018-12/msg00186.html> that image 
scaling via Xrender is surprisingly simple. So perhaps an X11 expert 
could investigate doing that for the X Window System, when ImageMagick 
scaling is not available or not used. My impression is that the Xrender 
extension (introduced in 2000) is reasonably popular among X11 servers 
these days.

Scaling on the server could also be faster (e.g., with hardware 
acceleration) and/or more reliable, so quite possibly it'd be better to 
use Xrender to scale even if ImageMagick is available.