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

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

[Léo Le Bouter]

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

Hello!

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.

We had a user reporting that Inkscape stopped working after the graft (
https://logs.guix.gnu.org/guix/2021-03-18.log#100200), after which we
decided on IRC with rekado we might cheat by symlinking the shared
libraries, which I've done in commit
2e0ff59f0cd836b156f1ef2e78791d864ce3cfcd, from a glance it didnt seem
the soname change caused backwards incompatible changes but only
forward incompatible changes.

Let's see some abidiff output now:

$ ./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
Functions changes summary: 0 Removed, 0 Changed, 0 Added function
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
Function symbols changes summary: 0 Removed, 0 Added function symbol
not referenced by debug info
Variable symbols changes summary: 0 Removed, 1 Added variable symbol
not referenced by debug info

SONAME changed from 'libMagickCore-6.Q16.so.6' to 'libMagickCore-
6.Q16.so.7'

1 Added variable symbol not referenced by debug info:

  [A] .gomp_critical_user_analyzeImage


$ ./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/libMagick++-6.Q16.so.8 $(./pre-inst-env guix build 
imagemagick@6.9.12-2g | grep -v doc)/lib/libMagick++-6.Q16.so.9
ELF SONAME changed
Functions changes summary: 0 Removed, 0 Changed, 0 Added function
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

SONAME changed from 'libMagick++-6.Q16.so.8' to 'libMagick++-
6.Q16.so.9'

$ ./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/libMagickWand-6.Q16.so.6 $(./pre-inst-env guix build 
imagemagick@6.9.12-2g | grep -v doc)/lib/libMagickWand-6.Q16.so.7
ELF SONAME changed
Functions changes summary: 0 Removed, 0 Changed, 0 Added function
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

SONAME changed from 'libMagickWand-6.Q16.so.6' to 'libMagickWand-
6.Q16.so.7'

Any more ABI diff-ing/testing, information, etc.. on whether this is
safe or not is welcome, it sounds to me it could be fine but there is
some amount of doubt still.

If we can't graft ImageMagick we shall revert all commits and then it
means we would have to apply patches for each and every CVE which can
be tedious to create and maintain and to me leaving the package as-is
without patching is not really OK :-/

To graft or not to graft?

Thank you,
Léo

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Julien Lepiller]

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

I don't think I understand the problem fully, but it looks like there is nothing wrong with the graft now that you symlinked tge library, so it's fine to keep the graft. Hopefully we can ungraft shortly during the "ungraftathon" next week :)

