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 503907 - Add WAF support
Add WAF support
Status: RESOLVED FIXED
Product: jhbuild
Classification: Infrastructure
Component: general
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: Jhbuild maintainers
Jhbuild QA
Depends on:
Blocks:
 
 
Reported: 2007-12-16 19:46 UTC by Gustavo Carneiro
Modified: 2009-06-21 21:15 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (15.25 KB, patch)
2007-12-16 19:46 UTC, Gustavo Carneiro
reviewed Details | Review
patch v2 (14.99 KB, patch)
2007-12-16 21:02 UTC, Gustavo Carneiro
none Details | Review
patch (9.72 KB, patch)
2007-12-16 21:18 UTC, Gustavo Carneiro
none Details | Review
patch v3 (8.97 KB, patch)
2007-12-17 11:53 UTC, Gustavo Carneiro
needs-work Details | Review
patch v4 (9.45 KB, patch)
2007-12-17 15:22 UTC, Gustavo Carneiro
committed Details | Review
implement skip_configure() for waf modules (1.23 KB, patch)
2007-12-17 23:24 UTC, Gustavo Carneiro
committed Details | Review

Description Gustavo Carneiro 2007-12-16 19:46:22 UTC
I would like to add support for building gnome-python and gnome-python-desktop using WAF[1].

The patch that I will attach includes:

1. A 'scriptinstall' module that lets you install modules that require running a simple installation command instead of configure/make/make install.  I need this because waf itself is installed with one such command.

2. A 'waf' module type, like autotools but uses waf instead of autogen and make.

3. Patches boostrap.modules to checkout waf from svn and install it

4. Patches the gnome 2.22 module set to build gnome-python and gnome-python-desktop using waf instead of autotools (note, users may need a clean checkout before the waf build works).

[1] http://code.google.com/p/waf/
Comment 1 Gustavo Carneiro 2007-12-16 19:46:44 UTC
Created attachment 101072 [details] [review]
patch
Comment 2 Frederic Peters 2007-12-16 20:05:18 UTC
There have been changes in a recent commit to the do_checkout and
get_revision methods, could you look at [1] and port them to your patch ?

[1] http://svn.gnome.org/viewvc/jhbuild/trunk/jhbuild/modtypes/autotools.py?r1=1734&r2=1733&pathrev=1734

Thanks to a GHOP student JHBuild got its moduleset DTD updated to match current code, could you update it to match your changes ?

