search

google / UIforETW

User interface for recording and managing ETW traces
https://randomascii.wordpress.com/2015/04/14/uiforetw-windows-performance-made-easier/
Apache License 2.0
1.57k stars 201 forks source link

UIForETW doesn't detect depot_tools' python #53

Closed Chubab closed 9 years ago

Chubab commented 9 years ago

While saving traces to disk I get:

"Preprocessing trace to identify Chrome processes... Couldn't find python."

I have python, but like most Chrome Windows developers, it's depot_tools' which actually means I have python.bat (not python.exe itself) in my PATH.

Cheers, Gab

randomascii commented 9 years ago

My Chromium development machine has two python related directories in the path:

C:\python_27_amd64\files c:\src\depot_tools

The first one contains python.exe. I'm not sure how that directory got added to my path. Its placement in the path environment variable strongly suggests that I did not add it manually.

I checked on another Chromium developer's machine and they had the exact same paths, on a machine that was set up last week, and they were certain that they did not manually add the paths.

So, perhaps your setup is non-standard? Or perhaps its worth adding the Python directory to your path.

Chubab commented 9 years ago

I think if one explicitly installs python on Windows via its installer, it may automatically add it to PATH.

I didn't explicitly install python because I just use depot_tools'; is there any way we can make UIForETW fallback to using python.bat if in path?

On Tue, Sep 1, 2015 at 4:27 PM Bruce Dawson notifications@github.com wrote:

My Chromium development machine has two python related directories in the path:

C:\python_27_amd64\files c:\src\depot_tools

The first one contains python.exe. I'm not sure how that directory got added to my path. Its placement in the path environment variable strongly suggests that I did not add it manually.

I checked on another Chromium developer's machine and they had the exact same paths, on a machine that was set up last week, and they were certain that they did not manually add the paths.

So, perhaps your setup is non-standard? Or perhaps its worth adding the Python directory to your path.

— Reply to this email directly or view it on GitHub https://github.com/google/UIforETW/issues/53#issuecomment-136851978.

randomascii commented 9 years ago

I'm sure we can do something. UIforETW could parse python.bat (just the depot_tools version) and use it to locate python.exe - should be easy enough.

It's just weird that your install doesn't have python in the path. Probably they changed it in the last year to add python.exe to the path - just a guess.

Chubab commented 9 years ago

I actually just re-did my setup for Win10 (although I didn't have to reinstall depot_tools per se as it was on D: and stayed around despite flashing the OS).

Don't see how I would end up with python in my path without installing it anyways though?

On Wed, Sep 2, 2015 at 1:26 PM Bruce Dawson notifications@github.com wrote:

I'm sure we can do something. UIforETW could parse python.bat (just the depot_tools version) and use it to locate python.exe - should be easy enough.

It's just weird that your install doesn't have python in the path. Probably they changed it in the last year to add python.exe to the path - just a guess.

— Reply to this email directly or view it on GitHub https://github.com/google/UIforETW/issues/53#issuecomment-137179099.

randomascii commented 9 years ago

I never installed Python - I assumed that the depot_tools setup process did, somehow, but I'm not sure how/when.

randomascii commented 9 years ago

How's this?

https://github.com/google/UIforETW/commit/c7891955a0cf5ef77f62c3c37245f097126c9daf

It's somewhat hardcoded - it expects to find a particular syntax to invoke python.exe - but I'm not sure I can make it more generic without knowing what variations of python.bat exist.

I'll probably reduce the indentation level before committing to master, but the nested if statements made testing easier. It does work.

Chubab commented 9 years ago

I guess that works, thanks (though I'd think this should be a fallback if python.exe is not found rather than being first).

Or could you have done a simple search for python.bat the same way you look for python.exe and invoke python.bat directly (I.e., why is an exe required?)

Le jeu. 3 sept. 2015 01:17, Bruce Dawson notifications@github.com a écrit :

How's this?

c789195 https://github.com/google/UIforETW/commit/c7891955a0cf5ef77f62c3c37245f097126c9daf

It's somewhat hardcoded - it expects to find a particular syntax to invoke python.exe - but I'm not sure I can make it more generic without knowing what variations of python.bat exist.

I'll probably reduce the indentation level before committing to master, but the nested if statements made testing easier. It does work.

— Reply to this email directly or view it on GitHub https://github.com/google/UIforETW/issues/53#issuecomment-137334503.

randomascii commented 9 years ago

I was definitely going to look for python.exe first. For some reason I hadn't considered launching python.bat directly. I guess I'd have to launch cmd.exe /c python.bat or something like that - I'm not sure, but I doubt python.bat can be launched directly. Thanks for the feedback.

Chubab commented 9 years ago

Or just ShellExecute the path which apparently works with either .exe or .bat? (ref.

http://stackoverflow.com/questions/16621911/shellexecute-not-working-with-bat-file

which is ironically titled the opposite of a working solution with shellexecute :-)).

On Thu, Sep 3, 2015 at 10:55 AM Bruce Dawson notifications@github.com wrote:

I was definitely going to look for python.exe first. For some reason I hadn't considered launching python.bat directly. I guess I'd have to launch cmd.exe /c python.bat or something like that - I'm not sure, but I doubt python.bat can be launched directly. Thanks for the feedback.

— Reply to this email directly or view it on GitHub https://github.com/google/UIforETW/issues/53#issuecomment-137476021.

randomascii commented 9 years ago

I have to capture the output which I don't think ShellExecute supports, because the whole point is to put the output in the trace information window.

I guess I could ShellExecute "python.bat IdentifyChromeProcesses.py >%temp%\identify_output.txt" or something like that. Hmmm - too many possibilities.

randomascii commented 9 years ago

Huh. It was way easier than expected. Apparently CreateProcess just works with python.bat. The issue is resolved by this change:

https://github.com/google/UIforETW/commit/36dff98cdf1dfa18327fd9253972d94e5f167100

I have not yet created a release but the next release will have the change.

Chubab commented 9 years ago

Nice :-), thanks!

On Sat, Sep 5, 2015 at 7:03 PM Bruce Dawson notifications@github.com wrote:

Huh. It was way easier than expected. Apparently CreateProcess just works with python.bat. The issue is resolved by this change:

36dff98 https://github.com/google/UIforETW/commit/36dff98cdf1dfa18327fd9253972d94e5f167100

I have not yet created a release but the next release will have the change.

— Reply to this email directly or view it on GitHub https://github.com/google/UIforETW/issues/53#issuecomment-138006101.

randomascii commented 9 years ago

The v 1.17 release has this change.