bug#47479: inkscape retains a reference to imagemagick, even though it is in native-inputs

bug#47479: inkscape retains a reference to imagemagick, even though it is in native-inputs

[Maxime Devos]

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

Hi Guix,

On
$ guix --version
> guix (GNU Guix) 510e24f973a918391d8122fd6ad515c0567bf23e

with
$ guix graph --type=references inkscape

it can be seen inkscape retains the reference to imagemagick,
even though imagemagick is in native-inputs.

Greetings,
Maxime.

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

bug#47479: inkscape retains a reference to imagemagick, even though it is in native-inputs

[Mark H Weaver]

Hi Maxime,

Maxime Devos <maximedevos@telenet.be> writes:

> On
> $ guix --version
>> guix (GNU Guix) 510e24f973a918391d8122fd6ad515c0567bf23e
>
> with
> $ guix graph --type=references inkscape
>
> it can be seen inkscape retains the reference to imagemagick,
> even though imagemagick is in native-inputs.

I believe this is incorrect.  On my Guix system, I see this:

--8<---------------cut here---------------start------------->8---
mhw@jojen ~$ guix package -I inkscape
inkscape	0.92.4	out	/gnu/store/rx26f9xn14fng2gnzsigr74492lrkydl-inkscape-0.92.4
mhw@jojen ~$ guix gc -R /gnu/store/rx26f9xn14fng2gnzsigr74492lrkydl-inkscape-0.92.4 | grep -i imagemagick
--8<---------------cut here---------------end--------------->8---

I guess that "guix graph --type=references" is not a reliable indicator
of what we need to check for.

FYI, this is what I do to check that my system and user profile do not
have references to imagemagick:

--8<---------------cut here---------------start------------->8---
mhw@jojen ~$ guix gc -R $(readlink /run/current-system) | grep -i imagemagick
mhw@jojen ~$ guix gc -R $(readlink -f ~/.guix-profile) | grep -i imagemagick
--8<---------------cut here---------------end--------------->8---

    Regards,
      Mark

bug#47479: inkscape retains a reference to imagemagick, even though it is in native-inputs

[Efraim Flashner]

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

On Mon, Mar 29, 2021 at 05:16:01PM -0400, Mark H Weaver wrote:
> Hi Maxime,
> 
> Maxime Devos <maximedevos@telenet.be> writes:
> 
> > On
> > $ guix --version
> >> guix (GNU Guix) 510e24f973a918391d8122fd6ad515c0567bf23e
> >
> > with
> > $ guix graph --type=references inkscape
> >
> > it can be seen inkscape retains the reference to imagemagick,
> > even though imagemagick is in native-inputs.
> 
> I believe this is incorrect.  On my Guix system, I see this:
> 
> --8<---------------cut here---------------start------------->8---
> mhw@jojen ~$ guix package -I inkscape
> inkscape	0.92.4	out	/gnu/store/rx26f9xn14fng2gnzsigr74492lrkydl-inkscape-0.92.4
> mhw@jojen ~$ guix gc -R /gnu/store/rx26f9xn14fng2gnzsigr74492lrkydl-inkscape-0.92.4 | grep -i imagemagick
> --8<---------------cut here---------------end--------------->8---

It is the case for inkscape@1.0.2

(ins)efraim@3900XT ~$ guix package -A inkscape
inkscape        1.0.2   out     gnu/packages/inkscape.scm:121:2
inkscape        0.92.4  out     gnu/packages/inkscape.scm:56:2
(ins)efraim@3900XT ~$ guix gc --references $(guix build inkscape@1) | grep -i imagemagick
/gnu/store/y4rym48vihmh3jk9qnll0zqfnxzi9n6v-imagemagick-6.9.12-4

I believe it comes from the glib-or-gtk-wrap phase where it wraps the
XDG_DATA_DIRS and likely grabs more than it needs.

> I guess that "guix graph --type=references" is not a reliable indicator
> of what we need to check for.
> 
> FYI, this is what I do to check that my system and user profile do not
> have references to imagemagick:
> 
> --8<---------------cut here---------------start------------->8---
> mhw@jojen ~$ guix gc -R $(readlink /run/current-system) | grep -i imagemagick
> mhw@jojen ~$ guix gc -R $(readlink -f ~/.guix-profile) | grep -i imagemagick
> --8<---------------cut here---------------end--------------->8---
> 
>     Regards,
>       Mark
> 
> 
> 

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

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

