Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1564503002:
Plumb system_libdir variable from GN to pkg-config.py (Closed)
DescriptionIntroduce a GN arg controlling the path prefix in pkg-config and modify pkg-config.py to use it.
Committed: https://crrev.com/a7efe0d67a564d1dbe540d70de2bf1f4ef409bb4
Cr-Commit-Position: refs/heads/master@{#367987}
Patch Set 1 #Patch Set 2 : better #
Total comments: 2
Patch Set 3 : improved the comments #
Messages
Total messages: 20 (7 generated)
rjkroege@chromium.org changed reviewers: + dpranke@chromium.org
Maybe this is the wrong approach but PKG_CONFIG_PATH needs to include the lib64/pkgconfig directory from the sysroot for GN to successfully a 64bit CrOS board. Testing: I verified that with this patch, Chrome builds for both a 32bit arm and 64bit intel CrOS board. PTAL
On 2016/01/06 19:44:17, rjkroege wrote: So with CrOS we have sigle sysroot with 32 + 64 bit installation? How is this currently working with gyp? Is system_libdir=lib64 set somewhere? I'm not sure that change is correct since you never want both 'lib' and lib64' in the PKG_CONFIG_PATH at the same time do you? If you do this won't it always prefer the first one (32-bit?).
On 2016/01/06 19:53:14, Sam Clegg wrote: > On 2016/01/06 19:44:17, rjkroege wrote: > > So with CrOS we have sigle sysroot with 32 + 64 bit installation? > > How is this currently working with gyp? Is system_libdir=lib64 set somewhere? yes. It's set by the board-specific GYP_DEFINES to one of lib64 or lib. > > I'm not sure that change is correct since you never want both 'lib' and lib64' > in the PKG_CONFIG_PATH at the same time do you? If you do this won't it always > prefer the first one (32-bit?). Will there only ever be one of lib/pkgconfig or lib64/pkgconfig in the same sysroot? Otherwise, the variable needs to be plumbed in from the args.gn file instead. Probably I should do that instead?
Description was changed from ========== Flexibly handle 64bit targets in pkg-config.py Try both 64bit and 32bit pkgconfig directories in CrOS sysroots to support building both 32bit and 64bit CrOS targets. ========== to ========== Introduce a GN arg controlling the path prefix in pkg-config and modify pkg-config.py to use it. ==========
rjkroege@chromium.org changed reviewers: + sbc@chromium.org
wdyt? Better?
On 2016/01/06 23:50:20, rjkroege wrote: > wdyt? Better? lgtm (not an OWNER) However, I can't see where the system_libdir gyp variable is actually set.. at least chromium code search doesn't show anything for it.
On 2016/01/07 00:25:46, Sam Clegg wrote: > On 2016/01/06 23:50:20, rjkroege wrote: > > wdyt? Better? > > lgtm (not an OWNER) > > However, I can't see where the system_libdir gyp variable is actually set.. at > least chromium code search doesn't show anything for it. Ah. That I know: it's set as an environment variable by the cros chrome-sdk command (see http://www.chromium.org/chromium-os/how-tos-and-troubleshooting/building-chro... for more info) So, in a GN world, it's set in the args.gn for the build.
rjkroege@chromium.org changed reviewers: + brettw@chromium.org - dpranke@chromium.org
On 2016/01/07 00:36:12, rjkroege wrote: > On 2016/01/07 00:25:46, Sam Clegg wrote: > > On 2016/01/06 23:50:20, rjkroege wrote: > > > wdyt? Better? > > > > lgtm (not an OWNER) > > > > However, I can't see where the system_libdir gyp variable is actually set.. at > > least chromium code search doesn't show anything for it. > > Ah. That I know: it's set as an environment variable by the cros chrome-sdk > command (see > http://www.chromium.org/chromium-os/how-tos-and-troubleshooting/building-chro... > for more info) Do you know where in the code this is set? Just curious because I have previously though that nobody was setting this particular variable. Any idea why codesearch doesn't find it? > > So, in a GN world, it's set in the args.gn for the build.
lgtm https://codereview.chromium.org/1564503002/diff/20001/build/config/linux/pkg_... File build/config/linux/pkg_config.gni (right): https://codereview.chromium.org/1564503002/diff/20001/build/config/linux/pkg_... build/config/linux/pkg_config.gni:35: # CrOS builds set the `system_libdir` variable as part of building on a 64 or I'm a bit confused what "lib" is relative to. The root directory? The sysroot? It would be goot to mention that here an in the python script.
https://codereview.chromium.org/1564503002/diff/20001/build/config/linux/pkg_... File build/config/linux/pkg_config.gni (right): https://codereview.chromium.org/1564503002/diff/20001/build/config/linux/pkg_... build/config/linux/pkg_config.gni:35: # CrOS builds set the `system_libdir` variable as part of building on a 64 or On 2016/01/07 00:43:25, brettw wrote: > I'm a bit confused what "lib" is relative to. The root directory? The sysroot? > It would be goot to mention that here an in the python script. Done. Much more detailed comment now.
The CQ bit was checked by rjkroege@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from brettw@chromium.org, sbc@chromium.org Link to the patchset: https://codereview.chromium.org/1564503002/#ps40001 (title: "improved the comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1564503002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1564503002/40001
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Introduce a GN arg controlling the path prefix in pkg-config and modify pkg-config.py to use it. ========== to ========== Introduce a GN arg controlling the path prefix in pkg-config and modify pkg-config.py to use it. Committed: https://crrev.com/a7efe0d67a564d1dbe540d70de2bf1f4ef409bb4 Cr-Commit-Position: refs/heads/master@{#367987} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/a7efe0d67a564d1dbe540d70de2bf1f4ef409bb4 Cr-Commit-Position: refs/heads/master@{#367987} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
