[pacman-dev] [PATCH] Give informative error message when the to-be-downloaded file cannot be created
It can happen that the to-be-downloaded file cannot be created in cachedir. For example, I am an -Sup user, and it is comfortable to set --cachedir to /mnt/pendrive, which is a FAT filesystem, so files like capseo-1:0.3-2-i686.pkg.tar.xz cannot be downloaded to there. Before this patch, pacman didn't give clear output about what happens when the download code could not create the necessary file. This can be confusing with -Su. An example output: *** $ sudo pacman -S capseo bochs --cachedir /c/TEMP resolving dependencies... looking for inter-conflicts... Targets (2): bochs-2.4.6-1 capseo-1:0.3-2 Total Download Size: 0.61 MiB Total Installed Size: 2.61 MiB Proceed with installation? [Y/n] :: Retrieving packages from extra... warning: failed to retrieve some files from extra bochs-2.4.6-1-i686 611.5 KiB 118K/s 00:05 [------------------] 97% error: failed to commit transaction (unexpected error) Errors occurred, no packages were upgraded. *** After the patch, pacman will give more informative error message (and pm_errno is set properly): *** error: failed to create file '/c/TEMP/capseo-1:0.3-2-i686.pkg.tar.xz.part' for download error: failed to commit transaction (failed to retrieve some files) *** Unfortunately, the "failed to create file" error message is printed for every mirror (that can be dozens of lines), which is ugly, but at least informative... Without modifying the download logic (for example, by introducing -2 return value for _alpm_download() to indicate giving up), this ugliness cannot be eliminated. Signed-off-by: Nagy Gabor <ngaba@bibl.u-szeged.hu> --- lib/libalpm/dload.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index 97778c2..9b57b97 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -422,6 +422,9 @@ static int curl_download_internal(struct dload_payload *payload, if(localf == NULL) { localf = fopen(payload->tempfile_name, payload->tempfile_openmode); if(localf == NULL) { + handle->pm_errno = ALPM_ERR_RETRIEVE; + _alpm_log(handle, ALPM_LOG_ERROR, + _("failed to create file '%s' for download\n"), payload->tempfile_name); goto cleanup; } } -- 1.7.9
On Wed, Feb 8, 2012 at 10:52 PM, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
It can happen that the to-be-downloaded file cannot be created in cachedir. For example, I am an -Sup user, and it is comfortable to set --cachedir to /mnt/pendrive, which is a FAT filesystem, so files like capseo-1:0.3-2-i686.pkg.tar.xz cannot be downloaded to there.
Before this patch, pacman didn't give clear output about what happens when the download code could not create the necessary file. This can be confusing with -Su. An example output: *** $ sudo pacman -S capseo bochs --cachedir /c/TEMP
resolving dependencies... looking for inter-conflicts...
Targets (2): bochs-2.4.6-1 capseo-1:0.3-2
Total Download Size: 0.61 MiB Total Installed Size: 2.61 MiB
Proceed with installation? [Y/n] :: Retrieving packages from extra... warning: failed to retrieve some files from extra bochs-2.4.6-1-i686 611.5 KiB 118K/s 00:05 [------------------] 97% error: failed to commit transaction (unexpected error) Errors occurred, no packages were upgraded. ***
After the patch, pacman will give more informative error message (and pm_errno is set properly): *** error: failed to create file '/c/TEMP/capseo-1:0.3-2-i686.pkg.tar.xz.part' for download error: failed to commit transaction (failed to retrieve some files) ***
Unfortunately, the "failed to create file" error message is printed for every mirror (that can be dozens of lines), which is ugly, but at least informative... Without modifying the download logic (for example, by introducing -2 return value for _alpm_download() to indicate giving up), this ugliness cannot be eliminated. I'm all for modifying the download logic in this fashion.
Signed-off-by: Nagy Gabor <ngaba@bibl.u-szeged.hu> --- lib/libalpm/dload.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index 97778c2..9b57b97 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -422,6 +422,9 @@ static int curl_download_internal(struct dload_payload *payload, if(localf == NULL) { localf = fopen(payload->tempfile_name, payload->tempfile_openmode); if(localf == NULL) { + handle->pm_errno = ALPM_ERR_RETRIEVE; + _alpm_log(handle, ALPM_LOG_ERROR, + _("failed to create file '%s' for download\n"), payload->tempfile_name);
Not a huge fan of the phrasing of this message. In most cases, I'd rather the filename or variable part of the message be at the front or the end, not in the middle. Perhaps _("could not open file '%s': %s\n"), tempfile_name, strerror(errno)) ?
goto cleanup; } } -- 1.7.9
Unfortunately, the "failed to create file" error message is printed for every mirror (that can be dozens of lines), which is ugly, but at least informative... Without modifying the download logic (for example, by introducing -2 return value for _alpm_download() to indicate giving up), this ugliness cannot be eliminated.
I'm all for modifying the download logic in this fashion.
OK. But not today :-). Then some other cases can also get retval -2, so we can eliminate some other pointless retries for every mirrors.
Not a huge fan of the phrasing of this message. In most cases, I'd rather the filename or variable part of the message be at the front or the end, not in the middle. Perhaps _("could not open file '%s': %s\n"), tempfile_name, strerror(errno))
OK, done in my working branch: http://repo.or.cz/w/pacman-ng.git/shortlog/refs/heads/working NG
participants (2)
-
Dan McGee
-
Nagy Gabor