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 653842 - DESTDIR usage breaks installation of Pango, gdk-pixbuf, rsvg, etc
DESTDIR usage breaks installation of Pango, gdk-pixbuf, rsvg, etc
Status: RESOLVED FIXED
Product: jhbuild
Classification: Infrastructure
Component: general
unspecified
Other Linux
: Normal blocker
: ---
Assigned To: Jhbuild maintainers
Jhbuild QA
: 653977 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2011-07-01 22:10 UTC by Owen Taylor
Modified: 2011-07-13 15:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add post-installation scripts (9.37 KB, patch)
2011-07-05 17:22 UTC, Colin Walters
none Details | Review
Add post-installation trigger scripts (9.45 KB, patch)
2011-07-05 18:21 UTC, Colin Walters
none Details | Review
triggers: Add pango module trigger (1.85 KB, patch)
2011-07-05 18:21 UTC, Colin Walters
none Details | Review
Add post-installation trigger scripts (9.85 KB, patch)
2011-07-05 23:43 UTC, Colin Walters
none Details | Review
pango.trigger: New trigger for pango modules (1.86 KB, patch)
2011-07-05 23:44 UTC, Colin Walters
none Details | Review
glib.trigger: New trigger for GSettings schemas (1.84 KB, patch)
2011-07-05 23:44 UTC, Colin Walters
none Details | Review
pango.trigger: New trigger for pango modules (1.87 KB, patch)
2011-07-06 00:05 UTC, Colin Walters
none Details | Review
gtk+.trigger: New gtk+ trigger for icon cache (1.86 KB, patch)
2011-07-06 00:09 UTC, Colin Walters
none Details | Review
Add post-installation trigger scripts (10.11 KB, patch)
2011-07-10 17:26 UTC, Colin Walters
committed Details | Review
pango.trigger: New trigger for pango modules (1.90 KB, patch)
2011-07-10 17:26 UTC, Colin Walters
committed Details | Review
glib.trigger: New trigger for GSettings schemas (1.88 KB, patch)
2011-07-10 17:26 UTC, Colin Walters
committed Details | Review
gtk+.trigger: New gtk+ trigger for icon cache (1.90 KB, patch)
2011-07-10 17:26 UTC, Colin Walters
committed Details | Review

Description Owen Taylor 2011-07-01 22:10:24 UTC
Excerpted from gdk-pixbuf Makefile.am:

RUN_QUERY_LOADER_TEST=test -z "$(DESTDIR)"

install-data-hook: install-ms-lib install-def-file
        @if $(RUN_QUERY_LOADER_TEST) ; then \
          $(mkinstalldirs) $(DESTDIR)$(libdir)/gdk-pixbuf-2.0/$(GDK_PIXBUF_BINARY_VERSION) ; \
          $(top_builddir)/gdk-pixbuf/gdk-pixbuf-query-loaders > $(DESTDIR)$(libdir)/gdk-pixbuf-2.0/$(GDK_PIXBUF_BINARY_VERSION)/loaders.cache ; \
        else \
          echo "***" ; \
          echo "*** Warning: loaders.cache not built" ; \
          echo "***" ; \
          echo "*** Generate this file manually on host" ; \
	  echo "*** system using gdk-pixbuf-query-loaders" ; \
          echo "***" ; \
        fi

Same exact thing occurs in Pango. In rsvg, you'll find:

install-data-hook:
        if test -z "$(DESTDIR)"; then \
	        $(mkinstalldirs) $(DESTDIR)$(gdk_pixbuf_binarydir) ; \
                $(GDK_PIXBUF_QUERYLOADERS) > $(DESTDIR)$(gdk_pixbuf_cache_file) ; \
	fi

In gnome-icon-theme-standard, we have:

install-data-hook:
        if test -z "$(DESTDIR)" ; then \
                $(GTK_UPDATE_ICON_CACHE) -q $(DESTDIR)$(themedir); \
        fi

