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

macOS build fails on NativeRuntimeEventSource.cs with old python version #688

Closed
marek-safar opened this issue Dec 9, 2019 · 11 comments
Closed

Comments

@marek-safar
Copy link
Contributor

marek-safar commented Dec 9, 2019

Initial cost estimate: 2 days
Initial contacts: @trylek, @ViktorHofer

macOS build fails when old python is used without an easy way to see that. I have the default python installed by macOS (2.7) but that's not enough to build runtime master version. The build fails with following

CSC : error CS2001: Source file '/<path>/runtime/src/coreclr/../../artifacts/obj/coreclr/System.Private.CoreLib/x64/Debug/../../../Eventing/x64/Debug/NativeRuntimeEventSource.cs' could not be found. 

The error is gone after installing python3.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-Infrastructure-coreclr untriaged New issue has not been triaged by the area owner labels Dec 9, 2019
@hoyosjs
Copy link
Member

hoyosjs commented Dec 9, 2019

@sywhang just fyi

@jashook
Copy link
Contributor

jashook commented May 11, 2020

Python3 is a requirement, if we do not have an error message this issue should track adding one.

@jashook jashook removed the untriaged New issue has not been triaged by the area owner label May 11, 2020
@jashook jashook added this to the Future milestone Jul 17, 2020
@ViktorHofer
Copy link
Member

@jkotas do you think it would make sense to convert the https://github.com/dotnet/runtime/blob/15460311833f161bbe90b61ef94a8f339178840b/src/coreclr/src/scripts/genRuntimeEventSources.py script which runs during the CoreLib build to msbuild (probably via a C# msbuild task)?

@jkotas
Copy link
Member

jkotas commented Dec 8, 2020

Yes!!

I think we should be eliminating our dependencies on python, first in build, and eventually in tests too.

@trylek
Copy link
Member

trylek commented Dec 8, 2020

@ViktorHofer - in light of JanK's response please file a follow-up CoreCLR infra item to remove the Python script by rewriting it probably to C# per your suggestion. I guess that in such case it doesn't make too much sense to continue pursuing this work item. We'll need to update the CoreCLR infra backlog local dev innerloop epic to reflect the follow-up item, sadly this is still very much manual.

@ViktorHofer
Copy link
Member

Filed #45733. Let's close this one then.

@trylek
Copy link
Member

trylek commented Dec 8, 2020

Thanks Viktor, I have updated the backlog as appropriate. Feel free to adjust the initial cost estimate, just please let me know or update the dev innerloop epic in the infra backlog to reflect the change.

@ViktorHofer
Copy link
Member

ViktorHofer commented Dec 8, 2020

I don't think this should take longer than a week max. I wonder if @hoyosjs or @jkoritzinsky might be interested in picking this task up.

@hoyosjs
Copy link
Member

hoyosjs commented Dec 8, 2020

There's three scripts, it needs to be validated for ETW, LTTNG, and EventPipe. There's different behavior for each and different codegen for each. A week isn't realistic. I really don't know how much value there's to this right now. Diagnostics is pretty swamped on work and issues at the moment. We had considered this at some point, but just didn't see enough value on the time investment.

cc: @dotnet/dotnet-diag

@ViktorHofer
Copy link
Member

ViktorHofer commented Dec 8, 2020

There's three scripts, it needs to be validated for ETW, LTTNG, and EventPipe. There's different behavior for each and different codegen for each. A week isn't realistic.

Got it. Reminds me that I shouldn't estimate work for an area I'm not familiar with 🗡️

@ViktorHofer
Copy link
Member

Regarding impact, besides the overall goodness of one less build dependency, this lowers the first time contributor bar noticeably. Based on issues we've seen in the past, python is one of the tools that isn't installed on every dev's machine.

@ghost ghost locked as resolved and limited conversation to collaborators Jan 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants