This allows us to remove the sync_only flag, and also do the following steps in the future : 1) refresh the database (if asked) 2) do other stuff (eg checking if a newer pacman version is available) 3) start the actual transaction Currently when we detect a newer pacman version, we have to release the current transaction and start a new one. Signed-off-by: Chantry Xavier <shiningxc@gmail.com> --- src/pacman/sync.c | 56 ++++++++++++++++++++++++++++++++-------------------- 1 files changed, 34 insertions(+), 22 deletions(-) diff --git a/src/pacman/sync.c b/src/pacman/sync.c index 27218d6..bb2a8bb 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -202,6 +202,17 @@ static int sync_synctree(int level, alpm_list_t *syncs) alpm_list_t *i; int success = 0, ret; + if(alpm_trans_init(PM_TRANS_TYPE_SYNC, 0, cb_trans_evt, + cb_trans_conv, cb_trans_progress) == -1) { + fprintf(stderr, _("error: failed to init transaction (%s)\n"), + alpm_strerrorlast()); + if(pm_errno == PM_ERR_HANDLE_LOCK) { + printf(_(" if you're sure a package manager is not already\n" + " running, you can remove %s.\n"), alpm_option_get_lockfile()); + } + return(0); + } + for(i = syncs; i; i = alpm_list_next(i)) { pmdb_t *db = alpm_list_getdata(i); @@ -229,10 +240,18 @@ static int sync_synctree(int level, alpm_list_t *syncs) } } + if(alpm_trans_release() == -1) { + fprintf(stderr, _("error: failed to release transaction (%s)\n"), + alpm_strerrorlast()); + return(0); + } /* We should always succeed if at least one DB was upgraded - we may possibly * fail later with unresolved deps, but that should be rare, and would be * expected */ + if(!success) { + fprintf(stderr, _("error: failed to synchronize any databases\n")); + } return(success > 0); } @@ -469,7 +488,7 @@ static int sync_list(alpm_list_t *syncs, alpm_list_t *targets) return(0); } -static int sync_trans(alpm_list_t *targets, int sync_only) +static int sync_trans(alpm_list_t *targets) { int retval = 0; alpm_list_t *data = NULL; @@ -487,23 +506,8 @@ static int sync_trans(alpm_list_t *targets, int sync_only) return(1); } - if(config->op_s_sync) { - /* grab a fresh package list */ - printf(_(":: Synchronizing package databases...\n")); - alpm_logaction("synchronizing package lists\n"); - if(!sync_synctree(config->op_s_sync, sync_dbs)) { - fprintf(stderr, _("error: failed to synchronize any databases\n")); - retval = 1; - goto cleanup; - } - if(sync_only) { - goto cleanup; - } - } - if(config->op_s_upgrade) { alpm_list_t *pkgs, *i; - printf(_(":: Starting full system upgrade...\n")); alpm_logaction("starting full system upgrade\n"); if(alpm_trans_sysupgrade() == -1) { @@ -756,7 +760,6 @@ cleanup: int pacman_sync(alpm_list_t *targets) { alpm_list_t *sync_dbs = NULL; - int sync_only = 0; /* clean the cache */ if(config->op_s_clean) { @@ -772,18 +775,27 @@ int pacman_sync(alpm_list_t *targets) return(1); } - if(config->op_s_search || config->group - || config->op_s_info || config->op_q_list) { - sync_only = 1; - } else if(targets == NULL && !(config->op_s_sync || config->op_s_upgrade)) { + if(targets == NULL && !(config->op_s_sync || config->op_s_upgrade + || config->op_s_search || config->group + || config->op_s_info || config->op_q_list)) { /* don't proceed here unless we have an operation that doesn't require * a target list */ pm_printf(PM_LOG_ERROR, _("no targets specified (use -h for help)\n")); return(1); } + if(config->op_s_sync) { + /* grab a fresh package list */ + printf(_(":: Synchronizing package databases...\n")); + alpm_logaction("synchronizing package lists\n"); + if(!sync_synctree(config->op_s_sync, sync_dbs)) { + return(1); + } + config->op_s_sync = 0; + } + if(needs_transaction()) { - if(sync_trans(targets, sync_only) == 1) { + if(sync_trans(targets) == 1) { return(1); } } -- 1.5.4

On Feb 17, 2008 9:12 AM, Chantry Xavier <shiningxc@gmail.com> wrote:

This patch offers a way to fix FS#9228. By putting "CheckNewVersion = pacman" in pacman.conf, the version check will happen before the transaction really starts, and before any replacements is made. Otherwise, no version check is done.

The sync301 pactest was updated to use this CheckNewVersion option.

Example session with CheckNewVersion = pacman, and a newer pacman version available : $ pacman -Su (or pacman -S <any targets>) :: the following packages should be upgraded first : pacman :: Do you want to cancel the current operation :: and upgrade these packages now? [Y/n]

resolving dependencies... looking for inter-conflicts...

Targets: pacman-x.y.z-t

Total Download Size: x.xx MB Total Installed Size: x.xx MB

Proceed with installation? [Y/n] n

As Nagy previously noted, doing this check on any -S operations might look intrusive, but it can be required. For example, the case where you want to install a package with versioned provisions, using a pacman version which didn't support that feature yet (and there is already a newer pacman in sync db supporting it).

Hmm, so say I put glibc in my CheckNewVersion option list, and I am not running the version in the sync DB. If I were to do this: pacman -S mpd I would be prompted to upgrade glibc? This seems a bit overreaching, as this is exactly what I did today on my server box. :) I would prefer it only be invoked on a --sysupgrade operation. But you do have a valid point. I'm also not sure about "CheckNewVersion" from a naming standpoint. Maybe something more like "UpgradeFirst" or something? We check for a new version of every package, so the name seems a bit misleading.

Signed-off-by: Chantry Xavier <shiningxc@gmail.com> --- doc/pacman.conf.5.txt | 6 +++ pactest/pmtest.py | 3 +- pactest/tests/sync301.py | 4 ++- src/pacman/conf.c | 2 + src/pacman/conf.h | 1 + src/pacman/pacman.c | 8 ++++ src/pacman/sync.c | 81 ++++++++++++++++++++++------------------------ 7 files changed, 61 insertions(+), 44 deletions(-)

diff --git a/doc/pacman.conf.5.txt b/doc/pacman.conf.5.txt index e8f7454..c48ec68 100644 --- a/doc/pacman.conf.5.txt +++ b/doc/pacman.conf.5.txt @@ -80,6 +80,12 @@ Options Instructs pacman to ignore any upgrades for this package when performing a '\--sysupgrade'.

+*CheckNewVersion =* package ...:: + Instructs pacman to check for newer version of these packages before any + '-S' or '-Su' operation. The user will then have the choice to either cancel + the current operation and install these packages or go on with the current + operation. This could typically be done for pacman itself. + *IgnoreGroup =* group ...:: Instructs pacman to ignore any upgrades for all packages in this group when performing a '\--sysupgrade'. diff --git a/pactest/pmtest.py b/pactest/pmtest.py index d54d7ba..9b45384 100755 --- a/pactest/pmtest.py +++ b/pactest/pmtest.py @@ -83,7 +83,8 @@ class pmtest: "noupgrade": [], "ignorepkg": [], "ignoregroup": [], - "noextract": [] + "noextract": [], + "checknewversion": [], }

# Test rules diff --git a/pactest/tests/sync301.py b/pactest/tests/sync301.py index e8526b9..cde08c6 100644 --- a/pactest/tests/sync301.py +++ b/pactest/tests/sync301.py @@ -16,10 +16,12 @@ self.addpkg2db("local", lp) lp1 = pmpkg("pkg1", "1.0-1") self.addpkg2db("local", lp1)

+self.option["checknewversion"] = ["pacman"] + self.args = "-Su"

self.addrule("PACMAN_RETCODE=0") self.addrule("PKG_EXIST=pacman") self.addrule("PKG_VERSION=pacman|1.0-2") +self.addrule("PKG_VERSION=pkg1|1.0-1") self.addrule("PKG_EXIST=dep") -self.addrule("PKG_REQUIREDBY=dep|pacman") diff --git a/src/pacman/conf.c b/src/pacman/conf.c index bf3a462..72c1408 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -45,6 +45,7 @@ config_t *config_new(void) newconfig->rootdir = NULL; newconfig->dbpath = NULL; newconfig->logfile = NULL; + newconfig->checknewversion = NULL;

return(newconfig); } @@ -55,6 +56,7 @@ int config_free(config_t *oldconfig) return(-1); }

+ FREELIST(oldconfig->checknewversion); free(oldconfig->configfile); free(oldconfig->rootdir); free(oldconfig->dbpath); diff --git a/src/pacman/conf.h b/src/pacman/conf.h index f804f56..2bbfb10 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -70,6 +70,7 @@ typedef struct __config_t { unsigned short totaldownload; /* When downloading, display the amount downloaded, rate, ETA, and percent downloaded of the total download list */ + alpm_list_t *checknewversion; } config_t;

/* Operations */ diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index f185320..31af78d 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -515,6 +515,11 @@ static int parseargs(int argc, char *argv[]) return(0); }

