leandron commented on a change in pull request #6333: URL: https://github.com/apache/incubator-tvm/pull/6333#discussion_r476325378
########## File path: tests/lint/filter_untracked.py ########## @@ -0,0 +1,60 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + + +import os.path +import subprocess +import sys + + +def check_output(args, **kw): + proc = subprocess.Popen(args, **kw, stdout=subprocess.PIPE) + out, _ = proc.communicate() + if proc.returncode: + sys.stderr.write('exited with code %d: %s\n' % (proc.returncode, ' '.join(args))) + sys.exit(2) + + if sys.version_info[0] == 2: + return unicode(out, 'utf-8') + else: + return str(out, 'utf-8') + + +def main(): + script_dir = os.path.dirname(__file__) or os.getcwd() + toplevel_dir = check_output(['git', 'rev-parse', '--show-toplevel'], cwd=script_dir).strip('\n') + git_status_output = check_output(['git', 'status', '-s', '--ignored'], + cwd=toplevel_dir) + untracked = [line[3:] + for line in git_status_output.split('\n') + if line.startswith('?? ') or line.startswith('!! ')] + + for line in sys.stdin: + cleaned_line = line + if line[:2] == './': + cleaned_line = line[2:] + cleaned_line = cleaned_line.strip('\n') + if any((cleaned_line.startswith(u) if u[-1] == '/' else cleaned_line == u) + for u in untracked): + continue + + sys.stdout.write(line) + + +if __name__ == '__main__': + main() Review comment: minor indentation difference here? ########## File path: tests/lint/check_asf_header.sh ########## @@ -0,0 +1,48 @@ +#!/bin/bash -e +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + + +rat_output=/tmp/$$.apache-rat.txt + +filter_untracked=0 +if [ "$1" == "--local" ]; then + filter_untracked=1 +fi + +java -jar /bin/apache-rat.jar -E tests/lint/rat-excludes -d . | (grep -E "^== File" >${rat_output} || true) + +# Rat can't be configured to ignore untracked files, so filter them. +if [ ${filter_untracked} -eq 1 ]; then + echo "NOTE: --local flag present, filtering untracked files" + cat ${rat_output} | sed 's/^== File: //g' | \ + python3 $(dirname $0)/filter_untracked.py | \ Review comment: `$0` needs escaping, similar to what you already did in many others. ########## File path: tests/lint/check_asf_header.sh ########## @@ -0,0 +1,48 @@ +#!/bin/bash -e +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + + +rat_output=/tmp/$$.apache-rat.txt Review comment: To prevent this file to leak some space in local machines, I suggest adding a trap cleanup handler, and a generic tempdir. Something like: ``` tmpdir=$(mktemp -d) cleanup() { rm -rf "$tmpdir" } trap cleanup 0 rat_output=$tmpdir/$$.apache-rat.txt ``` ########## File path: tests/lint/pylint.sh ########## @@ -0,0 +1,21 @@ +#!/bin/bash -e +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + + +python3 -m pylint python/tvm --rcfile=$(dirname $0)/pylintrc +python3 -m pylint vta/python/vta --rcfile=$(dirname $0)/pylintrc Review comment: Suggest escaping `$0` with quotes here. ########## File path: tests/lint/cppdocs.sh ########## @@ -0,0 +1,28 @@ +#!/bin/bash -e +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + Review comment: Same comment about the trap cleanup to avoid disk space leak on local machine. ########## File path: docker/lint.sh ########## @@ -0,0 +1,74 @@ +#!/bin/bash -e +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + + +source "$(dirname $0)/dev_common.sh" + +function run_lint_step() { + validate_only=0 + if [ "$1" == "--validate-only" ]; then + validate_only=1 + shift + fi + + case "$1" in + file_type) + cmd=( python3 tests/lint/check_file_type.py ) + ;; + asf) + cmd=( tests/lint/check_asf_header.sh --local ) + ;; + clang_format) + cmd=( tests/lint/clang_format.sh ) + ;; + cpplint) + cmd=( tests/lint/cpplint.sh ) + ;; + pylint) + cmd=( tests/lint/pylint.sh ) + ;; + jnilint) + cmd=( tests/lint/jnilint.sh ) + ;; + cppdocs) + cmd=( tests/lint/cppdocs.sh ) + ;; + *) + echo "error: don't know how to run lint step $1" >&2 Review comment: Suggestion: This message could also `echo` the contents of `steps` list, so that it shows what are the available options. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
