Improve 'log' command: produce proper CSV and XML outputs, add -p/--p…

…atch option for the text output

behave/features/log.feature

@@ -0,0 +1,114 @@
+Feature: `osc log` command
+
+
+Scenario: Run `osc log` on a package
+   Given I execute osc with args "log test:factory/test-pkgA"
+    Then the exit code is 0
+     And stdout matches
+        """
+        ----------------------------------------------------------------------------
+        r3 | Admin | ....-..-.. ..:..:.. | dc997133b8ddfaf084b471b05c2643b3 | 3 | 
+
+        Version 3
+        ----------------------------------------------------------------------------
+        r2 | Admin | ....-..-.. ..:..:.. | 0ea55feb9cdd741ba7f523ed58a4f099 | 2 | 
+
+        Version 2
+        ----------------------------------------------------------------------------
+        r1 | Admin | ....-..-.. ..:..:.. | e675755e79e0d69483d311e96d6b719e | 1 | 
+
+        Initial commit
+        ----------------------------------------------------------------------------
+        """
+
+
+Scenario: Run `osc log` on single revision of a package
+   Given I execute osc with args "log test:factory/test-pkgA --revision=2"
+    Then the exit code is 0
+     And stdout matches
+        """
+        ----------------------------------------------------------------------------
+        r2 | Admin | ....-..-.. ..:..:.. | 0ea55feb9cdd741ba7f523ed58a4f099 | 2 | 
+
+        Version 2
+        ----------------------------------------------------------------------------
+        """
+
+
+Scenario: Run `osc log` on revision range of a package
+   Given I execute osc with args "log test:factory/test-pkgA --revision=1:2"
+    Then the exit code is 0
+     And stdout matches
+        """
+        ----------------------------------------------------------------------------
+        r2 | Admin | ....-..-.. ..:..:.. | 0ea55feb9cdd741ba7f523ed58a4f099 | 2 | 
+
+        Version 2
+        ----------------------------------------------------------------------------
+        r1 | Admin | ....-..-.. ..:..:.. | e675755e79e0d69483d311e96d6b719e | 1 | 
+
+        Initial commit
+        ----------------------------------------------------------------------------
+        """
+
+
+@wip
+Scenario: Run `osc log --patch` on revision range of a package
+   Given I execute osc with args "log test:factory/test-pkgA --revision=1:2 --patch"
+    Then the exit code is 0
+     And stdout matches
+        """
+        ----------------------------------------------------------------------------
+        r2 \| Admin \| ....-..-.. ..:..:.. \| 0ea55feb9cdd741ba7f523ed58a4f099 \| 2 \| 
+
+        Version 2
+
+
+        changes files:
+        --------------
+        --- test-pkgA.changes
+        \+\+\+ test-pkgA.changes
+        @@ -2 \+2 @@
+        -Tue Jan  4 11:22:33 UTC 2022 - Geeko Packager <[email protected]>
+        \+Mon Jan  3 11:22:33 UTC 2022 - Geeko Packager <[email protected]>
+        @@ -4 \+4 @@
+        -- Release upstream version 2
+        \+- Release upstream version 1
+
+        spec files:
+        -----------
+        --- test-pkgA.spec
+        \+\+\+ test-pkgA.spec
+        @@ -1,5 \+1,5 @@
+         Name:           test-pkgA
+        -Version:        2
+        \+Version:        1
+         Release:        1
+         License:        GPL-2.0
+         Summary:        Test package
+
+        ----------------------------------------------------------------------------
+        r1 \| Admin \| ....-..-.. ..:..:.. \| e675755e79e0d69483d311e96d6b719e \| 1 \| 
+
+        Initial commit
+
+
+        changes files:
+        --------------
+
+        \+\+\+\+\+\+ deleted changes files:
+        --- test-pkgA.changes
+
+        old:
+        ----
+          test-pkgA.changes
+          test-pkgA.spec
+
+        spec files:
+        -----------
+
+        \+\+\+\+\+\+ deleted spec files:
+        --- test-pkgA.spec
+
+
+        """

osc/commandline.py

