Run dartanalyze on the dartzip file structure, by default.
This patch relands b905249b657b3ff373878a8756e2bde47e1289e6, which
enabled running dartanalyze by default, and hopefully fixes the problems
seen in that first run:
Instead of setting --package-root to the current build's out/.../gen/
dir, after building the dartzip file, unzip it to a temporary directory
and then run the analyzer on that.
a) This allows us to check that all dependencies are actually expressed
in build rules.
b) This lets us run the dartanalyze in other repositories. (We may be
consuming dartzips in repository B which were generated in repository
A.)
c) This keeps us from being tied to the build output of one repository.
BUG=459376
R=tonyg@chromium.org, zra@google.com
Review URL: https://codereview.chromium.org/996483003
diff --git a/BUILD.gn b/BUILD.gn
index fa8060b..bdb6bb2 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -25,24 +25,6 @@
}
}
-# A group which invokes the dart checker on each dart package. This can take
-# multiple seconds per target, so we separate it out from the main building.
-# Eventually, we'd like this to be part of the default dart building, but for
-# now, to participate in checking, please add "{package_name}_analyze" to the
-# list of dependencies below.
-group("dartcheck") {
- testonly = true
- deps = [
- "//examples/dart/console_example:console_example_analyze",
- "//examples/dart/hello_world/hello:hello_analyze",
- "//examples/dart/hello_world/world:world_analyze",
- "//examples/dart/wget:wget_analyze",
- "//services/dart/test/echo:echo_analyze",
- "//services/dart/test/pingpong:pingpong_analyze",
- "//services/dart/test/pingpong_target:pingpong_target_analyze",
- ]
-}
-
# Deprecated name for the default build target.
group("root") {
testonly = true
diff --git a/mojo/public/dart/rules.gni b/mojo/public/dart/rules.gni
index e8d7c72..304f2f6 100644
--- a/mojo/public/dart/rules.gni
+++ b/mojo/public/dart/rules.gni
@@ -63,15 +63,16 @@
mojo_output = "$root_out_dir/" + target_name + ".mojo"
}
- action_foreach(package_analyze_sources) {
- sources = invoker.sources
-
+ action(package_analyze_sources) {
script = rebase_path("mojo/public/tools/dart_analyze.py", ".", mojo_root)
+ sources = [
+ package_output,
+ ]
+
args = [
- rebase_path(root_gen_dir),
- rebase_path("$target_gen_dir/{{source_name_part}}.stamp"),
- "{{source}}",
+ rebase_path(package_output),
+ rebase_path("$target_gen_dir/${target_name}_analyze.stamp"),
"--no-hints",
"--url-mapping=mojo:application,/" +
rebase_path("mojo/public/dart/application.dart", "/", mojo_root),
@@ -81,6 +82,22 @@
rebase_path("mojo/dart/embedder/builtin.dart", "/", mojo_root),
"--url-mapping=mojo:core,/" +
rebase_path("mojo/public/dart/core.dart", "/", mojo_root),
+
+ # The dart bindings refer to the autogenerated application interface in
+ # their source code; since that code is then packaged into the image, we
+ # need to manually resolve these package urls to the autogenerated code.
+ "--url-mapping=package:mojo/public/interfaces/application/application.mojom.dart,/" + rebase_path(
+ "mojo/public/interfaces/application/application.mojom.dart",
+ "/",
+ root_gen_dir + mojo_root),
+ "--url-mapping=package:mojo/public/interfaces/application/service_provider.mojom.dart,/" + rebase_path(
+ "mojo/public/interfaces/application/service_provider.mojom.dart",
+ "/",
+ root_gen_dir + mojo_root),
+ "--url-mapping=package:mojo/public/interfaces/application/shell.mojom.dart,/" + rebase_path(
+ "mojo/public/interfaces/application/shell.mojom.dart",
+ "/",
+ root_gen_dir + mojo_root),
]
if (defined(invoker.deps)) {
@@ -92,7 +109,7 @@
}
outputs = [
- "$target_gen_dir/{{source_name_part}}.stamp",
+ "$target_gen_dir/${target_name}_analyze.stamp",
]
}
@@ -123,9 +140,7 @@
]
deps = [
- # TODO(erg): When dartanalyze runs at an acceptable speed, add
- # ":$package_analyze_sources" as a dependency here and remove the
- # manual group("check") in the toplevel build file.
+ ":$package_analyze_sources",
":$package_name",
]
if (defined(invoker.deps)) {
diff --git a/mojo/public/tools/dart_analyze.py b/mojo/public/tools/dart_analyze.py
index 8b8117e..baad3e5 100755
--- a/mojo/public/tools/dart_analyze.py
+++ b/mojo/public/tools/dart_analyze.py
@@ -10,10 +10,14 @@
# directory, which we treat as the package root. The second is the stamp file
# to touch if we succeed. The rest are passed to the analyzer verbatim.
+import glob
import os
+import re
+import shutil
import subprocess
import sys
-import re
+import tempfile
+import zipfile
_ANALYZING_PATTERN = re.compile(r'^Analyzing \[')
_FINAL_REPORT_PATTERN = re.compile(r'^[0-9]+ errors and [0-9]+ warnings found.')
@@ -25,43 +29,57 @@
_THAT_ONE_BROKEN_CLOSE_IN_WEB_SOCKETS_PATTERN = re.compile(
r'^\[error\] The name \'close\' is already defined')
+
def main(args):
- cmd = [
- "../../third_party/dart-sdk/dart-sdk/bin/dartanalyzer",
- ]
-
- gen_dir = args.pop(0)
+ dartzip_file = args.pop(0)
stamp_file = args.pop(0)
- cmd.extend(args)
- cmd.append("--package-root=%s" % gen_dir)
- passed = True
+ # Unzip |dartzip_file| to a temporary directory.
try:
- subprocess.check_output(cmd, shell=False, stderr=subprocess.STDOUT)
- except subprocess.CalledProcessError as e:
- # Perform post processing on the output. Filter out non-error messages and
- # known problem patterns that we're working on.
- raw_lines = e.output.split('\n')
- # Remove the last empty line
- raw_lines.pop()
- filtered_lines = [i for i in raw_lines if (
- not re.match(_ANALYZING_PATTERN, i) and
- not re.match(_FINAL_REPORT_PATTERN, i) and
- # TODO(erg): Remove the rest of these as fixes land:
- not re.match(_WARNING_PATTERN, i) and
- not re.match(_NATIVE_ERROR_PATTERN, i) and
- not re.match(_THAT_ONE_BROKEN_CLOSE_IN_WEB_SOCKETS_PATTERN, i))]
- for line in filtered_lines:
- passed = False
- print >> sys.stderr, line
+ temp_dir = tempfile.mkdtemp()
+ zipfile.ZipFile(dartzip_file).extractall(temp_dir)
- if passed:
- # We passed cleanly, so touch the stamp file so that we don't run again.
- with open(stamp_file, 'a'):
- os.utime(stamp_file, None)
- return 0
- else:
- return -2
+ cmd = [
+ "../../third_party/dart-sdk/dart-sdk/bin/dartanalyzer",
+ ]
+
+ # Grab all the toplevel dart files in the archive.
+ dart_files = glob.glob(os.path.join(temp_dir, "*.dart"))
+
+ cmd.extend(dart_files)
+ cmd.extend(args)
+ cmd.append("--package-root=%s" % temp_dir)
+
+ passed = True
+ try:
+ subprocess.check_output(cmd, shell=False, stderr=subprocess.STDOUT)
+ except subprocess.CalledProcessError as e:
+ # Perform post processing on the output. Filter out non-error messages and
+ # known problem patterns that we're working on.
+ raw_lines = e.output.split('\n')
+ # Remove the last empty line
+ raw_lines.pop()
+ filtered_lines = [i for i in raw_lines if (
+ not re.match(_ANALYZING_PATTERN, i) and
+ not re.match(_FINAL_REPORT_PATTERN, i) and
+ # TODO(erg): Remove the rest of these as fixes land:
+ not re.match(_WARNING_PATTERN, i) and
+ not re.match(_NATIVE_ERROR_PATTERN, i) and
+ not re.match(_THAT_ONE_BROKEN_CLOSE_IN_WEB_SOCKETS_PATTERN, i))]
+ for line in filtered_lines:
+ passed = False
+ print >> sys.stderr, line
+
+ if passed:
+ # We passed cleanly, so touch the stamp file so that we don't run again.
+ with open(stamp_file, 'a'):
+ os.utime(stamp_file, None)
+ return 0
+ else:
+ return -2
+ finally:
+ shutil.rmtree(temp_dir)
+
if __name__ == '__main__':
sys.exit(main(sys.argv[1:]))
diff --git a/mojo/tools/mojob.py b/mojo/tools/mojob.py
index 7f60900..fb9e144 100755
--- a/mojo/tools/mojob.py
+++ b/mojo/tools/mojob.py
@@ -112,14 +112,6 @@
return subprocess.call(['ninja', '-C', out_dir])
-def dartcheck(config):
- """Runs the dart analyzer on code for the given config."""
-
- out_dir = _get_out_dir(config)
- print 'Checking dart code in %s ...' % out_dir
- return subprocess.call(['ninja', '-C', out_dir, 'dartcheck'])
-
-
def _run_tests(config, test_types):
"""Runs the tests of the given type(s) for the given config."""
@@ -252,10 +244,6 @@
help='Run NaCl unit tests (does not build).')
nacltest_parser.set_defaults(func=nacltest)
- dartcheck_parser = subparsers.add_parser('dartcheck', parents=[parent_parser],
- help='Run the dart source code analyzer to check for warnings.')
- dartcheck_parser.set_defaults(func=dartcheck)
-
args = parser.parse_args()
config = _args_to_config(args)
return args.func(config)
