madecoste / swarming

Automatically exported from code.google.com/p/swarming
Apache License 2.0
0 stars 1 forks source link

subprocess.call invocation in run_isolated.py is wrong on Windows #65

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
The documentation for subprocess.call clearly states that the program can not 
be specified relative to cwd. Currently, the isolate build steps do produce 
relative paths (typically to e.g. '..\\out\\Release/[exe name]'), and the 
subprocess.call step fails to find these.

It happens that most of the isolates invoke their targets via test_env.py, and 
that fixes up the path to the target. However, run_isolated.py should do this 
same path normalization so that a true .exe can be specified.

Repro (on Windows):
1. run_isolated.py -H da96999872b785acad12c78f231f69776bbfefe5 -I 
https://isolateserver.appspot.com

Expected:

gl_tests.exe runs.

Actual:

DEBUG    run_isolated(180): make_tree_read_only(D:\src\chrome\src\cache)
 INFO           tools( 88): Profiling: Section Setup took 0.918 seconds
DEBUG   isolateserver(1630): 
IsolatedFile(da96999872b785acad12c78f231f69776bbfefe5)
DEBUG   isolateserver(1654): 
IsolatedFile.load(da96999872b785acad12c78f231f69776bbfefe5)
DEBUG   isolateserver(1672): 
fetch_files(da96999872b785acad12c78f231f69776bbfefe5)
DEBUG   isolateserver(1680): fetching out\Release\libGLESv2.dll
DEBUG   isolateserver(1680): fetching out\Release\libEGL.dll
DEBUG   isolateserver(1680): fetching out\Release\gl_tests.exe
 INFO           tools( 88): Profiling: Section GetIsolateds took 0.004 seconds
DEBUG   isolateserver(251): 
create_directories(D:\src\chrome\src\run_tha_test8ylnsz, 3)
 INFO   isolateserver(1833): Retrieving remaining files (0 of them)...
DEBUG    run_isolated(118): Mapping 
D:\src\chrome\src\cache\45a283add99f13f9d6f620bbf5fa8785c4a06248 to 
D:\src\chrome\src\run_tha_
test8ylnsz\out\Release\libEGL.dll
DEBUG    run_isolated(118): Mapping 
D:\src\chrome\src\cache\ae0ea0983bbad67ef0dbaae9a58ac3a71d64f878 to 
D:\src\chrome\src\run_tha_
test8ylnsz\out\Release\gl_tests.exe
DEBUG    run_isolated(118): Mapping 
D:\src\chrome\src\cache\c1bf8d30c3024eb5fe3656b42010866f9b9d7d14 to 
D:\src\chrome\src\run_tha_
test8ylnsz\out\Release\libGLESv2.dll
 INFO           tools( 88): Profiling: Section GetRest took 0.012 seconds
 INFO    run_isolated(392):     0 (       0kb) added
 INFO    run_isolated(396):  4507 (  162325kb) current
 INFO    run_isolated(399):     0 (       0kb) removed
 INFO    run_isolated(402):        78546232kb free
 INFO           tools( 88): Profiling: Section CleanupTrimming took 0.045 seconds
DEBUG    run_isolated(220): 
make_tree_writeable(D:\src\chrome\src\run_tha_test8ylnsz)
 INFO    run_isolated(667): Running [u'..\\out\\Release\\gl_tests.exe'], cwd=D:\src\chrome\src\run_tha_test8ylnsz\chrome
 INFO           tools( 88): Profiling: Section RunTest took 0.000 seconds
[------ Swarming Error ------]
Failed to run [u'..\\out\\Release\\gl_tests.exe']; 
cwd=D:\src\chrome\src\run_tha_test8ylnsz\chrome [Error 2] The system cannot fin
d the file specified
[----------------------------]

This is reproducible if the isolate is unpacked locally, the Command Prompt is 
cd'd to anything except the cwd, and the subprocess.call is made manually.

Original issue reported on code.google.com by kbr@chromium.org on 17 Jan 2014 at 8:01

GoogleCodeExporter commented 9 years ago
vadimsh@ debugged the above with me and the realization that we came to is that 
the only reason isolates are working for the current Chrome tests on Windows is 
that they're all invoking .../testing/test_env.py as the command, causing 
run_isolated to manually inject the Python executable as the "real" command and 
sidestepping this bug with relative paths.

Original comment by kbr@chromium.org on 17 Jan 2014 at 8:03

GoogleCodeExporter commented 9 years ago

Original comment by vadimsh@chromium.org on 17 Jan 2014 at 8:12

GoogleCodeExporter commented 9 years ago
Ok, fix is here: https://codereview.appspot.com/53850043/
I'll wait for canary to run a cycle.

Should I roll DEPs for this or it can wait (and be bundled with other changes)? 
wdyt?

Original comment by vadimsh@chromium.org on 17 Jan 2014 at 9:21

GoogleCodeExporter commented 9 years ago
I'm in the process of working around the issue in the GPU isolates, and am 
definitely going to land those changes. But the sooner this can land, the 
sooner we can delete that workaround.

Original comment by kbr@chromium.org on 17 Jan 2014 at 9:37

GoogleCodeExporter commented 9 years ago

Original comment by kbr@chromium.org on 17 Jan 2014 at 11:19

GoogleCodeExporter commented 9 years ago
On second thought, since I'm still testing the GPU isolate workarounds, I'll 
probably just wait for the swarming_client DEPS roll to land.

Original comment by kbr@chromium.org on 17 Jan 2014 at 11:20

GoogleCodeExporter commented 9 years ago
Rolled to chromium at 245701.

Original comment by vadimsh@chromium.org on 18 Jan 2014 at 2:14