Mailinglist Archive: zypp-devel (115 mails)

< Previous Next >
[zypp-devel] [PATCH] repo media handling and verifiers for review
  • From: "Duncan Mac-Vicar P." <dmacvicar@xxxxxxx>
  • Date: Tue, 24 Jul 2007 00:49:02 +0200
  • Message-id: <200707240049.02448.dmacvicar@xxxxxxx>

This patch does the following:

- Adds a RepoMediaAccess class, which contains a modified version of the old 
function repo::provideFile as a member.
- The old function is modified, to just instanciate a RepoMediaAccessClass, 
provide the file and return.

The class allows to be instanciated once at begining of commit and passed to 
PackageProvider. It can serve file request from multiple repositories, and 
keeps MediaSetAccess objects for different urls, it sets the verifiers 
automatically, and reset them if a diiferent repo access the same url other 
repo used before. That means, any repo can provide any file from any url, and 
only one media will be opened per url.

FAQ:
- Why not implement it in MediaManager (automatic remember of opened medias)
A: ask Marius :-P

Please comment, otherwise I will merge my local branch commits to svn trunk 
asap (we need this)

Duncan
diff --git a/zypp/RepoInfo.cc b/zypp/RepoInfo.cc
index f57d44d..3771e8d 100644
--- a/zypp/RepoInfo.cc
+++ b/zypp/RepoInfo.cc
@@ -54,7 +54,6 @@ namespace zypp
     std::string alias;
     std::string name;
     Pathname filepath;
-    Pathname metadatapath;
   public:
 
   private:
@@ -168,12 +167,6 @@ namespace zypp
     _pimpl->filepath = filepath;
     return *this;
   }
-  
-  RepoInfo & RepoInfo::setMetadataPath( const Pathname &path )
-  {
-    _pimpl->metadatapath = path;
-    return *this;
-  }
 
   tribool RepoInfo::enabled() const
   { return _pimpl->enabled; }
@@ -193,9 +186,6 @@ namespace zypp
   Pathname RepoInfo::filepath() const
   { return _pimpl->filepath; }
 
-  Pathname RepoInfo::metadataPath() const
-  { return _pimpl->metadatapath; }
-  
   repo::RepoType RepoInfo::type() const
   { return _pimpl->type; }
 
diff --git a/zypp/RepoInfo.h b/zypp/RepoInfo.h
index 0afdf29..5956e86 100644
--- a/zypp/RepoInfo.h
+++ b/zypp/RepoInfo.h
@@ -174,14 +174,6 @@ namespace zypp
      * infos created in memory.
      */
      Pathname filepath() const;
-     
-     /**
-     * \short Path where this repo metadata was read from
-     *
-     * \note could be an empty pathname for repo
-     * infos created in memory.
-     */
-     Pathname metadataPath() const;
       
      /**
      * \short Whether to check or not this repository with gpg
@@ -268,16 +260,6 @@ namespace zypp
      * \param path File path
      */
     RepoInfo & setFilepath( const Pathname &filename );
