Closed Bug 1298740 Opened 8 years ago Closed 8 years ago

Python pkg_check_modules doesn't seem to work with PKG_CONFIG_PATH

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox51 affected, firefox52 fixed)

RESOLVED FIXED
mozilla52
Tracking Status
firefox51 --- affected
firefox52 --- fixed

People

(Reporter: hectorz, Assigned: chmanchester)

References

Details

Attachments

(1 file)

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%
Assignee: nobody → cmanchester
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).
Attachment #8798596 - Flags: review?(mh+mozilla)
Comment on attachment 8798596 [details]
Bug 1298740 - Populate Python environment with variables from mozconfig.

https://reviewboard.mozilla.org/r/84014/#review84352
Attachment #8798596 - Flags: review?(mh+mozilla) → review+
Pushed by cmanchester@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e984459f2b0f
Populate Python environment with variables from mozconfig. r=glandium
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.
Flags: needinfo?(cmanchester)
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.
Attachment #8798596 - Flags: review+ → review?(mh+mozilla)
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 on attachment 8798596 [details]
Bug 1298740 - Populate Python environment with variables from mozconfig.

This is updated with the suggestion from comment 10.
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.
Attachment #8798596 - Flags: review?(mh+mozilla) → review+
Also note you have a mq comment in the commit message.
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.
Pushed by cmanchester@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/55ba6df1445e
Populate Python environment with variables from mozconfig. r=glandium
https://hg.mozilla.org/mozilla-central/rev/55ba6df1445e
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: