diff options
| -rw-r--r-- | load/load.cxx | 237 |
1 files changed, 146 insertions, 91 deletions
diff --git a/load/load.cxx b/load/load.cxx index 53f1f25..31230a7 100644 --- a/load/load.cxx +++ b/load/load.cxx @@ -4,6 +4,8 @@ #include <signal.h> // signal() #include <cerrno> +#include <chrono> +#include <thread> // this_thread::sleep_for() #include <cstring> // strncmp() #include <iostream> #include <algorithm> // find(), find_if() @@ -36,6 +38,7 @@ using std::cout; using std::cerr; using std::endl; +using namespace std::this_thread; using namespace odb::core; using namespace butl; using namespace bpkg; @@ -53,6 +56,17 @@ static const char* help_info ( static const path packages ("packages.manifest"); static const path repositories ("repositories.manifest"); +// Retry executing bpkg on recoverable errors for about 10 seconds. +// +// Should we just exit with some "bpkg recoverable" code instead and leave it +// to the caller to perform retries? Feels like it's better to handle such +// errors ourselves rather than to complicate every caller. Note that having +// some frequently updated prerequisite repository can make these errors quite +// probable, even if the internal repositories are rarely updated. +// +static const size_t bpkg_retries (10); +static const std::chrono::seconds bpkg_retry_timeout (1); + struct internal_repository { repository_location location; @@ -877,20 +891,39 @@ load_repositories (const options& lo, // args.push_back ("--trust-no"); - process p (repository_info (lo, pr->location.string (), args)); - - try + // Retry bpkg-rep-info on recoverable errors, for a while. + // + for (size_t i (0);; ++i) { - // @@ Note that bpkg-rep-info can fail with the 'repository manifest - // file checksum mismatch' error if the repository was changed - // during the fetch. We will need to retry when bpkg is fixed to - // exit with some recognizable code on this failure. - // - if (!p.wait ()) + if (i != 0) + { + // Let's follow up the bpkg's diagnostics with the number of + // retries left. + // + cerr << bpkg_retries - i + 1 << " retries left" << endl; + sleep_for (bpkg_retry_timeout); + } + + process p (repository_info (lo, pr->location.string (), args)); + + try { + // Bail out from the retry loop on success. + // + if (p.wait ()) + break; + // Assume the child issued diagnostics if terminated normally. // - if (!p.exit->normal ()) + if (p.exit->normal ()) + { + // Retry the manifests fetch on a recoverable error, unless the + // retries limit is reached. + // + if (p.exit->code () == 2 && i != bpkg_retries) + continue; + } + else cerr << "process " << lo.bpkg () << " " << *p.exit << endl; cerr << "error: unable to fetch manifests for " @@ -900,13 +933,13 @@ load_repositories (const options& lo, throw failed (); } - } - catch (const process_error& e) - { - cerr << "error: unable to fetch manifests for " - << pr->canonical_name << ": " << e << endl; + catch (const process_error& e) + { + cerr << "error: unable to fetch manifests for " + << pr->canonical_name << ": " << e << endl; - throw failed (); + throw failed (); + } } // Note that this is a non-pkg repository cache and so we create the @@ -1186,108 +1219,130 @@ certificate_info (const options& lo, const repository_location& rl, const optional<string>& fp) { - try - { - cstrings args { - "--cert-fingerprint", - "--cert-name", - "--cert-organization", - "--cert-email", - "-q"}; // Don't print info messages. + cstrings args { + "--cert-fingerprint", + "--cert-name", + "--cert-organization", + "--cert-email", + "-q"}; // Don't print info messages. - const char* trust ("--trust-no"); + const char* trust ("--trust-no"); - if (fp) + if (fp) + { + if (!fp->empty ()) { - if (!fp->empty ()) - { - args.push_back ("--trust"); - args.push_back (fp->c_str ()); - } - else - trust = "--trust-yes"; + args.push_back ("--trust"); + args.push_back (fp->c_str ()); + } + else + trust = "--trust-yes"; - if (!rl.remote ()) - { - args.push_back ("--auth"); - args.push_back ("all"); - } + if (!rl.remote ()) + { + args.push_back ("--auth"); + args.push_back ("all"); } + } - args.push_back (trust); + args.push_back (trust); - process pr (repository_info (lo, rl.string (), args)); + // Retry bpkg-rep-info on recoverable errors, for a while. + // + for (size_t i (0);; ++i) + { + if (i != 0) + { + // Let's follow up the bpkg's diagnostics with the number of retries + // left. + // + cerr << bpkg_retries - i + 1 << " retries left" << endl; + sleep_for (bpkg_retry_timeout); + } try { - ifdstream is ( - move (pr.in_ofd), - ifdstream::failbit | ifdstream::badbit | ifdstream::eofbit); + process pr (repository_info (lo, rl.string (), args)); - optional<certificate> cert; + try + { + ifdstream is ( + move (pr.in_ofd), + ifdstream::failbit | ifdstream::badbit | ifdstream::eofbit); - string fingerprint; - getline (is, fingerprint); + optional<certificate> cert; - if (!fingerprint.empty ()) - { - cert = certificate (); - cert->fingerprint = move (fingerprint); - getline (is, cert->name); - getline (is, cert->organization); - getline (is, cert->email); + string fingerprint; + getline (is, fingerprint); + + if (!fingerprint.empty ()) + { + cert = certificate (); + cert->fingerprint = move (fingerprint); + getline (is, cert->name); + getline (is, cert->organization); + getline (is, cert->email); + } + else + { + // Read out empty lines. + // + string s; + getline (is, s); // Name. + getline (is, s); // Organization. + getline (is, s); // Email. + } + + // Check that EOF is successfully reached. + // + is.exceptions (ifdstream::failbit | ifdstream::badbit); + if (is.peek () != ifdstream::traits_type::eof ()) + throw io_error (""); + + is.close (); + + if (pr.wait ()) + return cert; + + // Fall through. + // } - else + catch (const io_error&) { - // Read out empty lines. + // Child exit status doesn't matter. Just wait for the process + // completion and fall through. // - string s; - getline (is, s); // Name. - getline (is, s); // Organization. - getline (is, s); // Email. + pr.wait (); } - // Check that EOF is successfully reached. + // Assume the child issued diagnostics if terminated normally. // - is.exceptions (ifdstream::failbit | ifdstream::badbit); - if (is.peek () != ifdstream::traits_type::eof ()) - throw io_error (""); - - is.close (); + if (pr.exit->normal ()) + { + // Retry the certificate fetch on a recoverable error, unless the + // retries limit is reached. + // + if (pr.exit->code () == 2 && i != bpkg_retries) + continue; + } + else + cerr << "process " << lo.bpkg () << " " << *pr.exit << endl; - if (pr.wait ()) - return cert; + cerr << "error: unable to fetch certificate information for " + << rl.canonical_name () << endl; // Fall through. - // } - catch (const io_error&) + catch (const process_error& e) { - // Child exit status doesn't matter. Just wait for the process - // completion and fall through. - // - pr.wait (); - } + cerr << "error: unable to fetch certificate information for " + << rl.canonical_name () << ": " << e << endl; - // Assume the child issued diagnostics if terminated normally. - // - if (!pr.exit->normal ()) - cerr << "process " << lo.bpkg () << " " << *pr.exit << endl; - - cerr << "error: unable to fetch certificate information for " - << rl.canonical_name () << endl; - - // Fall through. - } - catch (const process_error& e) - { - cerr << "error: unable to fetch certificate information for " - << rl.canonical_name () << ": " << e << endl; + // Fall through. + } - // Fall through. + throw failed (); } - - throw failed (); } int |