Because it of course doesn't make sense to update the icon cache if we aren't putting anything there. Basically, the DESTDIR doesn't work without some sort of "post" mechanism to handle stuff that has to be done after putting files into their final location. Maybe this is just hardcoding a bunch of rules into the Python code for all the things that we want to install. Maybe it's:

 <trigger directory="<....>">
    <exec command="">
 </trigger>

rule in the moduleset. I don't know. Need to figure this or revert the DESTDIR stuff temporarily, since it's generating lots of obscure problems that most potential builders of GNOME aren't prepared to handle.
Comment 1 Owen Taylor 2011-07-05 13:49:21 UTC
*** Bug 653965 has been marked as a duplicate of this bug. ***
Comment 2 Colin Walters 2011-07-05 17:22:07 UTC
Created attachment 191332 [details] [review]
Add post-installation scripts

Several things in the GNOME stack have file-based caches, like
gdk-pixbuf's loaders.cache.  These need to be regenerated, and
historically that worked in jhbuild because these modules include
bits in their Makefile to run then relevant commands, if DESTDIR is
not set.

Package managers have post-installation scripts in packages, and OS
builders typically support this as well.

Jhbuild historically relied on the post-installation scripts in
modules, but in (see bug 647231) we switched to using DESTDIR for all
module types.  So we need to move closer to the package model here.

This patch adds a proof-of-concept post-installation script for
gdk-pixbuf loaders.
Comment 3 Colin Walters 2011-07-05 18:21:35 UTC
Created attachment 191349 [details] [review]
Add post-installation trigger scripts

Several things in the GNOME stack have file-based caches, like
gdk-pixbuf's loaders.cache.  These need to be regenerated, and
historically that worked in jhbuild because these modules include bits
in their Makefile to run then relevant commands, if DESTDIR is not
set.

Package managers have post-installation scripts in packages, and OS
builders typically support this as well.

Jhbuild historically relied on the post-installation scripts in
modules, but in (see bug 647231) we switched to using DESTDIR for all
module types.  So we need to move closer to the package model here.

We call them "triggers" as they're run after any kind of build
operation (including "uninstall").

This patch adds a proof-of-concept trigger script for gdk-pixbuf
loaders.
Comment 4 Colin Walters 2011-07-05 18:21:42 UTC
Created attachment 191350 [details] [review]
triggers: Add pango module trigger
Comment 5 Frederic Peters 2011-07-05 20:27:14 UTC
Does it really need to match on regexp? can't it match on directory/file? $libdir/gdk-pixbuf-2.0/2.10.0/loaders/ would be more readable imho.

Also I ponder mentioning required triggers in the moduleset, to avoid "your jhbuild was too old, please pull a new version to get new triggers"; just like modulesets are taken live from git.gnome.org nowadays.

Going that way, perhaps the trigger should be part of the moduleset itself, like adding such a snippet to the gdk-pixbuf module:
  <trigger filename="$libdir/gdk-pixbuf…"><command>gdk-pixbuf-query-loaders …</command></trigger>.
