Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2215693003:
gn: Make mini_installer link in static asan builds. (Closed)
|
Created:
4 years, 4 months ago by Nico Modified:
4 years, 4 months ago CC:
chromium-reviews, grt+watch_chromium.org, pennymac+watch_chromium.org, wfh+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptiongn: Make mini_installer link in static asan builds.
The target removes the default executable_config, so it doesn't receive the
benefit of https://codereview.chromium.org/2208093003 in static-library
builds automatically. Explicitly make it depend on sanitizers:link_executable.
BUG=598761
Committed: https://crrev.com/3148c7f3469d5f4b2d653fd4bc8c990ac43f759b
Cr-Commit-Position: refs/heads/master@{#409938}
Patch Set 1 #Patch Set 2 : comments #Patch Set 3 : rebase #Patch Set 4 : better rebase #Patch Set 5 : better better rebase #Messages
Total messages: 35 (23 generated)
The CQ bit was checked by thakis@chromium.org to run a CQ dry run
Description was changed from ========== gn: Make sure mini_installer links to asan runtime. The target removes the default executable_config, so it doesn't receive the benefit of https://codereview.chromium.org/2208093003 in static-library builds automatically. Explicitly make it depend on sanitizers:link_executable. BUG=598761 ========== to ========== gn: Make mini_installer link in static asan builds. The target removes the default executable_config, so it doesn't receive the benefit of https://codereview.chromium.org/2208093003 in static-library builds automatically. Explicitly make it depend on sanitizers:link_executable. BUG=598761 ==========
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
thakis@chromium.org changed reviewers: + etienneb@chromium.org, rnk@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
Looks right, but CQ gn doesn't seem to like it.
I tried that solution, but they were missing dependencies due to the asan runtime. The mini-installer is removing all useless libraries.
> Looks right, but CQ gn doesn't seem to like it.
Whoops. Fixed, I think.
On 2016/08/04 20:18:11, etienneb wrote:
> I tried that solution, but they were missing dependencies due to the asan
> runtime.
>
> The mini-installer is removing all useless libraries.
Ah, good catch. Instead of adding that too, I'm moving the library setup to
default_sanitizer_ldflags which is added as public_deps in "deps" – that seems
to be how we deal with this on non-Windows. See the comment:
# Even when a target removes default_sanitizer_flags, it may be depending
# on a library that did not remove default_sanitizer_flags. Thus, we need
# to add the ldflags here as well as in default_sanitizer_flags.
":default_sanitizer_ldflags",
That should do it?
The CQ bit was checked by thakis@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: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by thakis@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_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...) cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...) linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
The CQ bit was checked by thakis@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 checked by thakis@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.
Looks like at least the trybots are happy now. PTAL, it'd be cool to land this today so that all the bots could cycle over night.
yes. It looks like right. But, unfortunately I do not have access to a windows computer anymore (until tomm).
yes. It looks like right. But, unfortunately I do not have access to a windows computer anymore (until tomm).
On 2016/08/04 22:43:46, etienneb wrote: > yes. It looks like right. > But, unfortunately I do not have access to a windows computer anymore (until > tomm). I think it's ok to land and let the waterfall bots check if it helps. It can't possibly break things more given that things already don't work :-)
lgtm And we should keep eyes on bots.
The CQ bit was checked by thakis@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== gn: Make mini_installer link in static asan builds. The target removes the default executable_config, so it doesn't receive the benefit of https://codereview.chromium.org/2208093003 in static-library builds automatically. Explicitly make it depend on sanitizers:link_executable. BUG=598761 ========== to ========== gn: Make mini_installer link in static asan builds. The target removes the default executable_config, so it doesn't receive the benefit of https://codereview.chromium.org/2208093003 in static-library builds automatically. Explicitly make it depend on sanitizers:link_executable. BUG=598761 ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== gn: Make mini_installer link in static asan builds. The target removes the default executable_config, so it doesn't receive the benefit of https://codereview.chromium.org/2208093003 in static-library builds automatically. Explicitly make it depend on sanitizers:link_executable. BUG=598761 ========== to ========== gn: Make mini_installer link in static asan builds. The target removes the default executable_config, so it doesn't receive the benefit of https://codereview.chromium.org/2208093003 in static-library builds automatically. Explicitly make it depend on sanitizers:link_executable. BUG=598761 Committed: https://crrev.com/3148c7f3469d5f4b2d653fd4bc8c990ac43f759b Cr-Commit-Position: refs/heads/master@{#409938} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/3148c7f3469d5f4b2d653fd4bc8c990ac43f759b Cr-Commit-Position: refs/heads/master@{#409938} |
