Install link wrapper in runhooks, and turn incremental on by default

Install link wrapper in runhooks, and turn incremental on by default

Installs supalink over link.exe by default via DEPS runhooks. Removes usage of
chrome_incremental_dll flag, and turns Use Library Dependency Inputs on for
places where it works. The flag is still in common.gypi until usage of it is
removed from WebKit repo.

BUG=94837
TEST=No link errors on all configs

[scottmg, 2011-09-06 23:26:19]

Turning incremental + Use Library Dependency Inputs on by default for Windows
build.

Possibly controversial part here is replacing the system's link.exe with a
wrapper, which is required to make that flag work.

LMK if you think there's others who would want to review/object.

[bradn, 2011-09-06 23:56:19]

Intriguing fix.
I'm concerned about using the DEPS like this to deploy it (likely won't work on
the bots given lack of the right privs). Also, this will really piss people off
that we're doing this without asking on their msvs install.

Could we instead:
1. Add a some means for the presence of the shim to be detected quickly from gyp
generation:
  a. registry
  b. file in some well known location
  c. maybe a script to try to run the linker, but I'd rather avoid all the
current assumptions about where msvs is installed.
2. Land source + binary + installer for the shim (for the purpose of capturing a
suggested speedup).
3. Land gyp conditional change of settings based on gyp time detection of the
presence of the shim.
4. update online docs to suggest the fix for new windows setups, email the team.
5. File an infrastructure ticket to gradually deploy the fix to the bot fleet,
eventually adding it to the base system images.

How's that sound?

http://codereview.chromium.org/7792103/diff/7001/DEPS
File DEPS (right):

http://codereview.chromium.org/7792103/diff/7001/DEPS#newcode429
DEPS:429: import sys
Lots of stuff (more than should) processes deps, lets not do arbitrary python
here. Can we make this run unconditionally and then do the platform check inside
the script?

http://codereview.chromium.org/7792103/diff/7001/tools/supalink/install_supal...
File tools/supalink/install_supalink.py (right):

http://codereview.chromium.org/7792103/diff/7001/tools/supalink/install_supal...
tools/supalink/install_supalink.py:13: f = open("temp.bat", "w")
Please match the python style guide.

http://codereview.chromium.org/7792103/diff/7001/tools/supalink/install_supal...
tools/supalink/install_supalink.py:54: shutil.copyfile(link, link_backup)
The bots don't run with admin privs, so this won't work as a deploy strategy.

http://codereview.chromium.org/7792103/diff/7001/tools/supalink/install_supal...
tools/supalink/install_supalink.py:65: /D_CRT_SECURE_NO_WARNINGS /EHsc
supalink.cpp\
Can we just check this program in?

[jam, 2011-09-07 01:29:32]

(btw I'm jam@chromium.org, autocomplete doesn't work with my email, but it does
with John Abd-El-Malek. need to investigate that someday)

I think we don't want to change a user's linker without their permission, at the
very least. Also, what happens if the user isn't running as admin? I had to run
supalink's install bat file from a command prompt that was run as admin, so
wouldn't this fail for everyone unless they were running from an admin cmd
prompt?

Can we file a ticket with MS to see if it's possible to change the path to the
linker from a vcproj? I have an msdn subscription if you want to use mine to ask
them.

[scottmg, 2011-09-07 03:03:12]

On 2011/09/06 23:56:19, bradn wrote:
> Intriguing fix.
> I'm concerned about using the DEPS like this to deploy it (likely won't work
on
> the bots given lack of the right privs).

That's what I figured too, but I've already "rooted" the trybots a few times.

> Also, this will really piss people off
> that we're doing this without asking on their msvs install.

