diff --git a/git_cl.py b/git_cl.py index 1075bef44..8cda7dc11 100755 --- a/git_cl.py +++ b/git_cl.py @@ -58,6 +58,7 @@ import git_new_branch import google_java_format import metrics import metrics_utils +import metrics_xml_format import newauth import owners_client import owners_finder @@ -6765,7 +6766,7 @@ def _RunMojomFormat(opts, paths, top_dir, upstream_commit): return ret -def _FormatXml(opts, paths, top_dir, upstream_commit): +def _RunMetricsXMLFormat(opts, paths, top_dir, upstream_commit): # Skip the metrics formatting from the global presubmit hook. These files # have a separate presubmit hook that issues an error if the files need # formatting, whereas the top-level presubmit script merely issues a @@ -6776,14 +6777,11 @@ def _FormatXml(opts, paths, top_dir, upstream_commit): return_value = 0 for path in paths: - xml_dir = GetMetricsDir(path) - if not xml_dir: + pretty_print_tool = metrics_xml_format.FindMetricsXMLFormatterTool(path) + if not pretty_print_tool: continue - tool_dir = os.path.join(top_dir, xml_dir.replace('/', os.path.sep)) - pretty_print_tool = os.path.join(tool_dir, 'pretty_print.py') cmd = [shutil.which('vpython3'), pretty_print_tool, '--non-interactive'] - # If the XML file is histograms.xml or enums.xml, add the xml path # to the command as histograms/pretty_print.py now needs a relative # path argument after splitting the histograms into multiple @@ -6792,13 +6790,8 @@ def _FormatXml(opts, paths, top_dir, upstream_commit): # tools/metrics/histogrmas, pretty-print should be run with an # additional relative path argument, like: $ python pretty_print.py # metadata/UMA/histograms.xml $ python pretty_print.py enums.xml - if xml_dir == 'tools/metrics/histograms': - if os.path.basename(path) not in ('histograms.xml', 'enums.xml', - 'histogram_suffixes_list.xml'): - # Skip this XML file if it's not one of the known types. - continue + if metrics_xml_format.GetMetricsDir(path) == 'tools/metrics/histograms': cmd.append(path.replace('/', os.path.sep)) - if opts.dry_run or opts.diff: cmd.append('--diff') @@ -6935,7 +6928,7 @@ def CMDformat(parser, args): formatters = [ (GN_EXTS, _RunGnFormat), - (['.xml'], _FormatXml), + (['.xml'], _RunMetricsXMLFormat), ] if not opts.no_java: formatters += [(['.java'], _RunGoogleJavaFormat)] @@ -6962,19 +6955,6 @@ def CMDformat(parser, args): return return_value -def GetMetricsDir(diff_xml): - metrics_xml_dirs = [ - 'tools/metrics/actions', - 'tools/metrics/histograms', - 'tools/metrics/structured', - 'tools/metrics/ukm', - ] - for xml_dir in metrics_xml_dirs: - if diff_xml.startswith(xml_dir): - return xml_dir - return None - - @subcommand.usage('') @metrics.collector.collect_metrics('git cl checkout') def CMDcheckout(parser, args): diff --git a/metrics-xml-format b/metrics-xml-format new file mode 100755 index 000000000..9ce896ba7 --- /dev/null +++ b/metrics-xml-format @@ -0,0 +1,8 @@ +#!/usr/bin/env bash +# Copyright 2024 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +base_dir=$(dirname "$0") + +PYTHONDONTWRITEBYTECODE=1 exec python3 "$base_dir/metrics_xml_format.py" "$@" diff --git a/metrics_xml_format.py b/metrics_xml_format.py new file mode 100755 index 000000000..f511ee6cf --- /dev/null +++ b/metrics_xml_format.py @@ -0,0 +1,121 @@ +#!/usr/bin/env vpython3 +# Copyright (c) 2024 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. +"""Redirects to the version of the metrics XML formatter in the Chromium tree. +""" +import gclient_paths +import os +import shutil +import subprocess +import sys + + +def GetMetricsDir(path): + metrics_xml_dirs = [ + 'tools/metrics/actions', + 'tools/metrics/histograms', + 'tools/metrics/structured', + 'tools/metrics/ukm', + ] + normalized_abs_dirname = os.path.dirname(os.path.abspath(path)).replace( + os.sep, '/') + for xml_dir in metrics_xml_dirs: + if normalized_abs_dirname.endswith(xml_dir): + return xml_dir + return None + + +def IsSupportedHistogramsXML(path): + supported_xmls = set([ + 'histograms.xml', + 'enums.xml', + 'histogram_suffixes_list.xml', + ]) + return os.path.basename(path) in supported_xmls + + +def log(msg, verbose): + if verbose: + print(msg) + + +def FindMetricsXMLFormatterTool(path, verbose=False): + """Returns a path to the metrics XML formatter executable.""" + top_dir = gclient_paths.GetPrimarySolutionPath() + if not top_dir: + log('Not executed in a Chromium checkout; skip formatting', verbose) + return '' + xml_dir = GetMetricsDir(path) + if not xml_dir: + log(f'{path} is not a metric XML; skip formatting', verbose) + return '' + # Just to ensure that the given file is located in the current checkout + # folder. If not, skip the formatting. + if not os.path.abspath(path).startswith(os.path.abspath(top_dir)): + log( + f'{path} is not located in the current Chromium checkout; ' + 'skip formatting', verbose) + return '' + + if xml_dir == 'tools/metrics/histograms': + # Skips the formatting, if the XML file is not one of the known types. + if not IsSupportedHistogramsXML(path): + log(f'{path} is not a supported histogram XML; skip formatting', + verbose) + return '' + + # top_dir is already formatted with the OS specific path separator, whereas + # xml_dir is not yet. + tool_dir = os.path.join(top_dir, xml_dir.replace('/', os.path.sep)) + return os.path.join(tool_dir, 'pretty_print.py') + + +usage_text = """Usage: %s [option] filepath + +Format a given metrics XML file with the metrics XML formatters in the Chromium +checkout. Noop, if executed out of a Chromium checkout. + +Note that not all the options are understood by all the formatters. +Find the formatter binaries for all the options supported by each formatter. + +positional arguments: + filepath if the path is not under tools/metrics, + no formatter will be run. + +options:, + -h, --help show this help message and exit' + --presubmit + --diff""" + + +def _print_help(): + print(usage_text % sys.argv[0]) + + +def main(args): + path = next((arg for arg in args if not arg.startswith('-')), None) + if not path: + _print_help() + return 0 + if not os.path.exists(path): + raise FileNotFoundError(f'{path} does not exist.') + + tool = FindMetricsXMLFormatterTool(path, verbose=True) + if not tool: + # Fail (almost) silently. + return 0 + + subprocess.run([ + shutil.which('vpython3'), + tool, + ] + args) + return 0 + + +if __name__ == '__main__': + try: + sys.exit(main(sys.argv[1:])) + except KeyboardInterrupt: + sys.stderr.write('interrupted\n') + sys.exit(1) diff --git a/tests/metrics_xml_format_test.py b/tests/metrics_xml_format_test.py new file mode 100755 index 000000000..0fbd8c926 --- /dev/null +++ b/tests/metrics_xml_format_test.py @@ -0,0 +1,113 @@ +#!/usr/bin/env vpython3 +# coding=utf-8 +# Copyright (c) 2012 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +import os +import sys +import unittest +from unittest import mock + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +import gclient_paths_test +import metrics_xml_format + +norm = lambda path: path.replace('/', os.sep) +join = os.path.join + + +class TestBase(gclient_paths_test.TestBase): + + def setUp(self): + super().setUp() + + # os.path.abspath() doesn't seem to use os.path.getcwd() to compute + # the abspath of a given path. + # + # This mock os.path.abspath such that it uses the mocked getcwd(). + mock.patch('os.path.abspath', self.abspath).start() + # gclient_paths.GetPrimarysolutionPath() defaults to src. + self.make_file_tree({'.gclient': ''}) + self.cwd = join(self.cwd, 'src') + + def abspath(self, path): + if os.path.isabs(path): + return path + + return join(self.getcwd(), path) + + +class GetMetricsDirTest(TestBase): + + def testWithAbsolutePath(self): + get = lambda path: metrics_xml_format.GetMetricsDir(norm(path)) + self.assertTrue(get('/src/tools/metrics/actions/abc.xml')) + self.assertTrue(get('/src/tools/metrics/histograms/abc.xml')) + self.assertTrue(get('/src/tools/metrics/structured/abc.xml')) + self.assertTrue(get('/src/tools/metrics/ukm/abc.xml')) + + self.assertFalse(get('/src/tools/metrics/actions/next/abc.xml')) + self.assertFalse(get('/src/tools/metrics/histograms/next/abc.xml')) + self.assertFalse(get('/src/tools/metrics/structured/next/abc.xml')) + self.assertFalse(get('/src/tools/metrics/ukm/next/abc.xml')) + + def testWithRelativePaths(self): + get = lambda path: metrics_xml_format.GetMetricsDir(norm(path)) + self.cwd = join(self.cwd, 'tools') + self.assertFalse(get('abc.xml')) + self.assertTrue(get('metrics/actions/abc.xml')) + + +class FindMetricsXMLFormatTool(TestBase): + + def testWithMetricsXML(self): + findTool = metrics_xml_format.FindMetricsXMLFormatterTool + + self.assertEqual( + findTool(norm('tools/metrics/actions/abc.xml')), + join(self.getcwd(), norm('tools/metrics/actions/pretty_print.py')), + ) + + # same test, but with an absolute path. + self.assertEqual( + findTool(join(self.getcwd(), + norm('tools/metrics/actions/abc.xml'))), + join(self.getcwd(), norm('tools/metrics/actions/pretty_print.py')), + ) + + def testWthNonMetricsXML(self): + findTool = metrics_xml_format.FindMetricsXMLFormatterTool + self.assertEqual(findTool('tools/metrics/actions/next/abc.xml'), '') + + def testWithNonCheckout(self): + findTool = metrics_xml_format.FindMetricsXMLFormatterTool + self.cwd = self.root + self.assertEqual(findTool('tools/metrics/actions/abc.xml'), '') + + def testWithDifferentCheckout(self): + findTool = metrics_xml_format.FindMetricsXMLFormatterTool + checkout2 = join(self.root, '..', self._testMethodName + '2', 'src') + self.assertEqual( + # this is the case the tool was given a file path that is located + # in a different checkout folder. + findTool(join(checkout2, norm('tools/metrics/actions/abc.xml'))), + '', + ) + + def testSupportedHistogramsXML(self): + findTool = metrics_xml_format.FindMetricsXMLFormatterTool + self.assertEqual( + findTool(norm('tools/metrics/histograms/enums.xml')), + join(self.getcwd(), + norm('tools/metrics/histograms/pretty_print.py')), + ) + + def testNotSupportedHistogramsXML(self): + findTool = metrics_xml_format.FindMetricsXMLFormatterTool + self.assertEqual(findTool(norm('tools/metrics/histograms/NO.xml')), '') + + +if __name__ == '__main__': + unittest.main()