Merge pull request #1219 from dmach/fix-pylint-errors

Fix pylint errors

.github/workflows/linters.yaml

@@ -35,6 +35,21 @@ jobs:
           src: "."
           version: "1.5.1"
 
+  pylint_errors:
+    name: 'Run pylint that reports only errors'
+    runs-on: 'ubuntu-latest'
+    steps:
+      - name: 'Install packages'
+        run: |
+            sudo apt-get -y update
+            sudo apt-get -y --no-install-recommends install pylint python3-rpm
+
+      - uses: actions/checkout@v3
+
+      - name: 'Run pylint --errors-only'
+        run: |
+            pylint --errors-only osc
+
   pylint_plugins:
     name: 'Diff pylint runs on osc plugin'
     runs-on: 'ubuntu-latest'

contrib/osc.spec

@@ -65,6 +65,9 @@ Requires:       %{use_python_pkg}-urllib3
 # needed for showing download progressbars
 Recommends:     %{use_python_pkg}-progressbar
 
+# needed for setting the default editor by distro
+Recommends:     %{use_python_pkg}-distro
+
 # needed for storing credentials in kwallet/gnome-keyring
 Recommends:     %{use_python_pkg}-keyring
 Recommends:     %{use_python_pkg}-keyring-keyutils

osc/_private/api_build.py

@@ -34,7 +34,7 @@ class BuildHistory:
         ]
         url_query = {}
         if self._limit and self._limit > 0:
-            query["limit"] = self._limit
+            url_query["limit"] = self._limit
 
         root = api.get(self.apiurl, url_path, url_query)
 

osc/_private/common.py

@@ -12,7 +12,7 @@ def print_msg(msg, print_to="debug"):
     elif print_to == "stdout":
         print(msg)
     else:
