GNOME Bugzilla – Bug 573525
Wrong shell to run config.status in Makefile.in.in
Last modified: 2009-03-01 09:03:55 UTC
Hi, config.status is generated based on the shell detected in configure and should be called with the CONFIG_SHELL instead of the default make SHELL (/bin/sh). This was discovered recently in gettext and fixed: http://lists.gnu.org/archive/html/bug-gnu-utils/2009-02/msg00054.html Original bug: https://bugs.launchpad.net/gtk/+bug/332840 Bye
Created attachment 129730 [details] [review] Proposed patch modeled on the gettext fix
Also, I find it more elegant to call config.status the way gettext does: Makefile: Makefile.in.in Makevars $(top_builddir)/config.status @POMAKEFILEDEPS@ cd $(top_builddir) \ && $(SHELL) ./config.status $(subdir)/$@.in po-directories and it has support for multiple po/ dirs -- which was the need of Gtk+ which triggered its own set of issues, see bug #573515. So I kind of wonder whether it makes sense to duplicate this Makefile.in.in infrastructure and whether it would be possible to move this part to upstream gettext nowadays?
I don't think that is the correct fix. We should just not be hardcoding /bin/sh here at all, and doing SHELL = @SHELL@ instead of SHELL = /bin/sh. Your patch 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. I'm committing a fix to SVN using SHELL = @SHELL@ and marking your patch as obsolete. Also, glib and gtk+ use the Makefile.in.in copied from glib-gettextize, which is a modified version of the original gettext Makefile.in.in as well, and does support multiple po directories. The intltool Makefile.in.in also supports multiple po directories already. I don't know that the pristine gettext version does though.
I trust the fix from upstream gettext and would recommend we avoid diverging further from their Makefile.in.in implementation. I suspect this is currently broken in intltool: stamp-it: Makefile.in.in $(top_builddir)/config.status POTFILES.in cd $(top_builddir) \ && CONFIG_FILES=$(subdir)/Makefile.in CONFIG_HEADERS= CONFIG_LINKS= \ $(SHELL) ./config.status instead you want something like: stamp-it: Makefile.in.in $(top_builddir)/config.status POTFILES.in cd $(top_builddir) \ && $(SHELL) ./config.status $(subdir)/Makefile.in I think this is a good example of why we should follow gettext instead of doing our own autofoo changes; we're not tracking subtle low level changes to autotools close enough to claim we will keep our files up-to-date IMHO.