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 573515 - po-properties/ special Makefile.in.in handling causes failure to build
po-properties/ special Makefile.in.in handling causes failure to build
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: .General
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2009-02-28 09:14 UTC by Loïc Minier
Modified: 2009-03-01 20:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Build log showing the problematic behavior (44.47 KB, text/plain)
2009-02-28 09:51 UTC, Loïc Minier
  Details
Proposed patch for the second part of the bug (1.11 KB, patch)
2009-02-28 09:54 UTC, Loïc Minier
none Details | Review
Proposed patch for the first half of the first part of the bug (542 bytes, patch)
2009-02-28 10:25 UTC, Loïc Minier
none Details | Review

Description Loïc Minier 2009-02-28 09:14:52 UTC
Hi,

Gtk+ was failing to build for me recently in an odd way, "libtool" was getting corrupted mid-build.  This turned out to be a combination of two things:
- a new class of bugs not limited to gtk+ affecting gettext's, intltool's, gnulib's, glib's Makefile.in.in and perhaps others -- the rule to rebuild po-properties/Makefile is broken in two ways:
  * config.status is called with make's SHELL (/bin/sh) while it's tailored to be run with another one (detected during configure, e.g. bash) -- I personally think we shouldn't use $(SHELL) to call config.status
  * config.status is called with no arguments and rebuilds all OUTPUT files, instead of the single needed one
- the po-properties copy-paste of glib-gettext's po/Makefile.in.in is incomplete: po/POTFILES is generated during configure by AM_GLIB_GNU_GETTEXT but po-properties/POTFILES is not

As a result of the second bug, it's very easy to trigger the bug in some SHELL combinations.

I initially reported this at https://bugs.launchpad.net/ubuntu/+source/gtk+2.0/+bug/332840 thinking it was some autoconf / dash regression, but I think this bug has been around for a while and I'm not sure why it was unnoticed for so long.

Bye
Comment 1 Loïc Minier 2009-02-28 09:51:06 UTC
Created attachment 129715 [details]
Build log showing the problematic behavior
Comment 2 Loïc Minier 2009-02-28 09:54:14 UTC
Created attachment 129716 [details] [review]
Proposed patch for the second part of the bug

This patch against SVN only makes sure po-properties/ is handled in the same way as AM_GLIB_GNU_GETTEXT handles po/ by generating POTFILES during configure.

I'll wait for the final fix on the gettext side to propose changes to the Makefile.in.in files in glib-gettext, intltool, and gtk+/po-properties.
Comment 3 Loïc Minier 2009-02-28 10:04:56 UTC
Hmm actually the gettext folks are really quick and proposed this change:
http://lists.gnu.org/archive/html/bug-gnu-utils/2009-02/msg00054.html
Comment 4 Loïc Minier 2009-02-28 10:25:58 UTC
Created attachment 129724 [details] [review]
Proposed patch for the first half of the first part of the bug

This is the fix for the most important issue in the first part of this bug: using the wrong shell to call config.status; it's the same change as the one merged in gettext.
Comment 5 Loïc Minier 2009-02-28 10:27:02 UTC
Fixing the seconf half of the first part of this bug is lower priority for me; it's just inefficient to call "config.status" on all files, but I guess it shouldn't break anything.

I'll look into fixing the other instances of this first.
Comment 6 Loïc Minier 2009-02-28 10:28:25 UTC
I tested each patch independently with a full rebuild of gtk+ and confirm that either of them fixes the build (but both need to be applied).
Comment 7 Rodney Dawes 2009-03-01 03:49:52 UTC
I think the patch to Makefile.in.in should change the SHELL = line to be SHELL = @SHELL@ instead of replacing the $(SHELL) usage with @SHELL@, which would leave an opening to similar problems if other things are called with $(SHELL) in the future. Studying the Makefile.in produced by automake for other directories, and the resulting Makefile from autoconf, it seems like SHELL = @SHELL@ and using $(SHELL) is the correct and consistent way to fix this.
Comment 8 Loïc Minier 2009-03-01 09:00:32 UTC
Well I think setting SHELL is more intrusive and some files are to be called with /bin/sh, not @SHELL@; config.status is very specific in this regard, as explained by the upstream gettext maintainer who fixed this issue:
http://lists.gnu.org/archive/html/bug-gnu-utils/2009-02/msg00054.html

I would personally recommend applying the gettext fix to not diverge further from gettext's Makefile.in.in; for instance this is now broken:
Makefile: Makefile.in.in ../config.status POTFILES
        cd .. \
          && CONFIG_FILES=$(subdir)/$@.in CONFIG_HEADERS= \
               $(SHELL) ./config.status

it wont update only po/Makefile, but will also update all other config.status files -- which is why this bug was originally triggered.  Would we have been using upstream gettext, we wouldn't have seen the bug at all.
Comment 9 Matthias Clasen 2009-03-01 19:18:35 UTC
> for instance this is now broken

That is not the current code.

All the other uses of $(SHELL) in po/Makefile are invocations of mkinstalldirs which are done with SHELL = @SHELL@ in all other Makefiles....
Comment 10 Loïc Minier 2009-03-01 20:44:09 UTC
Yeah, the example I gave is from glib-gettext which I think is where the gtk+ po-properties thingy was copied from; but po-properties is doing it right.