From 68a83a64cd424385b613f58e23c03e262d840b91 Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Fri, 6 Mar 2020 13:07:55 +0100 Subject: GetLock: No strerror if it's just another process holding the lock This improves the locking message, getting rid of useless details. If we have a process holding the lock, we got that because the lock is being hold by it, so there's no point telling the people the reason for not getting the lock is the EAGAIN error and displaying its strerrror(). --- apt-pkg/contrib/fileutl.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc index db5463be2..045dbe17d 100644 --- a/apt-pkg/contrib/fileutl.cc +++ b/apt-pkg/contrib/fileutl.cc @@ -302,13 +302,15 @@ int GetLock(string File,bool Errors) if (Errors == true) { + // We only do the lookup in the if ((errno == EACCES || errno == EAGAIN)) + // case, so we do not need to show the errno strerrr here... if (fl.l_pid != -1) { auto name = GetProcessName(fl.l_pid); if (name.empty()) - _error->Errno("open", _("Could not get lock %s. It is held by process %d"), File.c_str(), fl.l_pid); + _error->Error(_("Could not get lock %s. It is held by process %d"), File.c_str(), fl.l_pid); else - _error->Errno("open", _("Could not get lock %s. It is held by process %d (%s)"), File.c_str(), fl.l_pid, name.c_str()); + _error->Error(_("Could not get lock %s. It is held by process %d (%s)"), File.c_str(), fl.l_pid, name.c_str()); } else _error->Errno("open", _("Could not get lock %s"), File.c_str()); -- cgit v1.2.3 From 1b81f6bd13bb31e59da3f53cfdc7caab43abf887 Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Fri, 6 Mar 2020 13:10:04 +0100 Subject: Show absolute time while waiting for lock instead of %, rework message Showing a percentage for a timeout is pretty non-standard. Rework the progress class so it can show an absolute progress (currently hardcoded to use seconds as a unit). If there is a timeout (aka if it's not the maximum long long unsigned -1llu), then show the timeout, otherwise just count up seconds, e.g. Waiting for cache lock: Could not get lock /var/lib/dpkg/lock-frontend. It is held by process 33842 (apt)... 1/120s or Waiting for cache lock: Could not get lock /var/lib/dpkg/lock-frontend. It is held by process 33842 (apt)... 1s Also improve the error message to use "Waiting for cache lock: %s" instead of "... (%s)", as having multiple sentences inside parenthesis is super weird, as is having two closing parens. We pass the information via _config, as that's reasonably easy and avoids ABI hackage. It also provides an interesting debugging tool for other kinds of progress. --- apt-pkg/contrib/progress.cc | 16 ++++++++++++---- apt-pkg/contrib/progress.h | 1 + apt-pkg/deb/debsystem.cc | 7 ++++++- doc/examples/configure-index | 2 ++ 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/apt-pkg/contrib/progress.cc b/apt-pkg/contrib/progress.cc index 971198270..03f88d4ce 100644 --- a/apt-pkg/contrib/progress.cc +++ b/apt-pkg/contrib/progress.cc @@ -91,7 +91,10 @@ void OpProgress::SubProgress(unsigned long long SubTotal,const string &Op, an update the display would be swamped and the system much slower. This provides an upper bound on the update rate. */ bool OpProgress::CheckChange(float Interval) -{ +{ + // For absolute progress, we assume every call is relevant. + if (_config->FindB("APT::Internal::OpProgress::Absolute", false)) + return true; // New major progress indication if (Op != LastOp) { @@ -199,9 +202,14 @@ void OpTextProgress::Update() Write(S); cout << endl; } - - // Print the spinner - snprintf(S,sizeof(S),_("%c%s... %u%%"),'\r',Op.c_str(),(unsigned int)Percent); + + // Print the spinner. Absolute progress shows us a time progress. + if (_config->FindB("APT::Internal::OpProgress::Absolute", false) && Total != -1llu) + snprintf(S, sizeof(S), _("%c%s... %llu/%llus"), '\r', Op.c_str(), Current, Total); + else if (_config->FindB("APT::Internal::OpProgress::Absolute", false)) + snprintf(S, sizeof(S), _("%c%s... %llus"), '\r', Op.c_str(), Current); + else + snprintf(S, sizeof(S), _("%c%s... %u%%"), '\r', Op.c_str(), (unsigned int)Percent); Write(S); OldOp = Op; diff --git a/apt-pkg/contrib/progress.h b/apt-pkg/contrib/progress.h index 4d118ee99..d6a698af3 100644 --- a/apt-pkg/contrib/progress.h +++ b/apt-pkg/contrib/progress.h @@ -28,6 +28,7 @@ class Configuration; class APT_PUBLIC OpProgress { + friend class OpTextProgress; unsigned long long Current; unsigned long long Total; unsigned long long Size; diff --git a/apt-pkg/deb/debsystem.cc b/apt-pkg/deb/debsystem.cc index da7ae2e42..6904879b6 100644 --- a/apt-pkg/deb/debsystem.cc +++ b/apt-pkg/deb/debsystem.cc @@ -79,6 +79,11 @@ debSystem::~debSystem() checking of the updates directory. */ static int GetLockMaybeWait(std::string const &file, OpProgress *Progress, int &timeoutSec) { + struct ScopedAbsoluteProgress + { + ScopedAbsoluteProgress() { _config->Set("APT::Internal::OpProgress::Absolute", true); } + ~ScopedAbsoluteProgress() { _config->Set("APT::Internal::OpProgress::Absolute", false); } + } _scopedAbsoluteProgress; int fd = -1; if (timeoutSec == 0 || Progress == nullptr) return GetLock(file); @@ -102,7 +107,7 @@ static int GetLockMaybeWait(std::string const &file, OpProgress *Progress, int & _error->PopMessage(poppedError); _error->RevertToStack(); - strprintf(completeError, _("Waiting for cache lock (%s)"), poppedError.c_str()); + strprintf(completeError, _("Waiting for cache lock: %s"), poppedError.c_str()); sleep(1); Progress->OverallProgress(i, timeoutSec, 0, completeError); } diff --git a/doc/examples/configure-index b/doc/examples/configure-index index 4e0bd57ae..bf9efc109 100644 --- a/doc/examples/configure-index +++ b/doc/examples/configure-index @@ -826,3 +826,5 @@ dir::filelistdir ""; dir::dpkg::tupletable ""; dir::dpkg::triplettable ""; dir::dpkg::cputable ""; + +APT::Internal::OpProgress::Absolute ""; -- cgit v1.2.3