Le 19 mars 2021 05:40:45 GMT-04:00, "Léo Le Bouter" <lle-bout@zaclys.net> a écrit :
>Hello!
>
>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.
>
>We had a user reporting that Inkscape stopped working after the graft (
>https://logs.guix.gnu.org/guix/2021-03-18.log#100200), after which we
>decided on IRC with rekado we might cheat by symlinking the shared
>libraries, which I've done in commit
>2e0ff59f0cd836b156f1ef2e78791d864ce3cfcd, from a glance it didnt seem
>the soname change caused backwards incompatible changes but only
>forward incompatible changes.
>
>Let's see some abidiff output now:
>
>$ ./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
>Functions changes summary: 0 Removed, 0 Changed, 0 Added function
>Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
>Function symbols changes summary: 0 Removed, 0 Added function symbol
>not referenced by debug info
>Variable symbols changes summary: 0 Removed, 1 Added variable symbol
>not referenced by debug info
>
>SONAME changed from 'libMagickCore-6.Q16.so.6' to 'libMagickCore-
>6.Q16.so.7'
>
>1 Added variable symbol not referenced by debug info:
>
>  [A] .gomp_critical_user_analyzeImage
>
>
>$ ./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/libMagick++-6.Q16.so.8 $(./pre-inst-env guix build 
>imagemagick@6.9.12-2g | grep -v doc)/lib/libMagick++-6.Q16.so.9
>ELF SONAME changed
>Functions changes summary: 0 Removed, 0 Changed, 0 Added function
>Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
>
>SONAME changed from 'libMagick++-6.Q16.so.8' to 'libMagick++-
>6.Q16.so.9'
>
>$ ./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/libMagickWand-6.Q16.so.6 $(./pre-inst-env guix build 
>imagemagick@6.9.12-2g | grep -v doc)/lib/libMagickWand-6.Q16.so.7
>ELF SONAME changed
>Functions changes summary: 0 Removed, 0 Changed, 0 Added function
>Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
>
>SONAME changed from 'libMagickWand-6.Q16.so.6' to 'libMagickWand-
>6.Q16.so.7'
>
>Any more ABI diff-ing/testing, information, etc.. on whether this is
>safe or not is welcome, it sounds to me it could be fine but there is
>some amount of doubt still.
>
>If we can't graft ImageMagick we shall revert all commits and then it
>means we would have to apply patches for each and every CVE which can
>be tedious to create and maintain and to me leaving the package as-is
>without patching is not really OK :-/
>
>To graft or not to graft?
>
>Thank you,
>Léo

[-- Attachment #2: Type: text/html, Size: 4127 bytes --]

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’.

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

[Andreas Enge]

Am Fri, Mar 19, 2021 at 10:40:45AM +0100 schrieb Léo Le Bouter:
> We had a user reporting that Inkscape stopped working after the graft (
> https://logs.guix.gnu.org/guix/2021-03-18.log#100200), after which we
> decided on IRC with rekado we might cheat by symlinking the shared
> libraries, which I've done in commit
> 2e0ff59f0cd836b156f1ef2e78791d864ce3cfcd, from a glance it didnt seem
> the soname change caused backwards incompatible changes but only
> forward incompatible changes.

It happens I just wanted to use inkscape, started submitting a bug report:
   https://issues.guix.gnu.org/47315
and ended up realising that this is exactly the issue discussed on
guix-devel.

I cannot afford a "guix pull" right now, since with
   https://issues.guix.gnu.org/31719
this might mean a download of a few gigabytes, so I did not check whether
the symlinking fix does work.

But honestly, this feels like piling a cludge (symlinking) onto a cludge
(grafting), and that we are not in the high quality approach for which
I appreciate Guix.

Personally, I would suggest to revert the commits. If the CVE is sufficiently
important (it would be useful if the commit log or the diff itself contained
its number), maybe we can update the imagemagick version on the wip-release
branch, which is supposed to be built soon and merged back to master?

And please let us agree that in the future, we only backport fixes in grafts
and do not update version numbers.

Andreas

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

[zimoun]

Hi,

On Sun, 21 Mar 2021 at 15:04, Ludovic Courtès <ludo@gnu.org> wrote:

> 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.

I agree.  It sounds reasonable.  In the same time, is it possible to
start to build the branch wip-next-release? then an ungrafted version
could be pushed there and if all the substitutes are availaible on time,
we could cherry-pick for the release.


Cheers,
simon

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

[Ludovic Courtès]

Hi again!

Ludovic Courtès <ludo@gnu.org> skribis:

> 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:
>
> $ 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];
>
> }
>
> 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.

I was plain wrong—apologies for the confusion!

Running:

  guix graph -t referrers /gnu/store/cnyiwi6mn53jwmjh7kdvnlmagf3frsa3-imagemagick-6.9.12-2g | xdot -

on my laptop, I see at least emacs-w3m, pstoedit, skribilo, and (of
course) inkscape.

So grafting makes sense.

Consequently, the way forward IMO is to get a 6.9.11 backport of
whatever CVEs it is we are patching and to use such a patched 6.9.11
variant as the replacement.

Does that make sense?

Ludo’.

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

[Mark H Weaver]

Hi Andreas,

Andreas Enge <andreas@enge.fr> writes:
> And please let us agree that in the future, we only backport fixes in grafts
> and do not update version numbers.

I agree that in this particular case, that's what should have been done,
and that we should still try to do it.  It will be several days at least
before I'm able to look at it, though.  Would someone else like to try?

I also agree that in general, we should be more careful to check for ABI
compatibility before grafting.  Moreover, if an update includes
substantial changes other than bug fixes, I agree that backporting the
fixes is highly preferable for grafting purposes.

However, I think it would be going too far to adopt your proposal as a
general rule for all grafts.  In some cases, it can clearly be seen that
an upstream release includes little more than bug fixes.  For example,
if the recent gvfs-1.40.2 security update had required grafting, I would
not have hesitated to do so, and that would have been much simpler and
IMO cleaner than importing the upstream patches into our tree.

I'll also note that fewer imported patches makes for less review work by
those of us who try to keep an eye on changes made to Guix, to help
guard against the possibility of malicious "fixes" being introduced by
our growing list of committers.  Note that this could happen without any
ill intent on the part of the committer, if their development machine is
compromised by a third party.

What do you think?

    Regards,
      Mark

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

[Léo Le Bouter]

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

In general my opinion is that backporting fixes is time-consuming and
that if we have to do it each time I wont be able to keep up with the
load. I'd rather update things to a version that already includes fixes
and is supported by upstream even at the cost of world rebuilds. I
can't deal with upstreams who either do not backport fixes, or don't
integrate fixes at all.

That's how I feel I can keep up with fixing security issues in GNU Guix
considering how overworked we are in general.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Ludovic Courtès]

Hi,

Mark H Weaver <mhw@netris.org> skribis:

> Andreas Enge <andreas@enge.fr> writes:
>> And please let us agree that in the future, we only backport fixes in grafts
>> and do not update version numbers.
>
> I agree that in this particular case, that's what should have been done,
> and that we should still try to do it.  It will be several days at least
> before I'm able to look at it, though.  Would someone else like to try?
>
> I also agree that in general, we should be more careful to check for ABI
> compatibility before grafting.

More specifically, for libraries, we should only ever use replacements
that are ABI-compatible.  (That usually means that the major and minor
version numbers are the same, which may be what Andreas had in mind.)
Anything else is most likely broken.

For command-line tools, I think we should only graft with replacements
that are backward-compatible at the CLI level.  That requires manual
review of the changes, I guess.

Thanks,
Ludo’.

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

[Andreas Enge]

Hello Mark and Léo,

Am Mon, Mar 22, 2021 at 05:12:35PM -0400 schrieb Mark H Weaver:
> However, I think it would be going too far to adopt your proposal as a
> general rule for all grafts.  In some cases, it can clearly be seen that
> an upstream release includes little more than bug fixes.  For example,
> if the recent gvfs-1.40.2 security update had required grafting, I would
> not have hesitated to do so, and that would have been much simpler and
> IMO cleaner than importing the upstream patches into our tree.

Am Tue, Mar 23, 2021 at 02:34:52PM +0100 schrieb Léo Le Bouter:
> In general my opinion is that backporting fixes is time-consuming and
> that if we have to do it each time I wont be able to keep up with the
> load. I'd rather update things to a version that already includes fixes
> and is supported by upstream even at the cost of world rebuilds. I
> can't deal with upstreams who either do not backport fixes, or don't
> integrate fixes at all.

these are very good arguments, which I understand and share. But moving
to another version is problematic even when there is no soname bump, as
I wrote in my bug report https://issues.guix.gnu.org/47315; grafts with
different version numbers lead to a command line behaviour that is not
understandable:

$ guix package -A imagemagick
imagemagick	6.9.12-2g	out,doc	gnu/packages/imagemagick.scm:132:2
imagemagick	6.9.11-48	out,doc	gnu/packages/imagemagick.scm:48:2

$ guix build imagemagick@6.9.11
guix build: error: imagemagick: package not found for version 6.9.11

$ guix build imagemagick@6.9.11-48
/gnu/store/c30y49vg735g6b4hh590zrc9fmvcsy0w-imagemagick-6.9.12-2g-doc
/gnu/store/l3hr0fimip6v7vmkgxbqygsglxaxasy0-imagemagick-6.9.12-2g

From a user's perspective, inkscape@6.9.11 is at the time there and not
there; it is shown by "guix package", but then not accessible for install-
ation, but silently "glossed over" in favour of a different version.