+/* helper for being used with setrepeatingoption */ +void option_add_checknewversion(const char *name) { + config->checknewversion = alpm_list_add(config->checknewversion, strdup(name)); +} + /** Add repeating options such as NoExtract, NoUpgrade, etc to libalpm * settings. Refactored out of the parseconfig code since all of them did * the exact same thing and duplicated code. @@ -675,6 +680,9 @@ static int _parseconfig(const char *file, const char *givensection, } else if(strcmp(key, "HoldPkg") == 0 || strcmp(upperkey, "HOLDPKG") == 0) { setrepeatingoption(ptr, "HoldPkg", alpm_option_add_holdpkg); + } else if(strcmp(key, "CheckNewVersion") == 0 + || strcmp(upperkey, "CHECKNEWVERSION") == 0) { + setrepeatingoption(ptr, "CheckNewVersion", option_add_checknewversion); } else if(strcmp(key, "DBPath") == 0 || strcmp(upperkey, "DBPATH") == 0) { /* don't overwrite a path specified on the command line */ if(!config->dbpath) { diff --git a/src/pacman/sync.c b/src/pacman/sync.c index bb2a8bb..66bd598 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -488,6 +488,24 @@ static int sync_list(alpm_list_t *syncs, alpm_list_t *targets) return(0); }