Yeah. :( I was hoping the benefit outweighed the distastefulness. Does gyp know
if it's running interactively so we could ask if it's OK?

> Could we instead:
> 1. Add a some means for the presence of the shim to be detected quickly from
gyp
> generation:
>   a. registry
>   b. file in some well known location
>   c. maybe a script to try to run the linker, but I'd rather avoid all the
> current assumptions about where msvs is installed.
> 2. Land source + binary + installer for the shim (for the purpose of capturing
a
> suggested speedup).
> 3. Land gyp conditional change of settings based on gyp time detection of the
> presence of the shim.
> 4. update online docs to suggest the fix for new windows setups, email the
team.
> 5. File an infrastructure ticket to gradually deploy the fix to the bot fleet,
> eventually adding it to the base system images.
> 
> How's that sound?

Yes, I could make it semi-optional but slightly easier. That would take it from
2 steps down to 1 (i.e. if the user installs the shim then gyp can detect that
and turn the link flag on).

I was hoping to get it to be the default though because: 1) that way everyone
benefits from the speed improvement (many people are too busy to bother
otherwise); and 2) not to have use supporting 2 different configs. There are
cases where flag on will break compile, where flag off will not, so I wanted to
force the issue by forcing it on.


> 
> http://codereview.chromium.org/7792103/diff/7001/DEPS
> File DEPS (right):
> 
> http://codereview.chromium.org/7792103/diff/7001/DEPS#newcode429
> DEPS:429: import sys
> Lots of stuff (more than should) processes deps, lets not do arbitrary python
> here. Can we make this run unconditionally and then do the platform check
inside
> the script?

Thanks, will do.

>
http://codereview.chromium.org/7792103/diff/7001/tools/supalink/install_supal...
> File tools/supalink/install_supalink.py (right):
> 
>
http://codereview.chromium.org/7792103/diff/7001/tools/supalink/install_supal...
> tools/supalink/install_supalink.py:13: f = open("temp.bat", "w")
> Please match the python style guide.

Sorry, I haven't written much Python at Google. I'm not sure what's better here?
I didn't get any presubmit failure, and can't find a conflicting rule here
http://google-styleguide.googlecode.com/svn/trunk/pyguide.html. "f" is too brief
maybe?

> 
>
http://codereview.chromium.org/7792103/diff/7001/tools/supalink/install_supal...
> tools/supalink/install_supalink.py:54: shutil.copyfile(link, link_backup)
> The bots don't run with admin privs, so this won't work as a deploy strategy.

Comment above, though I'm not disagreeing that maybe they ought to. :)

>
http://codereview.chromium.org/7792103/diff/7001/tools/supalink/install_supal...
> tools/supalink/install_supalink.py:65: /D_CRT_SECURE_NO_WARNINGS /EHsc
> supalink.cpp\
> Can we just check this program in?

Thanks, will do.

[scottmg, 2011-09-07 03:11:13]

On 2011/09/07 01:29:32, John Abd-El-Malek wrote:

> Can we file a ticket with MS to see if it's possible to change the path to the
> linker from a vcproj? I have an msdn subscription if you want to use mine to
ask
> them.

Unfortunately it's not possible in a vcproj in 2008. If we could convince
Microsoft to fix something I'd get them to fix the silly bug in the linker
instead. :/

ref:
http://social.msdn.microsoft.com/Forums/en/vcgeneral/thread/716d0024-d900-4ff...

I poked around a bit more, and I'm pretty sure it *is* possible to avoid
stomping the linker if we get everyone to change the global VS settings in Tools
> Options > Projects and Settings > VC++ Directories, and insert the path to the
linker shim as the first item for "Executable Files". That data is stored in a
per-user file, so I guess we could have runhooks add our path in there instead?
http://blogs.msdn.com/b/vsproject/archive/2009/07/07/vc-directories.aspx

