Re: imagemagick@6.9.11-48 to graft or not to graft with 6.9.12-2

[Ludovic Courtès]

Hi Léo,

Léo Le Bouter <lle-bout@zaclys.net> skribis:

> See commit: 82e887ba48c2ba91b17aa9b6b17501e3e0ef4aef
>
> Following discussion around whether it is safe to graft and whether we
> should do so or not, first, I apologize for not doing as rigorous
> checking on this issue as I should have, and also requesting more peer-
> review, I initially believed those two ImageMagick version were ABI
> compatible with unchanged soname so it turns out it would be a rather
> uncontroversial graft to make but now it turns out we have a changed
> soname but whether it is binary (backwards) compatible or not remains a
> question.

Mistakes happen, that’s okay.  However, the manual explicitly mentions
“trivial changes” are acceptable without peer review, but as I wrote,
those security updates rarely, if ever, qualify as “trivial”:

  https://guix.gnu.org/manual/devel/en/html_node/Commit-Access.html

> $ ./pre-inst-env guix environment --ad-hoc libabigail -- abidiff
> $(./pre-inst-env guix build --no-grafts imagemagick@6.9.11-48 | grep -v
> doc)/lib/libMagickCore-6.Q16.so.6 $(./pre-inst-env guix build 
> imagemagick@6.9.12-2g | grep -v doc)/lib/libMagickCore-6.Q16.so.7
> ELF SONAME changed

If upstream changed the SONAME, they probably had a reason.  A library
with a different SONAME cannot be used as a replacement, period.

It’s also unclear to me that ImageMagick can be meaningfully grafted.
Are there users of libMagick*.so in external packages?  That seems
unlikely.

On berlin, I see this:

--8<---------------cut here---------------start------------->8---
$ guix graph -t referrers 
/gnu/store/7iwx7rj1ipsbgb9wgimrrflniyxpilw3-imagemagick-6.9.12-2g 
digraph "Guix referrers" {
  "/gnu/store/7iwx7rj1ipsbgb9wgimrrflniyxpilw3-imagemagick-6.9.12-2g" [label = 
"imagemagick-6.9.12-2g", shape = box, fontname = sans];
  "/gnu/store/7iwx7rj1ipsbgb9wgimrrflniyxpilw3-imagemagick-6.9.12-2g" -> 
"/gnu/store/7iwx7rj1ipsbgb9wgimrrflniyxpilw3-imagemagick-6.9.12-2g" [color = 
darkviolet];
  "/gnu/store/7iwx7rj1ipsbgb9wgimrrflniyxpilw3-imagemagick-6.9.12-2g" -> 
"/gnu/store/wsw9an4lsnqxalwkvycxaa3y0ybp8rxp-ecl-ltk-0.992" [color = 
darkviolet];
  "/gnu/store/wsw9an4lsnqxalwkvycxaa3y0ybp8rxp-ecl-ltk-0.992" [label = 
"ecl-ltk-0.992", shape = box, fontname = sans];
  "/gnu/store/wsw9an4lsnqxalwkvycxaa3y0ybp8rxp-ecl-ltk-0.992" -> 
"/gnu/store/wsw9an4lsnqxalwkvycxaa3y0ybp8rxp-ecl-ltk-0.992" [color = 
peachpuff4];

}
--8<---------------cut here---------------end--------------->8---

That means ‘ecl-ltk’ is the only package that keeps a reference to
ImageMagick, and thus, it’s the only one that would benefit from the
graft.  The graft is useless.

To me that means we should revert this patch series (perhaps with the
exception of bb2427fa28):

  2e0ff59f0c gnu: imagemagick/fixed: Redirect old sonames to new sonames.
  bb2427fa28 gnu: ImageMagick: Refer to the version number in a more robust way.
  bb5d84a048 gnu: ImageMagick: Fix version number in build configuration of 
grafted replacement.
  852ba914a4 gnu: imagemagick/fixed: Retain version length for successful 
grafting.
  82e887ba48 gnu: imagemagick: Update to 6.9.12-2 [security fixes].

After that, what we can do, is introduce 6.9.12-2 as an additional
public version of imagemagick.  That way, users who run:

  guix install imagemagick

get the newer version, the one that includes security fixes.

Could you look into this?

Thanks,
Ludo’.