Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1683793003:
gn/win: Copy dbghelp.dll to the output dir. (Closed)
Descriptiongn/win: Copy dbghelp.dll to the output dir.
This was done for gyp in https://codereview.chromium.org/951083003/.
It was missing in gn, but base.isolate depends on this file. This makes
"isolate tests" fail on Windows bots that use gn and swarming.
BUG=460506, 461160, 498033
Committed: https://crrev.com/29ecee7df186eeefaab53189b7a2888a41183342
Cr-Commit-Position: refs/heads/master@{#374672}
Patch Set 1 #
Messages
Total messages: 22 (7 generated)
thakis@chromium.org changed reviewers: + scottmg@chromium.org
Failing bot link: https://build.chromium.org/p/chromium.fyi/builders/CrWinClang64%28dbg%29/buil... [found] [hashed/size/to hash] [looked up/to lookup] [uploaded/size/to upload/size] 01:46:57.502492 Root: E:\b\build\slave\CrWinClang64_dbg_\build\src 01:46:57.502492 Root: E:\b\build\slave\CrWinClang64_dbg_\build\src 01:46:57.503492 Root: E:\b\build\slave\CrWinClang64_dbg_\build\src 01:46:57.506492 Root: E:\b\build\slave\CrWinClang64_dbg_\build\src 01:46:57.509493 Root: E:\b\build\slave\CrWinClang64_dbg_\build\src ipc_tests GetFileAttributesEx E:\b\build\slave\CrWinClang64_dbg_\build\src\out\Debug_x64\dbghelp.dll: The system cannot find the file specified. wtf_unittests GetFileAttributesEx E:\b\build\slave\CrWinClang64_dbg_\build\src\out\Debug_x64\dbghelp.dll: The system cannot find the file specified. setup_unittests GetFileAttributesEx E:\b\build\slave\CrWinClang64_dbg_\build\src\out\Debug_x64\dbghelp.dll: The system cannot find the file specified. 01:46:57.511493 PushDirectory(E:\b\build\slave\CrWinClang64_dbg_\build\src\google_apis\test\) = 13 files google_apis_unittests GetFileAttributesEx E:\b\build\slave\CrWinClang64_dbg_\build\src\out\Debug_x64\dbghelp.dll: The system cannot find the file specified. blink_platform_unittests GetFileAttributesEx E:\b\build\slave\CrWinClang64_dbg_\build\src\out\Debug_x64\dbghelp.dll: The system cannot find the file specified. 01:46:57.515493 Root: E:\b\build\slave\CrWinClang64_dbg_\build\src 01:46:57.517493 Root: E:\b\build\slave\CrWinClang64_dbg_\build\src 01:46:57.518493 Root: E:\b\build\slave\CrWinClang64_dbg_\build\src
From https://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng...: [1/5535] COPY ../../build/win/dbghelp_xp/dbghelp.dll nacl_win64/dbghelp.dll [5/5535] COPY ../../build/win/dbghelp_xp/dbghelp.dll dbghelp.dll I guess that's ok and possibly even desirable (?)
maruel@chromium.org changed reviewers: + maruel@chromium.org
rubberstamp lgtm. We'll need other files too, like VS2015 files Bruce recently added.
The CQ bit was checked by thakis@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1683793003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1683793003/1
On 2016/02/10 16:16:15, M-A Ruel wrote: > rubberstamp lgtm. Thanks! > We'll need other files too, like VS2015 files Bruce recently added. Oh hmm, that's a good point. With gyp, I think this happens during gyp_chromium. Let me see if we do this in gn too already…
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
The CQ bit was checked by thakis@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1683793003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1683793003/1
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== gn/win: Copy dbghelp.dll to the output dir. This was done for gyp in https://codereview.chromium.org/951083003/. It was missing in gn, but base.isolate depends on this file. This makes "isolate tests" fail on Windows bots that use gn and swarming. BUG=460506,461160,498033 ========== to ========== gn/win: Copy dbghelp.dll to the output dir. This was done for gyp in https://codereview.chromium.org/951083003/. It was missing in gn, but base.isolate depends on this file. This makes "isolate tests" fail on Windows bots that use gn and swarming. BUG=460506,461160,498033 Committed: https://crrev.com/29ecee7df186eeefaab53189b7a2888a41183342 Cr-Commit-Position: refs/heads/master@{#374672} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/29ecee7df186eeefaab53189b7a2888a41183342 Cr-Commit-Position: refs/heads/master@{#374672}
Message was sent while issue was closed.
dpranke@chromium.org changed reviewers: + dpranke@chromium.org
Message was sent while issue was closed.
I'm a bit confused ... do the binaries actually depend on dbghelp.dll? If so, how did they link at all if the file wasn't in the build directory?
Message was sent while issue was closed.
I think it's a runtime dependency, not a link time one (from reading the tea leaves, I think for getting stack traces on XP, so maybe we can remove this from base.isolate soonish, but for now it's still there)
Message was sent while issue was closed.
> Oh hmm, that's a good point. With gyp, I think this happens during gyp_chromium. > Let me see if we do this in gn too already… scottmg tells me that copy_dlls (here: https://code.google.com/p/chromium/codesearch#chromium/src/build/toolchain/wi...) takes care of copying the runtime dlls already. Phew! (that does check that the current toolchain is the default_toolchain, so maybe this here should too…Scott, shout if you think I should change this)
Message was sent while issue was closed.
On 2016/02/10 17:20:46, Nico wrote: > > Oh hmm, that's a good point. With gyp, I think this happens during > gyp_chromium. > > Let me see if we do this in gn too already… > > scottmg tells me that copy_dlls (here: > https://code.google.com/p/chromium/codesearch#chromium/src/build/toolchain/wi...) > takes care of copying the runtime dlls already. Phew! > > (that does check that the current toolchain is the default_toolchain, so maybe > this here should too…Scott, shout if you think I should change this) I'm not really sure what those are, but I guess maybe it should check the same thing. Otherwise we'd also be copying it when we build for NaCl or something?
Message was sent while issue was closed.
On 2016/02/10 18:58:32, scottmg wrote: > On 2016/02/10 17:20:46, Nico wrote: > > > Oh hmm, that's a good point. With gyp, I think this happens during > > gyp_chromium. > > > Let me see if we do this in gn too already… > > > > scottmg tells me that copy_dlls (here: > > > https://code.google.com/p/chromium/codesearch#chromium/src/build/toolchain/wi...) > > takes care of copying the runtime dlls already. Phew! > > > > (that does check that the current toolchain is the default_toolchain, so maybe > > this here should too…Scott, shout if you think I should change this) > > I'm not really sure what those are, but I guess maybe it should check the same > thing. Otherwise we'd also be copying it when we build for NaCl or something? Right, see https://codereview.chromium.org/1683793003/#msg3
Message was sent while issue was closed.
On 2016/02/10 19:01:29, Nico wrote: > On 2016/02/10 18:58:32, scottmg wrote: > > On 2016/02/10 17:20:46, Nico wrote: > > > > Oh hmm, that's a good point. With gyp, I think this happens during > > > gyp_chromium. > > > > Let me see if we do this in gn too already… > > > > > > scottmg tells me that copy_dlls (here: > > > > > > https://code.google.com/p/chromium/codesearch#chromium/src/build/toolchain/wi...) > > > takes care of copying the runtime dlls already. Phew! > > > > > > (that does check that the current toolchain is the default_toolchain, so > maybe > > > this here should too…Scott, shout if you think I should change this) > > > > I'm not really sure what those are, but I guess maybe it should check the same > > thing. Otherwise we'd also be copying it when we build for NaCl or something? > > Right, see https://codereview.chromium.org/1683793003/#msg3 Oh, oops. I think it's probably not that useful (it's an x86 dll, so it's not going to help nacl_win64 anyway), but probably not harmful if it doesn't add duplicate edges. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
