manifest: generalize --json as --format=<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 <gavinmak@google.com>
Commit-Queue: Mike Frysinger <vapier@google.com>
Tested-by: Mike Frysinger <vapier@google.com>
diff --git a/man/repo-manifest.1 b/man/repo-manifest.1
index f2f7290..74e0914 100644
--- a/man/repo-manifest.1
+++ b/man/repo-manifest.1
@@ -30,8 +30,8 @@
 (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 @@
 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 bb6dc93..9786580 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 @@
 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 @@
 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 @@
             "(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 @@
         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 @@
 
             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 @@
                     "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 0000000..9b1ffb3
--- /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 = """\
+<?xml version="1.0" encoding="UTF-8"?>
+<manifest>
+  <remote name="test-remote" fetch="http://localhost" />
+  <default remote="test-remote" revision="refs/heads/main" />
+  <project name="repohooks" path="src/repohooks"/>
+  <repo-hooks in-project="repohooks" enabled-list="a, b"/>
+</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())
+        == """<?xml version="1.0" encoding="UTF-8"?>
+<manifest>
+<remote name="test-remote" fetch="http://localhost"/>
+<default remote="test-remote" revision="refs/heads/main"/>
+<project name="repohooks" path="src/repohooks"/>
+<repo-hooks in-project="repohooks" enabled-list="a b"/>
+</manifest>"""
+    )
+
+
+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)
+        == """<?xml version="1.0" encoding="UTF-8"?>
+<manifest>
+<remote name="test-remote" fetch="http://localhost"/>
+<default remote="test-remote" revision="refs/heads/main"/>
+<project name="repohooks" path="src/repohooks"/>
+<repo-hooks in-project="repohooks" enabled-list="a b"/>
+</manifest>"""
+    )
+
+
+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"
+  }
+}
+"""
+    )