[pacman-dev] [PATCH 2/2] Remove duplicate code shared between sync and upgrade
Dan McGee
dan at archlinux.org
Fri Jul 22 13:19:59 EDT 2011
Pacman did a great job of having almost (but not quite) duplicate code
paths through the sync and upgrade code. We can use the same logic in
both upgrade in sync once the targets are resolved, so extract a
function and delete a bunch of code.
Signed-off-by: Dan McGee <dan at archlinux.org>
---
Saw this when writing the previous patch and decided it should really get
fixed. Duplicate code sucks.
src/pacman/pacman.h | 1 +
src/pacman/sync.c | 19 +++++---
src/pacman/upgrade.c | 113 +------------------------------------------------
3 files changed, 16 insertions(+), 117 deletions(-)
diff --git a/src/pacman/pacman.h b/src/pacman/pacman.h
index 762e112..8bb9cb2 100644
--- a/src/pacman/pacman.h
+++ b/src/pacman/pacman.h
@@ -32,6 +32,7 @@ int pacman_query(alpm_list_t *targets);
int pacman_remove(alpm_list_t *targets);
/* sync.c */
int pacman_sync(alpm_list_t *targets);
+int sync_prepare_execute(void);
/* upgrade.c */
int pacman_upgrade(alpm_list_t *targets);
diff --git a/src/pacman/sync.c b/src/pacman/sync.c
index 0facd6c..eff0cea 100644
--- a/src/pacman/sync.c
+++ b/src/pacman/sync.c
@@ -712,9 +712,6 @@ cleanup:
static int sync_trans(alpm_list_t *targets)
{
- int retval = 0;
- alpm_list_t *data = NULL;
- alpm_list_t *packages = NULL;
alpm_list_t *i;
/* Step 1: create a new transaction... */
@@ -726,8 +723,8 @@ static int sync_trans(alpm_list_t *targets)
for(i = targets; i; i = alpm_list_next(i)) {
char *targ = alpm_list_getdata(i);
if(process_target(targ) == 1) {
- retval = 1;
- goto cleanup;
+ trans_release();
+ return 1;
}
}
@@ -736,11 +733,19 @@ static int sync_trans(alpm_list_t *targets)
alpm_logaction(config->handle, "starting full system upgrade\n");
if(alpm_sync_sysupgrade(config->handle, config->op_s_upgrade >= 2) == -1) {
pm_fprintf(stderr, ALPM_LOG_ERROR, "%s\n", alpm_strerror(alpm_errno(config->handle)));
- retval = 1;
- goto cleanup;
+ trans_release();
+ return 1;
}
}
+ return sync_prepare_execute();
+}
+
+int sync_prepare_execute(void)
+{
+ alpm_list_t *i, *packages, *data = NULL;
+ int retval = 0;
+
/* Step 2: "compute" the transaction based on targets and flags */
if(alpm_trans_prepare(config->handle, &data) == -1) {
enum _alpm_errno_t err = alpm_errno(config->handle);
diff --git a/src/pacman/upgrade.c b/src/pacman/upgrade.c
index 55b5fa9..e4b2e2b 100644
--- a/src/pacman/upgrade.c
+++ b/src/pacman/upgrade.c
@@ -41,9 +41,8 @@
*/
int pacman_upgrade(alpm_list_t *targets)
{
- alpm_list_t *i, *data = NULL;
+ alpm_list_t *i;
alpm_siglevel_t level = alpm_option_get_default_siglevel(config->handle);
- int retval = 0;
if(targets == NULL) {
pm_printf(ALPM_LOG_ERROR, _("no targets specified (use -h for help)\n"));
@@ -91,114 +90,8 @@ int pacman_upgrade(alpm_list_t *targets)
}
}
- /* Step 2: "compute" the transaction based on targets and flags */
- /* TODO: No, compute nothing. This is stupid. */
- if(alpm_trans_prepare(config->handle, &data) == -1) {
- enum _alpm_errno_t err = alpm_errno(config->handle);
- pm_fprintf(stderr, ALPM_LOG_ERROR, _("failed to prepare transaction (%s)\n"),
- alpm_strerror(err));
- switch(err) {
- case ALPM_ERR_PKG_INVALID_ARCH:
- for(i = data; i; i = alpm_list_next(i)) {
- char *pkg = alpm_list_getdata(i);
- printf(_(":: package %s does not have a valid architecture\n"), pkg);
- }
- break;
- case ALPM_ERR_UNSATISFIED_DEPS:
- for(i = data; i; i = alpm_list_next(i)) {
- alpm_depmissing_t *miss = alpm_list_getdata(i);
- char *depstring = alpm_dep_compute_string(miss->depend);
-
- /* TODO indicate if the error was a virtual package or not:
- * :: %s: requires %s, provided by %s
- */
- printf(_(":: %s: requires %s\n"), miss->target, depstring);
- free(depstring);
- }
- break;
- case ALPM_ERR_CONFLICTING_DEPS:
- for(i = data; i; i = alpm_list_next(i)) {
- alpm_conflict_t *conflict = alpm_list_getdata(i);
- if(strcmp(conflict->package1, conflict->reason) == 0 ||
- strcmp(conflict->package2, conflict->reason) == 0) {
- printf(_(":: %s and %s are in conflict\n"),
- conflict->package1, conflict->package2);
- } else {
- printf(_(":: %s and %s are in conflict (%s)\n"),
- conflict->package1, conflict->package2, conflict->reason);
- }
- }
- break;
- default:
- break;
- }
- trans_release();
- FREELIST(data);
- return 1;
- }
-
- /* Step 3: perform the installation */
- alpm_list_t *packages = alpm_trans_get_add(config->handle);
-
- if(config->print) {
- print_packages(packages);
- trans_release();
- return 0;
- }
-
- /* print targets and ask user confirmation */
- if(packages == NULL) { /* we are done */
- printf(_(" there is nothing to do\n"));
- trans_release();
- return retval;
- }
- display_targets();
- printf("\n");
- int confirm = yesno(_("Proceed with installation?"));
- if(!confirm) {
- trans_release();
- return retval;
- }
-
- if(alpm_trans_commit(config->handle, &data) == -1) {
- enum _alpm_errno_t err = alpm_errno(config->handle);
- pm_fprintf(stderr, ALPM_LOG_ERROR, _("failed to commit transaction (%s)\n"),
- alpm_strerror(err));
- switch(err) {
- case ALPM_ERR_FILE_CONFLICTS:
- for(i = data; i; i = alpm_list_next(i)) {
- alpm_fileconflict_t *conflict = alpm_list_getdata(i);
- switch(conflict->type) {
- case ALPM_FILECONFLICT_TARGET:
- printf(_("%s exists in both '%s' and '%s'\n"),
- conflict->file, conflict->target, conflict->ctarget);
- break;
- case ALPM_FILECONFLICT_FILESYSTEM:
- printf(_("%s: %s exists in filesystem\n"),
- conflict->target, conflict->file);
- break;
- }
- }
- break;
- case ALPM_ERR_PKG_INVALID:
- case ALPM_ERR_DLT_INVALID:
- for(i = data; i; i = alpm_list_next(i)) {
- char *filename = alpm_list_getdata(i);
- printf(_("%s is invalid or corrupted\n"), filename);
- }
- break;
- default:
- break;
- }
- FREELIST(data);
- trans_release();
- return 1;
- }
-
- if(trans_release() == -1) {
- retval = 1;
- }
- return retval;
+ /* now that targets are resolved, we can hand it all off to the sync code */
+ return sync_prepare_execute();
}
/* vim: set ts=2 sw=2 noet: */
--
1.7.6
More information about the pacman-dev
mailing list