I just noticed that I can do this:
$ guix build imagemagick@6.9.11-48 --no-grafts
/gnu/store/wlnciwhn6llwqwywf4hq739v5bbcrq3h-imagemagick-6.9.11-48-doc
/gnu/store/vlix7fclb7ifjgmxgpwr1pvraff89w7b-imagemagick-6.9.11-48
But I can also do this:
$ guix build imagemagick@6.9.12-2g --no-grafts
/gnu/store/4s20df0zjmmys8zvlvynksrwz5xqk9ls-imagemagick-6.9.12-2g-doc
/gnu/store/7iwx7rj1ipsbgb9wgimrrflniyxpilw3-imagemagick-6.9.12-2g
where I do not know what I would have expected - the ungrafted version
of 6.9.12 is 6.9.11, no? At the same time, for once it respects my
wish for a specific version.

Otherwise said, grafting to different versions breaks our semantic for
designating packages, in which version numbers play an important role,
and replaces it by a mess which even with the examples above I have a
hard time understanding.
 
Caeterum censeo:
The real fix is probably to do less grafts and more rebuilds...

Andreas

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

[Léo Le Bouter]

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

On Tue, 2021-03-23 at 15:22 +0100, Andreas Enge wrote:
> I wrote in my bug report https://issues.guix.gnu.org/47315; grafts
> with
> different version numbers lead to a command line behaviour that is
> not
> understandable:
> 
> $ guix package -A imagemagick
> imagemagick	6.9.12-2g	out,doc	gnu/packages/imagemagick.scm:132:2
> imagemagick	6.9.11-48	out,doc	gnu/packages/imagemagick.scm:48:2
> 
> $ guix build imagemagick@6.9.11
> guix build: error: imagemagick: package not found for version 6.9.11
> 
> $ guix build imagemagick@6.9.11-48
> /gnu/store/c30y49vg735g6b4hh590zrc9fmvcsy0w-imagemagick-6.9.12-2g-doc
> /gnu/store/l3hr0fimip6v7vmkgxbqygsglxaxasy0-imagemagick-6.9.12-2g
> 
> From a user's perspective, inkscape@6.9.11 is at the time there and
> not
> there; it is shown by "guix package", but then not accessible for
> install-
> ation, but silently "glossed over" in favour of a different version.
> 
> I just noticed that I can do this:
> $ guix build imagemagick@6.9.11-48 --no-grafts
> /gnu/store/wlnciwhn6llwqwywf4hq739v5bbcrq3h-imagemagick-6.9.11-48-doc
> /gnu/store/vlix7fclb7ifjgmxgpwr1pvraff89w7b-imagemagick-6.9.11-48
> But I can also do this:
> $ guix build imagemagick@6.9.12-2g --no-grafts
> /gnu/store/4s20df0zjmmys8zvlvynksrwz5xqk9ls-imagemagick-6.9.12-2g-doc
> /gnu/store/7iwx7rj1ipsbgb9wgimrrflniyxpilw3-imagemagick-6.9.12-2g
> where I do not know what I would have expected - the ungrafted
> version
> of 6.9.12 is 6.9.11, no? At the same time, for once it respects my
> wish for a specific version.
> 
> Otherwise said, grafting to different versions breaks our semantic
> for
> designating packages, in which version numbers play an important
> role,
> and replaces it by a mess which even with the examples above I have a
> hard time understanding.

For this, the problem is not grafting but that the replacement package
definition has been made public, this is an "issue" (?) that is known
and I try to not make replacement package definitions public now.

>  
> Caeterum censeo:
> The real fix is probably to do less grafts and more rebuilds...

Agreed, I would really like a security-updates branch for that, with
which we can buffer changes waiting for substitutes and then merge with
master, but I am afraid this enters in conflict with people not having
lots of bandwidth to download a new world again through substitutes or
very powerful machines for those who don't use substitutes.

Léo

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Leo Famulari]

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

On Tue, Mar 23, 2021 at 02:34:52PM +0100, Léo Le Bouter wrote:
> In general my opinion is that backporting fixes is time-consuming and
> that if we have to do it each time I wont be able to keep up with the
> load. I'd rather update things to a version that already includes fixes
> and is supported by upstream even at the cost of world rebuilds. I
> can't deal with upstreams who either do not backport fixes, or don't
> integrate fixes at all.

