[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