[pacman-dev] [PATCH] Add a fetch callback to allow front-end download support

[Dan McGee]

On Sat, Apr 4, 2009 at 3:30 AM, Sebastian Nowicki<sebnow at gmail.com> wrote:
> I fixed the warnings and errors and tested it a bit more. Everything
> seems to be working fine. Using dynamic allocation for the xfercommand
> string removed the problem with PATH_MAX. The previously inlined
> functions are now separate functions as before, with minor changes due
> to the lack of macros from libalpm (see diff below).

So I finally got around to this. I made a few small changes, detailed
in the diff below, that I included in the final patch.

diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
index 31e1f45..ce8c691 100644
--- a/lib/libalpm/alpm.h
+++ b/lib/libalpm/alpm.h
@@ -151,9 +151,6 @@ void alpm_option_add_ignoregrp(const char *grp);
 void alpm_option_set_ignoregrps(alpm_list_t *ignoregrps);
 int alpm_option_remove_ignoregrp(const char *grp);

-const char *alpm_option_get_xfercommand();
-void alpm_option_set_xfercommand(const char *cmd);
-
>>> You simply forgot to kill these here.

 unsigned short alpm_option_get_nopassiveftp();
 void alpm_option_set_nopassiveftp(unsigned short nopasv);
 void alpm_option_set_usedelta(unsigned short usedelta);
diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
index 9aa6141..6b163ce 100644
--- a/lib/libalpm/dload.c
+++ b/lib/libalpm/dload.c
@@ -267,12 +267,17 @@ static int download(const char *url, const char
*localpath,
 		time_t mtimeold, time_t *mtimenew) {
 	if(handle->fetchcb == NULL) {
 #if defined(INTERNAL_DOWNLOAD)
-		handle->fetchcb = download_internal;
+		return(download_internal(url, localpath, mtimeold, mtimenew));
 #else
 		RET_ERR(PM_ERR_EXTERNAL_DOWNLOAD, -1);
 #endif
+	} else {
+		int ret = handle->fetchcb(url, localpath, mtimeold, mtimenew);
+		if(ret == -1) {
+			RET_ERR(PM_ERR_EXTERNAL_DOWNLOAD, -1);
+		}
+		return(ret);
 	}
-	return handle->fetchcb(url, localpath, mtimeold, mtimenew);
 }
>>> Structuring the code this way ensures the correct pm_errno value will propagate up the call stack.

 /*
diff --git a/src/pacman/conf.h b/src/pacman/conf.h
index 2bf952a..2d3de98 100644
--- a/src/pacman/conf.h
+++ b/src/pacman/conf.h
@@ -73,7 +73,7 @@ typedef struct __config_t {
 	unsigned short cleanmethod; /* select -Sc behavior */
 	alpm_list_t *holdpkg;
 	alpm_list_t *syncfirst;
-	char *xfercommand; /* external download command */
+	char *xfercommand;
 } config_t;

 /* Operations */
diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
index 7cf1396..7cecd90 100644
--- a/src/pacman/pacman.c
+++ b/src/pacman/pacman.c
@@ -657,7 +657,7 @@ int download_with_xfercommand(const char *url,
const char *localpath,
 	/* cwd to the download directory */
 	getcwd(cwd, PATH_MAX);
 	if(chdir(localpath)) {
-		pm_printf(PM_LOG_DEBUG, "could not chdir to %s\n", localpath);
+		pm_printf(PM_LOG_WARNING, "could not chdir to %s\n", localpath);
 		ret = -1;
 		goto cleanup;
 	}
@@ -666,7 +666,7 @@ int download_with_xfercommand(const char *url,
const char *localpath,
 	retval = system(parsedCmd);

 	if(retval == -1) {
-		pm_printf(PM_LOG_DEBUG, "running XferCommand: fork failed!\n");
+		pm_printf(PM_LOG_WARNING, "running XferCommand: fork failed!\n");
>>> These two were warnings before; they are kind of a big deal.

 		ret = -1;
 	} else if(retval != 0) {
 		/* download failed */

-Dan