I agree, backporting is more time-consuming (and energy-consuming) than
upgrading.

But, you don't have to keep up with the load.

We can only do our best, and it's important for each of us to find a
level of commitment that we are able to sustain.

When we compare Guix to other volunteer distros, there have been times
when Guix did more security updates than any other distro, and times
when we were average, and then times when we were below average. At no
time did I perceive that it made a difference to how many people use
Guix.

Ultimately, I think the winning strategy is to work in a way that makes
it easy for other people to help. For example, by filing bug reports
about security updates being available, so that others can write the
patches. The idea is that, over time, we will build a team of people
writing security update patches.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Leo Famulari]

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

On Tue, Mar 23, 2021 at 03:38:02PM +0100, Léo Le Bouter wrote:
> For this, the problem is not grafting but that the replacement package
> definition has been made public, this is an "issue" (?) that is known
> and I try to not make replacement package definitions public now.

The replacement should be public in this case. We want people to get the
updated ImageMagick when they do `guix install imagemagick`.

> > Caeterum censeo:
> > The real fix is probably to do less grafts and more rebuilds...
> 
> Agreed, I would really like a security-updates branch for that, with
> which we can buffer changes waiting for substitutes and then merge with
> master, but I am afraid this enters in conflict with people not having
> lots of bandwidth to download a new world again through substitutes or
> very powerful machines for those who don't use substitutes.

We should definitely try to rebuild more often, but currently we are
only able to do this for x86_64 and i686. We also support ARM, and we
have to balance these different priorities somehow.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Mark H Weaver]

Hi Leo,

Leo Famulari <leo@famulari.name> writes:

> On Tue, Mar 23, 2021 at 03:38:02PM +0100, Léo Le Bouter wrote:
>> For this, the problem is not grafting but that the replacement package
>> definition has been made public, this is an "issue" (?) that is known
>> and I try to not make replacement package definitions public now.
>
> The replacement should be public in this case. We want people to get the
> updated ImageMagick when they do `guix install imagemagick`.

That should happen anyway, even without making the replacement package
public.  I certainly *hope* that's what happens.  If not, that's a
serious security flaw in Guix.

Also, I'm not sure why you qualify your suggestion with "in this case".
What is it that distinguishes ImageMagick from, e.g. glib, for purposes
of this question?  Would it be any less bad for "guix install glib" to
install a glib with security flaws?

It would be good to reach agreement on whether replacement packages
should be made public.  I haven't thought much about it, so I don't know
what the relevant issues are.

      Regards,
        Mark

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

[Mark H Weaver]

Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> writes:

> Mark H Weaver <mhw@netris.org> skribis:
>
>> Andreas Enge <andreas@enge.fr> writes:
>>> And please let us agree that in the future, we only backport fixes in grafts
>>> and do not update version numbers.
>>
>> I agree that in this particular case, that's what should have been done,
>> and that we should still try to do it.  It will be several days at least
>> before I'm able to look at it, though.  Would someone else like to try?
>>
>> I also agree that in general, we should be more careful to check for ABI
>> compatibility before grafting.
>
> More specifically, for libraries, we should only ever use replacements
> that are ABI-compatible.  (That usually means that the major and minor
> version numbers are the same, which may be what Andreas had in mind.)
> Anything else is most likely broken.
>
> For command-line tools, I think we should only graft with replacements
> that are backward-compatible at the CLI level.  That requires manual
> review of the changes, I guess.

I wholeheartedly agree.

      Thanks,
        Mark

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

[Mark H Weaver]

Hi Andreas,

Andreas Enge <andreas@enge.fr> writes:

