GNOME Bugzilla – Bug 571591
[build] srcdir != builddir breaks
Last modified: 2015-02-07 16:59:27 UTC
Please describe the problem: girscanner can't reliably find config.py when building, because it's in the builddir, not the srcdir. When builddir == srcdir, then it's fine, but not for out-of-tree builds. Steps to reproduce: 1. cd gobject-introspection 2. mkdir build 3. ../autogen.sh 4. ./configure 5. make Actual results: ImportError Expected results: Does this happen every time? Other information:
My take on this is that if we're going to actually support srcdir != builddir, it needs to be the *default*, or otherwise it will keep regressing. If there's a way to make the autogoo enforce that it'd be great.
"make distcheck" does a srcdir != builddir build, so as long as you use that to build your release tarballs (which You Really Should), then it gets tested at least once a release
Yeah, but then the burden is on whoever is running "make distcheck" every few months to fix up everyone else's patches since the last time they ran it. The point is that the burden for these thing should be distributed across the people writing patches, at the time they're writing the patches, not on the single person making a release. In other words distcheck isn't useful.
If you use jhbuild and set buildroot (and maybe builddir_pattern) variables, you'll get non-srcdir build if the module hasn't been configured to disable it Maybe ask for introspection to be added to build.gnome.org ?
I also saw this bug. It's weird that adding $(top_builddir)/giscanner to PYTHONPATH in common.mk doesn't help.
Created attachment 165018 [details] [review] Move config.py.in to top level From my experiments, my hypothesis is that giscanner is already bound to $(top_srcdir)/giscanner in Python module table, and $(top_builddir)/giscanner is never searched. A weird fix worked for me: by changing "from .config import ..." to "from config import ..." (i.e. removing the dot before "config") in giscanner/transformer.py and adding $(top_builddir)/giscanner to sys.path (instead of just $(top_builddir)) in tools/g-ir-scanner.py.in. A further step is that, while we have config.h.in practice with C/C++, why not with Python? In the fix mentioned above, the "config" module is made non-submodule but still referenced under a module directory. And its contents look not specific to giscanner at all. Moving it to top level like config.h.in in C/C++ practice just makes sense to me. The summarized patch is attached. Please consider if it's appropriate.
Review of attachment 165018 [details] [review]: This is wrong, config needs to be accessed when gobject-introspection is installed. It should be placed inside the giscanner/ directory. There are two ways to solve this, 1) Modify giscanner.__path__ to also look inside the builddir 2) Copy over everything to to the builddir and make it take precedence over srcdir when importing giscanner.
(In reply to comment #7) > This is wrong, config needs to be accessed when gobject-introspection is > installed. > It should be placed inside the giscanner/ directory. Indeed. My build just worked by accident, due to the relic of the previous round. > There are two ways to solve this, > 1) Modify giscanner.__path__ to also look inside the builddir This will be temporary for the build time. When actually installed, builddir is no longer relevant. > 2) Copy over everything to to the builddir and make it take precedence over > srcdir when importing giscanner. This sounds simpler, although looking awkward.
Created attachment 165095 [details] [review] Symlink python files to builddir So, this patch creates symlinks for giscanner python files to builddir if they do not exist. And since the builddir is already preferred by tools/g-ir-scanner, no more change is needed to make it take precedence.
Review of attachment 165095 [details] [review]: This looks better, but it still has to be cleaned up in clean., distcheck won't pass otherwise.
Created attachment 165165 [details] [review] Also clean symlinks This patch also cleans the created symlinks. Note: - I avoid touching config.py because it's generated by configure script, so it should be cleaned with "make distclean", not "make clean". - I choose to check builddir != srcdir rather than testing symlinks (in contrast to regular files) before removing because some systems may implement $(LN_S) with copying. - *.pyc are still there even after 'make distclean'. This is not relevant to this bug/patch, but should they also be cleaned?
(In reply to comment #11) > Created an attachment (id=165165) [details] [review] > Also clean symlinks > > This patch also cleans the created symlinks. This patch needs to be update for git head, it doesn't apply any longer. Please make sure that make distcheck passes with the patch.
(In reply to comment #12) > This patch needs to be update for git head, it doesn't apply any longer. I've just checked it today, and it still applies. > Please make sure that make distcheck passes with the patch. Even without the patch, make distcheck doesn't pass when built outside source tree, either: - Without running 'make' beforehand, 'make distcheck' will fail, due to gtk-doc build process. (I'll just ignore this.) - On building HTML doc, gtk-doc.make refers to $(HTML_IMAGES) relatively to $(srcdir), but it was defined in docs/reference/Makefile.am with $(srcdir) prefix. This will cause failure if $(srcdir) is not absolute path, as it's the case for 'make distcheck'. Removing $(srcdir) fixes it. - On check target in tests/scanner, the pre-check rule copies the giscanner/*.py from srcdir to builddir again. But they have already been symlinked in giscanner/Makefile.am the patch. So, it fails due to copying the files over themselves. This is a side-effect of the patch. Removing the pre-check and post-check rules fixes this. - The, it assertion-fails in the check in tests/repository, which is beyond my current knowledge how to solve it: ---8<--- make[1]: Entering directory `/home/thep/build/gnome_git/gi-dist/gobject-introspection-0.10.0/_build/tests/repository' ** ERROR:../../../tests/repository/gitestrepo.c:56:main: assertion failed: (ret) /bin/bash: line 5: 22291 Aborted env top_builddir="../.." XDG_DATA_DIRS="../../../gir:/home/gnome3/share:/usr/share/gnome:/usr/local/share/:/usr/share/:/usr/share/gdm/" ${dir}$tst FAIL: gitestrepo ** ERROR:../../../tests/repository/gitestthrows.c:30:main: assertion failed: (ret != NULL) /bin/bash: line 5: 22310 Aborted env top_builddir="../.." XDG_DATA_DIRS="../../../gir:/home/gnome3/share:/usr/share/gnome:/usr/local/share/:/usr/share/:/usr/share/gdm/" ${dir}$tst FAIL: gitestthrows ============================================================================================= 2 of 2 tests failed Please report to http://bugzilla.gnome.org/enter_bug.cgi?product=glib&component=introspection ============================================================================================= make[1]: *** [check-TESTS] Error 1 make[1]: Leaving directory `/home/thep/build/gnome_git/gi-dist/gobject-introspection-0.10.0/_build/tests/repository' make: *** [check-am] Error 2 ---8<--- - With this test skipped, the next error is the *.pyc left after distclean in giscanner. So, I add *.pyc to CLEANFILES.
Created attachment 173102 [details] [review] Updated patch
Review of attachment 173102 [details] [review]: ::: giscanner/Makefile.am @@ +13,3 @@ scannerlexer.c \ + scannerlexer.h \ + *.pyc Apparently there's an option in Python 2.6 now to skip writing the .pyc files; see: http://bugs.python.org/issue602345 We should prefer doing that. @@ +63,3 @@ +pkgpyexec_PYTHON = \ + $(giscanner_sources) \ + $(giscanner_built_sources) This part looks fine. @@ +72,3 @@ + $(LN_S) $(srcdir)/$$file $$file; \ + fi \ + done This is definitely gross. We could alternatively pass top_builddir to the scanner as an environment variable, from which it could set sys.path, right?
Colin, top_buildir is already getting passed as UNINSTALLED_INTROSPECTION_BUILDDIR, and adding that to sys.path (in any order) doesn't actually make it load config.py from builddir. As Theppitak mentioned, the path to "giscanner.*" seems to be set based on wherever python find's the giscanner/__init__.py, so trying to import giscanner.X will always look under the directory where it found __init__.py (and nowhere else). From PEP 302: Deeper down in the mechanism, a dotted name import is split up by its components. For "import spam.ham", first an "import spam" is done, and only when that succeeds is "ham" imported as a submodule of "spam". Unfortunately, there isn't a way to load giscanner.config other than as a submodule, and the submodule routine has the limitations noted above. Unfortunately, the only two options I see are either: 1. Writing a custom build-time finder (PEP 302) that will look in both srcdir and builddir, and utilize it via sys.meta_path 2. The symlink hack. Option #1 has the added advantage of being generally useful, but I've no idea where a "generally useful" version would live.
Created attachment 175292 [details] [review] Get past "import .config" OK, found a slightly better option in pkgutil.extend_path. Still some ugliness (we need an identical __init__.py in both srcdir and builddir, so I used configure.ac) but it makes it past the import .config stage. Unfortunately there's errors in gicomp now, but it's getting there.
Created attachment 175293 [details] [review] Fully working version OK, fixed the compiler errors, build gets all the way through now---it was able to generate GdkPixbuf-2.0.gir without bombing, at least. I didn't run make check, since has it's own issues with builddir vs. srcdir :-)
This got fixed.
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]