Does that sound better? I could try to rewrite the shim to work based on that
assumption if that sounds less yucky. It would effectively be the same (in that
we're sort of sneakily redirecting link.exe globally), but wouldn't actually
involve screwing with the linker exe.

[M-A Ruel, 2011-09-07 15:39:43]

There's 5 potential solutions here:

1. Replace system's link.exe, I think it's a bad idea.

2. Insert an executable directory to global VS settings and put the shim there,
I think it's a bad idea.

3. Insert an executable directory to the project's settings and put the shim
there, the best idea IMHO.
http://msdn.microsoft.com/library/microsoft.visualstudio.vcproject.vcdirector....

4. Not use VCLinkerTool at all and use a new .rule file. Could be complex to
implement. I also fear hitting some hard coded things in the IDE. Path to hell.

5. Do not bother much and instead accelerate update to VS2010.


I agree with brad, check in the precompiled tool.

I'll send an experiment about #3 in a separate message since it's verbose.

http://codereview.chromium.org/7792103/diff/7001/DEPS
File DEPS (right):

http://codereview.chromium.org/7792103/diff/7001/DEPS#newcode429
DEPS:429: import sys
On 2011/09/06 23:56:19, bradn wrote:
> Lots of stuff (more than should) processes deps, lets not do arbitrary python
> here. Can we make this run unconditionally and then do the platform check
inside
> the script?

Exact.

http://codereview.chromium.org/7792103/diff/7001/tools/supalink/README
File tools/supalink/README (right):

http://codereview.chromium.org/7792103/diff/7001/tools/supalink/README#newcode20
tools/supalink/README:20: Scott Graham <scottmg@chromium.org>
Remove, we know who you are. :)

[M-A Ruel, 2011-09-07 15:43:38]

>
>
http://codereview.chromium.**org/7792103/<http://codereview.chromium.org/7792...
>


Open MSVC, press Alt-F11 to open the macro editor, paste that in a new VB
module, open the command window in the VS IDE,
type Macros.MyMacros.Module1.foo, it should popup the executable directories
in a message box.

The annoying thing is that it looks like it doesn't persist in the .vcproj
... (!) Crap.

M-A

--- CUT HERE ---

Imports System
Imports EnvDTE
Imports Microsoft.VisualStudio.VCProjectEngine

Public Module Module1
    Function NavigateSolution(ByVal name As String) As EnvDTE.Project
        Try
            If Not DTE.Solution.IsOpen Then
                MsgBox("Please load or create a solution")
            Else
                Return NavigateProjects(DTE.Solution.Projects, name)
            End If
        Catch exception As System.Exception
            MsgBox(exception.ToString())
        End Try
        Return Nothing
    End Function

    Function NavigateProjects(ByVal projects As EnvDTE.Projects, ByVal name
As String) As EnvDTE.Project
        For Each project As EnvDTE.Project In projects
            Dim found As EnvDTE.Project = NavigateProject(project, name)
            If Not found Is Nothing Then
                Return found
            End If
        Next
        Return Nothing
    End Function

    Function NavigateProject(ByVal project As EnvDTE.Project, ByVal name As
String) As EnvDTE.Project
        If project.Name = name Then
            Return project
        End If
        For Each item As EnvDTE.ProjectItem In project.ProjectItems
            If Not item.SubProject() Is Nothing Then
                Dim found As EnvDTE.Project =
NavigateProject(item.SubProject(), name)
                If Not found Is Nothing Then
                    Return found
                End If
            End If
        Next
        Return Nothing
    End Function

    Sub foo()
        Dim chrome_dll As EnvDTE.Project = NavigateSolution("chrome_dll")
        ' If this line is shown as an error, add a reference to
        ' Microsoft.VisualStudio.VCProjectEngine(first!)
        Dim chrome_dll_vc As VCProject = chrome_dll.Object
        For Each platform As VCPlatform In chrome_dll_vc.Platforms
            If platform.Name = "Win32" Then
                'platform.ExecutableDirectories = "d:\foo;" &
platform.ExecutableDirectories
                MsgBox(platform.Name & ": " &
platform.ExecutableDirectories)
            End If
        Next
    End Sub
