| Summary: | [PENDING] Result caching in PKG_CHECK_MODULES() is broken / causes breakage | ||
|---|---|---|---|
| Product: | pkg-config | Reporter: | James Henstridge <james> |
| Component: | src | Assignee: | Tollef Fog Heen <tfheen> |
| Status: | RESOLVED NOTABUG | QA Contact: | |
| Severity: | normal | ||
| Priority: | high | CC: | dmacks, sbrabec, slawomir.czarko |
| Version: | unspecified | ||
| Hardware: | x86 (IA32) | ||
| OS: | Linux (All) | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
2005-06-26 Tollef Fog Heen <tfheen@err.no> * pkg.m4: Get rid of caching again. This breaks too much stuff, and pkg-config doesn't take much time to run. pkg-config 0.18 has now been released with a fix for this bug. I can reproduce with version 0.20: ----- configure.ac ----- AC_INIT PKG_CHECK_MODULES([PKGCONF], [pkg-config >= 0.1]) PKG_CHECK_MODULES([PKGCONF], [pkg-config < 0.1]) ------------------------ sbrabec> aclocal ; autoconf ; ./configure checking for pkg-config... /usr/bin/pkg-config checking pkg-config is at least version 0.9.0... yes checking for PKGCONF... yes checking for PKGCONF... yes sbrabec> pkg-config --version 0.20 It is because you set FOO_CFLAGS resp. FOO_LIBS, and then you skip the check, if FOO_CFLAGS resp. FOO_LIBS is not empty. I guess, that you can revert original implementation, add new variable pkg_cv_FOO_check, which will cache check string and then reply either: checking for PKGCONF... yes checking for PKGCONF... configure: error: PKG_CHECK_MODULES called twice with the same variable PKGCONF but different checks. or in case if both tests are equal: checking for PKGCONF... yes checking for PKGCONF... yes (cached) or in case if variables are set: checking for FOO... yes (using predefined FOO_CFLAGS, FOO_LDFLAGS) (In reply to comment #1) > 2005-06-26 Tollef Fog Heen <tfheen@err.no> > > * pkg.m4: Get rid of caching again. This breaks too much stuff, > and pkg-config doesn't take much time to run. > > I have a case when pkg-config takes over 30 seconds to run. Is that normal or does that indicate problem with my .pc files? Hardware is: 3.3 GHz CPU, SSD and 16 GB of RAM. I ran callgrind on pkg-config for the case which takes over 30 seconds and I got this: 99.33% of total time was spent in g_slist_last (almost 400K calls). g_slist_last is called mostly by g_slist_concat (almost 1.2M calls, 99.36% of total time). Would it make sense to use some other data structure which allows for faster list concatenation? (In reply to comment #3) > I can reproduce with version 0.20: > > ----- configure.ac ----- > AC_INIT > PKG_CHECK_MODULES([PKGCONF], [pkg-config >= 0.1]) > PKG_CHECK_MODULES([PKGCONF], [pkg-config < 0.1]) > ------------------------ > sbrabec> aclocal ; autoconf ; ./configure > checking for pkg-config... /usr/bin/pkg-config > checking pkg-config is at least version 0.9.0... yes > checking for PKGCONF... yes > checking for PKGCONF... yes > sbrabec> pkg-config --version > 0.20 > > It is because you set FOO_CFLAGS resp. FOO_LIBS, and then you skip the check, if > FOO_CFLAGS resp. FOO_LIBS is not empty. This is old now, but I'm inclinced to close this NOTABUG and just document not to do this unless you understand the limitations. The reason is because we definitely want to allow the user to override FOO_CFLAGS/LIBS from the environment and we cannot differentiate this case from repeated PKG_CHECK_MODULES(FOO, ...). Once the PKG_CHECK_MODULES succeeds, it needs to set FOO_CFLAGS/LIBS. The next invocation sees this and wouldn't have any way to know that it wasn't the user that supplied them. This is actually no different from how AC_CHECK_PROG and friends work except they have an explicit cache variable to use. If you call AC_CHECK_PROG repeatedly with the same variable, you'll get the first answer back everytime. So, I don't think PKG_CHECK_MODULES is doing anything unusual here. What we could do is warn the developer about this situation: diff --git a/pkg.m4 b/pkg.m4 index f26f84c..87687cc 100644 --- a/pkg.m4 +++ b/pkg.m4 @@ -105,6 +105,13 @@ fi[]dnl # -------------------------------------------------------------- AC_DEFUN([PKG_CHECK_MODULES], [AC_REQUIRE([PKG_PROG_PKG_CONFIG])dnl + +dnl Warn if we've already seen this tag +m4_ifdef([_PKG_SEEN_TAG_$1], + [m4_warn([syntax], + [$0 tag $1 is already registered and should probably not be reused.])], + [m4_define([_PKG_SEEN_TAG_$1])]) + AC_ARG_VAR([$1][_CFLAGS], [C compiler flags for $1, overriding pkg-config])dnl AC_ARG_VAR([$1][_LIBS], [linker flags for $1, overriding pkg-config])dnl However, this could be obnoxious since there are legitimate ways to use the same tag repeatedly if you know that you only want the first result or each use is only used conditionally. (In reply to comment #5) > I ran callgrind on pkg-config for the case which takes over 30 seconds and I > got this: > > 99.33% of total time was spent in g_slist_last (almost 400K calls). > > g_slist_last is called mostly by g_slist_concat (almost 1.2M calls, 99.36% of > total time). This is a different bug that has nothing to do with caching in PKG_CHECK_MODULES. Please open a separate bug with a specific test case if you're still hitting this problem. This would seem to indicate some extreme recursion that is definitely not the normal case. > Would it make sense to use some other data structure which allows for faster > list concatenation? It might not be smart for us to be concatenating the lists, but I'd like to see the specific case first. I added some documentation in pkg-config(1) in 1baefdf discouraging the use repeated variable prefixes. I think that's the best we can do without breaking legitimate uses. (In reply to comment #7) > (In reply to comment #5) > > I ran callgrind on pkg-config for the case which takes over 30 seconds and I > > got this: > > > > 99.33% of total time was spent in g_slist_last (almost 400K calls). > > > > g_slist_last is called mostly by g_slist_concat (almost 1.2M calls, 99.36% of > > total time). > > This is a different bug that has nothing to do with caching in > PKG_CHECK_MODULES. Please open a separate bug with a specific test case if > you're still hitting this problem. This would seem to indicate some extreme > recursion that is definitely not the normal case. > > > Would it make sense to use some other data structure which allows for faster > > list concatenation? > > It might not be smart for us to be concatenating the lists, but I'd like to see > the specific case first. New bug created - bug 54716 |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.
One of the new features added to pkg.m4 in 0.16 was caching of PKG_CHECK_MODULES() results. This caching is done on the basis of the variable name prefix, which means that consecutive calls using the same variable name but different package list will get the same result even when they shouldn't. This can lead to the following problems: 1. In evolution (before I patched it), it passed "EVOLUTION" as the variable prefix for a number of unrelated lists of package lists. The caching caused them all to get the same set of cflags/libs. 2. In eog, it does the following: PKG_CHECK_MODULES(EXIF, libexif >= $LIBEXIF_REQUIRED, have_exif=yes, have_exif=no) PKG_CHECK_MODULES(EXIF, libexif = $LIBEXIF_REQUIRED, have_old_libexif=yes, have_old_libexif=no) The second call sets have_old_libexif=yes even if you have a more recent version because of the result caching. If the macro is going to do result caching, it should be on the basis of the package list passed in rather than the variable name. However it is a bit difficult to construct an appropriate shell variable to use as the cache key (package requirement lists can be quite long, so you might run into variable name length limits too). Given that pkg-config invocations are pretty cheap, the easy option would be to just get rid of the caching behaviour entirely. What do you think?