-    
-    /**
-     * \short set the path where the local metadata is stored
-     *
-     * The path to the metadata of this repository
-     * was defined, or empty if nowhere.
-     *
-     * \param path directory path
-     */
-    RepoInfo & setMetadataPath( const Pathname &path );
 
     /**
      * \short Whether to check or not this repository with gpg
diff --git a/zypp/RepoManager.cc b/zypp/RepoManager.cc
index b06d962..70c48c4 100644
--- a/zypp/RepoManager.cc
+++ b/zypp/RepoManager.cc
@@ -253,18 +253,7 @@ namespace zypp
     MIL << endl;
 
     if ( PathInfo(_pimpl->options.knownReposPath).isExist() )
-    {
-      RepoInfoList repos = repositories_in_dir(_pimpl->options.knownReposPath);
-      for ( RepoInfoList::iterator it = repos.begin();
-            it != repos.end();
-            ++it )
-      {
-        // set the metadata path for the repo
-        Pathname metadata_path = rawcache_path_for_repoinfo(_pimpl->options, (*it));
-        (*it).setMetadataPath(metadata_path);
-      }
-      return repos;
-    }
+      return repositories_in_dir(_pimpl->options.knownReposPath);
     else
       return std::list<RepoInfo>();
 
@@ -844,7 +833,8 @@ namespace zypp
           {
             ZYPP_THROW(RepoException("Can't delete " + todelete.filepath().asString()));
           }
-          MIL << todelete.alias() << " sucessfully deleted." << endl;
+         MIL << todelete.alias() << " sucessfully deleted." << endl;
+          return;
         }
         else
         {
@@ -868,19 +858,17 @@ namespace zypp
             if ( (*fit).alias() != todelete.alias() )
               (*fit).dumpRepoOn(file);
           }
-        }
 
-        // now delete it from cache
-        cache::CacheStore store(_pimpl->options.repoCachePath);
+          cache::CacheStore store(_pimpl->options.repoCachePath);
 
-        if ( store.isCached( todelete.alias() ) ) {
-          MIL << "repository was cached. cleaning cache" << endl;
-          store.cleanRepository(todelete.alias());
-          store.commit();
-        }
+          if ( store.isCached( todelete.alias() ) ) {
+            MIL << "repository was cached. cleaning cache" << endl;
+            store.cleanRepository(todelete.alias());
+          }
 
-        MIL << todelete.alias() << " sucessfully deleted." << endl;
-        return;
+         MIL << todelete.alias() << " sucessfully deleted." << endl;
+          return;
+        }
       } // else filepath is empty
 
     }
diff --git a/zypp/repo/PackageProvider.cc b/zypp/repo/PackageProvider.cc
index 633518f..b935892 100644
--- a/zypp/repo/PackageProvider.cc
+++ b/zypp/repo/PackageProvider.cc
@@ -71,15 +71,13 @@ namespace zypp
       /////////////////////////////////////////////////////////////////
     } // namespace source
     ///////////////////////////////////////////////////////////////////
-    PackageProvider::PackageProvider(  RepoMediaAccess &access,
-                                      const Package::constPtr & package,
+    PackageProvider::PackageProvider( const Package::constPtr & package,
                                       const DeltaCandidates & deltas,
                                       const PackageProviderPolicy & policy_r )
     : _policy( policy_r )
     , _package( package )
     , _implPtr( detail::ImplConnect::resimpl( _package ) )
     , _deltas(deltas)
-    , _access(access)
     {}
 
     PackageProvider::~PackageProvider()
@@ -188,7 +186,8 @@ namespace zypp
       ProvideFilePolicy policy;
       policy.progressCB( bind( &PackageProvider::progressPackageDownload, this, _1 ) );
       policy.failOnChecksumErrorCB( bind( &PackageProvider::failOnChecksumError, this ) );
-      return _access.provideFile( _package->repository(), loc, policy );
+
+      return repo::provideFile( _package->repository(), loc, policy );
     }
 
     ManagedFile PackageProvider::tryDelta( const DeltaRpm & delta_r ) const
@@ -207,7 +206,7 @@ namespace zypp
         {
           ProvideFilePolicy policy;
           policy.progressCB( bind( &PackageProvider::progressDeltaDownload, this, _1 ) );
-          delta = _access.provideFile( _package->repository(), delta_r.location(), policy );
+          delta = repo::provideFile( _package->repository(), delta_r.location(), policy );
         }
       catch ( const Exception & excpt )
         {
@@ -258,7 +257,7 @@ namespace zypp
         {
           ProvideFilePolicy policy;
           policy.progressCB( bind( &PackageProvider::progressPatchDownload, this, _1 ) );
-          patch = _access.provideFile( _package->repository(), patch_r.location(), policy );
+          patch = repo::provideFile( _package->repository(), patch_r.location(), policy );
         }
       catch ( const Exception & excpt )
         {
diff --git a/zypp/repo/PackageProvider.h b/zypp/repo/PackageProvider.h
index 28b5e07..14265f2 100644
--- a/zypp/repo/PackageProvider.h
+++ b/zypp/repo/PackageProvider.h
@@ -20,7 +20,7 @@
 #include "zypp/Repository.h"
 #include "zypp/Package.h"
 #include "zypp/ManagedFile.h"
-#include "zypp/repo/RepoProvideFile.h"
+
 #include "zypp/repo/DeltaCandidates.h"
 
 ///////////////////////////////////////////////////////////////////
@@ -74,8 +74,7 @@ namespace zypp
 
     public:
       /** Ctor taking the Package to provide. */