bug#47479: inkscape retains a reference to imagemagick, even though it is in native-inputs

[Mark H Weaver]

Hi Efraim,

Efraim Flashner <efraim@flashner.co.il> writes:
> It is the case for inkscape@1.0.2

I see now that I'm using an older version, although I would have
preferred the newer one.  I refer to the variable name 'inkscape' from
my manifest file, and I expected that to point to the latest stable
version.  However, it seems that one must use the 'inkscape-1.0'
variable to get the latest stable version.  That's seems suboptimal.

I wonder if the 'inkscape' variable should be renamed 'inkscape/stable'
(for use in packages such as 'dblatex/stable'), and then 'inkscape'
could be repurposed to point to the latest stable version.  Thoughts?

> (ins)efraim@3900XT ~$ guix package -A inkscape
> inkscape        1.0.2   out     gnu/packages/inkscape.scm:121:2
> inkscape        0.92.4  out     gnu/packages/inkscape.scm:56:2
> (ins)efraim@3900XT ~$ guix gc --references $(guix build inkscape@1) | grep -i imagemagick
> /gnu/store/y4rym48vihmh3jk9qnll0zqfnxzi9n6v-imagemagick-6.9.12-4
>
> I believe it comes from the glib-or-gtk-wrap phase where it wraps the
> XDG_DATA_DIRS and likely grabs more than it needs.

Good catch!

So, for now, we shouldn't use 'imagemagick/stable' in 'inkscape', even
though it's in 'native-inputs'.  This shows that we'll have to be very
careful about this, at least until we have better tooling to catch these
problems automatically.

I think the fundamental problem here is that the build-side code cannot
distinguish between 'inputs' and 'native-inputs' unless you are
cross-compiling.  When compiling natively, the 'inputs' keyword argument
passed to the build phases includes the packages listed in the
'native-inputs' package field, and the 'native-inputs' keyword argument
is not passed at all.

This is why we must write (or native-inputs inputs) in so many places:
because to find the location of a package listed in the 'native-inputs'
package field from within build-side code, you must look in one of two
places depending on whether you're cross-compiling.  If you're
cross-compiling you must look in 'native-inputs'.  When compiling
natively, that argument doesn't even exist, and you must look in the
'inputs' keyword argument instead.

I don't know why it was done this way.  It seems to me an error-prone
design, but at this point it would be hard to change it.

Now we see another disadvantage.  The 'glib-or-gtk-wrap' phase should be
iterating over the 'inputs' but not the 'native-inputs'.  It's not
obvious how to fix this given the current design.

What do you think?

    Regards,
      Mark

bug#47479: inkscape retains a reference to imagemagick, even though it is in native-inputs

[Efraim Flashner]

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

On Tue, Mar 30, 2021 at 04:55:13AM -0400, Mark H Weaver wrote:
> Hi Efraim,
> 
> Efraim Flashner <efraim@flashner.co.il> writes:
> > It is the case for inkscape@1.0.2
> 
> I see now that I'm using an older version, although I would have
> preferred the newer one.  I refer to the variable name 'inkscape' from
> my manifest file, and I expected that to point to the latest stable
> version.  However, it seems that one must use the 'inkscape-1.0'
> variable to get the latest stable version.  That's seems suboptimal.
> 
> I wonder if the 'inkscape' variable should be renamed 'inkscape/stable'
> (for use in packages such as 'dblatex/stable'), and then 'inkscape'
> could be repurposed to point to the latest stable version.  Thoughts?

In the past we've kept the most up-to-date version as the 'main version'
and given the version suffix to the older version(s). Except for when
they all get version suffixes and then a separate (define rust
rust-1.45) package added.

