Skip to content

Conversation

@JunTaoLuo
Copy link
Contributor

@JunTaoLuo JunTaoLuo requested a review from dougbu March 14, 2019 22:07
@JunTaoLuo
Copy link
Contributor Author

I'm blocked on how to fix this here. The problem looks like https://github.com/aspnet/BuildTools/blob/release/2.2/files/KoreBuild/KoreBuild.Common.props#L38 where the normalized directory RepositoryRoot somehow ends up with a " in the value and that's throwing off the rest of MSBuild. I'm not familiar enough with MSBuild to figure out why or how to fix it.

@dougbu
Copy link
Contributor

dougbu commented Mar 15, 2019

@anurse since the problem you're having is Windows-specific, it's more likely the problem is in the PowerShell files. Having a look at https://dev.azure.com/dnceng/_apis/resources/Containers/1248307?itemPath=artifacts-Windows-Release%2Flogs%2Fmsbuild.rsp, I see /p:RepositoryRoot="F:\vsagent\81\s\\" i.e. things are messed up before MSBuild starts. Haven't tracked the issue down and will leave that you to you (unless you're also not familiar w/ PowerShell 😺).

@analogrelay
Copy link
Contributor

analogrelay commented Mar 15, 2019

I made some changes here in https://github.com/aspnet/BuildTools/pull/943/files#diff-6e1b5dc361d448d7a7e81c27825c08a1R297 that may be affecting things here, but in general, the \\" is expected because of how Windows command-line parsing works. We want a trailing \ but doing so next to the quote requires this extra escape. See https://devblogs.microsoft.com/oldnewthing/?p=12833 for some background.

It's possible my change made things a little funky though. The Convert-Path that I added is there to convert the path from a PowerShell path (which may use the "pseudo-drives" that PowerShell lets you create) into a native file system path, but it does add a trailing \ at the end. Because of the trailing slash added by Convert-Path, when we forward the Repo Root down to MSBuild using "$Path\\" later on in KoreBuild.psm1 you can end up with \\\" at the end which is treated as an escaped \ followed by and escaped ", which might explain this.

I also added logic to strip the trailing \ though so I'm a little surprised it's doing this. It's possible the build.ps1 is interacting with this in an unexpected way.

@JunTaoLuo
Copy link
Contributor Author

Ignore my previous comment. I tried to build locally and as @anurse described, anything other than \\" fails the build. Still trying to repro this locally and I'm starting think it has something to do with EF specifically.

@JunTaoLuo
Copy link
Contributor Author

JunTaoLuo commented Mar 15, 2019

Looks like the version of build tools used by EF is out of date. Will try to update that and see if it fixes this build. dotnet/efcore#15036

@dougbu
Copy link
Contributor

dougbu commented Mar 15, 2019

@JunTaoLuo bumping up BuildTools in EF Core was all well and good. But, how does that affect AspNetCore? I'd expect the submodule to be built using the version chosen here and / or for EF to use its version but not affect the larger build.

dougbu pushed a commit to dotnet-maestro-bot/AspNetCore that referenced this pull request Mar 15, 2019
* Handle OPTIONS requests without a handler in Razor Pages

Fixes dotnet#7438
@dougbu
Copy link
Contributor

dougbu commented Mar 15, 2019

In any case, the EF Core change is in. Need to rev the submodule here to pick that up.

@dougbu
Copy link
Contributor

dougbu commented Mar 15, 2019

@smitpatel @ajcvickers this picks up the #14887 fix. Without the submodule update, that change would not have shipped.

@dougbu
Copy link
Contributor

dougbu commented Mar 15, 2019

Looks as if the blob store containing the JDKs we need on Linux and Windows went awry. Hopefully this is temporary and that builds succeed on a retry.

@JunTaoLuo
Copy link
Contributor Author

Yea I figured. Already triggered retries.

@dougbu
Copy link
Contributor

dougbu commented Mar 15, 2019

@JunTaoLuo will need another AspNetCore-pr-validation-temp retry once Linux build completes:

C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\MSBuild\15.0\Bin\Microsoft.Common.CurrentVersion.targets(4551,5): warning MSB3026: Could not copy "F:\vsagent\82\s\src\Servers\IIS\AspNetCoreModuleV2\InProcessRequestHandler\bin\Release\Win32\aspnetcorev2_inprocess.dll" to "F:\vsagent\82\s\src\Servers\testassets\ServerComparison.TestSites\bin\Release\net461\x86\aspnetcorev2_inprocess.dll". Beginning retry 1 in 1000ms. The process cannot access the file 'F:\vsagent\82\s\src\Servers\testassets\ServerComparison.TestSites\bin\Release\net461\x86\aspnetcorev2_inprocess.dll' because it is being used by another process.  [F:\vsagent\82\s\src\Servers\testassets\ServerComparison.TestSites\ServerComparison.TestSites.csproj]
C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\MSBuild\15.0\Bin\Microsoft.Common.CurrentVersion.targets(4551,5): warning MSB3026: Could not copy "F:\vsagent\82\s\src\Servers\IIS\AspNetCoreModuleV2\InProcessRequestHandler\bin\Release\Win32\aspnetcorev2_inprocess.dll" to "bin\Release\netcoreapp2.2\x86\aspnetcorev2_inprocess.dll". Beginning retry 1 in 1000ms. The process cannot access the file 'bin\Release\netcoreapp2.2\x86\aspnetcorev2_inprocess.dll' because it is being used by another process.  [F:\vsagent\82\s\src\Servers\IIS\IIS\test\IIS.Tests\IIS.Tests.csproj]
F:\vsagent\82\s\build\RepositoryBuild.targets(143,5): error : Building EntityFrameworkCore failed: build.cmd exited code 1 [F:\vsagent\82\.dotnet\buildtools\korebuild\2.2.1-build-20190219.3\KoreBuild.proj]

@dougbu
Copy link
Contributor

dougbu commented Mar 15, 2019

@aspnet-hello
Copy link

This comment was made automatically. If there is a problem contact ryanbrandenburg.

I've triaged the above build.

Path must not contain trailing slashes apparently
@JunTaoLuo
Copy link
Contributor Author

Ok apparently the buildtools scripts were updated so they were relying on no trailing slashes but 2.2 submodule builds didn't get the memo. Fixed the submodule build scripts and the build is passing on a local repro.

@dougbu
Copy link
Contributor

dougbu commented Mar 15, 2019

Excellent ❕ Passed the debugging and back to real PR validations.

Sad note that the Windows validation is stuck behind 23 other jobs in AzDO.


<Exec
Command="./$(_BuildScriptToExecute) -Path $(BuildRepositoryRoot) $(BuildArguments)"
Command="./$(_BuildScriptToExecute) -Path $(BuildRepositoryRoot.TrimEnd('\')) $(BuildArguments)"
Copy link
Contributor Author

@JunTaoLuo JunTaoLuo Mar 15, 2019

Choose a reason for hiding this comment

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

Trimming here since the _BuildScriptToExecute expects no trailing slashes but everywhere else in this targets file the BuildRepositoryRoot is used to compute other paths and require a trailing slash. I'm not gonna try to make things more consistent and will just fix the path issues whenever they come up in the easiest way possible. AKA I'm gonna take the lazy approach.

@dougbu
Copy link
Contributor

dougbu commented Mar 16, 2019

@JunTaoLuo the AspNetCore-pr-validation-temp got to test signing but that failed with a new error:

F:\vsagent\68\.dotnet\buildtools\korebuild\2.2.1-build-20190219.3\modules\KoreBuild.Tasks\CodeSign.targets(51,5): error : Could not load file or assembly 'System.Private.CoreLib.dll' or one of its dependencies. An attempt was made to load a program with an incorrect format. [F:\vsagent\68\.dotnet\buildtools\korebuild\2.2.1-build-20190219.3\KoreBuild.proj]

Are we now passed one problem but hitting another?

The AspNetCore-ci failed much earlier due to a known flaky build error -- projects build out of order sometimes.

I restarted both builds in hopes we see Windows success somewhere!

@dougbu
Copy link
Contributor

dougbu commented Mar 16, 2019

Looks like this change is necessary but not sufficient. Seeing the CodeSign.targets in both Windows builds on retry. Probably need to look at a binary log to see where the "program with an incorrect format" is coming from.

@JunTaoLuo
Copy link
Contributor Author

JunTaoLuo commented Mar 16, 2019

My theory is that we need to update the version of Internal.AspNetCore.Sdk when we update the build tools version. Gonna look through the history to see how the version numbers are correlated.

@JunTaoLuo JunTaoLuo force-pushed the johluo/update-build-tools branch from e0c8db2 to e736a9a Compare March 16, 2019 05:48
@JunTaoLuo
Copy link
Contributor Author

@aspnet-hello
Copy link

This comment was made automatically. If there is a problem contact ryanbrandenburg.

I've triaged the above build.

@JunTaoLuo
Copy link
Contributor Author

Finally got a local repro for this behaviour. Seems like the failure occurs when trying to sign System.Private.CoreLib.dll. Are we supposed to sign this binary? Or is it supposed to be ignored? I'll scour the targets a bit more tomorrow to see if I can find some clues.

@dougbu
Copy link
Contributor

dougbu commented Mar 16, 2019

Seems like the failure occurs when trying to sign System.Private.CoreLib.dll. Are we supposed to sign this binary? Or is it supposed to be ignored?

