From 1acbc14c34f264e0158436dc3d0265d500848462 Mon Sep 17 00:00:00 2001 From: Mike Frysinger Date: Wed, 30 Apr 2025 13:29:20 -0400 Subject: [PATCH] manifest: generalize --json as --format= This will make it easier to add more formats without exploding the common --xxx space and checking a large set of boolean flags. Also fill out the test coverage while we're here. Bug: b/412725063 Change-Id: I754013dc6cb3445f8a0979cefec599d55dafdcff Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/471941 Reviewed-by: Gavin Mak Commit-Queue: Mike Frysinger Tested-by: Mike Frysinger --- man/repo-manifest.1 | 8 +- subcmds/manifest.py | 39 +++++++-- tests/test_subcmds_manifest.py | 156 +++++++++++++++++++++++++++++++++ 3 files changed, 195 insertions(+), 8 deletions(-) create mode 100644 tests/test_subcmds_manifest.py diff --git a/man/repo-manifest.1 b/man/repo-manifest.1 index f2f7290d..74e09145 100644 --- a/man/repo-manifest.1 +++ b/man/repo-manifest.1 @@ -30,8 +30,8 @@ if in \fB\-r\fR mode, do not write the dest\-branch field (only of use if the branch names for a sha1 manifest are sensitive) .TP -\fB\-\-json\fR -output manifest in JSON format (experimental) +\fB\-\-format\fR=\fI\,FORMAT\/\fR +output format: xml, json (default: xml) .TP \fB\-\-pretty\fR format output for humans to read @@ -78,6 +78,10 @@ set to the ref we were on when the manifest was generated. The 'dest\-branch' attribute is set to indicate the remote ref to push changes to via 'repo upload'. .PP +Multiple output formats are supported via \fB\-\-format\fR. The default output is XML, +and formats are generally "condensed". Use \fB\-\-pretty\fR for more human\-readable +variations. +.PP repo Manifest Format .PP A repo manifest describes the structure of a repo client; that is the diff --git a/subcmds/manifest.py b/subcmds/manifest.py index bb6dc930..9786580a 100644 --- a/subcmds/manifest.py +++ b/subcmds/manifest.py @@ -12,7 +12,9 @@ # See the License for the specific language governing permissions and # limitations under the License. +import enum import json +import optparse import os import sys @@ -23,6 +25,16 @@ from repo_logging import RepoLogger logger = RepoLogger(__file__) +class OutputFormat(enum.Enum): + """Type for the requested output format.""" + + # Canonicalized manifest in XML format. + XML = enum.auto() + + # Canonicalized manifest in JSON format. + JSON = enum.auto() + + class Manifest(PagedCommand): COMMON = False helpSummary = "Manifest inspection utility" @@ -42,6 +54,10 @@ revisions set to the current commit hash. These are known as In this case, the 'upstream' attribute is set to the ref we were on when the manifest was generated. The 'dest-branch' attribute is set to indicate the remote ref to push changes to via 'repo upload'. + +Multiple output formats are supported via --format. The default output +is XML, and formats are generally "condensed". Use --pretty for more +human-readable variations. """ @property @@ -86,11 +102,21 @@ to indicate the remote ref to push changes to via 'repo upload'. "(only of use if the branch names for a sha1 manifest are " "sensitive)", ) + # Replaced with --format=json. Kept for backwards compatibility. + # Can delete in Jun 2026 or later. p.add_option( "--json", - default=False, - action="store_true", - help="output manifest in JSON format (experimental)", + action="store_const", + dest="format", + const=OutputFormat.JSON.name.lower(), + help=optparse.SUPPRESS_HELP, + ) + formats = tuple(x.lower() for x in OutputFormat.__members__.keys()) + p.add_option( + "--format", + default=OutputFormat.XML.name.lower(), + choices=formats, + help=f"output format: {', '.join(formats)} (default: %default)", ) p.add_option( "--pretty", @@ -121,6 +147,8 @@ to indicate the remote ref to push changes to via 'repo upload'. if opt.manifest_name: self.manifest.Override(opt.manifest_name, False) + output_format = OutputFormat[opt.format.upper()] + for manifest in self.ManifestList(opt): output_file = opt.output_file if output_file == "-": @@ -135,8 +163,7 @@ to indicate the remote ref to push changes to via 'repo upload'. manifest.SetUseLocalManifests(not opt.ignore_local_manifests) - if opt.json: - logger.warning("warning: --json is experimental!") + if output_format == OutputFormat.JSON: doc = manifest.ToDict( peg_rev=opt.peg_rev, peg_rev_upstream=opt.peg_rev_upstream, @@ -152,7 +179,7 @@ to indicate the remote ref to push changes to via 'repo upload'. "separators": (",", ": ") if opt.pretty else (",", ":"), "sort_keys": True, } - fd.write(json.dumps(doc, **json_settings)) + fd.write(json.dumps(doc, **json_settings) + "\n") else: manifest.Save( fd, diff --git a/tests/test_subcmds_manifest.py b/tests/test_subcmds_manifest.py new file mode 100644 index 00000000..9b1ffb30 --- /dev/null +++ b/tests/test_subcmds_manifest.py @@ -0,0 +1,156 @@ +# Copyright (C) 2025 The Android Open Source Project +# +# Licensed 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. + +"""Unittests for the subcmds/manifest.py module.""" + +import json +from pathlib import Path +from unittest import mock + +import manifest_xml +from subcmds import manifest + + +_EXAMPLE_MANIFEST = """\ + + + + + + + +""" + + +def _get_cmd(repodir: Path) -> manifest.Manifest: + """Instantiate a manifest command object to test.""" + manifests_git = repodir / "manifests.git" + manifests_git.mkdir() + (manifests_git / "config").write_text( + """ +[remote "origin"] +\turl = http://localhost/manifest +""" + ) + client = manifest_xml.RepoClient(repodir=str(repodir)) + git_event_log = mock.MagicMock(ErrorEvent=mock.Mock(return_value=None)) + return manifest.Manifest( + repodir=client.repodir, + client=client, + manifest=client.manifest, + outer_client=client, + outer_manifest=client.manifest, + git_event_log=git_event_log, + ) + + +def test_output_format_xml_file(tmp_path): + """Test writing XML to a file.""" + path = tmp_path / "manifest.xml" + path.write_text(_EXAMPLE_MANIFEST) + outpath = tmp_path / "output.xml" + cmd = _get_cmd(tmp_path) + opt, args = cmd.OptionParser.parse_args(["--output-file", str(outpath)]) + cmd.Execute(opt, args) + # Normalize the output a bit as we don't exactly care. + normalize = lambda data: "\n".join( + x.strip() for x in data.splitlines() if x.strip() + ) + assert ( + normalize(outpath.read_text()) + == """ + + + + + +""" + ) + + +def test_output_format_xml_stdout(tmp_path, capsys): + """Test writing XML to stdout.""" + path = tmp_path / "manifest.xml" + path.write_text(_EXAMPLE_MANIFEST) + cmd = _get_cmd(tmp_path) + opt, args = cmd.OptionParser.parse_args(["--format", "xml"]) + cmd.Execute(opt, args) + # Normalize the output a bit as we don't exactly care. + normalize = lambda data: "\n".join( + x.strip() for x in data.splitlines() if x.strip() + ) + stdout = capsys.readouterr().out + assert ( + normalize(stdout) + == """ + + + + + +""" + ) + + +def test_output_format_json(tmp_path, capsys): + """Test writing JSON.""" + path = tmp_path / "manifest.xml" + path.write_text(_EXAMPLE_MANIFEST) + cmd = _get_cmd(tmp_path) + opt, args = cmd.OptionParser.parse_args(["--format", "json"]) + cmd.Execute(opt, args) + obj = json.loads(capsys.readouterr().out) + assert obj == { + "default": {"remote": "test-remote", "revision": "refs/heads/main"}, + "project": [{"name": "repohooks", "path": "src/repohooks"}], + "remote": [{"fetch": "http://localhost", "name": "test-remote"}], + "repo-hooks": {"enabled-list": "a b", "in-project": "repohooks"}, + } + + +def test_output_format_json_pretty(tmp_path, capsys): + """Test writing pretty JSON.""" + path = tmp_path / "manifest.xml" + path.write_text(_EXAMPLE_MANIFEST) + cmd = _get_cmd(tmp_path) + opt, args = cmd.OptionParser.parse_args(["--format", "json", "--pretty"]) + cmd.Execute(opt, args) + stdout = capsys.readouterr().out + assert ( + stdout + == """\ +{ + "default": { + "remote": "test-remote", + "revision": "refs/heads/main" + }, + "project": [ + { + "name": "repohooks", + "path": "src/repohooks" + } + ], + "remote": [ + { + "fetch": "http://localhost", + "name": "test-remote" + } + ], + "repo-hooks": { + "enabled-list": "a b", + "in-project": "repohooks" + } +} +""" + )