add metrics_xml_formatter

git cl format supports formatting certain xml files under tools/metrics.
This CL adds metrics_xml_formatter so that the formatter can be run
with git cl format.

Bug: b/369827156
Change-Id: I5922cd79304aa8e06917aacc9f2d9bd3ed548c2f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6072565
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Scott Lee <ddoman@chromium.org>

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)
+        pretty_print_tool = metrics_xml_format.FindMetricsXMLFormatterTool(path)
-        if not xml_dir:
+        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 metrics_xml_format.GetMetricsDir(path) == '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
             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('<codereview url or issue id>')
 @metrics.collector.collect_metrics('git cl checkout')
 def CMDcheckout(parser, args):

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" "$@"

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)

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()