Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

System.Private.CoreLib's fast up-to-date check is broken #68698

Closed
ViktorHofer opened this issue Apr 29, 2022 · 8 comments · Fixed by #69240
Closed

System.Private.CoreLib's fast up-to-date check is broken #68698

ViktorHofer opened this issue Apr 29, 2022 · 8 comments · Fixed by #69240
Labels
area-Infrastructure-coreclr
Milestone
Future

Comments

@ViktorHofer
Copy link
Member

When building System.Runtime's src project, which depends on System.Private.CoreLib without making any changes to any of the assemblies, Systme.Private.CoreLib always incrementally builds:

1>------ Build started: Project: System.Private.CoreLib, Configuration: Debug x64 ------
1>System.Private.CoreLib -> C:\git\runtime5\artifacts\bin\coreclr\windows.x64.Debug\IL\System.Private.CoreLib.dll
========== Build: 1 succeeded, 0 failed, 5 up-to-date, 0 skipped ==========

1>------ Build started: Project: System.Private.CoreLib, Configuration: Debug x64 ------
1>System.Private.CoreLib -> C:\git\runtime5\artifacts\bin\coreclr\windows.x64.Debug\IL\System.Private.CoreLib.dll
========== Build: 1 succeeded, 0 failed, 5 up-to-date, 0 skipped ==========

1>------ Build started: Project: System.Private.CoreLib, Configuration: Debug x64 ------
1>System.Private.CoreLib -> C:\git\runtime5\artifacts\bin\coreclr\windows.x64.Debug\IL\System.Private.CoreLib.dll
========== Build: 1 succeeded, 0 failed, 5 up-to-date, 0 skipped ==========

This shouldn't happen as Visual Studio caches inputs and outputs and makes sure that a project's build isn't even triggered when nothing changed.

cc @dotnet/runtime-infrastructure @trylek @agocke @jkotas
@ghost
Copy link

ghost commented Apr 29, 2022

Tagging subscribers to this area: @hoyosjs
See info in area-owners.md if you want to be subscribed.

Issue Details

When building System.Runtime's src project, which depends on System.Private.CoreLib without making any changes to any of the assemblies, Systme.Private.CoreLib always incrementally builds:

1>------ Build started: Project: System.Private.CoreLib, Configuration: Debug x64 ------
1>System.Private.CoreLib -> C:\git\runtime5\artifacts\bin\coreclr\windows.x64.Debug\IL\System.Private.CoreLib.dll
========== Build: 1 succeeded, 0 failed, 5 up-to-date, 0 skipped ==========

1>------ Build started: Project: System.Private.CoreLib, Configuration: Debug x64 ------
1>System.Private.CoreLib -> C:\git\runtime5\artifacts\bin\coreclr\windows.x64.Debug\IL\System.Private.CoreLib.dll
========== Build: 1 succeeded, 0 failed, 5 up-to-date, 0 skipped ==========

1>------ Build started: Project: System.Private.CoreLib, Configuration: Debug x64 ------
1>System.Private.CoreLib -> C:\git\runtime5\artifacts\bin\coreclr\windows.x64.Debug\IL\System.Private.CoreLib.dll
========== Build: 1 succeeded, 0 failed, 5 up-to-date, 0 skipped ==========

This shouldn't happen as Visual Studio caches inputs and outputs and makes sure that a project's build isn't even triggered when nothing changed.

cc @dotnet/runtime-infrastructure @trylek @agocke @jkotas

Author: ViktorHofer
Assignees: -
Labels:

area-Infrastructure-coreclr
Milestone: -

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Apr 29, 2022
@ViktorHofer ViktorHofer mentioned this issue Apr 29, 2022
Closed
@ViktorHofer
Copy link
Member Author

@drewnoakes @davkean is it possible to diagnose why the fast up-to-date check fails for System.Private.CoreLib via some knobs?

@ViktorHofer
Copy link
Member Author

I already found the relevant documentation: https://github.com/dotnet/project-system/blob/main/docs/up-to-date-check.md#sdk-style-projects.

3>FastUpToDate: Output 'C:\git\runtime5\artifacts\bin\coreclr\windows.x64.Debug\IL\System.Private.CoreLib.pdb' does not exist, not up-to-date. (System.Private.CoreLib)