Comment 6 Colin Walters 2011-07-05 20:51:43 UTC
(In reply to comment #5)
> Does it really need to match on regexp? can't it match on directory/file?
> $libdir/gdk-pixbuf-2.0/2.10.0/loaders/ would be more readable imho.

Sure, I guess.  I'll keep the current REMatch key, and add a new LiteralMatch.

> Also I ponder mentioning required triggers in the moduleset, to avoid "your
> jhbuild was too old, please pull a new version to get new triggers"; just like
> modulesets are taken live from git.gnome.org nowadays.

Hmmmm.  I can't see us introducing modules which have triggers too often.  There are only ~3 in the stack right now.

> Going that way, perhaps the trigger should be part of the moduleset itself,
> like adding such a snippet to the gdk-pixbuf module:
>   <trigger filename="$libdir/gdk-pixbuf…"><command>gdk-pixbuf-query-loaders
> …</command></trigger>.

Urgh =/  Shell script in XML will get ugly (like redirections > and <).
Comment 7 Frederic Peters 2011-07-05 21:00:40 UTC
(In reply to comment #6)
> > Going that way, perhaps the trigger should be part of the moduleset itself,
> > like adding such a snippet to the gdk-pixbuf module:
> >   <trigger filename="$libdir/gdk-pixbuf…"><command>gdk-pixbuf-query-loaders
> > …</command></trigger>.
> 
> Urgh =/  Shell script in XML will get ugly (like redirections > and <).

Well, it's no worse than escaping dots in regular expressions :) and there's CDATA section. Really I'd like triggers not to be tied to jhbuild sources, like patches that can live next to a remote moduleset (so it may not be <command> but <script>triggers/gdk-pixbuf</script>, that would be found relatively to the moduleset).
Comment 8 Colin Walters 2011-07-05 23:43:52 UTC
Created attachment 191372 [details] [review]
Add post-installation trigger scripts

* Load from jhbuild install prefix (PKGDATADIR), not self.config.prefix
* Support LiteralMatch in addition to REMatch
Comment 9 Colin Walters 2011-07-05 23:44:02 UTC
Created attachment 191373 [details] [review]
pango.trigger: New trigger for pango modules

Reattaching for ordering
Comment 10 Colin Walters 2011-07-05 23:44:07 UTC
Created attachment 191374 [details] [review]
glib.trigger: New trigger for GSettings schemas
Comment 11 Colin Walters 2011-07-06 00:05:45 UTC
Created attachment 191376 [details] [review]
pango.trigger: New trigger for pango modules

Fix installation directory
Comment 12 Colin Walters 2011-07-06 00:09:51 UTC
Created attachment 191377 [details] [review]
gtk+.trigger: New gtk+ trigger for icon cache
Comment 13 Matthias Clasen 2011-07-06 01:38:34 UTC
Review of attachment 191372 [details] [review]:

::: triggers/README
@@ +5,3 @@
+A .trigger file is a set of regular expressions which are evaluated
+against files installed after any "make install DESTDIR=", combined
+with a shell script to run if any of them match.

Elsewhere you say that the triggers are also run on uninstall - what gives ?

Also, it would be good to be a bit more specific as to what the matches match against here - paths of files or directories that are created / deleted / modified ? any of the above, or just the first ? Is the literal match a substring match ? or a match thats restricted to path components ?

What exactly is JHBUILD_PREFIX set to ? the prefix where jhbuild is installed, or the installation prefix ?

Also, it is probably worth mentioning that a trigger for module A can be run for an install of module B - or is that not the case ? I think it would be required for this to make sense...
Comment 14 Matthias Clasen 2011-07-06 01:38:42 UTC
Review of attachment 191372 [details] [review]:

::: triggers/README
@@ +5,3 @@
+A .trigger file is a set of regular expressions which are evaluated
+against files installed after any "make install DESTDIR=", combined
+with a shell script to run if any of them match.

Elsewhere you say that the triggers are also run on uninstall - what gives ?

Also, it would be good to be a bit more specific as to what the matches match against here - paths of files or directories that are created / deleted / modified ? any of the above, or just the first ? Is the literal match a substring match ? or a match thats restricted to path components ?

What exactly is JHBUILD_PREFIX set to ? the prefix where jhbuild is installed, or the installation prefix ?

Also, it is probably worth mentioning that a trigger for module A can be run for an install of module B - or is that not the case ? I think it would be required for this to make sense...
Comment 15 Colin Walters 2011-07-06 01:57:47 UTC
(In reply to comment #14)
> 
> Elsewhere you say that the triggers are also run on uninstall - what gives ?

Well...they're more run on any change to the manifest which contains files that match the lines.

What we should probably change is on uninstall, look at both the *old* manifest and the *new* one.

And we want this anyways to implement the whole rationale for DESTDIR in the first place, which was to remove no-longer-shipped files on upgrades.

> Also, it would be good to be a bit more specific as to what the matches match
> against here - paths of files or directories that are created / deleted /
> modified ? any of the above, or just the first ? Is the literal match a
> substring match ? or a match thats restricted to path components ?

I'll do so.

> What exactly is JHBUILD_PREFIX set to ? the prefix where jhbuild is installed,
> or the installation prefix ?

That was added in bug 653048.
 
> Also, it is probably worth mentioning that a trigger for module A can be run
> for an install of module B - or is that not the case ? I think it would be
> required for this to make sense...

Yep, it is.
Comment 16 Colin Walters 2011-07-06 02:00:49 UTC
One issue matthias raised that is important to fix is the case of partial builds.  If say we're *not* building glib, we still need to run the *system* glib-compile-schemas if the module ships schemas.

I think the easiest way to handle this is to drop the current logic associating a trigger to a module.  Basically, we just:

test -x glib-compile-schemas && ...

So we transparently pick up the system one if we have it, otherwise we use the jhbuild one.

Note that we really should be adding dependencies in our moduleset where triggers may be used.  For example as matthias said, gnome-icon-theme doesn't depend on gtk+, but it probably should.
Comment 17 Matthias Clasen 2011-07-06 02:10:05 UTC
(In reply to comment #16)

> 
> test -x glib-compile-schemas && ...

Of course, this requires your shell fragment to be simplistic enough to be treated this way. Alternatively, you could add the moral equivalent of a TryExec key...
Comment 18 Owen Taylor 2011-07-06 17:05:01 UTC
*** Bug 653977 has been marked as a duplicate of this bug. ***
Comment 19 Matthias Clasen 2011-07-08 19:41:05 UTC
Colin, any update on this ?
Comment 20 Colin Walters 2011-07-10 17:26:01 UTC
Created attachment 191639 [details] [review]
Add post-installation trigger scripts

* Correctly run after each module build, instead of after a whole operation
* Support IfExecutable
* Update README a bit
Comment 21 Colin Walters 2011-07-10 17:26:13 UTC
Created attachment 191640 [details] [review]
pango.trigger: New trigger for pango modules

Use IfExecutable
Comment 22 Colin Walters 2011-07-10 17:26:19 UTC
Created attachment 191641 [details] [review]
glib.trigger: New trigger for GSettings schemas

Use IfExecutable
Comment 23 Colin Walters 2011-07-10 17:26:30 UTC
Created attachment 191642 [details] [review]
gtk+.trigger: New gtk+ trigger for icon cache

Use IfExecutable
Comment 24 Colin Walters 2011-07-12 21:23:33 UTC
Attachment 191639 [details] pushed as c350169 - Add post-installation trigger scripts
Attachment 191640 [details] pushed as d3809d1 - pango.trigger: New trigger for pango modules
Attachment 191641 [details] pushed as a79c568 - glib.trigger: New trigger for GSettings schemas
Attachment 191642 [details] pushed as aa84ad1 - gtk+.trigger: New gtk+ trigger for icon cache
Comment 25 Thomas Andersen 2011-07-12 23:09:56 UTC
I get a crash at buildscript.py:run_triggers because PKGDATADIR is None.
(with 'jhbuild buildone gnome-games')

  • File "/home/phomes/jhbuild/jhbuild/frontends/buildscript.py", line 213 in run_triggers
    all_triggers = trigger.load_all(os.path.join(PKGDATADIR, 'triggers'))
  • File "/usr/lib64/python2.7/posixpath.py", line 68 in join
    elif path == '' or path.endswith('/'):
AttributeError: 'NoneType' object has no attribute 'endswith'

Comment 26 Colin Walters 2011-07-13 15:02:40 UTC
(In reply to comment #25)
> I get a crash at buildscript.py:run_triggers because PKGDATADIR is None.
> (with 'jhbuild buildone gnome-games')

Fixed, but please open *new* bugs for things like this.  

See bug 654555.