Merge pull request #1311 from dmach/fix-diff-specify-files

Fix 'diff' command to support diffing selected files only

osc/commandline.py

@@ -4646,9 +4646,10 @@ Please submit there instead, or use --nodevelproject to force direct submission.
                 for i in pac.get_diff(rev1):
                     diff += b''.join(i)
             else:
+                files = args
                 diff += server_diff_noex(pac.apiurl, pac.prjname, pac.name, rev1,
                                          pac.prjname, pac.name, rev2,
-                                         not opts.plain, opts.missingok, opts.meta, not opts.unexpand)
+                                         not opts.plain, opts.missingok, opts.meta, not opts.unexpand, files=files)
         run_pager(diff)
 
     @cmdln.option('--issues-only', action='store_true',

osc/connection.py

@@ -135,12 +135,12 @@ def http_request_wrap_file(func):
     Turn file path into a file object and close it automatically
     by using a context manager.
     """
-    def new_func(method, url, headers=None, data=None, file=None):
+    def new_func(method, url, headers=None, data=None, file=None, retry_on_400: bool = True):
         if file:
             with open(file, "rb") as f:
-                return func(method, url, headers, data, file=f)
+                return func(method, url, headers, data, f, retry_on_400)
         else:
-            return func(method, url, headers, data, file)
+            return func(method, url, headers, data, file, retry_on_400)
 
     new_func.__name__ = func.__name__
     new_func.__doc__ = func.__doc__
@@ -148,7 +148,7 @@ def http_request_wrap_file(func):
 
 
 @http_request_wrap_file
-def http_request(method: str, url: str, headers=None, data=None, file=None):
+def http_request(method: str, url: str, headers=None, data=None, file=None, retry_on_400: bool = True):
     """
     Send a HTTP request to a server.
 
@@ -168,6 +168,7 @@ def http_request(method: str, url: str, headers=None, data=None, file=None):
     :param headers: Dictionary of custom headers to send.
     :param data: Data to send in the request body (conflicts with `file`).
     :param file: Path to a file to send as data in the request body (conflicts with `data`).
+    :param retry_on_400: Whether to retry on receiving HTTP status code 400.
     """
 
     purl = urllib3.util.parse_url(url)
@@ -227,16 +228,22 @@ def http_request(method: str, url: str, headers=None, data=None, file=None):
         else:
             retries_kwargs = {"method_whitelist": None}
 
+
+        status_forcelist = (
+            500,  # Internal Server Error
+            502,  # Bad Gateway
+            503,  # Service Unavailable
+            504,  # Gateway Timeout
+        )
+        if retry_on_400:
+            status_forcelist = (
+                400,  # Bad Request; retry on 400: service in progress
+            ) + status_forcelist
+
         pool_kwargs["retries"] = urllib3.Retry(
             total=int(conf.config["http_retries"]),
             backoff_factor=2,
-            status_forcelist=(
-                400,  # Bad Request; retry on 400: service in progress
-                500,  # Internal Server Error
-                502,  # Bad Gateway
-                503,  # Service Unavailable
-                504,  # Gateway Timeout
-            ),
+            status_forcelist=status_forcelist,
             # don't raise because we want an actual response rather than a MaxRetryError with "too many <status_code> error responses" message
             raise_on_status=False,
             **retries_kwargs,

osc/core.py

@@ -11,6 +11,7 @@ __store_version__ = '1.0'
 
 
 import codecs
+import copy
 import datetime
 import difflib
 import errno
@@ -3574,6 +3575,21 @@ def pathjoin(a, *p):
     return path
 
 
+def osc_urlencode(data):
+    """
+    An urlencode wrapper that encodes dictionaries in OBS compatible way:
+    {"file": ["foo", "bar"]} -> &file[]=foo&file[]=bar
+    """
+    data = copy.deepcopy(data)
+    if isinstance(data, dict):
+        for key, value in list(data.items()):
+            if isinstance(value, list):
+                del data[key]
+                data[f"{key}[]"] = value
+
+    return urlencode(data, doseq=True)
+
+
 def makeurl(baseurl: str, l, query=None):
     """Given a list of path compoments, construct a complete URL.
 
@@ -3589,7 +3605,7 @@ def makeurl(baseurl: str, l, query=None):
     if isinstance(query, list):
         query = '&'.join(query)
     elif isinstance(query, dict):
-        query = urlencode(query)
+        query = osc_urlencode(query)
 
     scheme, netloc, path = urlsplit(baseurl)[0:3]
     return urlunsplit((scheme, netloc, '/'.join([path] + list(l)), query, ''))
@@ -5283,6 +5299,7 @@ def server_diff(
     onlyissues=False,
     full=True,
     xml=False,
+    files: list = None,
 ):
     query: Dict[str, Union[str, int]] = {"cmd": "diff"}
     if expand:
@@ -5308,9 +5325,11 @@ def server_diff(
         query['onlyissues'] = 1
         query['view'] = 'xml'
         query['unified'] = 0
+    if files:
+        query["file"] = files
 
     u = makeurl(apiurl, ['source', new_project, new_package], query=query)
-    f = http_POST(u)
+    f = http_POST(u, retry_on_400=False)
     if onlyissues and not xml:
         del_issue_list = []
         add_issue_list = []
@@ -5345,12 +5364,13 @@ def server_diff_noex(
     expand=True,
     onlyissues=False,
     xml=False,
+    files: list = None,
 ):
     try:
         return server_diff(apiurl,
                            old_project, old_package, old_revision,
                            new_project, new_package, new_revision,
-                           unified, missingok, meta, expand, onlyissues, True, xml)
+                           unified, missingok, meta, expand, onlyissues, True, xml, files=files)
     except HTTPError as e:
         msg = None
         body = None
@@ -5367,7 +5387,7 @@ def server_diff_noex(
                 rdiff += server_diff_noex(apiurl,
                                           old_project, old_package, old_revision,
                                           new_project, new_package, new_revision,
-                                          unified, missingok, meta, False)
+                                          unified, missingok, meta, False, files=files)
             except:
                 elm = ET.fromstring(body).find('summary')
                 summary = ''