@@ -39,6 +39,7 @@
 from .grabber import OscFileGrabber
 from .meter import create_text_meter
 from .output import get_user_input
+from .output import pipe_to_pager
 from .util import cpio, rpmquery, safewriter
 from .util.helper import _html_escape, format_table
 
@@ -7661,13 +7662,15 @@ def do_jobhistory(self, subcmd, opts, *args):
 
     @cmdln.option('-r', '--revision', metavar='rev',
                         help='show log of the specified revision')
+    @cmdln.option("-p", "--patch", action="store_true",
+                        help='show patch for each revision; NOTE: use this option carefully because it loads patches on demand in a pager')
     @cmdln.option('', '--csv', action='store_true',
-                  help='generate output in CSV (separated by |)')
+                  help='generate output in CSV')
     @cmdln.option('', '--xml', action='store_true',
                   help='generate output in XML')
-    @cmdln.option('-D', '--deleted', action='store_true',
+    @cmdln.option('-D', '--deleted', action='store_true', default=None,
                         help='work on deleted package')
-    @cmdln.option('-M', '--meta', action='store_true',
+    @cmdln.option('-M', '--meta', action='store_true', default=None,
                         help='checkout out meta data instead of sources')
     def do_log(self, subcmd, opts, *args):
         """
@@ -7695,8 +7698,8 @@ def do_log(self, subcmd, opts, *args):
         if opts.xml:
             format = 'xml'
 
-        log = '\n'.join(get_commitlog(apiurl, project, package, rev, format, opts.meta, opts.deleted, rev_upper))
-        run_pager(log)
+        lines = get_commitlog(apiurl, project, package, rev, format, opts.meta, opts.deleted, rev_upper, patch=opts.patch)
+        pipe_to_pager(lines, add_newlines=True)
 
     @cmdln.option('-v', '--verbose', action='store_true',
                   help='verbose run of local services for debugging purposes')

osc/core.py

@@ -6,6 +6,7 @@
 
 import codecs
 import copy
+import csv
 import datetime
 import difflib
 import errno
@@ -4657,82 +4658,97 @@ def print_jobhistory(apiurl: str, prj: str, current_package: str, repository: st
 
 
 def get_commitlog(
-    apiurl: str, prj: str, package: str, revision, format="text", meta=False, deleted=False, revision_upper=None
+    apiurl: str,
+    prj: str,
+    package: str,
+    revision: Optional[str],
+    format: str = "text",
+    meta: Optional[bool] = None,
+    deleted: Optional[bool] = None,
+    revision_upper: Optional[str] = None,
+    patch: Optional[bool] = None,
 ):
     if package is None:
         package = "_project"
 
-    query = {}
-    if deleted:
-        query['deleted'] = 1
-    if meta:
-        query['meta'] = 1
+    from . import obs_api
+    revision_list = obs_api.Package.get_revision_list(apiurl, prj, package, deleted=deleted, meta=meta)
 
-    u = makeurl(apiurl, ['source', prj, package, '_history'], query)
-    f = http_GET(u)
-    root = ET.parse(f).getroot()
+    # TODO: consider moving the following block to Package.get_revision_list()
+    # keep only entries matching the specified revision
+    if not revision_is_empty(revision):
+        if isinstance(revision, str) and len(revision) == 32:
+            # revision is srcmd5
+            revision_list = [i for i in revision_list if i.srcmd5 == revision]
+        else:
+            revision = int(revision)
+            if revision_is_empty(revision_upper):
+                revision_list = [i for i in revision_list if i.rev == revision]
+            else:
+                revision_upper = int(revision_upper)
+                revision_list = [i for i in revision_list if i.rev <= revision_upper and i.rev >= revision]
+
+    if format == "csv":
+        f = io.StringIO()
+        writer = csv.writer(f, dialect="unix")
+        for revision in reversed(revision_list):
+            writer.writerow(
+                (
+                    revision.rev,
+                    revision.user,
+                    revision.get_time_str(),
+                    revision.srcmd5,
+                    revision.comment,
+                    revision.requestid,
+                )
+            )
+        f.seek(0)
+        yield from f.read().splitlines()
+        return
 
-    r = []
-    if format == 'xml':
-        r.append('<?xml version="1.0"?>')
-        r.append('<log>')
-    revisions = root.findall('revision')
-    revisions.reverse()
-    for node in revisions:
-        srcmd5 = node.find('srcmd5').text
-        try:
-            rev = int(node.get('rev'))
-            # vrev = int(node.get('vrev')) # what is the meaning of vrev?
-            try:
-                if not revision_is_empty(revision) and revision_upper is not None:
-                    if rev > int(revision_upper) or rev < int(revision):
-                        continue
-                elif not revision_is_empty(revision) and rev != int(revision):
-                    continue
-            except ValueError:
-                if revision != srcmd5:
-                    continue
-        except ValueError:
-            # this part should _never_ be reached but...
-            return ['an unexpected error occured - please file a bug']
-        version = node.find('version').text
-        user = node.find('user').text
-        try:
-            comment = node.find('comment').text.encode(locale.getpreferredencoding(), 'replace')
-        except:
-            comment = b'<no message>'
-        try:
-            requestid = node.find('requestid').text.encode(locale.getpreferredencoding(), 'replace')
-        except:
-            requestid = ""
-        t = time.gmtime(int(node.find('time').text))
-        t = time.strftime('%Y-%m-%d %H:%M:%S', t)
+    if format == "xml":
+        root = ET.Element("log")
+        for revision in reversed(revision_list):
+            entry = ET.SubElement(root, "logentry")
+            entry.attrib["revision"] = str(revision.rev)
+            entry.attrib["srcmd5"] = revision.srcmd5
+            ET.SubElement(entry, "author").text = revision.user
+            ET.SubElement(entry, "date").text = revision.get_time_str()
+            ET.SubElement(entry, "requestid").text = str(revision.requestid) if revision.requestid else ""
+            ET.SubElement(entry, "msg").text = revision.comment or ""
+        xmlindent(root)
+        yield from ET.tostring(root, encoding="utf-8").decode("utf-8").splitlines()
+        return
 
-        if format == 'csv':
-            s = '%s|%s|%s|%s|%s|%s|%s' % (rev, user, t, srcmd5, version,
-                                          decode_it(comment).replace('\\', '\\\\').replace('\n', '\\n').replace('|', '\\|'), requestid)
-            r.append(s)
-        elif format == 'xml':
-            r.append('<logentry')
-            r.append(f'   revision="{rev}" srcmd5="{srcmd5}">')
-            r.append(f'<author>{user}</author>')
-            r.append(f'<date>{t}</date>')
-            r.append(f'<requestid>{requestid}</requestid>')
-            r.append(f'<msg>{_private.api.xml_escape(decode_it(comment))}</msg>')
-            r.append('</logentry>')
-        else:
-            if requestid:
-                requestid = decode_it(b"rq" + requestid)
-            s = '-' * 76 + \
-                f'\nr{rev} | {user} | {t} | {srcmd5} | {version} | {requestid}\n' + \
-                '\n' + decode_it(comment)
-            r.append(s)
-
-    if format not in ['csv', 'xml']:
-        r.append('-' * 76)
-    if format == 'xml':
-        r.append('</log>')
-    return r
+    if format == "text":
+        for revision in reversed(revision_list):
+            entry = (
+                f"r{revision.rev}",
+                revision.user,
+                revision.get_time_str(),
+                revision.srcmd5,
+                revision.version,
+                f"rq{revision.requestid}" if revision.requestid else ""
+            )
+            yield 76 * "-"
+            yield " | ".join(entry)
+            yield ""
+            yield revision.comment or "<no message>"
+            yield ""
+            if patch:
+                rdiff = server_diff_noex(
+                    apiurl,
+                    prj,
+                    package,
+                    revision.rev,
+                    prj,
+                    package,
+                    revision.rev - 1,
+                )
+                yield highlight_diff(rdiff).decode("utf-8", errors="replace")
+        return
+
+    raise ValueError(f"Invalid format: {format}")
 
 
 def runservice(apiurl: str, prj: str, package: str):