> these are very good arguments, which I understand and share. But moving
> to another version is problematic even when there is no soname bump, as
> I wrote in my bug report https://issues.guix.gnu.org/47315; grafts with
> different version numbers lead to a command line behaviour that is not
> understandable:
>
> $ guix package -A imagemagick
> imagemagick	6.9.12-2g	out,doc	gnu/packages/imagemagick.scm:132:2
> imagemagick	6.9.11-48	out,doc	gnu/packages/imagemagick.scm:48:2
>
> $ guix build imagemagick@6.9.11
> guix build: error: imagemagick: package not found for version 6.9.11
>
> $ guix build imagemagick@6.9.11-48
> /gnu/store/c30y49vg735g6b4hh590zrc9fmvcsy0w-imagemagick-6.9.12-2g-doc
> /gnu/store/l3hr0fimip6v7vmkgxbqygsglxaxasy0-imagemagick-6.9.12-2g
>
> From a user's perspective, inkscape@6.9.11 is at the time there and not
> there; it is shown by "guix package", but then not accessible for install-
> ation, but silently "glossed over" in favour of a different version.
[...]
> Otherwise said, grafting to different versions breaks our semantic for
> designating packages, in which version numbers play an important role,
> and replaces it by a mess which even with the examples above I have a
> hard time understanding.

To my mind this suggests a bug, or at least suboptimal behavior, in
"guix package".  I don't think it's appropriate to set grafting policy
to work around it.

How about changing "guix package -A" and "guix package -s" to display
information about the package's replacement, if it has one?

Alternatively, those commands could somehow explicitly indicate that the
package has been grafted, and show the version number of the
replacement, in such a way that is less confusing to users.

What do you think?

     Regards,
       Mark

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

[Leo Famulari]

On Tue, Mar 23, 2021 at 07:05:42PM -0400, Mark H Weaver wrote:
> Also, I'm not sure why you qualify your suggestion with "in this case".
> What is it that distinguishes ImageMagick from, e.g. glib, for purposes
> of this question?  Would it be any less bad for "guix install glib" to
> install a glib with security flaws?

I forgot the reason that end-user applications should have public
replacements, and why it's less important for the replacements of
libraries to be public.

