Python pkg_check_modules doesn't seem to work with PKG_CONFIG_PATH

RESOLVED FIXED in Firefox 52

Status

()

Core
Build Config
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: hectorz, Assigned: chmanchester)

Tracking

Trunk
mozilla52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox51 affected, firefox52 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

a year ago
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)

Updated

a year ago
Blocks: 1305145
(Assignee)

Updated

a year ago
Assignee: nobody → cmanchester
Comment hidden (mozreview-request)

Comment 2

a year ago
mozreview-review
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 hidden (mozreview-request)

Comment 4

a year ago
mozreview-review
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+

Comment 5

a year ago
Pushed by cmanchester@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e984459f2b0f
Populate Python environment with variables from mozconfig. r=glandium
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
Flags: needinfo?(cmanchester)
(Assignee)

Comment 7

a year ago
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 hidden (mozreview-request)
(Assignee)

Comment 9

a year ago
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 hidden (mozreview-request)
(Assignee)

Comment 12

a year ago
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

a year ago
mozreview-review
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.
(Assignee)

Comment 15

a year ago
mozreview-review-reply
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

a year ago
Pushed by cmanchester@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/55ba6df1445e
Populate Python environment with variables from mozconfig. r=glandium

Comment 17

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/55ba6df1445e
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.