+static alpm_list_t *checknewversion() { + alpm_list_t *i, *res = NULL; + + for(i = config->checknewversion; i; i = alpm_list_next(i)) { + char *pkgname = alpm_list_getdata(i); + pmpkg_t *pkg = alpm_db_get_pkg(alpm_option_get_localdb(), pkgname); + if(pkg == NULL) { + continue; + } + + if(alpm_sync_newversion(pkg, alpm_option_get_syncdbs())) { + res = alpm_list_add(res, strdup(pkgname)); + } + } + + return(res); +} + static int sync_trans(alpm_list_t *targets) { int retval = 0; @@ -507,7 +525,6 @@ static int sync_trans(alpm_list_t *targets) }

if(config->op_s_upgrade) { - alpm_list_t *pkgs, *i; printf(_(":: Starting full system upgrade...\n")); alpm_logaction("starting full system upgrade\n"); if(alpm_trans_sysupgrade() == -1) { @@ -515,46 +532,6 @@ static int sync_trans(alpm_list_t *targets) retval = 1; goto cleanup; } - - if(!(alpm_trans_get_flags() & (PM_TRANS_FLAG_DOWNLOADONLY | PM_TRANS_FLAG_PRINTURIS))) { - /* check if pacman itself is one of the packages to upgrade. - * this can prevent some of the "syntax error" problems users can have - * when sysupgrade'ing with an older version of pacman. - */ - pkgs = alpm_trans_get_pkgs(); - for(i = pkgs; i; i = alpm_list_next(i)) { - pmsyncpkg_t *sync = alpm_list_getdata(i); - pmpkg_t *spkg = alpm_sync_get_pkg(sync); - /* TODO pacman name should probably not be hardcoded. In addition, we - * have problems on an -Syu if pacman has to pull in deps, so recommend - * an '-S pacman' operation */ - if(strcmp("pacman", alpm_pkg_get_name(spkg)) == 0) { - printf("\n"); - printf(_(":: pacman has detected a newer version of itself.\n")); - if(yesno(_(":: Do you want to cancel the current operation\n" - ":: and install the new pacman version now? [Y/n] "))) { - if(alpm_trans_release() == -1) { - fprintf(stderr, _("error: failed to release transaction (%s)\n"), - alpm_strerrorlast()); - retval = 1; - goto cleanup; - } - if(alpm_trans_init(PM_TRANS_TYPE_SYNC, config->flags, - cb_trans_evt, cb_trans_conv, cb_trans_progress) == -1) { - fprintf(stderr, _("error: failed to init transaction (%s)\n"), - alpm_strerrorlast()); - return(1); - } - if(alpm_trans_addtarget("pacman") == -1) { - fprintf(stderr, _("error: pacman: %s\n"), alpm_strerrorlast()); - retval = 1; - goto cleanup; - } - break; - } - } - } - } Oh so much cleaner now. :)

} else { alpm_list_t *i;

@@ -795,7 +772,27 @@ int pacman_sync(alpm_list_t *targets) }