It's about the Guix user interface, that is, `guix show` and `guix
search`.

`guix show gnutls` won't show a meaningful result for a gnutls/fixed
replacement that cherry-picks some patches. Everything is the same about
the replacement package, except some very narrow bug fixing.

But `guix show imagemagick` will show the new version, available as a
replacement, in its results, and users should see it in the UI.

> It would be good to reach agreement on whether replacement packages
> should be made public.  I haven't thought much about it, so I don't know
> what the relevant issues are.

Based on those examples, I'd suggest that replacements that update the
package's version should be public.

It's been suggested before that all the package variables should be
publicly exported, but using the hidden-package procedure. I don't
remember the exact reason.

Sorry for the unreliable communication!

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

[zimoun]

Hi Mark,

On Tue, 23 Mar 2021 at 19:42, Mark H Weaver <mhw@netris.org> wrote:
> Andreas Enge <andreas@enge.fr> writes:

>> $ guix package -A imagemagick
>> imagemagick	6.9.12-2g	out,doc	gnu/packages/imagemagick.scm:132:2
>> imagemagick	6.9.11-48	out,doc	gnu/packages/imagemagick.scm:48:2
>>
>> $ guix build imagemagick@6.9.11
>> guix build: error: imagemagick: package not found for version 6.9.11
>>
>> $ guix build imagemagick@6.9.11-48
>> /gnu/store/c30y49vg735g6b4hh590zrc9fmvcsy0w-imagemagick-6.9.12-2g-doc
>> /gnu/store/l3hr0fimip6v7vmkgxbqygsglxaxasy0-imagemagick-6.9.12-2g

Here, there are several points.

--8<---------------cut here---------------start------------->8---
$ guix search imagemagick | recsel -p name,version | head -5
name: imagemagick
version: 6.9.12-2g

name: imagemagick
version: 6.9.11-48

$ guix build imagemagick
/gnu/store/b91y6ji9ypx8abk00jd33jglxbnjq4dy-imagemagick-6.9.12-2g-doc
/gnu/store/l0asah1mggmgli85sp673bnp2yc71g0j-imagemagick-6.9.12-2g
--8<---------------cut here---------------end--------------->8---

All is fine, security speaking.


Then, there is a bug on how Guix handles the version.  Well, 6.9.11
should be understood as 6.9.11-48 and it seems not:

--8<---------------cut here---------------start------------->8---
$ guix build imagemagick@6.9.11
guix build: error: imagemagick: package not found for version 6.9.11

$ guix build imagemagick@6.9.11-48
/gnu/store/b91y6ji9ypx8abk00jd33jglxbnjq4dy-imagemagick-6.9.12-2g-doc
/gnu/store/l0asah1mggmgli85sp673bnp2yc71g0j-imagemagick-6.9.12-2g
--8<---------------cut here---------------end--------------->8---

…but all is fine security speaking.  And the --no-grafts allows to get
the so-well named option. :-)

--8<---------------cut here---------------start------------->8---
$ guix build imagemagick@6.9.11-48 --no-grafts
/gnu/store/wlnciwhn6llwqwywf4hq739v5bbcrq3h-imagemagick-6.9.11-48-doc
/gnu/store/vlix7fclb7ifjgmxgpwr1pvraff89w7b-imagemagick-6.9.11-48
--8<---------------cut here---------------end--------------->8---

However, I am confused by,

--8<---------------cut here---------------start------------->8---
$ guix build imagemagick --no-grafts
/gnu/store/aby97j4d27zm6ilpcqrdm1lcw34xhcpj-imagemagick-6.9.12-2g-doc
/gnu/store/mzlng0n9s811abilzffa3v6pslv184yj-imagemagick-6.9.12-2g
--8<---------------cut here---------------end--------------->8---


> To my mind this suggests a bug, or at least suboptimal behavior, in
> "guix package".  I don't think it's appropriate to set grafting policy
> to work around it.

If I understand correctly all that, I agree with the 2 parts from your
comment made elsewhere:

      It would be good to reach agreement on whether replacement packages
      should be made public.  I haven't thought much about it, so I don't know
      what the relevant issues are.

<https://yhetil.org/guix/877dlxjwri.fsf@netris.org>


> How about changing "guix package -A" and "guix package -s" to display
> information about the package's replacement, if it has one?

The real question seems about ’replacement’ with upgraded version,
because otherwise it is already the expected behaviour, I mean the
behaviour that I personally expect. :-)

--8<---------------cut here---------------start------------->8---
$ guix build python
/gnu/store/wbci0x7f1q3k8rrc8d5qcckh59vl5zld-python-3.8.2
/gnu/store/r9f2bbavkbj17h2djjild5v5rd6yymcv-python-3.8.2-tk

$ guix build python --no-grafts
/gnu/store/rz42ba0my9vrgbkjpkzr2drmnjk5ah50-python-3.8.2
/gnu/store/r61fm38x5zwfaval9nf7ax960qmgsixf-python-3.8.2-tk

$ guix show python | recsel -p name,version
name: python
version: 3.8.2
outputs: out tk
--8<---------------cut here---------------end--------------->8---

> Alternatively, those commands could somehow explicitly indicate that the
> package has been grafted, and show the version number of the
> replacement, in such a way that is less confusing to users.

That’s the issue when updating a package using ’replacement’.  Is the
replaced package still visible at the CLI level?  If yes, we could try
to indicate something like:

--8<---------------cut here---------------start------------->8---
$ guix search imagemagick | recsel -p name,version | head -5
name: imagemagick
version: 6.9.12-2g

name: imagemagick
version: 6.9.11-48 (grafted by 6.9.12-2g)
--8<---------------cut here---------------end--------------->8---

but I do not know if it is possible / easy.  And even, there are still
clumsy behaviour as pointer above.


Cheers,
simon

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

[Andreas Enge]

Hello Mark,

Am Tue, Mar 23, 2021 at 07:42:48PM -0400 schrieb Mark H Weaver:
> To my mind this suggests a bug, or at least suboptimal behavior, in
> "guix package".  I don't think it's appropriate to set grafting policy
> to work around it.
> 
> How about changing "guix package -A" and "guix package -s" to display
> information about the package's replacement, if it has one?

I am not familiar enough with grafts and replacements to judge whether
this is a reasonable approach or whether it just adds more duct tape on
top of duct tape... Note that everything would have to be consistent,
from "guix package" to "guix build". And the "--no-grafts" also behaves
strangely, where it is not even clear what the optimal behaviour should be.
I still think it is a situation we should try to avoid as much as possible,
but maybe it is not worth to spend too much time fixing it.

Andreas