I am not quite motivated by <scriptinstall>, isn't it possible to ask WAF developers to provide a make target, or to use distutils ?
Comment 3 Gustavo Carneiro 2007-12-16 21:02:39 UTC
Created attachment 101077 [details] [review]
patch v2
Comment 4 Gustavo Carneiro 2007-12-16 21:07:08 UTC
(In reply to comment #2)
> Thanks to a GHOP student JHBuild got its moduleset DTD updated to match current
> code, could you update it to match your changes ?

Done
 
> I am not quite motivated by <scriptinstall>, isn't it possible to ask WAF
> developers to provide a make target, or to use distutils ?
 
I'm not thrilled about <scriptinstall> either, but to make do without it would require more than just a make target, it requires a ./configure that stores the --prefix somewhere and a 'make install' that installs it using the stored prefix.  The WAF maintainer does not seem to receptive to such changes.  How about adding more options to the <waf> module type, like <waf install-parameters='--prefix=%(prefix)s'> ... ?
Comment 5 Gustavo Carneiro 2007-12-16 21:18:40 UTC
Created attachment 101078 [details] [review]
patch

This new patch adds install-parameters option to the waf module type, and uses it to install waf itself.
Comment 6 Frederic Peters 2007-12-16 21:31:54 UTC
This is a kludge, I am currently looking at waf and I seriously think it wouldn't be that hard to add --prefix support to their minimal configure script, it would just have to copy parameters to the Makefile it creates:

--- configure
+++ configure
@@ -31,6 +31,7 @@
 #abs path
 WORKINGDIR=`pwd`
 cd $CUR_DIR
+ARGS="$*"
 
 # Checks for Python interpreter. Honours $PYTHON if set. Stores path to
 # interpreter in $PYTHON.
@@ -108,7 +109,7 @@
        @$WAF -p build
 
 install:
-       @$WAF install
+       @$WAF install $ARGS
 
 uninstall:
        @$WAF uninstall
<---- end of patch ---->

So the JHBuild can be kept as simple as in the first patch.

Would you mind proposing this to the WAF developers ?
Comment 7 Gustavo Carneiro 2007-12-16 21:52:48 UTC
Unfortunately that is not enough.  The autotools module type passes too many parameters:

*** Configuring waf *** [1/2]

./configure --prefix /opt/gnome-devel --libdir '${exec_prefix}/lib64'  --disable-static --enable-shared --enable-debug --disable-more-warnings --disable-gtk-doc --disable-maintainer-mode
Checking for Python			:  /opt/gnome-devel/bin/python
Checking for WAF			:  /home/gjc/projects/gnome/waf/waf
calling waf configure with parameters
------> Executing code from the top-level wscript <-----
Usage: waf [options] [commands ...]

* Main commands: configure build install clean dist distclean uninstall distcheck
* Example: ./waf build -j4

waf: error: no such option: --libdir

The explanation is that parameters like --libdir are not supported by default in waf; they are given by the 'gnome' waf module.  Anyway, the other parameters are not ignored by waf, as it uses the optparse python module which doesn't ignore unknown options.
Comment 8 Frederic Peters 2007-12-16 22:21:02 UTC
Ah, some sed/awk so it only keeps --prefix then.  I won't code this this evening though, but do you know if it will be accepted by WAF developers ?
Comment 9 Gustavo Carneiro 2007-12-17 00:31:23 UTC
To be honest, I really do not like so much filtering out unknown options...  I think it is one of the biggest misfeatures of configure scripts the fact that they blindly accept and ignore options that are not implemented.  I would not want to see WAF go down the same road.

I really like attachment 101078 [details] [review] better than this stuff.  In any case, I filed http://code.google.com/p/waf/issues/detail?id=71 to see how Thomas feels.
Comment 10 Frederic Peters 2007-12-17 10:05:41 UTC
Note I was just talking about that for their minimal configure script, not at all for the waf configure command.
Comment 11 Frederic Peters 2007-12-17 10:39:56 UTC
There is also the option of keeping a patch in jhbuild, like there is for some other modules.
Comment 12 Gustavo Carneiro 2007-12-17 11:09:37 UTC
Yes, but their minimal configure script is not really meant to configure WAF itself.  It is meant to ship with projects that want to provide a configure script and makefile for users used to those things, kind of like an autotools emulation layer.

The maintainer says:

"""
Ignoring command-line options is unacceptable (what if --profix is
passed instead of --prefix? continue quietly?)

Add a new wrapper in the utils folder (out of my sight and make certain
to name it with a funny name like jhbuild or alzheimer).
"""

So it would have to be done in another script, like an autogen.sh script that filters options and then calls configure.
Comment 13 Frederic Peters 2007-12-17 11:21:24 UTC
Oh, I thought their minimal configure script was just something they provided for themselves, not for other projects. (personal opinion: as an autotools emulation layer, I don't think it is worth it with such differences).

Note this feature of ignoring unknown options is definitely useful, this allows for us to give '--disable-gtk-doc' once and for all, and it will be accounted for in all modules using gtk-doc, and peacefully ignored by others.

Anyway, back to the tracks here and I certainly do not want to push waf in a direction they don't want, so I believe the course of action is now for JHBuild to get waf from a tarball and to apply a patch, the minimal patch necessary for it to install under $prefix, using the <autotools> module type.

I will tackle this if you do not want to do it (but it will probably have to wait for a few days), just tell me.
Comment 14 Gustavo Carneiro 2007-12-17 11:53:43 UTC
Created attachment 101118 [details] [review]
patch v3

I've added a autogen script to waf.  Now installing WAF is just a matter of:

<autotools id="waf" autogen-sh="utils/autogen.sh">
   <branch repo="waf.googlecode.com" module="trunk" checkoutdir="waf"/>
</autotools>

I also removed the install-parameters option, which is no longer needed.  Only waf-command is still there, because it is truly useful (some projects ship their own ./waf, other projects may ship ./waf-light plus wafadmin).
Comment 15 Frederic Peters 2007-12-17 12:23:42 UTC
Okay, great.  However I would still like waf downloaded as a tarball, please tell me when your autogen.sh change will be available in a tarball.

Some more comments on the patch itself then:

The call to register_lazy_module_type at the bottom of modtypes/__init__py. is not needed, calling register_module_type at the bottom of waf.py is enough.

It is best for the first parameter of buildscript.execute() to be a list, this prevents quoting errors; replace lines like:
 cmd = '%s check' % (self.waf_cmd)
 buildscript.execute(cmd, cwd=self.get_builddir(buildscript))
with:
 buildscript.execute([self.waf_cmd, 'check'], cwd=self.get_builddir(buildscript))

And don't forget to update modulesets.dtd.
Comment 16 Gustavo Carneiro 2007-12-17 12:44:21 UTC
(In reply to comment #15)
> Okay, great.  However I would still like waf downloaded as a tarball, please
> tell me when your autogen.sh change will be available in a tarball.

Does it really have to be a tarball? :(

I sometimes fix bugs in waf itself, and I can't wait for tarballs to be released ...  for instance, right now gnome-python-desktop does not work with the last released tarball of waf.

OTOH it gives some more stability guarantees, so I can understand that you want it.  I guess I'll need to persuade the admin to let me do tarball releases more often.. :P

> 
> Some more comments on the patch itself then:
> 
> The call to register_lazy_module_type at the bottom of modtypes/__init__py. is
> not needed, calling register_module_type at the bottom of waf.py is enough.
> 
> It is best for the first parameter of buildscript.execute() to be a list, this
> prevents quoting errors; replace lines like:
>  cmd = '%s check' % (self.waf_cmd)
>  buildscript.execute(cmd, cwd=self.get_builddir(buildscript))
> with:
>  buildscript.execute([self.waf_cmd, 'check'],
> cwd=self.get_builddir(buildscript))

Hm.. sure, totally agree.  But it was mostly copy pasted from the autotools module.. :)

> 
> And don't forget to update modulesets.dtd.

Will do.
Comment 17 Frederic Peters 2007-12-17 12:47:34 UTC
I ask for tarballs because I feel it is necessary if waf wants to be considered for use in more GNOME modules; I am fine keeping it as a subversion checkout for the moment.
Comment 18 Gustavo Carneiro 2007-12-17 14:40:15 UTC
Maintainer wants to fix some bugs before next release, mid january. :(

I was looking at doing a 1.3.0 tarball plus patch to add utils/autogen.sh and misc fixes.  Unfortunately patching does not make utils/autogen.sh executable.  What is the way forward, here?  Add support to <patch> for making it executable after patch?
Comment 19 Frederic Peters 2007-12-17 14:44:03 UTC
I think it is fine keeping it from subversion for a month, mid january is not that far :)
Comment 20 Gustavo Carneiro 2007-12-17 15:22:39 UTC
Created attachment 101126 [details] [review]
patch v4

In that case here's a new patch.  The DTD stuff I'm not 100% sure about, though.
Comment 21 Frederic Peters 2007-12-17 15:28:35 UTC
It's fine; it can be commited as is.  Thanks for your work on this issue!
Comment 22 Gustavo Carneiro 2007-12-17 15:59:25 UTC
Committed. Thanks.
So when waf 1.3.1 is released, we switch to a tarball...
Comment 23 Gustavo Carneiro 2007-12-17 23:23:18 UTC
Sorry, I forgot skip_configure..
Comment 24 Gustavo Carneiro 2007-12-17 23:24:21 UTC
Created attachment 101152 [details] [review]
implement skip_configure() for waf modules
Comment 25 Frederic Peters 2007-12-18 15:10:59 UTC
Fine (just remove the second blank line at the end)