-      PackageProvider( RepoMediaAccess &access,
-                       const Package::constPtr & package,
+      PackageProvider( const Package::constPtr & package,
                        const DeltaCandidates & deltas,
                        const PackageProviderPolicy & policy_r = PackageProviderPolicy() );
       ~PackageProvider();
@@ -108,7 +107,6 @@ namespace zypp
       mutable bool               _retry;
       mutable shared_ptr<Report> _report;
       DeltaCandidates _deltas;
-      RepoMediaAccess &_access;
     };
     ///////////////////////////////////////////////////////////////////
 
diff --git a/zypp/repo/RepoProvideFile.cc b/zypp/repo/RepoProvideFile.cc
index df6163c..3e22b25 100644
--- a/zypp/repo/RepoProvideFile.cc
+++ b/zypp/repo/RepoProvideFile.cc
@@ -16,13 +16,9 @@
 
 #include "zypp/base/Gettext.h"
 #include "zypp/base/Logger.h"
-#include "zypp/base/String.h"
 #include "zypp/repo/RepoProvideFile.h"
 #include "zypp/ZYppCallbacks.h"
 #include "zypp/MediaSetAccess.h"
-#include "zypp/ZConfig.h"
-#include "zypp/repo/SUSEMediaVerifier.h"
-#include "zypp/repo/RepoException.h"
 
 using std::endl;
 using std::set;
