On Fri, 2012-10-05 at 15:50 +0200, Zdeněk Pavlas wrote:
> ---
> cli.py | 45 +++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 45 insertions(+), 0 deletions(-)
>
> diff --git a/cli.py b/cli.py
> index 3334e49..3499273 100755
> --- a/cli.py
> +++ b/cli.py
> @@ -42,6 +42,7 @@ from yum import _, P_
> from yum.rpmtrans import RPMTransaction
> import signal
> import yumcommands
> +import tempfile
>
> from yum.i18n import to_unicode, to_utf8, exception2msg
>
> @@ -547,6 +548,31 @@ class YumBaseCli(yum.YumBase, output.YumOutput):
> msg += _(" aborting.")
> raise yum.Errors.YumBaseError(msg)
>
> + if self.conf.dlonly:
> + tmpdir = {}
> + lockfile = self._lockfile
> + for po in downloadpkgs:
> +
> + # create tmpdir
> + pkgdir, fn = po.localpath.rsplit('/', 1)
> + if pkgdir not in tmpdir:
> + tmpdir[pkgdir] = tempfile.mkdtemp(dir=pkgdir)
This worries me, as POSIX only guarantees rename() to work within a
directory IIRC.
Why did you create an extra directory, just so the filename could stay
the same (so you can more easily get it out at the end)? If so I'm
suggest two approaches:
1. Don't use tempfile at all, go for something simple using the real
filename + extension ... then just drop the extension.
2. Use a random filename, but then call localPkg() at the end to
"regenerate" the package name.
> + # update localpath
> + newpath = tmpdir[pkgdir] +'/'+ fn
> + try: os.rename(po.localpath, newpath)
> + except OSError: pass
> + po.localpath = newpath
> +
> + # cache all metadata we'll need
> + po.basepath
> + po.returnChecksums()
> +
> + # close DBs, unlock
> + self.repos.close()
> + self.closeRpmDB()
> + self.doUnlock()
This just seems like it'll be a world of pain. If anything triggers any
metadata download after this we are completely screwed (dito. some
problems with opening the rpmdb/sql again).
Why don't we just put all of this inside downloadPkgs() (after
predownload hook, and never call postdownload hook -- maybe having a
postdownload_unlocked hook if needed).
> # confirm with user
> if self._promptWanted():
> if self.conf.assumeno or not self.userconfirm():
> @@ -557,6 +583,25 @@ class YumBaseCli(yum.YumBase, output.YumOutput):
> _('Downloading Packages:'))
> problems = self.downloadPkgs(downloadpkgs,
> callback_total=self.download_callback_total_cb)
>
> + if self.conf.dlonly:
> + self.doLock(lockfile
Locking again is going to be a big no, we just can't know that this
will ever return and if it does we can't trust the repos. anyway (so
having the lock just gives a false sense of security).
The atomic rename's can be done without any locks, as can removing the
files that only we own.
> + for po in downloadpkgs:
> +
> + # move downloaded files back to pkgdir
> + pkgdir, tmp, fn = po.localpath.rsplit('/', 2)
> + newpath = pkgdir +'/'+ fn
> + if os.path.exists(po.localpath):
> + if os.path.exists(newpath):
> + # conflict: drop our file
> + os.unlink(po.localpath)
> + else:
> + os.rename(po.localpath, newpath)
> + po.localpath = newpath
> +
> + # remove temporary directories
> + for tmp in tmpdir.values():
> + os.rmdir(tmp)
> +
> if len(problems) > 0:
> errstring = ''
> errstring += _('Error Downloading Packages:\n')
_______________________________________________
Yum-devel mailing list
[email protected]
http://lists.baseurl.org/mailman/listinfo/yum-devel
