dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.25k stars 4.73k forks source link

ILC.exe runs twice for each Identity when running tests on uapaot #27119

Closed ViktorHofer closed 4 years ago

ViktorHofer commented 6 years ago

https://github.com/dotnet/buildtools/blob/master/src/Microsoft.DotNet.Build.Tasks/PackageFiles/tests.targets#L401-L406

This generates the commands twice in the RunTest.cmd because the echo is misplaced. The first time it will be executed as part of the template (echo --> to repro directly): https://github.com/dotnet/buildtools/blob/master/src/Microsoft.DotNet.Build.Tasks/PackageFiles/RunnerTemplate.Windows.txt#L16 and the second time as part of the execution itself.

echo
  call %RUNTIME_PATH%\TestILC\ilc.exe -ExeName xunit.console.exe -in %EXECUTION_DIR% -out 
  %EXECUTION_DIR%int\xunit.console.exe\ -usedefaultpinvoke -buildtype ret -v diag 
  set ILCERRORLEVEL=%ERRORLEVEL%
  if NOT [%ILCERRORLEVEL%] == [0] exit /b %ILCERRORLEVEL%
  robocopy /S /NP %EXECUTION_DIR%int\xunit.console.exe\ %EXECUTION_DIR%native\

This will be fixed in https://github.com/dotnet/buildtools/pull/2106.

cc @danmosemsft @safern @joperezr @MattGal

joperezr commented 6 years ago

Yeah it in fact does run twice, I believe @morganbr had already reported that and logged an issue for it but I'm having trouble finding it. Once I do I'll close this as a dupe of that one.

joperezr commented 6 years ago

Here it is: https://github.com/dotnet/buildtools/issues/2044 It looks like @morganbr closed that issue already, so perhaps this is fixed now. We should just double check

ViktorHofer commented 6 years ago

Weird, it didn't work for me and I had the latest changes. Doesn't really matter as it will change.

joperezr commented 6 years ago

Oh I see, looks like he hasn't merged his PR fixing this yet: https://github.com/dotnet/buildtools/pull/2095 Do make sure that you incorporate his fix on your changes.

ViktorHofer commented 6 years ago

I don't think that dotnet/buildtools#2095 is related to the fix at all? It only adds an argument in the line of trouble.

morganbr commented 6 years ago

https://github.com/dotnet/buildtools/pull/2085 was supposed to fix it, but maybe it didn't work (or the change hasn't propagated to CoreFX since I don't know how that happens). https://github.com/dotnet/buildtools/pull/2095 is unrelated.

safern commented 6 years ago

dotnet/buildtools#2085 was supposed to fix it, but maybe it didn't work (or the change hasn't propagated to CoreFX since I don't know how that happens)

That should happen automatically by a maestro auto-PR opened against corefx to update the version of buildtools once a merge build is green. Which by the date of your change, I would suspect it is already reflected in corefx. From what I see in the history of corefx/DotnetBuildToolsVersion.txt it has happened quite some times after your change went in.