[Yum-devel] Repo doSetup progress snafu (damn pylint highliting our bugs :)

James Antill james at fedoraproject.org
Sun Apr 5 05:20:47 UTC 2009


 I'd noticed within the last week or so that a new line was being
output, and was kind of confused about where it was coming from. This
was in the 3.2.22+ that is in the pipe for RHEL-5.4 so I had a little
investigate today, and it was this bit as part of the pylint cleanup:

diff --git a/yum/repos.py b/yum/repos.py
index 5185d98..0fba1c6 100644
--- a/yum/repos.py
+++ b/yum/repos.py
@@ -75,10 +75,9 @@ class RepoStorage:
         for repo in repos:
             repo.setup(self.ayum.conf.cache, self.ayum.mediagrabber,
                    gpg_import_func = self.gpg_import_func, confirm_func=self.co
-            num += 1
-            
-        if self.callback and len(repos) > 0:
-            self.callback.progressbar(num, len(repos), repo.id)
+            num += 1            
+            if self.callback and len(repos) > 0:
+                self.callback.progressbar(num, len(repos), repo.id)
 
         self._setup = True
         self.ayum.plugins.run('postreposetup')

...so not thinking much about it, figuring Tim had messed up the
indentation somehow, I just reverted this bit.
 Then I thought more about it later in the day and eventually (slowly)
realized what was going on, and that it was an almost fix that's kind of
bad :o.


 The code was initially introduced in commit:

84b5185a327db9002abc55ec279d2fa814b12b84
Author: Jeremy Katz <katzj at redhat.com>
Date:   Sat Dec 17 04:02:59 2005 +0000

...which was kinda correct, as (roughly):

         num = 1
         for repo in repos:
[...]
+            if self.repos.callback:
+                self.repos.callback.progressbar(num, len(repos), repo.id)
[...]
+            num += 1
 
+        if self.repos.callback:
+            self.repos.callback.progressbar(num, len(repos), repo.id)

...here the last progress call doesn't do anything due to
"num == len(repos) + 1", but the progress info. in the loop is good
(done before the num for each repo.).

 Then within a few weeks it changes a few times:

08fb8d6590a929f91d7a5eb49783dc98fb84bc35
548ce96fa71e4b845062bb302a998e652b40d961
658e9d3077877158f58b8fe64b2b5a452bb78a1d

...eventually ending up with the inner loop progressbar as a debug only
thing (so nothing is output). Then at some point I can't easily find,
most of the inner loop went away and in:

commit d805495b5d1e65c322d684663816615c18c93875
Author: Jeremy Katz <katzj at redhat.com>
Date:   Tue May 29 15:52:33 2007 +0000

...it moved to it's present location in repos.py.


 So, what to do. The "world" is kind of in a different place now. And we
have the following situation:

1. doSetup takes milliseconds for double digit numbers of repos.

2. Having the progress in the "correct" place¹ outputs ~80 bytes per.
repo., so takes more time to run than the function.

3. Correct output looks weird (very similar to repo. XML => sqlite
building output ... and humans only see the last repo).

4. Having the output in the wrong place is basically a "bug" (as it does
nothing, in text mode), but one we've had for 3.25 years :).

...so, what to do?
 The "obvious" thing is to just delete the progress output (as it
currently does nothing), however I don't want to just do that without
running it past everyone here ... is it possible yumex/etc. would want
this there? In theory it's an API ... ha!
 Any votes for keeping it and fixing it?


¹ Basically Tim's fix, but move relative to "num += 1", or we drop the
last repo.

-- 
James Antill <james at fedoraproject.org>
Fedora


More information about the Yum-devel mailing list