System.Private.CoreLib.dll is a native assembly. Every .NET Core application loads it. From the error message, I suspect the app is trying to load this assembly and not sign it.

Could have trouble loading if the chosen copy of Microsoft.DotNet.SignTool.dll is picked from the wrong SignTool/tools directory (net461 versus netcoreapp2.0) or if the Microsoft.DotNet.SignTool.deps.json file (if running in dotnet msbuild) is messed up.

@dougbu dougbu requested review from ajcvickers and removed request for ajcvickers March 17, 2019 21:01
@dougbu
Copy link
Contributor

dougbu commented Mar 17, 2019

@joeloff @mmitche we're iterating here and getting further but are not able to get passed the validation checks (yet). I'm hoping this is the last iteration. But, if you see something we're missing, let us know ASAP.

@mmitche
Copy link
Member

mmitche commented Mar 18, 2019

Okay thanks.

@dougbu dougbu added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Mar 18, 2019
@JunTaoLuo
Copy link
Contributor Author

Looks like one of our updates to the Microsoft.DotNet.SignTool broke our signing.

From the binlog I see:

Extracting file 'System.Private.CoreLib.dll' from 'C:\gh\AspNetCore2\artifacts\Debug\installers\aspnetcore-runtime-2.2.4-servicing-190316-99-win-x64.zip' to 'C:\gh\AspNetCore2\obj\ContainerSigning\634\shared\Microsoft.NETCore.App\2.2.3\System.Private.CoreLib.dll'.
Tracking file 'C:\gh\AspNetCore2\obj\ContainerSigning\634\shared\Microsoft.NETCore.App\2.2.3\System.Private.CoreLib.dll' isNested=True
Errors
    C:\Users\johluo\.dotnet\buildtools\korebuild\2.2.1-build-20190219.3\modules\KoreBuild.Tasks\CodeSign.targets(51,5): Could not load file or assembly 'System.Private.CoreLib.dll' or one of its dependencies. An attempt was made to load a program with an incorrect format. [C:\Users\johluo\.dotnet\buildtools\korebuild\2.2.1-build-20190219.3\KoreBuild.proj]

The CertificateName attribute of the file is set to None which means the file should be skipped as per https://github.com/dotnet/arcade/blob/master/src/Microsoft.DotNet.SignTool/src/Configuration.cs#L294 but instead there's a file load error. Which means the exception is thrown somewhere somewhere in https://github.com/dotnet/arcade/blob/master/src/Microsoft.DotNet.SignTool/src/Configuration.cs#L243-L297 since this is all the code that runs between Tracking file ... and File configured to not be signed ... messages.

This means two possible issues:

  1. The file is invalid. I cannot confirm if this is the issue since I don't know how to verify if the file is formatted correctly or not. I am able to open it in ILSpy and the assembly is indeed x64 but contains unmanaged code (crossgen probably). I did verify it's the same dll as the released System.Private.CoreLib.dll as the released 2.2.3 patch so I doubt this is the issue.
  2. A regression in the SignTool. This is more likely since I am able to test locally with version "1.0.0-beta.18603.2" and I was able to run the build successfully in my local repro. I briefly looked over the changes made to SignTool at https://github.com/dotnet/arcade/commits/master/src/Microsoft.DotNet.SignTool/src/Configuration.cs but didn't spot anything obvious.

Actions we can take to resolve this issue:

  1. Revert from "1.0.0-beta.19119.1" to an earlier version of SignTool. It doesn't have to be "1.0.0-beta.18603.2" since that's just an older version I had on hand for testing. I could try to find the latest version of the package that works if required. This will require:
    a. Updating the SignTool version like in https://github.com/aspnet/BuildTools/pull/936/files
    b. Updating to a new buildtool version with lowered SignTool depedency
  2. Contact the arcade team to figure out what changed and whether it's a tool issue or a file issue. Which can result in:
    a. Fix the tool
    b. Fix the System.Private.CoreLib.dll

@mmitche
Copy link
Member

mmitche commented Mar 18, 2019

/cc @JohnTortugo @tmat for potential signtool issue

@JohnTortugo
Copy link

Looking into it

@JohnTortugo
Copy link

@JunTaoLuo I couldn't check which version of SignTool you're using, but I assume you are using something built before this PR got merged: dotnet/arcade#2170

Could you please try using the latest version of SignTool? Microsoft.DotNet.SignTool.1.0.0-beta.19167.10

@JunTaoLuo
Copy link
Contributor Author

Oh my, a windows test is passing! I think we finally made it.

@JunTaoLuo
Copy link
Contributor Author

Woohoo we did it @dougbu to merge.

@dougbu dougbu merged commit de5310e into release/2.2 Mar 18, 2019
@dougbu dougbu deleted the johluo/update-build-tools branch March 18, 2019 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants