restore OutputFile for chrome.exe, do moving hardlink on dll instead

restore OutputFile for chrome.exe, do moving hardlink on dll instead

Hardlink from different dir for chrome.exe caused annoyance when running from
Visual Studio because $(TargetPath) the default in the .user file is not the
correct path. So, instead do the hardlink on the dll. To avoid the possible
expense on non-NTFS, and to enable a trybot in the future, do this hardlink
based on a gyp define.

BUG=92528, 92685
TEST=No link problems


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=97341

[scottmg, 2011-08-17 17:22:37]

Rollback the OutputFile change for chrome.exe because it's annoying in the
debugger because the default debug path is wrong (and can't be fixed in gyp
because it's stored in a .user file).

Instead, based on a gyp flag (incremental_chrome_dll), move chrome.dll's
OutputFile, and hardlink it back to OutDir as a postbuild.

[Roger McFarlane (Chromium), 2011-08-17 19:12:35]

http://codereview.chromium.org/7670031/diff/1/chrome/chrome_dll.gypi
File chrome/chrome_dll.gypi (right):

http://codereview.chromium.org/7670031/diff/1/chrome/chrome_dll.gypi#newcode157
chrome/chrome_dll.gypi:157: }], ['incremental_chrome_dll==1', {
If I understand what you intend, the flow of these conditions isn't doing what
you want it to.  The conditions here are equivalent to:

  if optimize_with_syzygy then
     set pdb and output files

  if incremental_chrome_dll then
     set output file and use library dependency
  else
     set pdb file


Per the docs (http://code.google.com/p/gyp/wiki/InputFormatReference#Merging)
you should be able to move the pdb setting out of the condition (just after the
ImportLibrary settings, above) and get the "correct" behavior.

http://codereview.chromium.org/7670031/diff/1/chrome/chrome_dll.gypi#newcode166
chrome/chrome_dll.gypi:166: 'conditions': [['incremental_chrome_dll==1', {
Perhaps 'incremental_chrome_dll==1 && optimize_with_syzygy==0'?

In principle, these two setting could co-exist (modulo some issues Syzygy has
groking incrementally linked binaries and PDB files).

Btw, does this work for Release builds, and for Official builds (Release plus
other optimizations)? Does the flag need to understand what configuration is
been used?

Also, formatting convention seems put the conditional on the next line.  I.e.,

  'conditions': [
    ['foo="bar"', {
      ...
    }],
  ],

http://codereview.chromium.org/7670031/diff/1/chrome/chrome_dll.gypi#newcode168
chrome/chrome_dll.gypi:168: # OutDiris a workaround to avoid having the .ilk for
chrome.exe
OutDiris -> Outdir is

[scottmg, 2011-08-17 19:54:12]

Thanks Roger,

>
http://codereview.chromium.org/7670031/diff/1/chrome/chrome_dll.gypi#newcode157
> chrome/chrome_dll.gypi:157: }], ['incremental_chrome_dll==1', {
> If I understand what you intend, the flow of these conditions isn't doing what
> you want it to.  The conditions here are equivalent to:

Hrm, yeah, I was confused. Something with keywords might be a bit easier to
read, eh? :) Yikes.

> Btw, does this work for Release builds, and for Official builds (Release plus
> other optimizations)? Does the flag need to understand what configuration is
> been used?

The flag should work fine/correctly for non-Debug builds (it's just linking to a
different dir and copying) but of course there's no benefit either. Doesn't seem
worthwhile to add the extra logic to explicitly disallow it for other builds
though.

[Roger McFarlane (Chromium), 2011-08-17 20:41:08]

For Release builds, incremental linking is explicitly turned off
(build/common.gypi at ~ line 1175) and /PROFILE is enabled. How does that
interact with the flag you're enabling?

http://codereview.chromium.org/7670031/diff/4001/chrome/chrome_dll.gypi
File chrome/chrome_dll.gypi (right):

http://codereview.chromium.org/7670031/diff/4001/chrome/chrome_dll.gypi#newco...
chrome/chrome_dll.gypi:159: 'OutputFile': '$(OutDir)\\incremental\\chrome.dll',
I'd also suggest reusing \\initial\\ to let the two flags co-exist.
optimize_with_syzygy==1 will put the dll into its rightful place in a downstream
step, but expects the output file to be in $(OutDir)\\initial.

[scottmg, 2011-08-17 21:05:21]

On 2011/08/17 20:41:08, rogerm wrote:
> For Release builds, incremental linking is explicitly turned off
> (build/common.gypi at ~ line 1175) and /PROFILE is enabled. How does that
> interact with the flag you're enabling?

It won't cause a problem, but it will not be an incremental link either.

I guess I could name the new flag something closer to what it actually does. It
*enables* an incremental link of chrome.dll, but doesn't specifically turn it
on.

None of the incremental link settings and gyp flags are really accomplishing
anything right now, so I'm not sure it matters.

> I'd also suggest reusing \\initial\\ to let the two flags co-exist.

Done, thanks. I misunderstood what you meant the first time around.

[Roger McFarlane (Chromium), 2011-08-18 13:05:54]

lgtm

[commit-bot: I haz the power, 2011-08-18 16:26:42]

No LGTM from valid reviewers yet.

[darin (slow to review), 2011-08-18 16:33:03]

LGTM

[M-A Ruel, 2011-08-18 17:43:46]

lgtm

[commit-bot: I haz the power, 2011-08-18 18:30:13]

Change committed as 97341