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.

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

All System.Private.Corelib.csproj to build successfully in VS #1284

Merged
merged 1 commit into from
Jan 3, 2020

Conversation

danmoseley
Copy link
Member

These two changes are necessary for the project to open and build in VS for me.

  1. NativeRuntimeEventSource.cs is necessary for compilation when FEATURE_PERFTRACING is defined. FEATURE_PERFTRACING is defined by default. So the file must be visible to VS.

Why did '$(BuildingInsideVisualStudio)' != 'true' " exist here? @stephentoub added it in dotnet/coreclr#26076 to fix https://github.com/dotnet/coreclr/issues/26075 because it required that a build had to happen outside VS to generate the file - this does not use T4 or something VS understands.

Why did that not cause these compilation errors at that time? If I sync back to that point in coreclr and open the corelib project, I see that VS does not consider FEATURE_PERFTRACING defined. It is not clear why. I see the define present in $(DefineConstants) and I see it going to the compiler in the build log that VS produces, but nevertheless the VS "squiggle" build is apparently not receiving this define. Using the same version of VS against the head of dotnet/runtime, it is defined. Something changed in the build between those times that means VS now gets initialized correctly.

I figure it is better to have to build that file outside VS, and have build then work in VS, than have build always broken in VS.

The message explaining how to create it is a hack. I can remove it, but the debt already exists (a file that VS cannot create) and it seems better to help contributors be successful.

  1. $(PYTHON) is set in build.cmd, but typically not defined in a VS context. It is not clear to me why this was not causing problems in the past, but it seems we can skip this build step inside VS. I did not condition on $(PYTHON) being defined, since that could mask a future problem outside VS.

@danmoseley danmoseley changed the title Make corelib work in VS All System.Private.Corelib.csproj to build successfully in VS Jan 3, 2020
@EgorBo
Copy link
Member

EgorBo commented Jan 3, 2020

$(PYTHON) is set in build.cmd, but typically not defined in a VS context. It is not clear to me why this was not causing problems in the past

I think it was: https://github.com/dotnet/coreclr/issues/24500

@danmoseley danmoseley requested a review from jkotas January 3, 2020 20:39
@danmoseley
Copy link
Member Author

ARM failures only.

@danmoseley danmoseley merged commit 4bd7e3a into dotnet:master Jan 3, 2020
@danmoseley danmoseley deleted the loadcorelib branch January 3, 2020 23:00
@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants