Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(18)

Issue 1144333005: Make compile_frontend.py work with default JVM 1.8 installation on Mac OS X (Closed)

Created:
4 years, 11 months ago by caseq
Modified:
4 years, 11 months ago
Reviewers:
dgozman, pfeldman
CC:
blink-reviews, apavlov, caseq+blink_chromium.org, yurys+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, sergeyv+blink_chromium.org, kozyatinskiy+blink_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Make compile_frontend.py work with default JVM 1.8 installation on Mac OS X Java 1.8 installs into a directory containing a space in its name on Mac OS, which breaks compile_frontend.py's lame handling of file names. This removes shell=True flag to subprocess.Popen() and ensures commands are represented as arrays of strings, not as space-separated strings. BUG= Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=196576

Patch Set 1 #

Patch Set 2 : rebased #

Patch Set 3 : rebased again #

Patch Set 4 : fixed execution on windows #

Total comments: 1

Patch Set 5 : #

Patch Set 6 : 'server ' -> 'server' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+43 lines, -30 lines) Patch
M Source/devtools/scripts/compile_frontend.py View 1 2 3 4 5 11 chunks +43 lines, -30 lines 0 comments Download

Messages

Total messages: 13 (8 generated)
caseq
4 years, 11 months ago (2015-06-03 16:31:14 UTC) #2
dgozman
lgtm https://codereview.chromium.org/1144333005/diff/80001/Source/devtools/scripts/compile_frontend.py File Source/devtools/scripts/compile_frontend.py (right): https://codereview.chromium.org/1144333005/diff/80001/Source/devtools/scripts/compile_frontend.py#newcode404 Source/devtools/scripts/compile_frontend.py:404: '--js', to_platform_path(injectedScriptSourceTmpFile), nit: extra comma
4 years, 11 months ago (2015-06-05 13:08:29 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1144333005/100001
4 years, 11 months ago (2015-06-05 13:48:13 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1144333005/120001
4 years, 11 months ago (2015-06-05 13:51:26 UTC) #12
commit-bot: I haz the power
4 years, 11 months ago (2015-06-05 14:47:24 UTC) #13
Message was sent while issue was closed.
Committed patchset #6 (id:120001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=196576

Powered by Google App Engine
This is Rietveld 408576698