> 
> > (ins)efraim@3900XT ~$ guix package -A inkscape
> > inkscape        1.0.2   out     gnu/packages/inkscape.scm:121:2
> > inkscape        0.92.4  out     gnu/packages/inkscape.scm:56:2
> > (ins)efraim@3900XT ~$ guix gc --references $(guix build inkscape@1) | grep -i imagemagick
> > /gnu/store/y4rym48vihmh3jk9qnll0zqfnxzi9n6v-imagemagick-6.9.12-4
> >
> > I believe it comes from the glib-or-gtk-wrap phase where it wraps the
> > XDG_DATA_DIRS and likely grabs more than it needs.
> 
> Good catch!
> 
> So, for now, we shouldn't use 'imagemagick/stable' in 'inkscape', even
> though it's in 'native-inputs'.  This shows that we'll have to be very
> careful about this, at least until we have better tooling to catch these
> problems automatically.
> 
> I think the fundamental problem here is that the build-side code cannot
> distinguish between 'inputs' and 'native-inputs' unless you are
> cross-compiling.  When compiling natively, the 'inputs' keyword argument
> passed to the build phases includes the packages listed in the
> 'native-inputs' package field, and the 'native-inputs' keyword argument
> is not passed at all.

I ran into this also with the cargo-build-system. I only wanted to
propagate regular inputs, not native inputs. It's probably worth
figuring out how to fix some of it on core-updates.

> This is why we must write (or native-inputs inputs) in so many places:
> because to find the location of a package listed in the 'native-inputs'
> package field from within build-side code, you must look in one of two
> places depending on whether you're cross-compiling.  If you're
> cross-compiling you must look in 'native-inputs'.  When compiling
> natively, that argument doesn't even exist, and you must look in the
> 'inputs' keyword argument instead.
> 
> I don't know why it was done this way.  It seems to me an error-prone
> design, but at this point it would be hard to change it.
> 
> Now we see another disadvantage.  The 'glib-or-gtk-wrap' phase should be
> iterating over the 'inputs' but not the 'native-inputs'.  It's not
> obvious how to fix this given the current design.
> 
> What do you think?

We can always try to make it better. In the mean time perhaps we can try
changing the way some of the wrappers work to only accept regular
inputs, or possibly to specify exactly which inputs to iterate over and
to use in the wrap phases.

>     Regards,
>       Mark

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

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

bug#47479: inkscape retains a reference to imagemagick, even though it is in native-inputs

[Leo Famulari]

On Tue, Mar 30, 2021 at 04:55:13AM -0400, Mark H Weaver wrote:
> I see now that I'm using an older version, although I would have
> preferred the newer one.  I refer to the variable name 'inkscape' from
> my manifest file, and I expected that to point to the latest stable
> version.  However, it seems that one must use the 'inkscape-1.0'
> variable to get the latest stable version.  That's seems suboptimal.
> 
> I wonder if the 'inkscape' variable should be renamed 'inkscape/stable'
> (for use in packages such as 'dblatex/stable'), and then 'inkscape'
> could be repurposed to point to the latest stable version.  Thoughts?

I think we should do this, or even remove the old Inkscape package now.

I'm guessing the reason for keeping the older release series is that
the Inkscape save-file format changed?

By the way, there is a new upstream release of the old series available,
0.92.5.

bug#47479: inkscape retains a reference to imagemagick, even though it is in native-inputs

[Mark H Weaver]

Hi Leo,

Leo Famulari <leo@famulari.name> writes:

> On Tue, Mar 30, 2021 at 04:55:13AM -0400, Mark H Weaver wrote:
>> I wonder if the 'inkscape' variable should be renamed 'inkscape/stable'
>> (for use in packages such as 'dblatex/stable'), and then 'inkscape'
>> could be repurposed to point to the latest stable version.  Thoughts?
>
> I think we should do this,

Thanks.

> or even remove the old Inkscape package now.

I don't think we can remove the old Inkscape, because 'inkscape' is an
input of 'dblatex/stable', which 'gtk-doc/stable' depends on.

There might or might not be other reasons to keep an older version of
'inkscape' around, but either way, if 'gtk-doc/stable' depends on
Inkscape, I think we'd better have an 'inkscape/stable' too, so that our
'inkscape' package can be freely updated on 'master'.

Going forward, if it turns out that Inkscape is not truly needed as an
input to 'dblatex/stable', and moreover, if _nothing_ deep in our stack
truly needs to depend on Inkscape, then we could perhaps eliminate
'inkscape/stable' altogether.  However, that would need to happen on
either 'core-updates' or 'staging' (or similar), because changing
'dblatex/stable' entails too many rebuilds.

What do you think?

    Thanks,
      Mark

bug#47479: inkscape retains a reference to imagemagick, even though it is in native-inputs

[Leo Famulari]

On Tue, Mar 30, 2021 at 06:48:16PM -0400, Mark H Weaver wrote:
> I don't think we can remove the old Inkscape, because 'inkscape' is an
> input of 'dblatex/stable', which 'gtk-doc/stable' depends on.
> 
> There might or might not be other reasons to keep an older version of
> 'inkscape' around, but either way, if 'gtk-doc/stable' depends on
> Inkscape, I think we'd better have an 'inkscape/stable' too, so that our
> 'inkscape' package can be freely updated on 'master'.
> 
> Going forward, if it turns out that Inkscape is not truly needed as an
> input to 'dblatex/stable', and moreover, if _nothing_ deep in our stack
> truly needs to depend on Inkscape, then we could perhaps eliminate
> 'inkscape/stable' altogether.  However, that would need to happen on
> either 'core-updates' or 'staging' (or similar), because changing
> 'dblatex/stable' entails too many rebuilds.
> 
> What do you think?

I didn't realize / remember that Inkscape was used that deep in the
package graph. I agree, we should delay this change, at least until a
rebuild cycle.

I do think it's suboptimal that an end-user application like Inkscape
is depended on by so many packages...

bug#47479: inkscape retains a reference to imagemagick, even though it is in native-inputs

[Mark H Weaver]

Hi Leo,

Leo Famulari <leo@famulari.name> writes:
> I didn't realize / remember that Inkscape was used that deep in the
> package graph. I agree, we should delay this change, at least until a
> rebuild cycle.

The removal of inkscape@0.92.4 should certainly be delayed, but I see no
reason why we couldn't immediately, on 'master', rename the variable
'inkscape' to 'inkscape/stable', and 'inkscape-1.0' to 'inkscape', with
'inkscape-1.0' made an alias to 'inkscape', if we can agree on it.
Do you see a reason to delay those changes?

> I do think it's suboptimal that an end-user application like Inkscape
> is depended on by so many packages...

Indeed, it's not good.  In fact, the question just occurred to me:

 "How is it that Inkscape, which clearly depends on Gtk+, can also be a
  dependency of Gtk+, via the path gtk+ -> at-spi2-atk -> at-spi2-core
  -> gtk-doc -> dblatex -> inkscape@0.92.4?"

It turns out that the only reason there's no cycle here is because:

(1) the older inkscape@0.92.4 uses gtk+-2 (not 3), and
(2) none of the dependencies of gtk+-2 use gtk-doc.

Both of these are likely suboptimal, but we will apparently be blocked
from fixing these issues while Inkscape is needed to build our core
graphics stack.

In my opinion, the best way to fix this is to split off documentation
generation for selected core libraries into separate packages.
Generating documentation often requires higher-level components, and yet
we should also generate documentation for our core libraries.  This
naturally leads to cycles unless the documentation is split off.  We
should use the core libraries (without docs) to build the documentation
generators, and then from there build the documentation for the core
libraries.

What do you think?

Thanks for the discussion,

      Mark

bug#47479: inkscape retains a reference to imagemagick, even though it is in native-inputs

[Maxim Cournoyer]

Hi Leo et al.,

Leo Famulari <leo@famulari.name> writes:

> On Tue, Mar 30, 2021 at 04:55:13AM -0400, Mark H Weaver wrote:
>> I see now that I'm using an older version, although I would have
>> preferred the newer one.  I refer to the variable name 'inkscape' from
>> my manifest file, and I expected that to point to the latest stable
>> version.  However, it seems that one must use the 'inkscape-1.0'
>> variable to get the latest stable version.  That's seems suboptimal.
>> 
>> I wonder if the 'inkscape' variable should be renamed 'inkscape/stable'
>> (for use in packages such as 'dblatex/stable'), and then 'inkscape'
>> could be repurposed to point to the latest stable version.  Thoughts?
>
> I think we should do this, or even remove the old Inkscape package now.
>
> I'm guessing the reason for keeping the older release series is that
> the Inkscape save-file format changed?

The reason inksape@0.92 is still kept around is becauseInkscape@1
doesn't build on ARM (more accurately, one of its dependencies,
lib2geom, doesn't).  It's been a while since I looked at the issue, and
it seems there may have been some activity in lib2geom upstream to try
to address the problem, so we should revisit it.

Maxim

bug#47479: inkscape retains a reference to imagemagick, even though it is in native-inputs

[Maxim Cournoyer]

Hello,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Hi Leo et al.,
>
> Leo Famulari <leo@famulari.name> writes:
>
>> On Tue, Mar 30, 2021 at 04:55:13AM -0400, Mark H Weaver wrote:
>>> I see now that I'm using an older version, although I would have
>>> preferred the newer one.  I refer to the variable name 'inkscape' from
>>> my manifest file, and I expected that to point to the latest stable
>>> version.  However, it seems that one must use the 'inkscape-1.0'
>>> variable to get the latest stable version.  That's seems suboptimal.
>>> 
>>> I wonder if the 'inkscape' variable should be renamed 'inkscape/stable'
>>> (for use in packages such as 'dblatex/stable'), and then 'inkscape'
>>> could be repurposed to point to the latest stable version.  Thoughts?
>>
>> I think we should do this, or even remove the old Inkscape package now.
>>
>> I'm guessing the reason for keeping the older release series is that
>> the Inkscape save-file format changed?
>
> The reason inksape@0.92 is still kept around is becauseInkscape@1
> doesn't build on ARM (more accurately, one of its dependencies,
> lib2geom, doesn't).  It's been a while since I looked at the issue, and
> it seems there may have been some activity in lib2geom upstream to try
> to address the problem, so we should revisit it.

That's not relevant anymore, but our current inkscape 1.2 depends on
imagemagick still.  Seeing how it now links directly to it, I've added
it to inputs as well in commit 552ebc47af and pushed to core-updates.

Closing!

-- 
Thanks,
Maxim

bug#47479: inkscape retains a reference to imagemagick, even though it is in native-inputs

[Maxim Cournoyer]

Hello,

[...]

>>> On Tue, Mar 30, 2021 at 04:55:13AM -0400, Mark H Weaver wrote:
>>>> I see now that I'm using an older version, although I would have
>>>> preferred the newer one.  I refer to the variable name 'inkscape' from
>>>> my manifest file, and I expected that to point to the latest stable
>>>> version.  However, it seems that one must use the 'inkscape-1.0'
>>>> variable to get the latest stable version.  That's seems suboptimal.
>>>> 
>>>> I wonder if the 'inkscape' variable should be renamed 'inkscape/stable'
>>>> (for use in packages such as 'dblatex/stable'), and then 'inkscape'
>>>> could be repurposed to point to the latest stable version.  Thoughts?
>>>
>>> I think we should do this, or even remove the old Inkscape package now.
>>>
>>> I'm guessing the reason for keeping the older release series is that
>>> the Inkscape save-file format changed?
>>
>> The reason inksape@0.92 is still kept around is becauseInkscape@1
>> doesn't build on ARM (more accurately, one of its dependencies,
>> lib2geom, doesn't).  It's been a while since I looked at the issue, and
>> it seems there may have been some activity in lib2geom upstream to try
>> to address the problem, so we should revisit it.
>
> That's not relevant anymore, but our current inkscape 1.2 depends on
> imagemagick still.  Seeing how it now links directly to it, I've added
> it to inputs as well in commit 552ebc47af and pushed to core-updates.

I've applied some patches from Maxime and refined my understanding of
what this was about; it was not just about retaining a reference to
imagemagick listed from native-inputs, it was about retaining a
reference to imagemagick for the *stable* variant of inkscape/stable,
which meant we couldn't use the imagemagick/stable insecure variant.

Tentatively fixed in b4a6b1ba93844d7373c58237cb0b742352dec954 ("gnu:
inkscape/stable: Build stable variant without imagemagick support.")
which builds on a series from Maxime Devos.  I haven't caught up with
rebuilding core-updates yet to validate it truly works, but we'll see
soon.

Thanks, Maxime!

-- 
Maxim