1298740 - Python pkg_check_modules doesn't seem to work with PKG_CONFIG_PATH

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Hector Zhao [:hectorz]]

packages-Mac-mini% ./mach build
 0:04.41 /usr/bin/make -f client.mk -s
 0:38.86 Automatically clobbering /path/to/gecko-dev/obj-x86_64-apple-darwin13.4.0
 0:38.86 Successfully completed auto clobber.
 0:41.87 Adding client.mk options from /path/to/gecko-dev/.mozconfig:
 0:41.87     AUTOCLOBBER=1
 0:41.87     MOZ_OBJDIR=/path/to/gecko-dev/obj-x86_64-apple-darwin13.4.0
 0:41.87     OBJDIR=/path/to/gecko-dev/obj-x86_64-apple-darwin13.4.0
 0:41.87     FOUND_MOZCONFIG=/path/to/gecko-dev/.mozconfig
 0:41.90 Generating /path/to/gecko-dev/configure
 0:41.94 Generating /path/to/gecko-dev/js/src/configure
 0:42.43 cd /path/to/gecko-dev/obj-x86_64-apple-darwin13.4.0
 0:42.43 /path/to/gecko-dev/configure
 0:42.84 Creating Python environment
 0:47.64 New python executable in /path/to/gecko-dev/obj-x86_64-apple-darwin13.4.0/_virtualenv/bin/python
 0:47.64 Installing setuptools, pip, wheel...done.
 0:48.46 running build_ext
 0:48.46 copying build/lib.macosx-10.9-intel-2.7/psutil/_psutil_osx.so -> psutil
 0:48.46 copying build/lib.macosx-10.9-intel-2.7/psutil/_psutil_posix.so -> psutil
 0:48.46
 0:48.46 Reexecuting in the virtualenv
 0:48.60 Adding configure options from /path/to/gecko-dev/.mozconfig
 0:48.60   --with-ccache=/usr/local/bin/ccache
 0:48.60   --with-system-nspr
 0:48.61   --with-system-libevent
 0:48.61   --with-system-nss
 0:48.61   --with-nss-prefix=/usr/local/opt/nss
 0:48.61   --with-system-jpeg=/usr/local/opt/jpeg-turbo
 0:48.61   --with-system-zlib=/usr/local/opt/zlib
 0:48.61   --with-system-bz2=/usr/local/opt/bzip2
 0:48.61   --with-system-png=/usr/local/opt/libpng
 0:48.61   --enable-system-hunspell
 0:48.61   --enable-system-ffi
 0:48.61   --with-system-libvpx
 0:48.61   --enable-signmar=no
 0:48.61   --enable-system-sqlite
 0:48.61   --enable-system-pixman
 0:48.61   --with-system-icu
 0:48.61   CC=/usr/local/opt/llvm/bin/clang -fcolor-diagnostics
 0:48.61   PKG_CONFIG_PATH=/usr/local/opt/icu4c/lib/pkgconfig:/usr/local/opt/libffi/lib/pkgconfig:/usr/local/opt/nss/lib/pkgconfig:/usr/local/opt/sqlite/lib/pkgconfig
 0:48.61   CXX=/usr/local/opt/llvm/bin/clang++ -fcolor-diagnostics