@ViktorHofer
Copy link
Member Author

ViktorHofer commented Apr 29, 2022 •
edited
Loading

Found the culprit. These two targets move the pdb from the build directory into a separate location and by that, invalidate the output:

runtime/src/coreclr/Directory.Build.targets

Lines 11 to 35 in 8656518

<!-- Target used to consolidate all PDBs into a single location, except the aotsdk
PDBs, as S.P.C will overwrite the coreclr PDBs -->
<Target Name="MoveSymbolFiles"
AfterTargets="Build"
Condition="Exists(@(_AllPdbs))"
Inputs="@(_AllPdbs)"
Outputs="@(_AllPdbs -> '$(RuntimeBinDir)PDB/%(Filename).pdb')">
<Move SourceFiles="@(_AllPdbs)"
DestinationFolder="$(RuntimeBinDir)PDB"
Condition="'%(_AllPdbs.FolderName)' != 'aotsdk'" />
</Target>
<Target Name="MoveAotSymbolFiles"
AfterTargets="Build"
Condition="Exists(@(_AllPdbs))"
Inputs="@(_AllPdbs)"
Outputs="@(_AllPdbs -> '$(RuntimeBinDir)PDB/aotsdk/%(Filename).pdb')">
<Move SourceFiles="@(_AllPdbs)"
DestinationFolder="$(RuntimeBinDir)PDB/aotsdk"
Condition="'%(_AllPdbs.FolderName)' == 'aotsdk'" />
</Target>

@trylek @agocke @jkotas does the pdb need to be moved or can it be copied instead?

@danmoseley
Copy link
Member

The comment there mentions that they'd otherwise be overwritten. Are any outputs being updated in place? Moves and updates in place both make incremental build harder.

@jkotas
Copy link
Member

jkotas commented Apr 29, 2022

I would be even better to place the .pdbs into the right directory so that they do not have to be moved or copied.

@agocke should be able to tell what can be done about this since he has investigated and changed this recently in #68063

@agocke
Copy link
Member

agocke commented May 2, 2022

Yeah, each could be placed in a separate directory for its project, I think. I was just working around the NativeAOT/IL S.P.C collision. The broad view is having separate directories, instead of just for AOT

@agocke agocke added this to the Future milestone May 2, 2022
@agocke agocke removed the untriaged New issue has not been triaged by the area owner label May 2, 2022
@ViktorHofer
Copy link
Member Author

ViktorHofer commented May 2, 2022 •
edited
Loading

If I have some time I would be willing to send a PR to fix this. @jkotas @agocke just to clarify, did you mean that the pdb should be placed into the $(RuntimeBinDir)PDB / $(RuntimeBinDir)PDB/aotsdk directory or that the pdb belongs into the output directory of System.Private.CoreLib and that whatever tool requires the pdbs, should look into the project's individual output directories?

ViktorHofer added a commit to ViktorHofer/runtime that referenced this issue May 12, 2022
@ViktorHofer
Fix CoreLib fast up-to-date check
7bf7cf9 
Fixes dotnet#68698

CoreLib's fast up-to-date check was broken because the output pdb was
moved into a different folder. Changing this to use a copy task instead
and consolidate the two targets into one.

I couldn't find a publicly exposed property that allows to change the
pdb's output path, hence I decided to copy.
@ViktorHofer ViktorHofer mentioned this issue May 12, 2022
Merged
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label May 12, 2022
ViktorHofer added a commit that referenced this issue May 12, 2022
@ViktorHofer
Fix CoreLib fast up-to-date check (#69240)
c96b440 
* Fix CoreLib fast up-to-date check

Fixes #68698

CoreLib's fast up-to-date check was broken because the output pdb was
moved into a different folder. Changing this to use a copy task instead
and consolidate the two targets into one.

I couldn't find a publicly exposed property that allows to change the
pdb's output path, hence I decided to copy.
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label May 12, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jun 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
area-Infrastructure-coreclr
Projects
None yet
Milestone
Future
Development

Successfully merging a pull request may close this issue.

Fix CoreLib fast up-to-date check ViktorHofer/runtime
4 participants
@agocke @danmoseley @jkotas @ViktorHofer