After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 781645 - Skip GSettings schema compile and icon cache update when DESTDIR is set
Skip GSettings schema compile and icon cache update when DESTDIR is set
Status: RESOLVED FIXED
Product: evolution
Classification: Applications
Component: general
3.24.x (obsolete)
Other Linux
: Normal critical
: ---
Assigned To: Evolution Shell Maintainers Team
Evolution QA team
Depends on:
Blocks:
 
 
Reported: 2017-04-24 01:03 UTC by Jeremy Bicha
Modified: 2017-10-02 18:38 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Jeremy Bicha 2017-04-24 01:03:52 UTC
evolution 3.24.1 on Fedora and Ubuntu

There are a very large number of errors in the build log like this:

Error copying file (if different) from "/builddir/build/BUILD/evolution-3.24.1/data/icons/hicolor_apps_16x16_im-msn.png" to "/usr/share/evolution/icons/hicolor/16x16/apps/im-msn.png".

I checked the resulting binary packages and those icons are not installed.

By the way, I saw similar errors for the gsettings schemas, but those do look like they are installed.

Error copying file (if different) from "/builddir/build/BUILD/evolution-3.24.1/_build/data/org.gnome.evolution.calendar.gschema.xml" to "/usr/share/glib-2.0/schemas/".

Full build logs
---------------
https://kojipkgs.fedoraproject.org//packages/evolution/3.24.1/1.fc27/data/logs/x86_64/build.log

https://launchpadlibrarian.net/316746199/buildlog_ubuntu-artful-amd64.evolution_3.24.1-0ubuntu0~artful1_BUILDING.txt.gz
Comment 1 Milan Crha 2017-04-25 16:39:07 UTC
Thanks for a bug report. I didn't notice it myself when the switch to CMake happened. It looks like that the custom scripts do not respect DESTDIR variable provided into "make install", while other parts do respect it. The evolution-data-server is also affected, at least with the schema files. Both evolution-ews and evolution-mapi are fine, according to Fedora logs.

Nonetheless, the packages are complete, there is not missing any file in them. The reason is a bit hacky way to allow schema files compilation and regeneration of icon cache, due to not "after install" hook in CMake. The scripts install those affected files twice, once through the standard CMake way, which respect DESTDIR and is there to support 'make uninstall' as well, the other time manually, where the "if different" notice beside the error in the 'make install' log takes into effect. The manual install is there to make sure that the schema compilation and icon cache update have all files in the correct places before invoked. CMake doesn't regenerate install scripts before 'make install', which is fine and expected, from my point of view.

These errors are harmless, which is the reason why CMake didn't stop on them, I believe.
Comment 2 Jeremy Bicha 2017-04-25 16:48:00 UTC
Ok, I see that the icons are still installed by looking at the Ubuntu build logs (the koji one confuses me since I'm less familiar with it).

Is there something you can do in the evolution source to not have those harmless errors then?
Comment 3 Milan Crha 2017-04-26 10:05:57 UTC
The sources can be configured with -DENABLE_SCHEMAS_COMPILE=OFF, which will avoid warnings on shema files install (both for eds and evo), but there is no such option for icons, it either can find gtk-update-icon-cache program, in which case it also calls these bits which produce the error message, or it cannot find that program and the icon cache compilation is skipped.

Maybe I could add ENABLE_ICON_CACHE_UPDATE option. What do you think?
Comment 4 Jeremy Bicha 2017-04-26 10:29:27 UTC
> What do you think?

I'm sorry I didn't understand your explanation of why the icon cache and gsettings schema compilation need to be run twice by default.
Comment 5 Milan Crha 2017-04-26 13:09:08 UTC
Because CMake has no "after install" hook and custom commands added to install target run in semi-random order, usually before those files are installed, not after, thus that double-install files is there to ensure that the schema compilation and icon cache update is called when all the files are there for sure. Not compiling schemas and not updating icon cache will/would avoid that double-installation and thus the error as well. It can be avoided when building a package, but it should not be avoided when installing on the target machine.
Comment 6 Sobhan Mohammadpour 2017-09-27 17:25:56 UTC
(In reply to Milan Crha from comment #1)
> Thanks for a bug report. I didn't notice it myself when the switch to CMake
> happened. It looks like that the custom scripts do not respect DESTDIR
> variable provided into "make install", while other parts do respect it. The
> evolution-data-server is also affected, at least with the schema files. Both
> evolution-ews and evolution-mapi are fine, according to Fedora logs.
> 
> Nonetheless, the packages are complete, there is not missing any file in
> them. The reason is a bit hacky way to allow schema files compilation and
> regeneration of icon cache, due to not "after install" hook in CMake. The
> scripts install those affected files twice, once through the standard CMake
> way, which respect DESTDIR and is there to support 'make uninstall' as well,
> the other time manually, where the "if different" notice beside the error in
> the 'make install' log takes into effect. The manual install is there to
> make sure that the schema compilation and icon cache update have all files
> in the correct places before invoked. CMake doesn't regenerate install
> scripts before 'make install', which is fine and expected, from my point of
> view.
> 
> These errors are harmless, which is the reason why CMake didn't stop on
> them, I believe.

https://bugs.gentoo.org/628460
looks related...sandbox violations...
these copy if different lines in cmake/modules/iconcache are not very sandbox friendly they do replace gtk-update-icon-cache with /bin/true tho...
Comment 7 Milan Crha 2017-09-29 07:26:47 UTC
(In reply to Sobhan Mohammadpour from comment #6)
> https://bugs.gentoo.org/628460

David Seifert's comment there, #17, is very "nice" and "positive".

> looks related...sandbox violations...

What is sandbox in this context, please? Note I have no single idea of Gentoo.

> these copy if different lines in cmake/modules/iconcache are not very
> sandbox friendly they do replace gtk-update-icon-cache with /bin/true tho...

The above sentence confuses me. Who 'they' is it about?

When making the CMake scripts, I tried to stay as close to autotools behaviour as possible. It seemed to me that autotools always rebuilds the icon cache. If I overlooked the fact that it does so only if DESTDIR is not defined, then it's my fault (caused by not using DESTDIR here at all). On the other hand, this behaviour feels hidden, thus I suggested comment #3, which makes things explicit and icon cache regeneration tight to a variable the same as GSettings scheme regeneration is. Eventually the GSettings scheme regeneration can be also tight to the DESTDIR existence, in addition to the variable (the inconsistency makes just trouble, from my point of view).

Even this bug is closed, I'm still willing to do the change, I only want a confirmation from you that it's the right way to do it and that it'll work for you. And if you prefer to tight the icon cache regeneration on the DESTDIR existence, then it's also fine. Just let me know and I'll attach a patch to test by you.
Comment 8 Milan Crha 2017-10-02 16:52:28 UTC
Looking more closely into the behaviour of autotools scripts (and hooks) I see that both GSettings schemas compilation and icon cache update are skipped automatically when DESTDIR is set. I agree with you that this behaviour should be kept, thus I'll make the change where needed.
Comment 9 Milan Crha 2017-10-02 17:36:59 UTC
Created commit_67f9cbdb3a in eds master (3.27.1+) [1]
Created commit_a9f72bd18c in evo master (3.27.1+) [2]
Created commit_62027ac9c9 in ews master (3.27.1+) [3]
Created commit_0ce3baadd0 in ema master (3.27.1+) [4]

Created commit_5406fcda08 in eds gnome-3-26 (3.26.2+)
Created commit_b40001cac7 in evo gnome-3-26 (3.26.2+)
Created commit_bdc3319e90 in ews gnome-3-26 (3.26.2+)
Created commit_0588464d30 in ema gnome-3-26 (3.26.2+)

[1] https://git.gnome.org/browse/evolution-data-server/commit/?id=67f9cbdb3a
[2] https://git.gnome.org/browse/evolution/commit/?id=a9f72bd18c
[3] https://git.gnome.org/browse/evolution-ews/commit/?id=62027ac9c9
[4] https://git.gnome.org/browse/evolution-mapi/commit/?id=0ce3baadd0
Comment 10 Sobhan Mohammadpour 2017-10-02 18:36:35 UTC
Thank you very much
Comment 11 Sobhan Mohammadpour 2017-10-02 18:38:16 UTC
(In reply to Milan Crha from comment #7)
> (In reply to Sobhan Mohammadpour from comment #6)
> > https://bugs.gentoo.org/628460
> 
> David Seifert's comment there, #17, is very "nice" and "positive".
> 
> > looks related...sandbox violations...
> 
> What is sandbox in this context, please? Note I have no single idea of
> Gentoo.
> 
> > these copy if different lines in cmake/modules/iconcache are not very
> > sandbox friendly they do replace gtk-update-icon-cache with /bin/true tho...
> 
> The above sentence confuses me. Who 'they' is it about?
> 
> When making the CMake scripts, I tried to stay as close to autotools
> behaviour as possible. It seemed to me that autotools always rebuilds the
> icon cache. If I overlooked the fact that it does so only if DESTDIR is not
> defined, then it's my fault (caused by not using DESTDIR here at all). On
> the other hand, this behaviour feels hidden, thus I suggested comment #3,
> which makes things explicit and icon cache regeneration tight to a variable
> the same as GSettings scheme regeneration is. Eventually the GSettings
> scheme regeneration can be also tight to the DESTDIR existence, in addition
> to the variable (the inconsistency makes just trouble, from my point of
> view).
> 
> Even this bug is closed, I'm still willing to do the change, I only want a
> confirmation from you that it's the right way to do it and that it'll work
> for you. And if you prefer to tight the icon cache regeneration on the
> DESTDIR existence, then it's also fine. Just let me know and I'll attach a
> patch to test by you.

It expects that the package does not try to change any thing outside the directory that it's being built