if(needs_transaction()) { - if(sync_trans(targets) == 1) { + alpm_list_t *targs = alpm_list_strdup(targets); + if(!(config->flags & (PM_TRANS_FLAG_DOWNLOADONLY | PM_TRANS_FLAG_PRINTURIS))) {

Presumably could just add a check for config->op_s_upgrade here. I just don't know that it makes sense to honor this variable for all -S ops and not just -Su.

+ /* check for newer packages version */ + alpm_list_t *newversion = checknewversion(); + if(newversion) { + printf(_(":: the following packages should be upgraded first :\n")); + list_display(" ", newversion); + if(yesno(_(":: Do you want to cancel the current operation\n" + ":: and upgrade these packages now? [Y/n] "))) { + FREELIST(targs); + targs = newversion; + config->flags = 0; + config->op_s_upgrade = 0; + } + printf("\n"); + } + } + + int ret = sync_trans(targs); + FREELIST(targs); + if(ret == 1) { return(1); } } -- 1.5.4

This patch offers a way to fix FS#9228. By putting "UpgradeFirst = pacman" in pacman.conf, the version check will happen before the transaction really starts, and before any replacements is made. Otherwise, no version check is done. The sync301 pactest was updated to use this UpgradeFirst option. Example session with UpgradeFirst = pacman, and a newer pacman version available : $ pacman -Su (or pacman -S <any targets>) :: the following packages should be upgraded first : pacman :: Do you want to cancel the current operation :: and upgrade these packages now? [Y/n] resolving dependencies... looking for inter-conflicts... Targets: pacman-x.y.z-t Total Download Size: x.xx MB Total Installed Size: x.xx MB Proceed with installation? [Y/n] n As Nagy previously noted, doing this check on any -S operations might look intrusive, but it can be required. For example, the case where you want to install a package with versioned provisions, using a pacman version which didn't support that feature yet (and there is already a newer pacman in sync db supporting it). Signed-off-by: Chantry Xavier <shiningxc@gmail.com> --- doc/pacman.conf.5.txt | 6 ++++ pactest/pmtest.py | 3 +- pactest/tests/sync301.py | 4 ++- src/pacman/conf.c | 2 + src/pacman/conf.h | 1 + src/pacman/pacman.c | 8 +++++ src/pacman/sync.c | 74 ++++++++++++++++++++++++--------------------- 7 files changed, 61 insertions(+), 37 deletions(-) diff --git a/doc/pacman.conf.5.txt b/doc/pacman.conf.5.txt index e8f7454..c3bbb02 100644 --- a/doc/pacman.conf.5.txt +++ b/doc/pacman.conf.5.txt @@ -80,6 +80,12 @@ Options Instructs pacman to ignore any upgrades for this package when performing a '\--sysupgrade'. +*UpgradeFirst =* package ...:: + Instructs pacman to check for newer version of these packages before any + '-S' or '-Su' operation. The user will then have the choice to either cancel + the current operation and upgrade these packages first or go on with the + current operation. This could typically be done for pacman itself. + *IgnoreGroup =* group ...:: Instructs pacman to ignore any upgrades for all packages in this group when performing a '\--sysupgrade'. diff --git a/pactest/pmtest.py b/pactest/pmtest.py index d54d7ba..db27410 100755 --- a/pactest/pmtest.py +++ b/pactest/pmtest.py @@ -83,7 +83,8 @@ class pmtest: "noupgrade": [], "ignorepkg": [], "ignoregroup": [], - "noextract": [] + "noextract": [], + "upgradefirst": [] } # Test rules diff --git a/pactest/tests/sync301.py b/pactest/tests/sync301.py index e8526b9..0a71b61 100644 --- a/pactest/tests/sync301.py +++ b/pactest/tests/sync301.py @@ -16,10 +16,12 @@ self.addpkg2db("local", lp) lp1 = pmpkg("pkg1", "1.0-1") self.addpkg2db("local", lp1) +self.option["upgradefirst"] = ["pacman"] + self.args = "-Su" self.addrule("PACMAN_RETCODE=0") self.addrule("PKG_EXIST=pacman") self.addrule("PKG_VERSION=pacman|1.0-2") +self.addrule("PKG_VERSION=pkg1|1.0-1") self.addrule("PKG_EXIST=dep") -self.addrule("PKG_REQUIREDBY=dep|pacman") diff --git a/src/pacman/conf.c b/src/pacman/conf.c index bf3a462..bfe2be3 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -45,6 +45,7 @@ config_t *config_new(void) newconfig->rootdir = NULL; newconfig->dbpath = NULL; newconfig->logfile = NULL; + newconfig->upgradefirst = NULL; return(newconfig); } @@ -55,6 +56,7 @@ int config_free(config_t *oldconfig) return(-1); } + FREELIST(oldconfig->upgradefirst); free(oldconfig->configfile); free(oldconfig->rootdir); free(oldconfig->dbpath); diff --git a/src/pacman/conf.h b/src/pacman/conf.h index f804f56..936abc3 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -70,6 +70,7 @@ typedef struct __config_t { unsigned short totaldownload; /* When downloading, display the amount downloaded, rate, ETA, and percent downloaded of the total download list */ + alpm_list_t *upgradefirst; } config_t; /* Operations */ diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index f185320..2969b2c 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -515,6 +515,11 @@ static int parseargs(int argc, char *argv[]) return(0); } +/* helper for being used with setrepeatingoption */ +void option_add_upgradefirst(const char *name) { + config->upgradefirst = alpm_list_add(config->upgradefirst, strdup(name)); +} + /** Add repeating options such as NoExtract, NoUpgrade, etc to libalpm * settings. Refactored out of the parseconfig code since all of them did * the exact same thing and duplicated code. @@ -675,6 +680,9 @@ static int _parseconfig(const char *file, const char *givensection, } else if(strcmp(key, "HoldPkg") == 0 || strcmp(upperkey, "HOLDPKG") == 0) { setrepeatingoption(ptr, "HoldPkg", alpm_option_add_holdpkg); + } else if(strcmp(key, "UpgradeFirst") == 0 + || strcmp(upperkey, "UPGRADEFIRST") == 0) { + setrepeatingoption(ptr, "UpgradeFirst", option_add_upgradefirst); } else if(strcmp(key, "DBPath") == 0 || strcmp(upperkey, "DBPATH") == 0) { /* don't overwrite a path specified on the command line */ if(!config->dbpath) { diff --git a/src/pacman/sync.c b/src/pacman/sync.c index c4c4090..aa83a66 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -501,6 +501,24 @@ static int sync_list(alpm_list_t *syncs, alpm_list_t *targets) return(0); } +static alpm_list_t *upgradefirst() { + alpm_list_t *i, *res = NULL; + + for(i = config->upgradefirst; i; i = alpm_list_next(i)) { + char *pkgname = alpm_list_getdata(i); + pmpkg_t *pkg = alpm_db_get_pkg(alpm_option_get_localdb(), pkgname); + if(pkg == NULL) { + continue; + } + + if(alpm_sync_newversion(pkg, alpm_option_get_syncdbs())) { + res = alpm_list_add(res, strdup(pkgname)); + } + } + + return(res); +} + static int sync_trans(alpm_list_t *targets) { int retval = 0; @@ -513,7 +531,6 @@ static int sync_trans(alpm_list_t *targets) } if(config->op_s_upgrade) { - alpm_list_t *pkgs, *i; printf(_(":: Starting full system upgrade...\n")); alpm_logaction("starting full system upgrade\n"); if(alpm_trans_sysupgrade() == -1) { @@ -521,39 +538,6 @@ static int sync_trans(alpm_list_t *targets) retval = 1; goto cleanup; } - - if(!(alpm_trans_get_flags() & (PM_TRANS_FLAG_DOWNLOADONLY | PM_TRANS_FLAG_PRINTURIS))) { - /* check if pacman itself is one of the packages to upgrade. - * this can prevent some of the "syntax error" problems users can have - * when sysupgrade'ing with an older version of pacman. - */ - pkgs = alpm_trans_get_pkgs(); - for(i = pkgs; i; i = alpm_list_next(i)) { - pmsyncpkg_t *sync = alpm_list_getdata(i); - pmpkg_t *spkg = alpm_sync_get_pkg(sync); - /* TODO pacman name should probably not be hardcoded. In addition, we - * have problems on an -Syu if pacman has to pull in deps, so recommend - * an '-S pacman' operation */ - if(strcmp("pacman", alpm_pkg_get_name(spkg)) == 0) { - printf("\n"); - printf(_(":: pacman has detected a newer version of itself.\n")); - if(yesno(_(":: Do you want to cancel the current operation\n" - ":: and install the new pacman version now? [Y/n] "))) { - if(sync_trans_release() == -1) { - return(1); - } - if(sync_trans_init(0) == -1) { - return(1); - } - if(alpm_trans_addtarget("pacman") == -1) { - fprintf(stderr, _("error: pacman: %s\n"), alpm_strerrorlast()); - return(1); - } - break; - } - } - } - } } else { alpm_list_t *i; @@ -792,7 +776,27 @@ int pacman_sync(alpm_list_t *targets) } if(needs_transaction()) { - if(sync_trans(targets) == 1) { + alpm_list_t *targs = alpm_list_strdup(targets); + if(!(config->flags & (PM_TRANS_FLAG_DOWNLOADONLY | PM_TRANS_FLAG_PRINTURIS))) { + /* check for newer versions of packages to be upgraded first */ + alpm_list_t *packages = upgradefirst(); + if(packages) { + printf(_(":: the following packages should be upgraded first :\n")); + list_display(" ", packages); + if(yesno(_(":: Do you want to cancel the current operation\n" + ":: and upgrade these packages now? [Y/n] "))) { + FREELIST(targs); + targs = packages; + config->flags = 0; + config->op_s_upgrade = 0; + } + printf("\n"); + } + } + + int ret = sync_trans(targs); + FREELIST(targs); + if(ret == 1) { return(1); } } -- 1.5.4.2

On Mon, Feb 18, 2008 at 12:15:37AM -0600, Dan McGee wrote:

...

diff --git a/src/pacman/sync.c b/src/pacman/sync.c index 27218d6..bb2a8bb 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -202,6 +202,17 @@ static int sync_synctree(int level, alpm_list_t *syncs) alpm_list_t *i; int success = 0, ret;

+ if(alpm_trans_init(PM_TRANS_TYPE_SYNC, 0, cb_trans_evt, + cb_trans_conv, cb_trans_progress) == -1) { + fprintf(stderr, _("error: failed to init transaction (%s)\n"), + alpm_strerrorlast()); + if(pm_errno == PM_ERR_HANDLE_LOCK) { + printf(_(" if you're sure a package manager is not already\n" + " running, you can remove %s.\n"), alpm_option_get_lockfile()); + } + return(0); + } +

Don't we have this same code in a few different places now? Can we refactor this somehow?

I will see what I can do.

...

for(i = syncs; i; i = alpm_list_next(i)) { pmdb_t *db = alpm_list_getdata(i);

@@ -229,10 +240,18 @@ static int sync_synctree(int level, alpm_list_t *syncs) } }

+ if(alpm_trans_release() == -1) { + fprintf(stderr, _("error: failed to release transaction (%s)\n"), + alpm_strerrorlast()); + return(0); + } /* We should always succeed if at least one DB was upgraded - we may possibly * fail later with unresolved deps, but that should be rare, and would be * expected */

What is this comment even referring to? I'm confused here. And you had code changes on either side of it.

...

+ if(!success) { + fprintf(stderr, _("error: failed to synchronize any databases\n")); + } return(success > 0); }

Well, it simply refers to that last part. If at least one DB was upgraded (success > 0), we return 1. Otherwise (success = 0), we print an error and return 0.