-        raise ValueError(f"Invalid value of the 'output' option: {output}")
+        raise ValueError(f"Invalid value of the 'print_to' option: {print_to}")
 
 
 def format_msg_project_package_options(

osc/babysitter.py

@@ -26,6 +26,7 @@ from .util.packagequery import PackageError
 
 try:
     # import as RPMError because the class "error" is too generic
+    # pylint: disable=E0611
     from rpm import error as RPMError
 except:
     # if rpm-python isn't installed (we might be on a debian system):

osc/build.py

@@ -221,6 +221,14 @@ class Pac:
 
     def __init__(self, node, buildarch, pacsuffix, apiurl, localpkgs=None):
         localpkgs = localpkgs or []
+
+        # set attributes to mute pylint error E1101: Instance of 'Pac' has no '<attr>' member (no-member)
+        self.project = None
+        self.name = None
+        self.canonname = None
+        self.repository = None
+        self.repoarch = None
+
         self.mp = {}
         for i in ['binary', 'package',
                   'epoch', 'version', 'release', 'hdrmd5',
@@ -292,7 +300,7 @@ class Pac:
         self.urllist = [url % self.mp for url in urllist]
 
     def __str__(self):
-        return self.name
+        return self.name or ""
 
     def __repr__(self):
         return "%s" % self.name
@@ -1177,11 +1185,11 @@ def main(apiurl, opts, argv):
                     def __del__(self):
                         self.cleanup()
 
-                    def __exit__(self):
+                    def __exit__(self, exc_type, exc_value, traceback):
                         self.cleanup()
 
                     def __str__(self):
-                        return self.name
+                        return self.name or ""
 
                 old_pkg_dir = mytmpdir(prefix='.build.oldpackages', dir=os.path.abspath(os.curdir))
                 if not os.path.exists(destdir):

osc/checker.py

@@ -19,6 +19,7 @@ class Checker:
         import rpm
         self.dbdir = mkdtemp(prefix='oscrpmdb')
         self.imported = {}
+        # pylint: disable=E1101
         rpm.addMacro('_dbpath', self.dbdir)
         self.ts = rpm.TransactionSet()
         self.ts.initDB()
@@ -29,6 +30,7 @@ class Checker:
     def readkeys(self, keys=None):
         import rpm
         keys = keys or []
+        # pylint: disable=E1101
         rpm.addMacro('_dbpath', self.dbdir)
         for key in keys:
             try:
@@ -40,6 +42,7 @@ class Checker:
             raise KeyError('', "no key imported")
 
         import rpm
+        # pylint: disable=E1101
         rpm.delMacro("_dbpath")
 
 # python is an idiot

osc/commandline.py

@@ -1856,7 +1856,7 @@ Please submit there instead, or use --nodevelproject to force direct submission.
 
         print('created request id', result)
         if conf.config['print_web_links']:
-            obs_url = _private.get_configuration_option(apiurl, "obs_url")
+            obs_url = _private.get_configuration_value(apiurl, "obs_url")
             print(f"{obs_url}/request/show/{result}")
 
         if supersede_existing:
@@ -3650,7 +3650,7 @@ Please submit there instead, or use --nodevelproject to force direct submission.
         r = create_maintenance_request(apiurl, source_project, source_packages, target_project, release_project, opt_sourceupdate, opts.message, opts.enforce_branching)
         print(r.reqid)
         if conf.config['print_web_links']:
-            obs_url = _private.get_configuration_option(apiurl, "obs_url")
+            obs_url = _private.get_configuration_value(apiurl, "obs_url")
             print(f"{obs_url}/request/show/{r.reqid}")
 
         if supersede_existing:
@@ -4578,7 +4578,7 @@ Please submit there instead, or use --nodevelproject to force direct submission.
         else:
             raise oscerr.WrongArgs('Wrong number of arguments')
 
-        download_url = _private.get_configuration_option(apiurl, "download_url")
+        download_url = _private.get_configuration_value(apiurl, "download_url")
 
         url_deb_tmpl = 'deb ' + download_url + '/%s/%s/ /'
         url_arch_tmpl = 'Server=' + download_url + '/%s/%s/$arch'

osc/core.py

@@ -2134,7 +2134,6 @@ class Package:
         def diff_add_delete(fname, add, revision):
             diff = []
             diff.append(diff_hdr % fname.encode())
-            tmpfile = None
             origname = fname
             if add:
                 diff.append(b'--- %s\t(revision 0)\n' % fname.encode())
@@ -2155,6 +2154,8 @@ class Package:
                 diff.append(b'+++ %s\t(working copy)\n' % fname.encode())
                 fname = os.path.join(self.storedir, fname)
 
+            fd = None
+            tmpfile = None
             try:
                 if revision is not None and not add:
                     (fd, tmpfile) = tempfile.mkstemp(prefix='osc_diff')
@@ -2180,8 +2181,9 @@ class Package:
                         lines.append(b'\n\\ No newline at end of file\n')
                 diff.extend(lines)
             finally:
-                if tmpfile is not None:
+                if fd is not None:
                     os.close(fd)
+                if tmpfile is not None and os.path.exists(tmpfile):
                     os.unlink(tmpfile)
             return diff
 
@@ -2226,6 +2228,7 @@ class Package:
             if revision is None:
                 yield get_source_file_diff(self.absdir, f.name, self.rev)
             else:
+                fd = None
                 tmpfile = None
                 diff = []
                 try:
@@ -2234,8 +2237,9 @@ class Package:
                     diff = get_source_file_diff(self.absdir, f.name, revision,
                                                 os.path.basename(tmpfile), os.path.dirname(tmpfile), f.name)
                 finally:
-                    if tmpfile is not None:
+                    if fd is not None:
                         os.close(fd)
+                    if tmpfile is not None and os.path.exists(tmpfile):
                         os.unlink(tmpfile)
                 yield diff
 
@@ -2617,7 +2621,8 @@ rev: %s
         elif filename in self.skipped:
             raise oscerr.OscIOError(None, 'file \'%s\' is marked as skipped and cannot be reverted' % filename)
         if filename in self.filenamelist and not os.path.exists(os.path.join(self.storedir, filename)):
-            raise oscerr.PackageInternalError('file \'%s\' is listed in filenamelist but no storefile exists' % filename)
+            msg = f"file '{filename}' is listed in filenamelist but no storefile exists"
+            raise oscerr.PackageInternalError(self.prjname, self.name, msg)
         state = self.status(filename)
         if not (state == 'A' or state == '!' and filename in self.to_be_added):
             shutil.copyfile(os.path.join(self.storedir, filename), os.path.join(self.absdir, filename))
@@ -4203,10 +4208,7 @@ def read_meta_from_spec(specfile, *args):
 def _get_linux_distro():
     if distro is not None:
         return distro.id()
-    elif sys.version_info >= (3, 8):
+    return None
-        return None
-    # compatibility for Python 2.6 to 3.7
-    return platform.linux_distribution()[0]
 
 
 def get_default_editor():

osc/fetch.py

@@ -86,6 +86,8 @@ class Fetcher:
                         pac = pkgs[decode_it(hdr.filename)]
 
                     # Extract a single file from the cpio archive
+                    fd = None
+                    tmpfile = None
                     try:
                         fd, tmpfile = tempfile.mkstemp(prefix='osc_build_file')
                         archive.copyin_file(hdr.filename,
@@ -93,8 +95,9 @@ class Fetcher:
                                             decode_it(os.path.basename(tmpfile)))
                         self.move_package(tmpfile, pac.localdir, pac)
                     finally:
-                        os.close(fd)
+                        if fd is not None:
-                        if os.path.exists(tmpfile):
+                            os.close(fd)
+                        if tmpfile is not None and os.path.exists(tmpfile):
                             os.unlink(tmpfile)
 
                 for pac in pkgs.values():

osc/oscerr.py

@@ -126,6 +126,21 @@ class WorkingCopyOutdated(OscBaseError):
                 % (self.args[0], self.args[1], self.args[2]))
 
 
+class ProjectError(OscBaseError):
+    """Base class for all Project related exceptions"""
+
+    def __init__(self, prj, msg=None):
+        super().__init__()
+        self.prj = prj
+        self.msg = msg
+
+    def __str__(self):
+        result = f"{self.__class__.__name__}: {self.prj}"
+        if self.msg:
+            result += f": {self.msg}"
+        return result
+
+
 class PackageError(OscBaseError):
     """Base class for all Package related exceptions"""