@@ -50,10 +46,10 @@ namespace zypp
       */
       struct DownloadFileReportHack : public callback::ReceiveReport<repo::RepoReport>
       {
-        virtual bool progress( const ProgressData &progress )
+        virtual bool progress( int value )
         {
           if ( _redirect )
-            return _redirect( progress.val() );
+            return _redirect( value );
           return true;
         }
         function<bool ( int )> _redirect;
@@ -67,126 +63,6 @@ namespace zypp
                              const OnMediaLocation & loc_r,
                              const ProvideFilePolicy & policy_r )
     {
-      RepoMediaAccess access;
-      return access.provideFile(repo_r, loc_r, policy_r );
-    }
-    
-    class RepoMediaAccess::Impl
-    {
-    public:
-      Impl()
-      {}
-      
-      ~Impl()
-      {
-        std::map<Url, shared_ptr<MediaSetAccess> >::iterator it;
-        for ( it = _medias.begin();
-              it != _medias.end();
-              ++it )
-        {
-          it->second->release();
-        }
-      }
-      
-      shared_ptr<MediaSetAccess> mediaAccessForUrl( const Url &url )
-      {
-        std::map<Url, shared_ptr<MediaSetAccess> >::const_iterator it;
-        it = _medias.find(url);
-        shared_ptr<MediaSetAccess> media;
-        if ( it != _medias.end() )
-        {
-          media = it->second;
-        }
-        else
-        {
-          media.reset( new MediaSetAccess(url) );
-          _medias[url] = media;
-        }
-        return media;
-      }
-      
-      void setVerifierForRepo( Repository repo, shared_ptr<MediaSetAccess> media )
-      {
-        RepoInfo info = repo.info();
-        // set a verifier if the repository has it
-        Pathname mediafile = info.metadataPath() + "/media.1/media";
-        if ( ! mediafile.empty() )
-        {
-          if ( PathInfo(mediafile).isExist() )
-          {
-            std::map<shared_ptr<MediaSetAccess>, Repository>::const_iterator it;
-            it = _verifier.find(media);
-            if ( it != _verifier.end() )
-            {
-              if ( it->second == repo )
-              {
-                // this media is already using this repo verifier
-                return;
-              }
-            }
-
-            std::ifstream str(mediafile.asString().c_str());
-            std::string vendor;
-            std::string mediaid;
-            std::string buffer;
-            if ( str )
-            {
-              getline(str, vendor);
-              getline(str, mediaid);
-              getline(str, buffer);
-              
-              unsigned media_nr = str::strtonum<unsigned>(buffer);
-              MIL << "Repository '" << info.alias() << "' has " << media_nr << " medias"<< endl;
-              
-              for ( int i=1; i <= media_nr; ++i )
-              {
-                media::MediaVerifierRef verifier( new repo::SUSEMediaVerifier(vendor,
-                                           mediaid,
-                                           media_nr));
-                
-                media->setVerifier( i, verifier);
-              }
-              _verifier[media] = repo;
-            }
-            else
-            {
-              ZYPP_THROW(RepoMetadataException(info));
-            }
-          }
-          else
-          {
-            WAR << "No media verifier for repo '" << info.alias() << endl;
-          }
-        }
-        else
-        {
-          MIL << "Unknown metadata path for repo '" << info.alias() << "'. Can't set media verifier."<< endl;
-        }
-      }
-      
-      std::map<shared_ptr<MediaSetAccess>, Repository> _verifier;
-      std::map<Url, shared_ptr<MediaSetAccess> > _medias;
-    };
-    
-    
-    
-    RepoMediaAccess::RepoMediaAccess()
-      : _impl( new Impl() )
-    {
-      
-    }
-    
-    RepoMediaAccess::~RepoMediaAccess()
-    {
-    
-    }
-    
-    
-    
-    ManagedFile RepoMediaAccess::provideFile( Repository repo_r,
-                                              const OnMediaLocation & loc_r,
-                                              const ProvideFilePolicy & policy_r )
-    {
       MIL << "provideFile " << loc_r << endl;
       // Arrange DownloadFileReportHack to recieve the source::DownloadFileReport
       // and redirect download progress triggers to call the ProvideFilePolicy
@@ -201,7 +77,7 @@ namespace zypp
       set<Url> urls = info.baseUrls();
       if ( urls.empty() )
         ZYPP_THROW(Exception(_("No url in repository.")));
-      
+
       for ( RepoInfo::urls_const_iterator it = urls.begin();
             it != urls.end();
             ++it )
@@ -209,12 +85,10 @@ namespace zypp
         url = *it;
         try
         {
-          MIL << "Providing file of repo '" << info.alias() 
-              << "' from " << url << endl;
-          shared_ptr<MediaSetAccess> access = _impl->mediaAccessForUrl(url);
-          _impl->setVerifierForRepo(repo_r, access);
 
-          ManagedFile ret( access->provideFile(loc_r) );
+          MediaSetAccess access(url);
+
+          ManagedFile ret( access.provideFile(loc_r) );
 
           std::string scheme( url.getScheme() );
           if ( scheme == "http" || scheme == "https" || scheme == "ftp" )
@@ -262,10 +136,7 @@ namespace zypp
         }
       } // iteration over urls
 
-      ZYPP_THROW(Exception(str::form(_("Can't provide file %s from repository %s"),
-                                       loc_r.filename().c_str(),
-                                       info.alias().c_str() ) ) );
-      
+      ZYPP_THROW(Exception(_("No more urls in repository.")));
       return ManagedFile(); // not reached
     }
 
diff --git a/zypp/repo/RepoProvideFile.h b/zypp/repo/RepoProvideFile.h
index f5a0bbb..b5d838c 100644
--- a/zypp/repo/RepoProvideFile.h
+++ b/zypp/repo/RepoProvideFile.h
@@ -14,7 +14,6 @@
 
 #include <iosfwd>
 
-#include "zypp/base/PtrTypes.h"
 #include "zypp/base/Function.h"
 #include "zypp/base/Functional.h"
 #include "zypp/Repository.h"
@@ -47,39 +46,6 @@ namespace zypp
                              const OnMediaLocation & loc_r,
                              const ProvideFilePolicy & policy_r = ProvideFilePolicy() );
 
