Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Move windows agents to DotNetCore-Build#16849

Merged
MattGal merged 1 commit into
dotnet:masterfrom
MattGal:use-dotnetcore-build
Mar 22, 2018
Merged

Move windows agents to DotNetCore-Build#16849
MattGal merged 1 commit into
dotnet:masterfrom
MattGal:use-dotnetcore-build

Conversation

@MattGal
Copy link
Copy Markdown
Member

@MattGal MattGal commented Mar 8, 2018

@vancem , @jashook FYI. If CoreCLR can build in a VS2017-only environment now, this should be all that's left to make that happen.

Ping me for guidance on testing this if needed.

@jashook
Copy link
Copy Markdown

jashook commented Mar 9, 2018

Will start testing the changes.

/cc @RussKeldorph @vancem

@jashook
Copy link
Copy Markdown

jashook commented Mar 9, 2018

@jashook
Copy link
Copy Markdown

jashook commented Mar 9, 2018

Run failed due to not having msdia120.dll registered. Marking as no merge until the machines are setup correctly.

@jashook jashook added the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label Mar 9, 2018
@vancem
Copy link
Copy Markdown

vancem commented Mar 12, 2018

This is likely related to this issue were the DacTableGen is using a old version of msdia (and VS 2017 does not register it by default (but it does carry the DLL).

https://github.com/dotnet/coreclr/issues/11305#issuecomment-343009330

This hit developers who set up clean machines with just VS 2017 (which is probably what is happening on the build machines.

Fixing DacTableGen (that is issue 11305) would be nice, but it relies on very old nuget packages that may take some effort to update (even finding their source code was non-trivial).

Instead as a work-around we added the following to build.cmd which tests if msdia120.dll was registered and if not tells the user how to do this (you need elevation to run the command, and we did not want builds to require elevation).

REM Make the work-around to a bug in the microsoft.dotnet.buildtools.coreclr package until it is fixed.  
reg query HKEY_CLASSES_ROOT\WOW6432Node\CLSID\{3BFCEA48-620F-4B6B-81F7-B9AF75454C7D}\InprocServer32 > NUL: 2>&1
if NOT '%ERRORLEVEL%' == '0' (
    echo.
    echo.**********************************************************************************
    echo.Error: We have detected that the msdia120.dll is not registered.   
    echo.This is necessary for the build to complete without a Class_Not_Registered error.
    echo.
    echo.You can fix this by 
    echo.  1. Launching the "Developer Command Prompt for VS2017" with Administrative privileges
    echo.  2. Running  regsvr32.exe "%%VSINSTALLDIR%%\Common7\IDE\msdia120.dll"  
    echo.
    echo.This will only need to be done once for the lifetime of the machine.
    echo.For more details see: https://github.com/dotnet/coreclr/issues/11305
    exit /b 1
)

Notice this gives the instructions (a regsvr32.exe command) that can be run on these machines (it only needs to run once, but it is OK if runs many times), that registers a copy of msdia120.dll that lives with the VS2017 distribution.

Can someone with more knowledge of the machine setup scripts comment? A simple work-around would be to add this regsvr32 command to the setup scripts (or event the build scripts.

@vancem
Copy link
Copy Markdown

vancem commented Mar 19, 2018

PR #17004 should resolve the failure because of msdia120.dll

@jashook can you see if we succeed now?

@jashook
Copy link
Copy Markdown

jashook commented Mar 19, 2018

@MattGal MattGal force-pushed the use-dotnetcore-build branch from 1e2dafd to 2cda0e8 Compare March 22, 2018 17:15
@MattGal MattGal removed the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label Mar 22, 2018
@MattGal
Copy link
Copy Markdown
Member Author

MattGal commented Mar 22, 2018

Ready to merge, clean build using 2017 agents: https://devdiv.visualstudio.com/DevDiv/_build/index?buildId=1508454&_a=summary

Copy link
Copy Markdown

@vancem vancem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We ran a private build of this and it worked.
I think we are good to go.

@vancem
Copy link
Copy Markdown

vancem commented Mar 22, 2018

LGTM

@MattGal MattGal merged commit f6cda3e into dotnet:master Mar 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants