[zypp-commit] <libzypp> master : Big refactoring of MediaSetAccess

ref: refs/heads/master commit 3111bd808dfc89af98b77f4b84cfef9ca58704c2 Author: Duncan Mac-Vicar P Date: Fri Feb 13 15:07:34 2009 +0100 Big refactoring of MediaSetAccess - deprecate MediaSetAccess::provdeOptionalFile - remove the mess of boolean cached, checkonly and stuff. - For non-interactive (except for media change) there is a flag NON_INTERACTIVE which is accepted by provide* - Downloaders don't need doesFileExist anymore. - Exceptions are thrown. If you need ignore functionality, Fetcher has it, using OnMediaLocation::optional() - refactor MediaSetAccess provideFile, provideDir in one common function removing tons of duplicated code Pending/Next: refactor doesFileExist somehow (may be templating) Fix doesFileExist in aria (may be with a curl external cmd line implementation) using the following command line options curl -I -A agent --basic --digest --connect-timeout --cookie-jar --anyauth header -H "Host:" --location --no-keepalive ? --proxy-basic / digest /negotiate --proxytunnerl -r 0-1 --user user:password --proxy-user user:pass --proxy --- devel/devel.dmacvicar/CMakeLists.txt | 3 + devel/devel.dmacvicar/testbed.cc | 56 +++++++---- zypp/Fetcher.cc | 2 + zypp/MediaSetAccess.cc | 184 +++++++++------------------------- zypp/MediaSetAccess.h | 62 ++++++++++-- zypp/media/MediaManager.cc | 6 +- zypp/media/MediaManager.h | 13 +-- zypp/repo/susetags/Downloader.cc | 23 ++-- zypp/repo/yum/Downloader.cc | 32 +++---- 9 files changed, 171 insertions(+), 210 deletions(-) diff --git a/devel/devel.dmacvicar/CMakeLists.txt b/devel/devel.dmacvicar/CMakeLists.txt index 9cabc61..f9efa10 100644 --- a/devel/devel.dmacvicar/CMakeLists.txt +++ b/devel/devel.dmacvicar/CMakeLists.txt @@ -3,6 +3,9 @@ ADD_DEFINITIONS(-DSRC_DIR=${CMAKE_CURRENT_SOURCE_DIR}) ADD_EXECUTABLE(rpmbuilder rpmbuilder.cc) TARGET_LINK_LIBRARIES(rpmbuilder zypp ) +ADD_EXECUTABLE(testbed testbed.cc) +TARGET_LINK_LIBRARIES(testbed zypp ) + FIND_PACKAGE(Zsync) IF(ZSYNC_FOUND) ADD_EXECUTABLE(zsync zsync.cc) diff --git a/devel/devel.dmacvicar/testbed.cc b/devel/devel.dmacvicar/testbed.cc index d2d317d..f0ebf04 100644 --- a/devel/devel.dmacvicar/testbed.cc +++ b/devel/devel.dmacvicar/testbed.cc @@ -9,50 +9,64 @@ #include "zypp/Product.h" #include "zypp/Package.h" - +#include "zypp/Fetcher.h" #include "zypp/TmpPath.h" #include "zypp/ProgressData.h" -#include "zypp/parser/yum/RepoParser.h" -#include "zypp/repo/yum/Downloader.h" #include "zypp/sat/Pool.h" -#include "zypp/PoolQuery.h" +#include "zypp/ZYppCallbacks.h" using namespace std; using namespace zypp; using namespace zypp::repo; +using zypp::media::MediaChangeReport; + bool result_cb( const ResObject::Ptr &r ) { cout << r << endl; } +struct MediaChangeReportReceiver : public zypp::callback::ReceiveReport<MediaChangeReport> + { + virtual MediaChangeReport::Action + requestMedia(zypp::Url & url, + unsigned mediumNr, + const std::string & label, + MediaChangeReport::Error error, + const std::string & description, + const std::vectorstd::string & devices, + unsigned int & index) + { + cout << std::endl; + MIL << "media problem, url: " << url.asString() << std::endl; + return MediaChangeReport::IGNORE; + } + }; + + int main(int argc, char **argv) { try { ZYpp::Ptr z = getZYpp(); - //z->initializeTarget("/"); - //z->target()->load(); - - sat::Pool::instance().addRepoSolv("./foo.solv"); - -// for ( ResPool::const_iterator it = z->pool().begin(); it != z->pool().end(); ++it ) -// { -// ResObject::constPtr res = it->resolvable(); -// if ( res->name() == "kde4-kcolorchooser") -// { -// cout << res << endl; -// cout << res->summary() << " | " << res->size() << endl; -// } -// } - - PoolQuery query(); - //query.execute("kde", &result_cb); + MediaChangeReportReceiver report; + report.connect(); + Fetcher fetcher; + MediaSetAccess access(Url("http://ftp.kernel.org/pub")); + filesystem::TmpDir tmp; + + OnMediaLocation loc; + loc.setLocation("/README2"); + loc.setOptional(true); + + fetcher.enqueue(loc); + fetcher.start(tmp.path(), access); + } catch ( const Exception &e ) { diff --git a/zypp/Fetcher.cc b/zypp/Fetcher.cc index a36d2b1..ed89a5d 100644 --- a/zypp/Fetcher.cc +++ b/zypp/Fetcher.cc @@ -556,7 +556,9 @@ namespace zypp try { Pathname tmp_file = media.provideFile(resource); + Pathname dest_full_path = dest_dir + resource.filename(); + if ( assert_dir( dest_full_path.dirname() ) != 0 ) ZYPP_THROW( Exception("Can't create " + dest_full_path.dirname().asString())); if ( filesystem::copy(tmp_file, dest_full_path ) != 0 ) diff --git a/zypp/MediaSetAccess.cc b/zypp/MediaSetAccess.cc index ba8653b..b1d0516 100644 --- a/zypp/MediaSetAccess.cc +++ b/zypp/MediaSetAccess.cc @@ -117,29 +117,40 @@ IMPL_PTR_TYPE(MediaSetAccess); media_mgr.dirInfo(media, retlist, dirname, dots); } - Pathname MediaSetAccess::provideFile( const OnMediaLocation & on_media_file ) + Pathname MediaSetAccess::provideFile( const OnMediaLocation & resource, ProvideFileOptions options ) { - return provideFile( on_media_file.filename(), on_media_file.medianr() ); + return provide( ProvideFile, resource, options ); } - - Pathname MediaSetAccess::provideFile(const Pathname & file, unsigned media_nr ) + Pathname MediaSetAccess::provideFile(const Pathname & file, unsigned media_nr, ProvideFileOptions options ) { - return provideFileInternal( file, media_nr, false, false); + OnMediaLocation resource; + resource.setLocation(file, media_nr); + return provide( ProvideFile, resource, options ); } Pathname MediaSetAccess::provideOptionalFile(const Pathname & file, unsigned media_nr ) { - return provideFileInternal( file, media_nr, false, true); + OnMediaLocation resource; + resource.setLocation(file, media_nr); + Pathname ret; + try { + ret = provide(ProvideFile, resource, NON_INTERACTIVE); + } + catch ( const Exception &e ) + { + ZYPP_CAUGHT(e); + } + return ret; } + bool MediaSetAccess::doesFileExist(const Pathname & file, unsigned media_nr ) { callback::SendReportmedia::MediaChangeReport report; media::MediaManager media_mgr; bool exists = false; - do { // get the mediaId, but don't try to attach it here @@ -256,8 +267,13 @@ IMPL_PTR_TYPE(MediaSetAccess); return exists; } - Pathname MediaSetAccess::provideFileInternal(const Pathname & file, unsigned media_nr, bool cached, bool checkonly ) + Pathname MediaSetAccess::provide( ProvideAction action, + const OnMediaLocation &resource, + ProvideFileOptions options ) { + Pathname file(resource.filename()); + unsigned media_nr(resource.medianr()); + callback::SendReportmedia::MediaChangeReport report; media::MediaManager media_mgr; @@ -275,7 +291,18 @@ IMPL_PTR_TYPE(MediaSetAccess); // try to attach the media if ( ! media_mgr.isAttached(media) ) media_mgr.attachDesiredMedia(media); - media_mgr.provideFile (media, file, false, false); + + switch (action) + { + case ProvideFile: + media_mgr.provideFile (media, file); break; + case ProvideDir: + media_mgr.provideDir (media, file); break; + case ProvideDirRecursive: + media_mgr.provideDirTree(media, file); break; + default: + ZYPP_THROW(Exception("Unknown action")); + } break; } catch ( media::MediaException & excp ) @@ -321,9 +348,12 @@ IMPL_PTR_TYPE(MediaSetAccess); reason = media::MediaChangeReport::IO_SOFT; } - // checkonly: only bother the user if wrong medium is in the drive - if (checkonly && reason != media::MediaChangeReport::WRONG) - return Pathname(); + // non interactive only bother the user if wrong medium is in the drive + // otherwise propagate the error + if ( ( options & NON_INTERACTIVE ) && reason != media::MediaChangeReport::WRONG) + { + ZYPP_RETHROW(excp); + } else { // release all media before requesting another (#336881) @@ -345,15 +375,11 @@ IMPL_PTR_TYPE(MediaSetAccess); if( user == media::MediaChangeReport::ABORT ) { DBG << "Aborting" << endl; - if (checkonly) - return Pathname(); ZYPP_RETHROW ( excp ); } else if ( user == media::MediaChangeReport::IGNORE ) { DBG << "Skipping" << endl; - if (checkonly) - return Pathname(); SkipRequestException nexcp("User-requested skipping of a file"); nexcp.remember(excp); ZYPP_THROW(nexcp); @@ -382,8 +408,6 @@ IMPL_PTR_TYPE(MediaSetAccess); else { DBG << "Don't know, let's ABORT" << endl; - if (checkonly) - return Pathname(); ZYPP_RETHROW ( excp ); } } while( user == media::MediaChangeReport::EJECT ); @@ -398,127 +422,13 @@ IMPL_PTR_TYPE(MediaSetAccess); Pathname MediaSetAccess::provideDir(const Pathname & dir, bool recursive, - unsigned media_nr) + unsigned media_nr, + ProvideFileOptions options ) { - callback::SendReportmedia::MediaChangeReport report; - media::MediaManager media_mgr; - media::MediaAccessId _media; - - do - { - // get the mediaId, but don't try to attach it here - _media = getMediaAccessId(media_nr); - - try - { - DBG << "Going to try provide direcotry " << dir - << (recursive ? " (recursively)" : "") - << " from media nr. " << media_nr << endl; - - // try to attach the media - if (!media_mgr.isAttached(_media)) - media_mgr.attachDesiredMedia(_media); - - _media = getMediaAccessId(media_nr); // in case of redirect - - if (recursive) - media_mgr.provideDirTree(_media, dir); - else - media_mgr.provideDir(_media, dir); - - break; // quit the retry loop - } - catch (media::MediaException & excp) - { - ZYPP_CAUGHT(excp); - media::MediaChangeReport::Action user; - - do - { - DBG << "Media couldn't provide dir " << dir << ", releasing." << endl; - try - { - media_mgr.release (_media); - } - catch (const Exception & excpt_r) - { - ZYPP_CAUGHT(excpt_r); - ERR << "Failed to release media " << _media << endl; - } - - // set up the reason - media::MediaChangeReport::Error reason = media::MediaChangeReport::INVALID; - - if (typeid(excp) == typeid(media::MediaFileNotFoundException) - || typeid(excp) == typeid(media::MediaNotAFileException)) - { - reason = media::MediaChangeReport::NOT_FOUND; - } - else if (typeid(excp) == typeid( media::MediaNotDesiredException) - || typeid(excp) == typeid(media::MediaNotAttachedException)) - { - reason = media::MediaChangeReport::WRONG; - } - - vector<string> devices; - unsigned int devindex; - media_mgr.getDetectedDevices(_media, devices, devindex); - - // release all media before requesting another (#336881) - media_mgr.releaseAll(); - - user = report->requestMedia(_url, - media_nr, - _label, - reason, - excp.asUserString(), - devices, - devindex); - - MIL << "ProvideDir exception caught, callback answer: " << user << endl; - - if (user == media::MediaChangeReport::ABORT) - { - DBG << "Aborting" << endl; - ZYPP_RETHROW ( excp ); - } - //! \todo FIXME media::MediaChangeReport::IGNORE handling - else if (user == media::MediaChangeReport::EJECT) - { - DBG << "Eject: try to release" << endl; - media_mgr.releaseAll(); - // eject - media_mgr.release (_media, - devindex < devices.size() ? devices[devindex] : ""); - } - else if (user == media::MediaChangeReport::RETRY || - user == media::MediaChangeReport::CHANGE_URL) - { - // retry - DBG << "Going to try again" << endl; - // invalidate current media access id - media_mgr.close(_media); - _medias.erase(media_nr); - - // not attaching, media set will do that for us - // this could generate uncaught exception (#158620) - - break; - } - else - { - DBG << "Don't know, let's ABORT" << endl; - - ZYPP_RETHROW (excp); - } - } - while (user == media::MediaChangeReport::EJECT); - } - // retry or change URL - } - while (true); + OnMediaLocation resource; + resource.setLocation(dir, media_nr); - return media_mgr.localPath(_media, dir); + return provide( recursive? ProvideDir : ProvideDirRecursive, resource, options ); } media::MediaAccessId MediaSetAccess::getMediaAccessId (media::MediaNr medianr) diff --git a/zypp/MediaSetAccess.h b/zypp/MediaSetAccess.h index 3f0accd..01cfaf2 100644 --- a/zypp/MediaSetAccess.h +++ b/zypp/MediaSetAccess.h @@ -17,6 +17,7 @@ #include "zypp/base/ReferenceCounted.h" #include "zypp/base/NonCopyable.h" +#include "zypp/base/Flags.h" #include "zypp/base/PtrTypes.h" #include "zypp/media/MediaManager.h" #include "zypp/Pathname.h" @@ -107,10 +108,20 @@ namespace zypp void setLabel( const std::string & label_r ) { _label = label_r; } + enum ProvideFileOption + { + /** + * The user is not asked anything, and the error + * exception is just propagated */ + NONE = 0x0, + NON_INTERACTIVE = 0x1 + }; + ZYPP_DECLARE_FLAGS(ProvideFileOptions,ProvideFileOption); + /** * Provides a file from a media location. * - * \param on_media_file location of the file on media + * \param resource location of the file on media * \return local pathname of the requested file * * \throws MediaException if a problem occured and user has chosen to @@ -120,9 +131,18 @@ namespace zypp * to skip the current operation. The calling code should continue * with the next one, if possible. * + * + * If the resource is marked as optional, no Exception is thrown + * and Pathname() is returned + * + * \note interaction with the user does not ocurr if + * \ref ProvideFileOptions::NON_INTERACTIVE is set. + * + * \note OnMediaLocation::optional() hint has no effect on the transfer. + * * \see zypp::media::MediaManager::provideFile() */ - Pathname provideFile( const OnMediaLocation & on_media_file ); + Pathname provideFile( const OnMediaLocation & resource, ProvideFileOptions options = NONE ); /** * Provides \a file from media \a media_nr. @@ -131,6 +151,11 @@ namespace zypp * \param media_nr the media number in the media set * \return local pathname of the requested file * + * \note interaction with the user does not ocurr if + * \ref ProvideFileOptions::NON_INTERACTIVE is set. + * + * \note OnMediaLocation::optional() hint has no effect on the transfer. + * * \throws MediaException if a problem occured and user has chosen to * abort the operation. The calling code should take care * to quit the current operation. @@ -139,24 +164,32 @@ namespace zypp * with the next one, if possible. * \see zypp::media::MediaManager::provideFile() */ - Pathname provideFile(const Pathname & file, unsigned media_nr = 1 ); + Pathname provideFile(const Pathname & file, unsigned media_nr = 1, ProvideFileOptions options = NONE ); /** + * \deprecated * The same as provideFile(Pathname,unsigned) but this method does not * call the user callbacks, except of the case of * wrong media in the drive, and it won't throw an exception in any case. * * \return Path to the provided file on success, an empty Pathname() otherwise. + * This method is obsolete. + * + * To avoid interaction with the user, + * use \ref provideFile with \ref ProvideFileOptions::NON_INTERACTIVE + * as an option. However you need to handle the exceptions yourself + * in case of error. If you need ignore functionality, try + * \ref Fetcher and set \ref OnMediaLocation::setOptional */ - Pathname provideOptionalFile( const Pathname & file, unsigned media_nr = 1 ); + ZYPP_DEPRECATED Pathname provideOptionalFile( const Pathname & file, unsigned media_nr = 1 ); /** * Release file from media. * This signal that file is not needed anymore. * - * \param on_media_file location of the file on media + * \param resource location of the file on media */ - void releaseFile( const OnMediaLocation & on_media_file ); + void releaseFile( const OnMediaLocation &resource ); /** @@ -185,7 +218,7 @@ namespace zypp * \see zypp::media::MediaManager::provideDir() * \see zypp::media::MediaManager::provideDirTree() */ - Pathname provideDir(const Pathname & dir, bool recursive, unsigned media_nr = 1); + Pathname provideDir(const Pathname & dir, bool recursive, unsigned media_nr = 1, ProvideFileOptions options = NONE ); /** * Checks if a file exists on the specified media, with user callbacks. @@ -248,7 +281,19 @@ namespace zypp * to skip the current operation. The calling code should continue * with the next one, if possible. */ - Pathname provideFileInternal(const Pathname & file, unsigned media_nr, bool checkonly, bool cached); + Pathname provideFileInternal( const OnMediaLocation &resource, ProvideFileOptions options ); + + //typedef function ProvideAction; + enum ProvideAction + { + ProvideFile, + ProvideDir, + ProvideDirRecursive, + ProvideFileExist, + }; + + Pathname provide( ProvideAction action, const OnMediaLocation &resource, ProvideFileOptions options ); + media::MediaAccessId getMediaAccessId (media::MediaNr medianr); virtual std::ostream & dumpOn( std::ostream & str ) const; @@ -275,6 +320,7 @@ namespace zypp VerifierMap _verifiers; }; /////////////////////////////////////////////////////////////////// + ZYPP_DECLARE_OPERATORS_FOR_FLAGS(MediaSetAccess::ProvideFileOptions); /** \relates MediaSetAccess Stream output */ inline std::ostream & operator<<( std::ostream & str, const MediaSetAccess & obj ) diff --git a/zypp/media/MediaManager.cc b/zypp/media/MediaManager.cc index 30f90f9..d4956d4 100644 --- a/zypp/media/MediaManager.cc +++ b/zypp/media/MediaManager.cc @@ -852,9 +852,7 @@ namespace zypp // --------------------------------------------------------------- void MediaManager::provideFile(MediaAccessId accessId, - const Pathname &filename, - bool cached, - bool checkonly) const + const Pathname &filename ) const { MutexLock glock(g_Mutex); @@ -862,7 +860,7 @@ namespace zypp ref.checkDesired(accessId); - ref.handler->provideFile(filename, cached, checkonly); + ref.handler->provideFile(filename); } // --------------------------------------------------------------- diff --git a/zypp/media/MediaManager.h b/zypp/media/MediaManager.h index 60c473e..84fc725 100644 --- a/zypp/media/MediaManager.h +++ b/zypp/media/MediaManager.h @@ -720,14 +720,6 @@ namespace zypp * * \param accessId The media access id to use. * \param filename The filename to provide, relative to localRoot(). - * \param cached If cached is set to true, the function checks, if - * the file already exists and doesn't download it again - * if it does. Currently only the existence is checked, - * no other file attributes. - * \param checkonly If this and 'cached' are set to true only the - * existence of the file is checked but it's not - * downloaded. If 'cached' is unset an errer is - * returned always. * * \throws MediaNotOpenException in case of invalid access id. * \throws MediaNotAttachedException in case, that the media is not attached. @@ -738,11 +730,10 @@ namespace zypp * \throws MediaSystemException in case a system operation fails. * \throws MediaException derived exception, depending on the url (handler). */ + void provideFile(MediaAccessId accessId, - const Pathname &filename, - bool cached = false, - bool checkonly = false) const; + const Pathname &filename ) const; /** * FIXME: see MediaAccess class. diff --git a/zypp/repo/susetags/Downloader.cc b/zypp/repo/susetags/Downloader.cc index 55868b0..1b19733 100644 --- a/zypp/repo/susetags/Downloader.cc +++ b/zypp/repo/susetags/Downloader.cc @@ -51,25 +51,26 @@ void Downloader::download( MediaSetAccess &media, SignatureFileChecker sigchecker/*(repoInfo().name())*/; Pathname sig = repoInfo().path() + "/content.asc"; - if ( media.doesFileExist(sig) ) - { - this->enqueue( OnMediaLocation( sig, 1 ) ); - this->start( dest_dir, media ); - this->reset(); - sigchecker = SignatureFileChecker( dest_dir + sig/*, repoInfo().name() */); - } + this->enqueue( OnMediaLocation( sig, 1 ) ); + this->start( dest_dir, media ); + // only if there is a signature in the destination directory + if ( PathInfo(dest_dir / sig ).isExist() ) + sigchecker = SignatureFileChecker( dest_dir + sig/*, repoInfo().name() */); + this->reset(); Pathname key = repoInfo().path() + "/content.key"; - if ( media.doesFileExist(key) ) + + this->enqueue( OnMediaLocation( key, 1 ) ); + this->start( dest_dir, media ); + // only if there is a key in the destination directory + if ( PathInfo(dest_dir / key).isExist() ) { KeyContext context; context.setRepoInfo(repoInfo()); - this->enqueue( OnMediaLocation( key, 1 ) ); - this->start( dest_dir, media ); - this->reset(); sigchecker.addPublicKey(dest_dir + key, context); } + this->reset(); if ( ! repoInfo().gpgCheck() ) { diff --git a/zypp/repo/yum/Downloader.cc b/zypp/repo/yum/Downloader.cc index acfbfc1..587f6e0 100644 --- a/zypp/repo/yum/Downloader.cc +++ b/zypp/repo/yum/Downloader.cc @@ -125,27 +125,23 @@ void Downloader::download( MediaSetAccess &media, #warning Do we need SignatureFileChecker(string descr)? SignatureFileChecker sigchecker/*(repoInfo().name())*/; - if ( _media_ptr->doesFileExist(sigpath) ) - { - this->enqueue( OnMediaLocation(sigpath,1).setOptional(true) ); - this->start( dest_dir, *_media_ptr); - this->reset(); - sigchecker = SignatureFileChecker(dest_dir + sigpath); - } - + this->enqueue( OnMediaLocation(sigpath,1).setOptional(true) ); + this->start( dest_dir, *_media_ptr); + // only add the signature if it exists + if ( PathInfo(dest_dir / sigpath).isExist() ) + sigchecker = SignatureFileChecker(dest_dir / sigpath); + this->reset(); - if ( _media_ptr->doesFileExist(keypath) ) + this->enqueue( OnMediaLocation(keypath,1).setOptional(true) ); + this->start( dest_dir, *_media_ptr); + // only add the key if it exists + if ( PathInfo(dest_dir / keypath).isExist() ) { - KeyContext context; - context.setRepoInfo(repoInfo()); - this->enqueue( OnMediaLocation(keypath,1).setOptional(true) ); - this->start( dest_dir, *_media_ptr); - this->reset(); - sigchecker.addPublicKey(dest_dir + keypath, context); + KeyContext context; + context.setRepoInfo(repoInfo()); + sigchecker.addPublicKey(dest_dir + keypath, context); } - - - this->start( dest_dir, *_media_ptr ); + this->reset(); if ( ! progress.tick() ) ZYPP_THROW(AbortRequestException()); -- To unsubscribe, e-mail: zypp-commit+unsubscribe@opensuse.org For additional commands, e-mail: zypp-commit+help@opensuse.org