From 23e5b799c41c38ec1e75bcdb4f7284c02983bac5 Mon Sep 17 00:00:00 2001 From: Benjamin Zeller Date: Tue, 14 Jun 2022 12:43:01 +0200 Subject: [PATCH] Add --download-max-retries and --download-retry-wait-time switches (bsc#1200370) --- src/Summary.cc | 2 +- src/Summary.h | 2 +- src/Table.cc | 2 +- src/Zypper.cc | 4 +- src/callbacks/media.h | 14 +- src/callbacks/repo.h | 11 +- src/commands/commonflags.h | 15 ++ src/commands/optionsets.cc | 7 +- src/commands/repos/refresh.cc | 5 + src/output/Out.h | 6 +- src/output/prompt.h | 1 + src/repos.cc | 336 ++++++++++++++++++---------------- src/solve-commit.cc | 2 +- src/utils/text.h | 2 +- tests/lib/TestSetup.h | 2 +- 15 files changed, 242 insertions(+), 169 deletions(-) diff --git a/src/Summary.cc b/src/Summary.cc index cb7b7b04e9..0ed47504e8 100644 --- a/src/Summary.cc +++ b/src/Summary.cc @@ -12,7 +12,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/src/Summary.h b/src/Summary.h index 213eb98e58..ceba4e6eec 100644 --- a/src/Summary.h +++ b/src/Summary.h @@ -14,7 +14,7 @@ #include #include -#include +#include #include #include diff --git a/src/Table.cc b/src/Table.cc index 5d6bbf12c7..4d7cabd796 100644 --- a/src/Table.cc +++ b/src/Table.cc @@ -4,7 +4,7 @@ #include #include -#include +#include #include "utils/colors.h" #include "utils/console.h" diff --git a/src/Zypper.cc b/src/Zypper.cc index 181cd127ce..68b4d39a61 100644 --- a/src/Zypper.cc +++ b/src/Zypper.cc @@ -24,8 +24,8 @@ #include #include -#include -#include +#include +#include #include #include diff --git a/src/callbacks/media.h b/src/callbacks/media.h index 183ba8825e..8cec9be4a8 100644 --- a/src/callbacks/media.h +++ b/src/callbacks/media.h @@ -15,6 +15,7 @@ #include #include #include +#include #include "Zypper.h" @@ -100,6 +101,7 @@ namespace ZmartRecipients { _last_reported = time(NULL); _last_drate_avg = -1; + _silentRetries = 0; Out & out = Zypper::instance().out(); @@ -154,12 +156,19 @@ namespace ZmartRecipients problem( const Url & uri, DownloadProgressReport::Error error, const std::string & description ) { DBG << "media problem" << std::endl; + if (_be_quiet) Zypper::instance().out().dwnldProgressEnd(uri, _last_drate_avg, true); Zypper::instance().out().error(zcb_error2str(error, description)); - Action action = (Action) read_action_ari( - PROMPT_ARI_MEDIA_PROBLEM, DownloadProgressReport::ABORT); + Action action = DownloadProgressReport::ABORT; + if ( _silentRetries < ZConfig::instance().download_max_silent_tries() ) { + action = (Action) read_action_ari_with_timeout( PROMPT_ARI_MEDIA_PROBLEM, 0, DownloadProgressReport::RETRY ); + if ( action == DownloadProgressReport::RETRY ) + _silentRetries++; + } else { + action = (Action) read_action_ari( PROMPT_ARI_MEDIA_PROBLEM, DownloadProgressReport::ABORT ); + } if (action == DownloadProgressReport::RETRY) Zypper::instance().requestExit(false); return action; @@ -179,6 +188,7 @@ namespace ZmartRecipients bool _be_quiet; time_t _last_reported; double _last_drate_avg; + int _silentRetries; }; diff --git a/src/callbacks/repo.h b/src/callbacks/repo.h index b8d7d50ccc..68dadff2c5 100644 --- a/src/callbacks/repo.h +++ b/src/callbacks/repo.h @@ -35,6 +35,7 @@ struct DownloadResolvableReportReceiver : public callback::ReceiveReport /** * \file Contains all flags that are commonly used in multiple commands but which are too simple for a OptionSet @@ -45,6 +46,20 @@ namespace CommonFlags _("Consider only patches which affect the package management itself.") }; } + + inline zypp::ZyppFlags::CommandOption downloadMaxRetriesFlag () { + return { + "download-max-retries", '\0', ZyppFlags::RequiredArgument, ZyppFlags::CallbackVal( []( const auto& opt, const boost::optional &in ){ + ZConfig::instance().set_download_max_silent_tries(ZyppFlags::argValueConvert(opt, in)); + }, ARG_INTEGER ), _("Number of times zypp should attempt a download in case it fails.") }; + } + + inline zypp::ZyppFlags::CommandOption downloadRetryWaitTimeFlag () { + return { + "download-retry-wait-time", '\0', ZyppFlags::RequiredArgument, ZyppFlags::CallbackVal( []( const auto& opt, const boost::optional &in ){ + ZConfig::instance().set_download_retry_wait_time(ZyppFlags::argValueConvert(opt, in)); + }, ARG_INTEGER ), _("Time in seconds zypp should wait before retrying a failed download.") }; + } } diff --git a/src/commands/optionsets.cc b/src/commands/optionsets.cc index 1a96d22ba4..29f5483eb8 100644 --- a/src/commands/optionsets.cc +++ b/src/commands/optionsets.cc @@ -5,6 +5,7 @@ |__/|_| |_| \*---------------------------------------------------------------------------*/ #include "optionsets.h" +#include "commonflags.h" #include "utils/flags/flagtypes.h" #include "global-settings.h" #include "Zypper.h" @@ -183,7 +184,9 @@ std::vector DownloadOptionSet::options() }, { "download-in-advance", '\0', ZyppFlags::NoArgument | ZyppFlags::Repeatable | ZyppFlags::Hidden, DownloadModeNoArgType( *this, DownloadMode::DownloadInAdvance ), "" }, { "download-in-heaps", '\0', ZyppFlags::NoArgument | ZyppFlags::Repeatable | ZyppFlags::Hidden, DownloadModeNoArgType( *this, DownloadMode::DownloadInHeaps ), "" }, - { "download-as-needed", '\0', ZyppFlags::NoArgument | ZyppFlags::Repeatable | ZyppFlags::Hidden, DownloadModeNoArgType( *this, DownloadMode::DownloadAsNeeded ), "" } + { "download-as-needed", '\0', ZyppFlags::NoArgument | ZyppFlags::Repeatable | ZyppFlags::Hidden, DownloadModeNoArgType( *this, DownloadMode::DownloadAsNeeded ), "" }, + CommonFlags::downloadMaxRetriesFlag(), + CommonFlags::downloadRetryWaitTimeFlag() }}}; } @@ -191,6 +194,8 @@ void DownloadOptionSet::reset() { _mode = ZConfig::instance().commit_downloadMode(); _wasSetBefore = false; + ZConfig::instance().reset_download_max_silent_tries(); + ZConfig::instance().reset_download_retry_wait_time(); } diff --git a/src/commands/repos/refresh.cc b/src/commands/repos/refresh.cc index 4f6f7237ec..dcf6a05ac7 100644 --- a/src/commands/repos/refresh.cc +++ b/src/commands/repos/refresh.cc @@ -8,6 +8,7 @@ #include "repos.h" #include "commands/conditions.h" #include "commands/services/refresh.h" +#include "commands/commonflags.h" #include "utils/messages.h" @@ -68,6 +69,8 @@ zypp::ZyppFlags::CommandGroup RefreshRepoCmd::cmdOptions() const // translators: -D, --download-only _("Only download raw metadata, don't build the database.") }, + CommonFlags::downloadMaxRetriesFlag(), + CommonFlags::downloadRetryWaitTimeFlag(), {"repo", 'r', ZyppFlags::RequiredArgument | ZyppFlags::Repeatable, ZyppFlags::StringVectorType( &that->_repos, ARG_REPOSITORY), // translators: -r, --repo @@ -87,6 +90,8 @@ void RefreshRepoCmd::doReset() _flags = Default; _repos.clear(); _services = false; + ZConfig::instance().reset_download_max_silent_tries(); + ZConfig::instance().reset_download_retry_wait_time(); } int RefreshRepoCmd::execute( Zypper &zypper , const std::vector &positionalArgs_r ) diff --git a/src/output/Out.h b/src/output/Out.h index 89b25d9060..64cdace66a 100644 --- a/src/output/Out.h +++ b/src/output/Out.h @@ -10,11 +10,11 @@ #include #include #include -#include -#include +#include +#include #include #include -#include +#include #include #include diff --git a/src/output/prompt.h b/src/output/prompt.h index 7104f08431..65cea7f9c8 100644 --- a/src/output/prompt.h +++ b/src/output/prompt.h @@ -72,6 +72,7 @@ typedef enum PR_ENUM(PROMPT_PACKAGEKIT_QUIT, 23) PR_ENUM(PROMPT_YN_CONTINUE_ON_FILECONFLICT, 24) PR_ENUM(PROMPT_YN_RUN_SEARCH_PACKAGES, 25) + PR_ENUM(PROMPT_ARI_RETRY_OPERATION, 26) #ifndef PROMPT_H_ #define PROMPT_H_ diff --git a/src/repos.cc b/src/repos.cc index b726e8e3bb..7108a63e8d 100644 --- a/src/repos.cc +++ b/src/repos.cc @@ -18,7 +18,7 @@ #include #include -#include +#include #include #include @@ -216,193 +216,222 @@ bool refresh_raw_metadata( Zypper & zypper, const RepoInfo & repo, bool force_do #define DISABLE_ScopedDisableMediaChangeReport_GUARD callback::TempConnect tempDisconnect; - try + int autoRetryCount = 0; + const auto maxRetries = ZConfig::instance().download_max_silent_tries(); + bool canRetry = true; + const auto &qualifiesForRetry = [&](){ + if ( autoRetryCount < maxRetries ) { + auto action = read_action_ari_with_timeout(PROMPT_ARI_RETRY_OPERATION, ZConfig::instance().download_retry_wait_time(), 1); + if ( action == 1 ) { + canRetry = true; + autoRetryCount++; + return true; + } + } + return false; + }; + + while ( canRetry ) { - if ( !force_download ) + canRetry = false; // we will decide in a per error case if retry is possible + + try { - // check whether libzypp indicates a refresh is needed, and if so, - // print a message - zypper.out().info( str::Format(_("Checking whether to refresh metadata for %s")) % repo.asUserString(), - Out::HIGH ); - if ( !repo.baseUrlsEmpty() ) + if ( !force_download ) { -#ifndef DISABLE_ScopedDisableMediaChangeReport_GUARD - Disabled because of fix for bsc#1123967 - // Suppress (interactive) media::MediaChangeReport if we in have multiple basurls (>1) - media::ScopedDisableMediaChangeReport guard( repo.baseUrlsSize() > 1 ); -#endif - - for ( RepoInfo::urls_const_iterator it = repo.baseUrlsBegin(); it != repo.baseUrlsEnd(); ) + // check whether libzypp indicates a refresh is needed, and if so, + // print a message + zypper.out().info( str::Format(_("Checking whether to refresh metadata for %s")) % repo.asUserString(), + Out::HIGH ); + if ( !repo.baseUrlsEmpty() ) { - try + #ifndef DISABLE_ScopedDisableMediaChangeReport_GUARD + Disabled because of fix for bsc#1123967 + // Suppress (interactive) media::MediaChangeReport if we in have multiple basurls (>1) + media::ScopedDisableMediaChangeReport guard( repo.baseUrlsSize() > 1 ); + #endif + + for ( RepoInfo::urls_const_iterator it = repo.baseUrlsBegin(); it != repo.baseUrlsEnd(); ) { - RepoManager::RefreshCheckStatus stat = manager.checkIfToRefreshMetadata( repo, *it, - zypper.command() == ZypperCommand::REFRESH || - zypper.command() == ZypperCommand::REFRESH_SERVICES ? - RepoManager::RefreshIfNeededIgnoreDelay : - RepoManager::RefreshIfNeeded ); - - do_refresh = ( stat == RepoManager::REFRESH_NEEDED ); - if ( !do_refresh - && ( zypper.command() == ZypperCommand::REFRESH || zypper.command() == ZypperCommand::REFRESH_SERVICES ) ) + try { - switch ( stat ) + RepoManager::RefreshCheckStatus stat = manager.checkIfToRefreshMetadata( repo, *it, + zypper.command() == ZypperCommand::REFRESH || + zypper.command() == ZypperCommand::REFRESH_SERVICES ? + RepoManager::RefreshIfNeededIgnoreDelay : + RepoManager::RefreshIfNeeded ); + + do_refresh = ( stat == RepoManager::REFRESH_NEEDED ); + if ( !do_refresh + && ( zypper.command() == ZypperCommand::REFRESH || zypper.command() == ZypperCommand::REFRESH_SERVICES ) ) { - case RepoManager::REPO_UP_TO_DATE: - { - TermLine outstr( TermLine::SF_SPLIT | TermLine::SF_EXPAND ); - outstr.lhs << str::Format(_("Repository '%s' is up to date.")) % repo.asUserString(); - //outstr.rhs << repoGpgCheckStatus( repo ); - zypper.out().infoLine( outstr ); - } - break; - case RepoManager::REPO_CHECK_DELAYED: - zypper.out().info( str::Format(_("The up-to-date check of '%s' has been delayed.")) % repo.asUserString(), - Out::HIGH ); - break; - default: - WAR << "new item in enum, which is not covered" << endl; + switch ( stat ) + { + case RepoManager::REPO_UP_TO_DATE: + { + TermLine outstr( TermLine::SF_SPLIT | TermLine::SF_EXPAND ); + outstr.lhs << str::Format(_("Repository '%s' is up to date.")) % repo.asUserString(); + //outstr.rhs << repoGpgCheckStatus( repo ); + zypper.out().infoLine( outstr ); + } + break; + case RepoManager::REPO_CHECK_DELAYED: + zypper.out().info( str::Format(_("The up-to-date check of '%s' has been delayed.")) % repo.asUserString(), + Out::HIGH ); + break; + default: + WAR << "new item in enum, which is not covered" << endl; + } } + break; // don't check all the urls, just the first successful. + } + catch ( const Exception & e ) + { + ZYPP_CAUGHT( e ); + Url badurl( *it ); + if ( ++it == repo.baseUrlsEnd() ) + ZYPP_RETHROW( e ); + ERR << badurl << " doesn't look good. Trying another url (" << *it << ")." << endl; } - break; // don't check all the urls, just the first successful. - } - catch ( const Exception & e ) - { - ZYPP_CAUGHT( e ); - Url badurl( *it ); - if ( ++it == repo.baseUrlsEnd() ) - ZYPP_RETHROW( e ); - ERR << badurl << " doesn't look good. Trying another url (" << *it << ")." << endl; } } } - } - else - { - zypper.out().info(_("Forcing raw metadata refresh")); - do_refresh = true; - } + else + { + zypper.out().info(_("Forcing raw metadata refresh")); + do_refresh = true; + } - if ( do_refresh ) - { - plabel = str::form(_("Retrieving repository '%s' metadata"), repo.asUserString().c_str() ); - zypper.out().progressStart( "raw-refresh", plabel, true ); + if ( do_refresh ) + { + plabel = str::form(_("Retrieving repository '%s' metadata"), repo.asUserString().c_str() ); + zypper.out().progressStart( "raw-refresh", plabel, true ); - // RepoManager::RefreshForced because we already know from checkIfToRefreshMetadata above - // that refresh is needed (or forced anyway). Forcing here prevents refreshMetadata from - // doing it's own checkIfToRefreshMetadata. Otherwise we'd download the stats twice. - manager.refreshMetadata( repo, RepoManager::RefreshForced ); + // RepoManager::RefreshForced because we already know from checkIfToRefreshMetadata above + // that refresh is needed (or forced anyway). Forcing here prevents refreshMetadata from + // doing it's own checkIfToRefreshMetadata. Otherwise we'd download the stats twice. + manager.refreshMetadata( repo, RepoManager::RefreshForced ); - //plabel += repoGpgCheckStatus( repo ); - zypper.out().progressEnd( "raw-refresh", plabel ); - plabel.clear(); + //plabel += repoGpgCheckStatus( repo ); + zypper.out().progressEnd( "raw-refresh", plabel ); + plabel.clear(); + } } - } - catch ( const AbortRequestException & e ) - { - ZYPP_CAUGHT( e ); - // rethrow ABORT exception, stop executing the command - zypper.setExitCode( ZYPPER_EXIT_ERR_ZYPP ); - ZYPP_RETHROW( e ); - } - catch ( const SkipRequestException & e ) - { - ZYPP_CAUGHT( e ); - - std::string question = str::Format(_("Do you want to disable the repository %s permanently?")) % repo.name(); - - if ( read_bool_answer( PROMPT_YN_MEDIA_CHANGE, question, false ) ) + catch ( const AbortRequestException & e ) + { + ZYPP_CAUGHT( e ); + // rethrow ABORT exception, stop executing the command + zypper.setExitCode( ZYPPER_EXIT_ERR_ZYPP ); + ZYPP_RETHROW( e ); + } + catch ( const SkipRequestException & e ) { - MIL << "Disabling repository " << repo.name().c_str() << " permanently." << endl; + ZYPP_CAUGHT( e ); - try - { - RepoInfo origRepo( manager.getRepositoryInfo(repo.alias()) ); + std::string question = str::Format(_("Do you want to disable the repository %s permanently?")) % repo.name(); - origRepo.setEnabled( false ); - manager.modifyRepository (repo.alias(), origRepo ); - } - catch ( const Exception & ex ) + if ( read_bool_answer( PROMPT_YN_MEDIA_CHANGE, question, false ) ) { - ZYPP_CAUGHT( ex ); - zypper.out().error( ex, str::Format(_("Error while disabling repository '%s'.")) % repo.alias() ); + MIL << "Disabling repository " << repo.name().c_str() << " permanently." << endl; + + try + { + RepoInfo origRepo( manager.getRepositoryInfo(repo.alias()) ); - ERR << "Error while disabling the repository." << endl; + origRepo.setEnabled( false ); + manager.modifyRepository (repo.alias(), origRepo ); + } + catch ( const Exception & ex ) + { + ZYPP_CAUGHT( ex ); + zypper.out().error( ex, str::Format(_("Error while disabling repository '%s'.")) % repo.alias() ); + + ERR << "Error while disabling the repository." << endl; + } } + // will disable repo in gData.repos + return true; } - // will disable repo in gData.repos - return true; - } - catch ( const media::MediaException & e ) - { - ZYPP_CAUGHT( e ); - if ( do_refresh ) + catch ( const media::MediaException & e ) { - zypper.out().progressEnd( "raw-refresh", plabel, true ); - plabel.clear(); - } - zypper.out().error( e, str::Format(_("Problem retrieving files from '%s'.")) % repo.asUserString(), - _("Please see the above error message for a hint.") ); + ZYPP_CAUGHT( e ); + if ( do_refresh ) + { + zypper.out().progressEnd( "raw-refresh", plabel, true ); + plabel.clear(); + } + zypper.out().error( e, str::Format(_("Problem retrieving files from '%s'.")) % repo.asUserString(), + _("Please see the above error message for a hint.") ); - return true; // error - } - catch ( const repo::RepoNoUrlException & e ) - { - ZYPP_CAUGHT( e ); - if ( do_refresh ) - { - zypper.out().progressEnd( "raw-refresh", plabel, true ); - plabel.clear(); + if ( qualifiesForRetry() ) + continue; + + return true; // error } - zypper.out().error( str::Format(_("No URIs defined for '%s'.")) % repo.asUserString() ); - if ( !repo.filepath().empty() ) + catch ( const repo::RepoNoUrlException & e ) + { + ZYPP_CAUGHT( e ); + if ( do_refresh ) + { + zypper.out().progressEnd( "raw-refresh", plabel, true ); + plabel.clear(); + } + zypper.out().error( str::Format(_("No URIs defined for '%s'.")) % repo.asUserString() ); + if ( !repo.filepath().empty() ) - zypper.out().info( - // TranslatorExplanation the first %s is a .repo file path - str::Format(_("Please add one or more base URI (baseurl=URI) entries to %s for repository '%s'.")) - % repo.filepath() % repo.asUserString() - ); + zypper.out().info( + // TranslatorExplanation the first %s is a .repo file path + str::Format(_("Please add one or more base URI (baseurl=URI) entries to %s for repository '%s'.")) + % repo.filepath() % repo.asUserString() + ); - return true; // error - } - catch ( const repo::RepoNoAliasException & e ) - { - ZYPP_CAUGHT( e ); - if ( do_refresh ) - { - zypper.out().progressEnd( "raw-refresh", plabel, true ); - plabel.clear(); + return true; // error } - zypper.out().error(_("No alias defined for this repository.") ); - report_a_bug( zypper.out() ); - return true; // error - } - catch ( const repo::RepoException & e ) - { - ZYPP_CAUGHT( e ); - if ( do_refresh ) + catch ( const repo::RepoNoAliasException & e ) { - zypper.out().progressEnd( "raw-refresh", plabel, true ); - plabel.clear(); + ZYPP_CAUGHT( e ); + if ( do_refresh ) + { + zypper.out().progressEnd( "raw-refresh", plabel, true ); + plabel.clear(); + } + zypper.out().error(_("No alias defined for this repository.") ); + report_a_bug( zypper.out() ); + return true; // error } - zypper.out().error( e, str::Format(_("Repository '%s' is invalid.")) % repo.asUserString(), - _("Please check if the URIs defined for this repository are pointing to a valid repository.") ); - return true; // error - } - catch ( const Exception & e ) - { - ZYPP_CAUGHT( e ); - if ( do_refresh ) + catch ( const repo::RepoException & e ) { - zypper.out().progressEnd( "raw-refresh", plabel, true ); - plabel.clear(); + ZYPP_CAUGHT( e ); + if ( do_refresh ) + { + zypper.out().progressEnd( "raw-refresh", plabel, true ); + plabel.clear(); + } + zypper.out().error( e, str::Format(_("Repository '%s' is invalid.")) % repo.asUserString(), + _("Please check if the URIs defined for this repository are pointing to a valid repository.") ); + + if ( qualifiesForRetry() ) + continue; + + return true; // error } - ERR << "Error reading repository '" << repo.asUserString() << "'" << endl; - zypper.out().error( e, str::Format(_("Error retrieving metadata for '%s':")) % repo.asUserString() ); + catch ( const Exception & e ) + { + ZYPP_CAUGHT( e ); + if ( do_refresh ) + { + zypper.out().progressEnd( "raw-refresh", plabel, true ); + plabel.clear(); + } + ERR << "Error reading repository '" << repo.asUserString() << "'" << endl; + zypper.out().error( e, str::Format(_("Error retrieving metadata for '%s':")) % repo.asUserString() ); - return true; // error - } + if ( qualifiesForRetry() ) + continue; + return true; // error + } + } return false; // no error } @@ -1950,4 +1979,3 @@ std::vector createTempRepoFromArgs( Zypper &zypper, std::vector #include #include -#include +#include #include #include diff --git a/src/utils/text.h b/src/utils/text.h index 4b1aae34fd..5909ffe735 100644 --- a/src/utils/text.h +++ b/src/utils/text.h @@ -12,7 +12,7 @@ #include #include -#include +#include #include //#include "output/Out.h" MOVED TO THE BOTTOM diff --git a/tests/lib/TestSetup.h b/tests/lib/TestSetup.h index df938aa1e6..0750412ae5 100644 --- a/tests/lib/TestSetup.h +++ b/tests/lib/TestSetup.h @@ -9,7 +9,7 @@ using boost::unit_test::test_case; #include "zypp/base/LogControl.h" #include "zypp/base/LogTools.h" -#include "zypp/base/InputStream.h" +#include "zypp-core/base/InputStream" #include "zypp/base/IOStream.h" #include "zypp/base/Flags.h" #include "zypp/ZYppFactory.h"