...
 0:56.62 checking for libffi > 3.0.9... no
 0:56.62 ERROR: Package libffi was not found in the pkg-config search path.
 0:56.62 ERROR: Perhaps you should add the directory containing `libffi.pc'
 0:56.62 ERROR: to the PKG_CONFIG_PATH environment variable
 0:56.62 ERROR: No package 'libffi' found
 0:56.64 *** Fix above errors and then restart with               "/Applications/Xcode.app/Contents/Developer/usr/bin/make -f client.mk build"
 0:56.64 make[2]: *** [configure] Error 1
 0:56.64 make[1]: *** [/path/to/gecko-dev/obj-x86_64-apple-darwin13.4.0/Makefile] Error 2
 0:56.64 make: *** [build] Error 2
 0:56.65 315 compiler warnings present.
 0:57.59 Failed to parse ccache stats output: cache hit rate                     29.32 %
packages-Mac-mini%

Comment 1

[Chris Manchester (limited bugmail, email directly)]

Attachment: Bug 1298740 - Populate Python environment with variables from mozconfig.

Review commit: https://reviewboard.mozilla.org/r/84014/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/84014/

Comment 2

[Mike Hommey [:glandium]]

Comment on attachment 8798596 [details]
Bug 1298740 - Populate Python environment with variables from mozconfig.

https://reviewboard.mozilla.org/r/84014/#review82776

::: build/moz.configure/init.configure:298
(Diff revision 1)
> +    def quote_value(value):
> +        if isinstance(value, (tuple, list)):
> +            return quote(*value)
> +        return quote(value)

We don't want to quote environment variables values. At best, we want them flattened if they are tuples or lists, but I don't think the mozconfig reader will ever give us that (and if it did, that would be a problem in the mozconfig_options function above)

All in all, you could just add os.environ[key] = value in 2 places in mozconfig_options and be done with it, although there's the fun thing where windows doesn't like unicode strings in os.environ sometimes (but that's not addressed by this patch either).

Comment 3

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8798596 [details]
Bug 1298740 - Populate Python environment with variables from mozconfig.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/84014/diff/1-2/

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 8798596 [details]
Bug 1298740 - Populate Python environment with variables from mozconfig.

https://reviewboard.mozilla.org/r/84014/#review84352

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e984459f2b0f
Populate Python environment with variables from mozconfig. r=glandium

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I had to back this out because it appears to have caused Windows builds to fail like https://treeherder.mozilla.org/logviewer.html#?job_id=37679014&repo=mozilla-inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/bb4cb6a2db95

Comment 7

[Chris Manchester (limited bugmail, email directly)]

I should have noticed this before attempting to land, but we're getting unix-style paths in PATH out of our mozconfigs and using them as our PATH in a windows process... which isn't going to work.

I'm not sure of the best way to handle this. For this specific case we may want to go back to the original patch from bug 1305145, but we do need a solution more generally for things like bug 1307561.

Comment 8

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8798596 [details]
Bug 1298740 - Populate Python environment with variables from mozconfig.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/84014/diff/2-3/

Comment 9

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8798596 [details]
Bug 1298740 - Populate Python environment with variables from mozconfig.

I can't find an example of converting from unix style back to unix style paths that would be convenient to use here. This already saw r+, but if there's a way to solve the problem more generally I'm not thinking of we should go with that.

Comment 10

[Mike Hommey [:glandium]]

How about changing mozconfig_loader to invoke a python program that would dump the environment in a similar manner `env` does, except that it would do a msys->win32 transition and get the translated values?

Comment 11

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8798596 [details]
Bug 1298740 - Populate Python environment with variables from mozconfig.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/84014/diff/3-4/

Comment 12

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8798596 [details]
Bug 1298740 - Populate Python environment with variables from mozconfig.

This is updated with the suggestion from comment 10.

Comment 13

[Mike Hommey [:glandium]]

Comment on attachment 8798596 [details]
Bug 1298740 - Populate Python environment with variables from mozconfig.

https://reviewboard.mozilla.org/r/84014/#review87192

::: python/mozbuild/mozbuild/dump_env.py:1
(Diff revision 4)
> +# This Source Code Form is subject to the terms of the Mozilla Public

This file would probably be better placed under mozbuild/action/

::: python/mozbuild/mozbuild/test/test_mozconfig.py:382
(Diff revision 4)
>              self.assertEqual(result['env']['added'], {
>                  'MY_EXPORTED': 'woot'})
>  
>      def test_read_modify_variables(self):
>          """Variables modified by mozconfig are detected."""
> -        os.environ[b'CC'] = b'/usr/bin/gcc'
> +        old_path = os.path.realpath(b'/usr/bin/gcc')

Why do you need these changes? The test runs fine without these changes on my machine.

Comment 14

[Mike Hommey [:glandium]]

Also note you have a mq comment in the commit message.

Comment 15

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8798596 [details]
Bug 1298740 - Populate Python environment with variables from mozconfig.

https://reviewboard.mozilla.org/r/84014/#review87192

> Why do you need these changes? The test runs fine without these changes on my machine.

This is for Windows. We're returining native paths from the mozconfig loader now, so comparisions with "/usr/bin/gcc" et al. are failing.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/55ba6df1445e
Populate Python environment with variables from mozconfig. r=glandium

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/55ba6df1445e