-    /**
-     * \short Provides files from different repos
-     *
-     * Class that allows to get files from repositories
-     * It handles automatically setting media verifiers if the
-     * repo is cached, and reuses media set access opened for
-     * repositories during its scope, so you can provide
-     * files from different repositories in different order
-     * without opening and closing medias all the time
-     */
-    class RepoMediaAccess
-    {
-    public:
-      RepoMediaAccess();
-      ~RepoMediaAccess();
-      
-      /** Provide a file from a Repository.
-      * Let \a source_r provide the file described by \a loc_r. In case
-      * \a loc_r contains a checksum, the file is verified. \a policy_r
-      * provides callback hooks for download progress reporting and behaviour
-      * on failed checksum verification.
-      *
-      * \throws Exception
-      */
-      ManagedFile provideFile( Repository repo_r,
-                               const OnMediaLocation & loc_r,
-                               const ProvideFilePolicy & policy_r = ProvideFilePolicy() );
-    private:
-      class Impl;
-       RW_pointer<Impl> _impl;
-      
-    };
-    
     /////////////////////////////////////////////////////////////////
   } // namespace repo
   ///////////////////////////////////////////////////////////////////
diff --git a/zypp/repo/SUSEMediaVerifier.cc b/zypp/repo/SUSEMediaVerifier.cc
index 1a91de6..3b84c3f 100644
--- a/zypp/repo/SUSEMediaVerifier.cc
+++ b/zypp/repo/SUSEMediaVerifier.cc
@@ -25,8 +25,7 @@ SUSEMediaVerifier::SUSEMediaVerifier(const std::string & vendor_r,
     , _media_nr(media_nr)
 {}
 
-SUSEMediaVerifier::SUSEMediaVerifier( int media_nr, const Pathname &path_r )
-  : _media_nr(media_nr)
+SUSEMediaVerifier::SUSEMediaVerifier( const Pathname &path_r )
 {
   std::ifstream str(path_r.asString().c_str());
   std::string vendor;
diff --git a/zypp/repo/SUSEMediaVerifier.h b/zypp/repo/SUSEMediaVerifier.h
index b415c33..f778c68 100644
--- a/zypp/repo/SUSEMediaVerifier.h
+++ b/zypp/repo/SUSEMediaVerifier.h
@@ -40,10 +40,8 @@ namespace zypp
       
       /**
        * \short creates a verifier from a media file
-       *
-       * \param path_r Path to media.1/media kind file
        */
-      SUSEMediaVerifier( int media_nr, const Pathname &path_r );
+      SUSEMediaVerifier( const Pathname &path_r );
       
       /**
         * \short Check if it is the desider media
diff --git a/zypp/target/TargetImpl.cc b/zypp/target/TargetImpl.cc
index 9f8ba72..7c50143 100644
--- a/zypp/target/TargetImpl.cc
+++ b/zypp/target/TargetImpl.cc
@@ -229,10 +229,8 @@ namespace zypp
     struct RepoProvidePackage
     {
       ResPool _pool;
-      repo::RepoMediaAccess &_access;
-
-      RepoProvidePackage( repo::RepoMediaAccess &access, ResPool pool_r )
-        : _pool(pool_r), _access(access)
+      RepoProvidePackage( ResPool pool_r )
+        : _pool(pool_r)
       {
       
       }
@@ -258,7 +256,7 @@ namespace zypp
         }
   
         repo::DeltaCandidates deltas(repos);
-        repo::PackageProvider pkgProvider( _access, p, deltas, packageProviderPolicy );
+        repo::PackageProvider pkgProvider( p, deltas, packageProviderPolicy );
         return pkgProvider.providePackage();
       }
     };
@@ -489,7 +487,7 @@ namespace zypp
                         const ResPool & pool_r )
     {
       TargetImpl::PoolItemList remaining;
-      repo::RepoMediaAccess access;
+
       MIL << "TargetImpl::commit(<list>" << policy_r << ")" << endl;
 
       bool abort = false;
@@ -497,7 +495,7 @@ namespace zypp
       // remember the last used source (if any)
       Repository lastUsedRepo;
 
-      RepoProvidePackage repoProvidePackage( access, pool_r);
+      RepoProvidePackage repoProvidePackage(pool_r);
       // prepare the package cache.
       CommitPackageCache packageCache( items_r.begin(), items_r.end(),
                                        root() / "tmp", repoProvidePackage );
< Previous Next >
Follow Ups