End Module

--- CUT HERE ---

[jam, 2011-09-07 16:04:46]

On 2011/09/07 15:39:43, Marc-Antoine Ruel wrote:
> There's 5 potential solutions here:
> 
> 1. Replace system's link.exe, I think it's a bad idea.
> 
> 2. Insert an executable directory to global VS settings and put the shim
there,
> I think it's a bad idea.
> 
> 3. Insert an executable directory to the project's settings and put the shim
> there, the best idea IMHO.
>
http://msdn.microsoft.com/library/microsoft.visualstudio.vcproject.vcdirector...).
> 
> 4. Not use VCLinkerTool at all and use a new .rule file. Could be complex to
> implement. I also fear hitting some hard coded things in the IDE. Path to
hell.
> 
> 5. Do not bother much and instead accelerate update to VS2010.

Linking in 2010 isn't faster than 2008, based on my experience. The GUI is much
slower though.

> 
> 
> I agree with brad, check in the precompiled tool.
> 
> I'll send an experiment about #3 in a separate message since it's verbose.
> 
> http://codereview.chromium.org/7792103/diff/7001/DEPS
> File DEPS (right):
> 
> http://codereview.chromium.org/7792103/diff/7001/DEPS#newcode429
> DEPS:429: import sys
> On 2011/09/06 23:56:19, bradn wrote:
> > Lots of stuff (more than should) processes deps, lets not do arbitrary
python
> > here. Can we make this run unconditionally and then do the platform check
> inside
> > the script?
> 
> Exact.
> 
> http://codereview.chromium.org/7792103/diff/7001/tools/supalink/README
> File tools/supalink/README (right):
> 
>
http://codereview.chromium.org/7792103/diff/7001/tools/supalink/README#newcode20
> tools/supalink/README:20: Scott Graham <mailto:scottmg@chromium.org>
> Remove, we know who you are. :)

[scottmg, 2011-09-07 16:17:37]

On Wed, Sep 7, 2011 at 8:39 AM,  <maruel@chromium.org> wrote:
> There's 5 potential solutions here:

Thanks for the summary. There's a lot of "not quite" solutions. :(

>
> 1. Replace system's link.exe, I think it's a bad idea.
>
> 2. Insert an executable directory to global VS settings and put the shim
> there,
> I think it's a bad idea.
>
> 3. Insert an executable directory to the project's settings and put the shim
> there, the best idea IMHO.
>
http://msdn.microsoft.com/library/microsoft.visualstudio.vcproject.vcdirector....

I'm not aware of any way to do this properly, but I'd be happy if you
could find a way. IIRC, CMake uses a first-run install process which
installs their helper macros. I guess gyp could do something like
this, but it's still going to be global, not attached to the vcproj.

> 4. Not use VCLinkerTool at all and use a new .rule file. Could be complex to
> implement. I also fear hitting some hard coded things in the IDE. Path to
> hell.
>
> 5. Do not bother much and instead accelerate update to VS2010.

Are we sure it's fixed? Did you get any useful follow up on this
issue?
http://connect.microsoft.com/VisualStudio/feedback/details/461987/use-library...
(assuming you're one of the commenters there)

I can do a quick test to see if the limit is still there in 2010. I'd
generally be fine with upgrading sooner if there's no reason not to,
though it's generally prudent to wait for a few more SPs/hotfixes.


>
>
> I agree with brad, check in the precompiled tool.
>
> I'll send an experiment about #3 in a separate message since it's verbose.
>
>
> http://codereview.chromium.org/7792103/diff/7001/DEPS
> File DEPS (right):
>
> http://codereview.chromium.org/7792103/diff/7001/DEPS#newcode429
> DEPS:429: import sys
> On 2011/09/06 23:56:19, bradn wrote:
>>
>> Lots of stuff (more than should) processes deps, lets not do arbitrary
>
> python
>>
>> here. Can we make this run unconditionally and then do the platform
>
> check inside
>>
>> the script?
>
> Exact.
>
> http://codereview.chromium.org/7792103/diff/7001/tools/supalink/README
> File tools/supalink/README (right):
>
>
http://codereview.chromium.org/7792103/diff/7001/tools/supalink/README#newcode20
> tools/supalink/README:20: Scott Graham <scottmg@chromium.org>
> Remove, we know who you are. :)

