|
|
Created:
3 years, 8 months ago by richard.townsend Modified:
3 years, 8 months ago CC:
chromium-reviews Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionIntroduce host_pkg_config variable for cross-compilation
When cross-compiling in the ChromiumOS chroot, it's possible for
host-based compiled_actions targets to pick up libraries in the board
sysroot which causes linker warnings (converted to errors via
--fatal-warnings). To address this, introduce a host_pkg_config
variable, which can override pkg_config when building tools used on
the host as part of the build process.
BUG=710841
Review-Url: https://codereview.chromium.org/2818523002
Cr-Commit-Position: refs/heads/master@{#466947}
Committed: https://chromium.googlesource.com/chromium/src/+/0dbb630385d7bd387348b00437e72b611287cb49
Patch Set 1 #Patch Set 2 : Fix native Linux builds #Patch Set 3 : Address incorrect sysroot handling #
Total comments: 4
Patch Set 4 : Cleanup of patchset #3 #Messages
Total messages: 34 (25 generated)
Description was changed from ========== Introduce host_pkg_config variable for cross-compilation When cross-compiling, in the ChromiumOS chroot, it's possible for host-based compiled_actions to pick up libraries in the board sysroot, which causes linker warnings (converted to errors via --fatal-warnings). To address this, introduce a host_pkg_config variable, which can override pkg_config for tools built to be used on the host as part of the build process. BUG=710841 ========== to ========== Introduce host_pkg_config variable for cross-compilation When cross-compiling in the ChromiumOS chroot, it's possible for host-based compiled_actions targets to pick up libraries in the board sysroot which causes linker warnings (converted to errors via --fatal-warnings). To address this, introduce a host_pkg_config variable, which can override pkg_config when building tools used on the host as part of the build process. BUG=710841 ==========
The CQ bit was checked by cavalcantii@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
gurchetansingh@chromium.org changed reviewers: + brettw@chromium.org, dpranke@chromium.org, scottmg@chromium.org
Patchset #2 (id:20001) has been deleted
The CQ bit was checked by cavalcantii@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
The CQ bit was checked by cavalcantii@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
richard.townsend@arm.com changed reviewers: + gurchetansingh@chromium.org, lgarron@chromium.org, vapier@chromium.org
This patch is ready to review, it maintains the existing pkg_config behaviour across all platforms for now, but a patch which builds on this and corrects the problematic behaviour displayed in #710841 is available at https://chromium-review.googlesource.com/c/483342/
seems ok, but i'm not a GN expert at all
Basically looks fine. The comments are slightly more than nits, but only slightly :). https://codereview.chromium.org/2818523002/diff/60001/build/config/linux/pkg_... File build/config/linux/pkg_config.gni (right): https://codereview.chromium.org/2818523002/diff/60001/build/config/linux/pkg_... build/config/linux/pkg_config.gni:88: # Only use the custom libdir when building with the target sysroot. Why did this comment move? https://codereview.chromium.org/2818523002/diff/60001/build/config/linux/pkg_... build/config/linux/pkg_config.gni:93: } I'd probably move this if down to line 97 and just have if (current == host) { args = host_pkg_config_args + invoker.packages } else { args = pkg_config_args + invoker.packages }
Awaiting someone with permission to check the CQ bit on patchset #4. https://codereview.chromium.org/2818523002/diff/60001/build/config/linux/pkg_... File build/config/linux/pkg_config.gni (right): https://codereview.chromium.org/2818523002/diff/60001/build/config/linux/pkg_... build/config/linux/pkg_config.gni:88: # Only use the custom libdir when building with the target sysroot. On 2017/04/21 00:25:25, Dirk Pranke wrote: > Why did this comment move? Done. https://codereview.chromium.org/2818523002/diff/60001/build/config/linux/pkg_... build/config/linux/pkg_config.gni:93: } On 2017/04/21 00:25:25, Dirk Pranke wrote: > I'd probably move this if down to line 97 and just have > > if (current == host) { > args = host_pkg_config_args + invoker.packages > } else { > args = pkg_config_args + invoker.packages > } Done.
The CQ bit was checked by cavalcantii@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm, thanks!
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by richard.townsend@arm.com
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by richard.townsend@arm.com
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 80001, "attempt_start_ts": 1493117056706170, "parent_rev": "387c2f19a653715fc14e8ee857fba3323fa92ba8", "commit_rev": "0dbb630385d7bd387348b00437e72b611287cb49"}
Message was sent while issue was closed.
Description was changed from ========== Introduce host_pkg_config variable for cross-compilation When cross-compiling in the ChromiumOS chroot, it's possible for host-based compiled_actions targets to pick up libraries in the board sysroot which causes linker warnings (converted to errors via --fatal-warnings). To address this, introduce a host_pkg_config variable, which can override pkg_config when building tools used on the host as part of the build process. BUG=710841 ========== to ========== Introduce host_pkg_config variable for cross-compilation When cross-compiling in the ChromiumOS chroot, it's possible for host-based compiled_actions targets to pick up libraries in the board sysroot which causes linker warnings (converted to errors via --fatal-warnings). To address this, introduce a host_pkg_config variable, which can override pkg_config when building tools used on the host as part of the build process. BUG=710841 Review-Url: https://codereview.chromium.org/2818523002 Cr-Commit-Position: refs/heads/master@{#466947} Committed: https://chromium.googlesource.com/chromium/src/+/0dbb630385d7bd387348b00437e7... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:80001) as https://chromium.googlesource.com/chromium/src/+/0dbb630385d7bd387348b00437e7... |