|
|
DescriptionPolymer: Update find_unused_elements.py
- Stop assuming that Uglify is installed globally, instead
use Uglify binary from third_party/node.
- Remove obsolete logic to skip generated files. Generated files
are no longer checked into the repository.
BUG=None
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2920003002
Cr-Commit-Position: refs/heads/master@{#477694}
Committed: https://chromium.googlesource.com/chromium/src/+/0638fc4291945fb6444537ad467da412e5d0711b
Patch Set 1 #
Total comments: 2
Patch Set 2 : Fix script to actually work #
Total comments: 5
Patch Set 3 : Done #Patch Set 4 : Rebase #
Total comments: 2
Patch Set 5 : Remove tempfile #
Depends on Patchset: Dependent Patchsets: Messages
Total messages: 24 (8 generated)
Description was changed from ========== Polymer, find_unused_elemnts: Remove obsolete logic to skip generated files. Generated files are no longer checked into the repository. BUG=None ========== to ========== Polymer, find_unused_elemnts: Remove obsolete logic to skip generated files. Generated files are no longer checked into the repository. BUG=None CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
dpapad@chromium.org changed reviewers: + dbeam@chromium.org
https://codereview.chromium.org/2920003002/diff/1/third_party/polymer/v1_0/fi... File third_party/polymer/v1_0/find_unused_elements.py (right): https://codereview.chromium.org/2920003002/diff/1/third_party/polymer/v1_0/fi... third_party/polymer/v1_0/find_unused_elements.py:61: proc = subprocess.Popen(['uglifyjs', filename], stdout=subprocess.PIPE) Actually there is more issues with this script. It assumes that uglifyjs is installed globally, or in this folder, instead of grabbing it from third_party. Will attempt to update as part of the same CL. It currently throws following error: Traceback (most recent call last): File "find_unused_elements.py", line 151, in <module> UnusedElementsDetector().Run() File "find_unused_elements.py", line 106, in Run not self.__IsImported(element, relevant_src_dirs)): File "find_unused_elements.py", line 145, in __IsImported os.path.join(dirpath, filename))): File "find_unused_elements.py", line 78, in __StripComments text = UnusedElementsDetector.__StripJsComments(filename) File "find_unused_elements.py", line 61, in __StripJsComments proc = subprocess.Popen(['uglifyjs', filename], stdout=subprocess.PIPE) File "/usr/lib/python2.7/subprocess.py", line 710, in __init__ errread, errwrite) File "/usr/lib/python2.7/subprocess.py", line 1327, in _execute_child raise child_exception OSError: [Errno 2] No such file or directory
Description was changed from ========== Polymer, find_unused_elemnts: Remove obsolete logic to skip generated files. Generated files are no longer checked into the repository. BUG=None CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Polymer: Update find_unused_elements.py - Stop assuming that uglifyjs is installed globally, instead use uglifyjs binary from third_party/node. - Remove obsolete logic to skip generated files. Generated files are no longer checked into the repository. BUG=None CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Updated script. PTAL
https://codereview.chromium.org/2920003002/diff/1/third_party/polymer/v1_0/fi... File third_party/polymer/v1_0/find_unused_elements.py (right): https://codereview.chromium.org/2920003002/diff/1/third_party/polymer/v1_0/fi... third_party/polymer/v1_0/find_unused_elements.py:61: proc = subprocess.Popen(['uglifyjs', filename], stdout=subprocess.PIPE) On 2017/06/02 at 18:19:26, dpapad wrote: > Actually there is more issues with this script. It assumes that uglifyjs is installed globally, or in this folder, instead of grabbing it from third_party. Will attempt to update as part of the same CL. It currently throws following error: > > Traceback (most recent call last): > File "find_unused_elements.py", line 151, in <module> > UnusedElementsDetector().Run() > File "find_unused_elements.py", line 106, in Run > not self.__IsImported(element, relevant_src_dirs)): > File "find_unused_elements.py", line 145, in __IsImported > os.path.join(dirpath, filename))): > File "find_unused_elements.py", line 78, in __StripComments > text = UnusedElementsDetector.__StripJsComments(filename) > File "find_unused_elements.py", line 61, in __StripJsComments > proc = subprocess.Popen(['uglifyjs', filename], stdout=subprocess.PIPE) > File "/usr/lib/python2.7/subprocess.py", line 710, in __init__ > errread, errwrite) > File "/usr/lib/python2.7/subprocess.py", line 1327, in _execute_child > raise child_exception > OSError: [Errno 2] No such file or directory Addressed in latest patch.
https://codereview.chromium.org/2920003002/diff/20001/third_party/polymer/v1_... File third_party/polymer/v1_0/find_unused_elements.py (left): https://codereview.chromium.org/2920003002/diff/20001/third_party/polymer/v1_... third_party/polymer/v1_0/find_unused_elements.py:61: proc = subprocess.Popen(['uglifyjs', filename], stdout=subprocess.PIPE) can we change to subprocess2 and use stderr=subprocess.VOID instead? https://cs.chromium.org/chromium/build/tests/masters_util.py?type=cs&q=stderr...
https://codereview.chromium.org/2920003002/diff/20001/third_party/polymer/v1_... File third_party/polymer/v1_0/find_unused_elements.py (left): https://codereview.chromium.org/2920003002/diff/20001/third_party/polymer/v1_... third_party/polymer/v1_0/find_unused_elements.py:61: proc = subprocess.Popen(['uglifyjs', filename], stdout=subprocess.PIPE) On 2017/06/02 at 21:49:08, Dan Beam wrote: > can we change to subprocess2 and use stderr=subprocess.VOID instead? > > https://cs.chromium.org/chromium/build/tests/masters_util.py?type=cs&q=stderr... We probably could (have not tried yet), but importing subproccess2 is a bit involved, see lines https://cs.chromium.org/chromium/build/tests/masters_util.py?type=cs&q=stderr.... Do you still think this is a better idea than just writing to a temp file? If not, I might not get to it today.
michaelpg@chromium.org changed reviewers: + michaelpg@chromium.org
https://codereview.chromium.org/2920003002/diff/20001/third_party/polymer/v1_... File third_party/polymer/v1_0/find_unused_elements.py (right): https://codereview.chromium.org/2920003002/diff/20001/third_party/polymer/v1_... third_party/polymer/v1_0/find_unused_elements.py:19: sys.path.append(os.path.join(_SRC_PATH, 'third_party', 'node')) remove uglify check in reproduce.sh
https://codereview.chromium.org/2920003002/diff/20001/third_party/polymer/v1_... File third_party/polymer/v1_0/find_unused_elements.py (right): https://codereview.chromium.org/2920003002/diff/20001/third_party/polymer/v1_... third_party/polymer/v1_0/find_unused_elements.py:19: sys.path.append(os.path.join(_SRC_PATH, 'third_party', 'node')) On 2017/06/05 at 19:22:36, michaelpg wrote: > remove uglify check in reproduce.sh Done.
lgtm
PTAL https://codereview.chromium.org/2920003002/diff/20001/third_party/polymer/v1_... File third_party/polymer/v1_0/find_unused_elements.py (left): https://codereview.chromium.org/2920003002/diff/20001/third_party/polymer/v1_... third_party/polymer/v1_0/find_unused_elements.py:61: proc = subprocess.Popen(['uglifyjs', filename], stdout=subprocess.PIPE) On 2017/06/02 at 21:49:08, Dan Beam wrote: > can we change to subprocess2 and use stderr=subprocess.VOID instead? > > https://cs.chromium.org/chromium/build/tests/masters_util.py?type=cs&q=stderr... This is not necessary anymore. I rebased this CL after https://codereview.chromium.org/2921793002 (which changes uglifyjs to uglify-es), and there is no need to use the temporary file anymore, see patch #4.
https://codereview.chromium.org/2920003002/diff/60001/third_party/polymer/v1_... File third_party/polymer/v1_0/find_unused_elements.py (right): https://codereview.chromium.org/2920003002/diff/60001/third_party/polymer/v1_... third_party/polymer/v1_0/find_unused_elements.py:15: import tempfile remove
https://codereview.chromium.org/2920003002/diff/60001/third_party/polymer/v1_... File third_party/polymer/v1_0/find_unused_elements.py (right): https://codereview.chromium.org/2920003002/diff/60001/third_party/polymer/v1_... third_party/polymer/v1_0/find_unused_elements.py:15: import tempfile On 2017/06/06 at 23:34:40, michaelpg wrote: > remove Done.
Description was changed from ========== Polymer: Update find_unused_elements.py - Stop assuming that uglifyjs is installed globally, instead use uglifyjs binary from third_party/node. - Remove obsolete logic to skip generated files. Generated files are no longer checked into the repository. BUG=None CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Polymer: Update find_unused_elements.py - Stop assuming that Uglify is installed globally, instead use Uglify binary from third_party/node. - Remove obsolete logic to skip generated files. Generated files are no longer checked into the repository. BUG=None CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
lgtm
still lgtm
The CQ bit was checked by dpapad@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 80001, "attempt_start_ts": 1496855691969140, "parent_rev": "986ad291791882d151f951824774a8f1e276df14", "commit_rev": "0638fc4291945fb6444537ad467da412e5d0711b"}
Message was sent while issue was closed.
Description was changed from ========== Polymer: Update find_unused_elements.py - Stop assuming that Uglify is installed globally, instead use Uglify binary from third_party/node. - Remove obsolete logic to skip generated files. Generated files are no longer checked into the repository. BUG=None CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Polymer: Update find_unused_elements.py - Stop assuming that Uglify is installed globally, instead use Uglify binary from third_party/node. - Remove obsolete logic to skip generated files. Generated files are no longer checked into the repository. BUG=None CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2920003002 Cr-Commit-Position: refs/heads/master@{#477694} Committed: https://chromium.googlesource.com/chromium/src/+/0638fc4291945fb6444537ad467d... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/0638fc4291945fb6444537ad467d... |