Oops, thanks. Was copied from my original github project.

>
> http://codereview.chromium.org/7792103/
>

[scottmg, 2011-09-07 16:23:36]

> Are we sure it's fixed? Did you get any useful follow up on this
> issue?
http://connect.microsoft.com/VisualStudio/feedback/details/461987/use-library...
> (assuming you're one of the commenters there)

Oh, nvm, of course it doesn't matter if it's fixed or not, as it'll be
easy to install the shim anyway via MSBuild I guess.

[bradn, 2011-09-20 22:16:06]

Didn't mean to derail this.
What's the status Scott?

-BradN


On Wed, Sep 7, 2011 at 9:23 AM, Scott Graham <scottmg@chromium.org> wrote:

> > Are we sure it's fixed? Did you get any useful follow up on this
> > issue?
>
http://connect.microsoft.com/VisualStudio/feedback/details/461987/use-library...
> > (assuming you're one of the commenters there)
>
> Oh, nvm, of course it doesn't matter if it's fixed or not, as it'll be
> easy to install the shim anyway via MSBuild I guess.
>

[scottmg, 2011-09-20 22:30:36]

Well, we're against replacing the system linker, but there's no other
working solution (other than just making people do that themselves).

So... current status is 5-10 people are using it locally, but we don't
have a good path to getting it on by default.

I'd like to verify if we need the linker hack on 2010 or not, as that
would be lower friction. I haven't had a chance yet, but I'll check in
the next few days. If that works, then I'll simply tie Use Library
Dependency Inputs to (VS2010 && Debug) and not bother pursuing default
2008 support. It seems to have been fairly low-pain for people using
it locally so those who care can just keep it on with a local flag for
2008.

On Tue, Sep 20, 2011 at 3:16 PM, Bradley Nelson <bradnelson@google.com> wrote:
> Didn't mean to derail this.
> What's the status Scott?
> -BradN
>
> On Wed, Sep 7, 2011 at 9:23 AM, Scott Graham <scottmg@chromium.org> wrote:
>>
>> > Are we sure it's fixed? Did you get any useful follow up on this
>> > issue?
>> >
http://connect.microsoft.com/VisualStudio/feedback/details/461987/use-library...
>> > (assuming you're one of the commenters there)
>>
>> Oh, nvm, of course it doesn't matter if it's fixed or not, as it'll be
>> easy to install the shim anyway via MSBuild I guess.
>
>

[scottmg, 2011-09-23 19:08:45]

The good news is that the linker hack doesn't appear to be necessary on 2010.

The bad news is that ULDI (i.e. incremental) is broken on 2010 (post
on gyp-developer with possible workarounds
http://groups.google.com/group/gyp-developer/browse_thread/thread/e42c26493bb...).
I would probably lean towards solution #2 in that thread. Anyone
support that style of project generation (and think it's worth the
effort), or have another suggestion?

I'm retreating to locally futzed 2008 for now :-( as non-incremental
is basically unusable.

On Tue, Sep 20, 2011 at 3:30 PM, Scott Graham <scottmg@chromium.org> wrote:
> Well, we're against replacing the system linker, but there's no other
> working solution (other than just making people do that themselves).
>
> So... current status is 5-10 people are using it locally, but we don't
> have a good path to getting it on by default.
>
> I'd like to verify if we need the linker hack on 2010 or not, as that
> would be lower friction. I haven't had a chance yet, but I'll check in
> the next few days. If that works, then I'll simply tie Use Library
> Dependency Inputs to (VS2010 && Debug) and not bother pursuing default
> 2008 support. It seems to have been fairly low-pain for people using
> it locally so those who care can just keep it on with a local flag for
> 2008.
>
> On Tue, Sep 20, 2011 at 3:16 PM, Bradley Nelson <bradnelson@google.com> wrote:
>> Didn't mean to derail this.
>> What's the status Scott?
>> -BradN
>>
>> On Wed, Sep 7, 2011 at 9:23 AM, Scott Graham <scottmg@chromium.org> wrote:
>>>
>>> > Are we sure it's fixed? Did you get any useful follow up on this
>>> > issue?
>>> >
http://connect.microsoft.com/VisualStudio/feedback/details/461987/use-library...
>>> > (assuming you're one of the commenters there)
>>>
>>> Oh, nvm, of course it doesn't matter if it's fixed or not, as it'll be
>>> easy to install the shim anyway via MSBuild I guess.
>>
>>
>

[M-A Ruel, 2011-09-23 19:14:43]

It'd be interested to have a script in tools/ to do whatever necessary to
manually install it. The user would have to run it manually once.

It would be beneficial for the build slaves, we just need the base image to
have it run once.

M-A

Le 23 septembre 2011 15:08, Scott Graham <scottmg@chromium.org> a écrit :

> The good news is that the linker hack doesn't appear to be necessary on
> 2010.
>
> The bad news is that ULDI (i.e. incremental) is broken on 2010 (post
> on gyp-developer with possible workarounds
>
>
http://groups.google.com/group/gyp-developer/browse_thread/thread/e42c26493bb...
> ).
> I would probably lean towards solution #2 in that thread. Anyone
> support that style of project generation (and think it's worth the
> effort), or have another suggestion?
>
> I'm retreating to locally futzed 2008 for now :-( as non-incremental
> is basically unusable.
>
> On Tue, Sep 20, 2011 at 3:30 PM, Scott Graham <scottmg@chromium.org>
> wrote:
> > Well, we're against replacing the system linker, but there's no other
> > working solution (other than just making people do that themselves).
> >
> > So... current status is 5-10 people are using it locally, but we don't
> > have a good path to getting it on by default.
> >
> > I'd like to verify if we need the linker hack on 2010 or not, as that
> > would be lower friction. I haven't had a chance yet, but I'll check in
> > the next few days. If that works, then I'll simply tie Use Library
> > Dependency Inputs to (VS2010 && Debug) and not bother pursuing default
> > 2008 support. It seems to have been fairly low-pain for people using
> > it locally so those who care can just keep it on with a local flag for
> > 2008.
> >
> > On Tue, Sep 20, 2011 at 3:16 PM, Bradley Nelson <bradnelson@google.com>
> wrote:
> >> Didn't mean to derail this.
> >> What's the status Scott?
> >> -BradN
> >>
> >> On Wed, Sep 7, 2011 at 9:23 AM, Scott Graham <scottmg@chromium.org>
> wrote:
> >>>
> >>> > Are we sure it's fixed? Did you get any useful follow up on this
> >>> > issue?
> >>> >
>
http://connect.microsoft.com/VisualStudio/feedback/details/461987/use-library...
> >>> > (assuming you're one of the commenters there)
> >>>
> >>> Oh, nvm, of course it doesn't matter if it's fixed or not, as it'll be
> >>> easy to install the shim anyway via MSBuild I guess.
> >>
> >>
> >
>

[scottmg, 2011-09-27 21:00:54]

On 2011/09/23 19:14:43, Marc-Antoine Ruel wrote:
> It'd be interested to have a script in tools/ to do whatever necessary to
> manually install it. The user would have to run it manually once.

Mistakenly uploaded from a different branch, but
http://codereview.chromium.org/8059024/ takes a combination of this approach and
the one suggested by Brad.