PRESUBMIT: Improve --check-only logic in tools/gen_xxx
This change introduces more uniform and reliable
handling of the presubmit-time check of generators.
Instead of delegating the check to the PRESUBMIT.py
script, it introduce a --check-only option.
Then it changes the behavior of generator to:
- Always write to xxx.swp files (which are .gitignore-d)
- If --check-only, diff-test and remove them
- If not, rename them into the final location
Test: manual (edit files, run git cl presubmit --force)
Change-Id: Ic156620c591bd434781c543803c9ab7aa3c2a33c
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index e2dd762..db40ea5 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -51,57 +51,30 @@
def CheckBuild(input_api, output_api):
+ tool = 'tools/gen_bazel'
# If no GN files were modified, bail out.
def build_file_filter(x): return input_api.FilterSourceFile(
x,
- white_list=('.*BUILD[.]gn$', '.*[.]gni$', 'tools/gen_bazel',
- 'BUILD\.extras'))
+ white_list=('.*BUILD[.]gn$', '.*[.]gni$', 'BUILD\.extras', tool))
if not input_api.AffectedSourceFiles(build_file_filter):
return []
-
- with open('BUILD') as f:
- current_build = f.read()
-
- new_build = subprocess.check_output(
- ['tools/gen_bazel', '--output', '/dev/stdout', '--output-proto',
- '/dev/null'])
-
- with open('protos/BUILD') as f:
- current_proto_build = f.read()
-
- new_proto_build = subprocess.check_output(
- ['tools/gen_bazel', '--output', '/dev/null', '--output-proto',
- '/dev/stdout'])
-
- if current_build != new_build or current_proto_build != new_proto_build:
- return [
- output_api.PresubmitError(
- 'BUILD and/or protos/BUILD is out of date. ' +
- 'Please run tools/gen_bazel to update it.')
- ]
+ if subprocess.call([tool, '--check-only']):
+ return [output_api.PresubmitError('Bazel BUILD(s) are out of date. Run '
+ + tool + ' to update them.')]
return []
def CheckAndroidBlueprint(input_api, output_api):
+ tool = 'tools/gen_android_bp'
# If no GN files were modified, bail out.
def build_file_filter(x): return input_api.FilterSourceFile(
x,
- white_list=('.*BUILD[.]gn$', '.*[.]gni$', 'tools/gen_android_bp'))
+ white_list=('.*BUILD[.]gn$', '.*[.]gni$', tool))
if not input_api.AffectedSourceFiles(build_file_filter):
return []
-
- with open('Android.bp') as f:
- current_blueprint = f.read()
-
- new_blueprint = subprocess.check_output(
- ['tools/gen_android_bp', '--output', '/dev/stdout'])
-
- if current_blueprint != new_blueprint:
- return [
- output_api.PresubmitError(
- 'Android.bp is out of date. Please run tools/gen_android_bp '
- 'to update it.')
- ]
+ if subprocess.call([tool, '--check-only']):
+ return [output_api.PresubmitError('Android build files are out of date. ' +
+ 'Run ' + tool + ' to update them.')]
return []
diff --git a/tools/gen_amalgamated b/tools/gen_amalgamated
index 06da7a3..a8d40da 100755
--- a/tools/gen_amalgamated
+++ b/tools/gen_amalgamated
@@ -44,7 +44,11 @@
# Arguments for the GN output directory (unless overridden from the command
# line).
-gn_args = 'is_debug=false perfetto_enable_ipc=true'
+gn_args = ' '.join([
+ 'is_debug=false',
+ 'perfetto_build_with_embedder=true',
+ 'perfetto_enable_ipc=true'
+])
# Compiler flags which aren't filtered out.
cflag_whitelist = r'^-(W.*|fno-exceptions|fPIC|std.*|fvisibility.*)$'
diff --git a/tools/gen_android_bp b/tools/gen_android_bp
index aa13178..dda247a 100755
--- a/tools/gen_android_bp
+++ b/tools/gen_android_bp
@@ -72,7 +72,12 @@
]
# Arguments for the GN output directory.
-gn_args = 'target_os="android" target_cpu="arm" is_debug=false perfetto_build_with_android=true'
+gn_args = ' '.join([
+ 'is_debug=false',
+ 'perfetto_build_with_android=true',
+ 'target_cpu="arm"',
+ 'target_os="android"',
+])
# All module names are prefixed with this string to avoid collisions.
module_prefix = 'perfetto_'
@@ -721,6 +726,9 @@
parser = argparse.ArgumentParser(
description='Generate Android.bp from a GN description.')
parser.add_argument(
+ '--check-only', help='Don\'t keep the generated files',
+ action='store_true')
+ parser.add_argument(
'--desc',
help=
'GN description (e.g., gn desc out --format=json --all-toolchains "//*"'
@@ -771,9 +779,18 @@
with open(args.extras, 'r') as r:
for line in r:
output.append(line.rstrip("\n\r"))
- with open(args.output, 'w') as f:
+
+ out_files = []
+
+ # Generate the Android.bp file.
+ out_files.append(args.output + '.swp')
+ with open(out_files[-1], 'w') as f:
f.write('\n'.join(output))
+ # Either check the contents or move the files to their final destination.
+ return gn_utils.check_or_commit_generated_files (out_files, args.check_only)
+
+
if __name__ == '__main__':
sys.exit(main())
diff --git a/tools/gen_bazel b/tools/gen_bazel
index 08a0f6a..4c57bf3 100755
--- a/tools/gen_bazel
+++ b/tools/gen_bazel
@@ -54,7 +54,11 @@
# Arguments for the GN output directory.
# host_os="linux" is to generate the right build files from Mac OS.
-gn_args = 'target_os="linux" is_debug=false host_os="linux"'
+gn_args = ' '.join([
+ 'host_os="linux"',
+ 'is_debug=false',
+ 'target_os="linux"',
+])
# Default targets to translate to the blueprint file.
default_targets = [
@@ -69,7 +73,7 @@
'//protos/perfetto/metrics:lite_gen'
]
-# Aliases to add to the BUILD file
+# Aliases to add to the BUILD file.
alias_targets = {
'//src/protozero:libprotozero': 'libprotozero',
'//src/trace_processor:trace_processor': 'trace_processor',
@@ -606,6 +610,9 @@
parser = argparse.ArgumentParser(
description='Generate BUILD from a GN description.')
parser.add_argument(
+ '--check-only', help='Don\'t keep the generated files',
+ action='store_true')
+ parser.add_argument(
'--desc',
help='GN description (e.g., gn desc out --format=json --all-toolchains "//*"'
)
@@ -641,22 +648,28 @@
else:
desc = gn_utils.create_build_description(gn_args, args.repo_root)
+ out_files = []
+
build_generator = BuildGenerator(desc)
build, proto_build = build_generator.create_build_for_targets(
args.targets or default_targets)
- with open(args.output, 'w') as f:
+
+ # Generate the main BUILD file.
+ out_files.append(args.output + '.swp')
+ with open(out_files[-1], 'w') as f:
writer = Writer(f)
build.write(writer)
writer.newline()
-
with open(args.extras, 'r') as r:
for line in r:
writer.line(line.rstrip("\n\r"))
- with open(args.output_proto, 'w') as f:
+ # Generate the protos/BUILD file.
+ out_files.append(args.output_proto + '.swp')
+ with open(out_files[-1], 'w') as f:
proto_build.write(Writer(f))
- return 0
+ return gn_utils.check_or_commit_generated_files(out_files, args.check_only)
if __name__ == '__main__':
diff --git a/tools/gn_utils.py b/tools/gn_utils.py
index 5c6d318..ca3e003 100644
--- a/tools/gn_utils.py
+++ b/tools/gn_utils.py
@@ -17,6 +17,7 @@
from __future__ import print_function
import errno
+import filecmp
import json
import os
import re
@@ -86,14 +87,17 @@
shutil.rmtree(out)
-def build_targets(out, targets):
+def build_targets(out, targets, quiet=False):
"""Runs ninja to build a list of GN targets in the given out directory.
Compiling these targets is required so that we can include any generated
source files in the amalgamated result.
"""
targets = [t.replace('//', '') for t in targets]
- subprocess.check_call([_tool_path('ninja')] + targets, cwd=out)
+ with open(os.devnull, 'rw') as devnull:
+ stdout = devnull if quiet else None
+ subprocess.check_call([_tool_path('ninja')] + targets, cwd=out,
+ stdout=stdout)
def compute_source_dependencies(out):
@@ -141,4 +145,25 @@
"""
name = re.sub(r'^//:?', '', label)
name = re.sub(r'[^a-zA-Z0-9_]', '_', name)
- return name
\ No newline at end of file
+ return name
+
+def check_or_commit_generated_files(tmp_files, check):
+ """Checks that gen files are unchanged or renames them to the final location
+
+ Takes in input a list of 'xxx.swp' files that have been written.
+ If check == False, it renames xxx.swp -> xxx.
+ If check == True, it just checks that the contents of 'xxx.swp' == 'xxx'.
+ Returns 0 if no diff was detected, 1 otherwise (to be used as exit code).
+ """
+ res = 0
+ for tmp_file in tmp_files:
+ assert(tmp_file.endswith('.swp'))
+ target_file = os.path.relpath(tmp_file[:-4])
+ if check:
+ if not filecmp.cmp(tmp_file, target_file):
+ sys.stderr.write('%s needs to be regenerated\n' % target_file)
+ res = 1
+ os.unlink(tmp_file)
+ else:
+ os.rename(tmp_file, target_file)
+ return res
\ No newline at end of file
