>From 01ba4fce1e7bf2c6058074c583e5db5147d31f02 Mon Sep 17 00:00:00 2001
From: Nagy Gabor <[email protected]>
Date: Fri, 2 Jan 2009 17:43:05 +0100
Subject: [PATCH] Fix for trans001.py (FS#9088)

>From now on _alpm_db_find_fileconflicts() works with upgrade and remove
target lists (like checkdeps), which makes it transaction independent
(we still need a trans param because of the progressbar). This is a small
step towards the universal transaction. So we call this function directly
from sync.c before commiting the remove transaction. This is much safer,
but we can get false fileconflict error alarms in some tricky cases
("symlinks puzzle" etc).

The patch on find_fileconflict looks complex, but it is mainly an
"indent-patch", the new code-part can be found after the
/* check remove list ... */ comment, and I modified something around the
"file has changed hand" case (see comment modifications in the code).

Unfortunately sync.c became more ugly, because we have to create 2 parallel
internal transactions: to avoid duplicated work, upgrade transaction is
used to load package data (filelists). This problem will disappear, when
we finally get rid of internal transactions.

Signed-off-by: Nagy Gabor <[email protected]>
---
 lib/libalpm/add.c         |    2 +-
 lib/libalpm/conflict.c    |   90 ++++++++++++++++++------------------
 lib/libalpm/conflict.h    |    3 +-
 lib/libalpm/sync.c        |  111 ++++++++++++++++++++++++++-------------------
 pactest/tests/trans001.py |    2 -
 5 files changed, 112 insertions(+), 96 deletions(-)

diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
index 5098bfe..8b82ce7 100644
--- a/lib/libalpm/add.c
+++ b/lib/libalpm/add.c
@@ -166,7 +166,7 @@ int _alpm_add_prepare(pmtrans_t *trans, pmdb_t *db, 
alpm_list_t **data)
                EVENT(trans, PM_TRANS_EVT_FILECONFLICTS_START, NULL, NULL);
 
                _alpm_log(PM_LOG_DEBUG, "looking for file conflicts\n");
-               lp = _alpm_db_find_fileconflicts(db, trans, handle->root);
+               lp = _alpm_db_find_fileconflicts(db, trans, trans->packages, 
NULL);
                if(lp != NULL) {
                        if(data) {
                                *data = lp;
diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c
index 499e55a..139176a 100644
--- a/lib/libalpm/conflict.c
+++ b/lib/libalpm/conflict.c
@@ -350,16 +350,16 @@ void _alpm_fileconflict_free(pmfileconflict_t *conflict)
 /* Find file conflicts that may occur during the transaction with two checks:
  * 1: check every target against every target
  * 2: check every target against the filesystem */
-alpm_list_t *_alpm_db_find_fileconflicts(pmdb_t *db, pmtrans_t *trans, char 
*root)
+alpm_list_t *_alpm_db_find_fileconflicts(pmdb_t *db, pmtrans_t *trans,
+                                        alpm_list_t *upgrade, alpm_list_t 
*remove)
 {
-       alpm_list_t *i, *conflicts = NULL;
-       alpm_list_t *targets = trans->packages;
-       int numtargs = alpm_list_count(targets);
+       alpm_list_t *i, *j, *conflicts = NULL;
+       int numtargs = alpm_list_count(upgrade);
        int current;
 
        ALPM_LOG_FUNC;
 
-       if(db == NULL || targets == NULL || root == NULL) {
+       if(db == NULL || upgrade == NULL) {
                return(NULL);
        }
 
@@ -367,8 +367,8 @@ alpm_list_t *_alpm_db_find_fileconflicts(pmdb_t *db, 
pmtrans_t *trans, char *roo
         * be possible with real transactions. Right now we only do half as much
         * here as we do when we actually extract files in add.c with our 12
         * different cases. */
-       for(current = 1, i = targets; i; i = i->next, current++) {
-               alpm_list_t *j, *k, *tmpfiles = NULL;
+       for(current = 1, i = upgrade; i; i = i->next, current++) {
+               alpm_list_t *k, *tmpfiles = NULL;
                pmpkg_t *p1, *p2, *dbpkg;
                char path[PATH_MAX+1];
 
@@ -392,7 +392,7 @@ alpm_list_t *_alpm_db_find_fileconflicts(pmdb_t *db, 
pmtrans_t *trans, char *roo
 
                        if(tmpfiles) {
                                for(k = tmpfiles; k; k = k->next) {
-                                       snprintf(path, PATH_MAX, "%s%s", root, 
(char *)k->data);
+                                       snprintf(path, PATH_MAX, "%s%s", 
handle->root, (char *)k->data);
                                        conflicts = add_fileconflict(conflicts, 
PM_FILECONFLICT_TARGET, path,
                                                        alpm_pkg_get_name(p1), 
alpm_pkg_get_name(p2));
                                }
@@ -408,7 +408,7 @@ alpm_list_t *_alpm_db_find_fileconflicts(pmdb_t *db, 
pmtrans_t *trans, char *roo
                _alpm_log(PM_LOG_DEBUG, "searching for filesystem conflicts: 
%s\n", p1->name);
                dbpkg = _alpm_db_get_pkgfromcache(db, p1->name);
 
-               /* Do two different checks here. f the package is currently 
installed,
+               /* Do two different checks here. If the package is currently 
installed,
                 * then only check files that are new in the new package. If 
the package
                 * is not currently installed, then simply stat the whole 
filelist */
                if(dbpkg) {
@@ -420,12 +420,10 @@ alpm_list_t *_alpm_db_find_fileconflicts(pmdb_t *db, 
pmtrans_t *trans, char *roo
                        tmpfiles = alpm_list_strdup(alpm_pkg_get_files(p1));
                }
 
-               /* loop over each file to be installed */
                for(j = tmpfiles; j; j = j->next) {
-                       int skip_conflict = 0;
                        filestr = j->data;
 
-                       snprintf(path, PATH_MAX, "%s%s", root, filestr);
+                       snprintf(path, PATH_MAX, "%s%s", handle->root, filestr);
 
                        /* stat the file - if it exists, do some checks */
                        if(_alpm_lstat(path, &lsbuf) != 0) {
@@ -436,48 +434,50 @@ alpm_list_t *_alpm_db_find_fileconflicts(pmdb_t *db, 
pmtrans_t *trans, char *roo
                        if(path[strlen(path)-1] == '/') {
                                if(S_ISDIR(lsbuf.st_mode)) {
                                        _alpm_log(PM_LOG_DEBUG, "%s is a 
directory, not a conflict\n", path);
-                                       skip_conflict = 1;
+                                       continue;
                                } else if(S_ISLNK(lsbuf.st_mode) && 
S_ISDIR(sbuf.st_mode)) {
                                        _alpm_log(PM_LOG_DEBUG,
                                                        "%s is a symlink to a 
dir, hopefully not a conflict\n", path);
-                                       skip_conflict = 1;
+                                       continue;
                                }
                        }
-                       if(!skip_conflict) {
-                               _alpm_log(PM_LOG_DEBUG, "checking possible 
conflict: %s\n", path);
-
-                               /* Look at all the targets to see if file has 
changed hands */
-                               int resolved_conflict = 0; /* have we acted on 
this conflict? */
-                               for(k = targets; k; k = k->next) {
-                                       p2 = k->data;
-                                       if(!p2 || strcmp(p1->name, p2->name) == 
0) {
-                                               continue;
-                                       }
+                       _alpm_log(PM_LOG_DEBUG, "checking possible conflict: 
%s\n", path);
 
-                                       pmpkg_t *localp2 = 
_alpm_db_get_pkgfromcache(db, p2->name);
-
-                                       /* Check if it used to exist in a 
package, but doesn't anymore */
-                                       alpm_list_t *pkgfiles, *localfiles; /* 
added for readability */
-                                       pkgfiles = alpm_pkg_get_files(p2);
-                                       localfiles = 
alpm_pkg_get_files(localp2);
-
-                                       if(localp2 && 
!alpm_list_find_str(pkgfiles, filestr)
-                                                && 
alpm_list_find_str(localfiles, filestr)) {
-                                               /* skip removal of file, but 
not add. this will prevent a second
-                                                * package from removing the 
file when it was already installed
-                                                * by its new owner (whether 
the file is in backup array or not */
-                                               trans->skip_remove = 
alpm_list_add(trans->skip_remove, strdup(path));
-                                               _alpm_log(PM_LOG_DEBUG, "file 
changed packages, adding to remove skiplist: %s\n", filestr);
-                                               resolved_conflict = 1;
-                                               break;
-                                       }
+                       int resolved_conflict = 0; /* have we acted on this 
conflict? */
+
+                       /* Check remove list (will we remove the conflicting 
local file?) */
+                       for(k = remove; k && !resolved_conflict; k = k->next) {
+                               pmpkg_t *rempkg = k->data;
+                               if(rempkg && 
alpm_list_find_str(alpm_pkg_get_files(rempkg), filestr)) {
+                                       _alpm_log(PM_LOG_DEBUG, "local file 
will be removed, not a conflict: %s\n", filestr);
+                                       resolved_conflict = 1;
                                }
-                               if(!resolved_conflict) {
-                                       _alpm_log(PM_LOG_DEBUG, "file found in 
conflict: %s\n", path);
-                                       conflicts = add_fileconflict(conflicts, 
PM_FILECONFLICT_FILESYSTEM,
-                                                       path, p1->name, NULL);
+                       }
+
+                       /* Look at all the targets to see if file has changed 
hands */
+                       for(k = upgrade; k && !resolved_conflict; k = k->next) {
+                               p2 = k->data;
+                               if(!p2 || strcmp(p1->name, p2->name) == 0) {
+                                       continue;
+                               }
+                               pmpkg_t *localp2 = 
_alpm_db_get_pkgfromcache(db, p2->name);
+
+                               /* localp2->files will be removed (target 
conflicts are handled by CHECK 1) */
+                               if(localp2 && 
alpm_list_find_str(alpm_pkg_get_files(localp2), filestr)) {
+                                       /* skip removal of file, but not add. 
this will prevent a second
+                                        * package from removing the file when 
it was already installed
+                                        * by its new owner (whether the file 
is in backup array or not */
+                                       trans->skip_remove = 
alpm_list_add(trans->skip_remove, strdup(path));
+                                       _alpm_log(PM_LOG_DEBUG, "file changed 
packages, adding to remove skiplist: %s\n", filestr);
+                                       resolved_conflict = 1;
                                }
                        }
+
+                       if(!resolved_conflict) {
+                               _alpm_log(PM_LOG_DEBUG, "file found in 
conflict: %s\n", path);
+                               conflicts = add_fileconflict(conflicts, 
PM_FILECONFLICT_FILESYSTEM,
+                                               path, p1->name, NULL);
+                       }
                }
                FREELIST(tmpfiles);
        }
diff --git a/lib/libalpm/conflict.h b/lib/libalpm/conflict.h
index 71ed579..1c67372 100644
--- a/lib/libalpm/conflict.h
+++ b/lib/libalpm/conflict.h
@@ -41,7 +41,8 @@ void _alpm_conflict_free(pmconflict_t *conflict);
 int _alpm_conflict_isin(pmconflict_t *needle, alpm_list_t *haystack);
 alpm_list_t *_alpm_innerconflicts(alpm_list_t *packages);
 alpm_list_t *_alpm_outerconflicts(pmdb_t *db, alpm_list_t *packages);
-alpm_list_t *_alpm_db_find_fileconflicts(pmdb_t *db, pmtrans_t *trans, char 
*root);
+alpm_list_t *_alpm_db_find_fileconflicts(pmdb_t *db, pmtrans_t *trans,
+                                        alpm_list_t *upgrade, alpm_list_t 
*remove);
 
 void _alpm_fileconflict_free(pmfileconflict_t *conflict);
 
diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
index 607057c..d3b5332 100644
--- a/lib/libalpm/sync.c
+++ b/lib/libalpm/sync.c
@@ -769,7 +769,7 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, 
alpm_list_t **data)
 {
        alpm_list_t *i, *j, *files = NULL;
        alpm_list_t *deltas = NULL;
-       pmtrans_t *tr = NULL;
+       pmtrans_t *tr_remove = NULL, *tr_upgrade = NULL;
        int replaces = 0;
        int errors = 0;
        const char *cachedir = NULL;
@@ -914,61 +914,44 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, 
alpm_list_t **data)
                return(0);
        }
 
-       /* remove conflicting and to-be-replaced packages */
        trans->state = STATE_COMMITING;
-       tr = _alpm_trans_new();
-       if(tr == NULL) {
+
+       /* Create remove and upgrade transactions */
+       tr_remove = _alpm_trans_new();
+       if(tr_remove == NULL) {
                _alpm_log(PM_LOG_ERROR, _("could not create removal 
transaction\n"));
                goto error;
        }
+       tr_upgrade = _alpm_trans_new();
+       if(tr_upgrade == NULL) {
+               _alpm_log(PM_LOG_ERROR, _("could not create transaction\n"));
+               goto error;
+       }
 
-       if(_alpm_trans_init(tr, PM_TRANS_TYPE_REMOVE, PM_TRANS_FLAG_NODEPS, 
NULL, NULL, NULL) == -1) {
+       if(_alpm_trans_init(tr_remove, PM_TRANS_TYPE_REMOVE, 
PM_TRANS_FLAG_NODEPS, NULL, NULL, NULL) == -1) {
                _alpm_log(PM_LOG_ERROR, _("could not initialize the removal 
transaction\n"));
                goto error;
        }
+       if(_alpm_trans_init(tr_upgrade, PM_TRANS_TYPE_UPGRADE, trans->flags, 
trans->cb_event, trans->cb_conv, trans->cb_progress) == -1) {
+               _alpm_log(PM_LOG_ERROR, _("could not initialize 
transaction\n"));
+               goto error;
+       }
 
+       /* adding targets */
        for(i = trans->packages; i; i = i->next) {
                pmsyncpkg_t *sync = i->data;
                alpm_list_t *j;
+               /* remove transaction */
                for(j = sync->removes; j; j = j->next) {
                        pmpkg_t *pkg = j->data;
-                       if(!_alpm_pkg_find(tr->packages, pkg->name)) {
-                               if(_alpm_trans_addtarget(tr, pkg->name) == -1) {
+                       if(!_alpm_pkg_find(tr_remove->packages, pkg->name)) {
+                               if(_alpm_trans_addtarget(tr_remove, pkg->name) 
== -1) {
                                        goto error;
                                }
                                replaces++;
                        }
                }
-       }
-       if(replaces) {
-               _alpm_log(PM_LOG_DEBUG, "removing conflicting and 
to-be-replaced packages\n");
-               if(_alpm_trans_prepare(tr, data) == -1) {
-                       _alpm_log(PM_LOG_ERROR, _("could not prepare removal 
transaction\n"));
-                       goto error;
-               }
-               /* we want the frontend to be aware of commit details */
-               tr->cb_event = trans->cb_event;
-               if(_alpm_trans_commit(tr, NULL) == -1) {
-                       _alpm_log(PM_LOG_ERROR, _("could not commit removal 
transaction\n"));
-                       goto error;
-               }
-       }
-       _alpm_trans_free(tr);
-       tr = NULL;
-
-       /* install targets */
-       _alpm_log(PM_LOG_DEBUG, "installing packages\n");
-       tr = _alpm_trans_new();
-       if(tr == NULL) {
-               _alpm_log(PM_LOG_ERROR, _("could not create transaction\n"));
-               goto error;
-       }
-       if(_alpm_trans_init(tr, PM_TRANS_TYPE_UPGRADE, trans->flags | 
PM_TRANS_FLAG_NODEPS, trans->cb_event, trans->cb_conv, trans->cb_progress) == 
-1) {
-               _alpm_log(PM_LOG_ERROR, _("could not initialize 
transaction\n"));
-               goto error;
-       }
-       for(i = trans->packages; i; i = i->next) {
-               pmsyncpkg_t *sync = i->data;
+               /* upgrade transaction */
                pmpkg_t *spkg = sync->pkg;
                const char *fname;
                char *fpath;
@@ -980,7 +963,7 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, 
alpm_list_t **data)
                /* Loop through the cache dirs until we find a matching file */
                fpath = _alpm_filecache_find(fname);
 
-               if(_alpm_trans_addtarget(tr, fpath) == -1) {
+               if(_alpm_trans_addtarget(tr_upgrade, fpath) == -1) {
                        FREE(fpath);
                        goto error;
                }
@@ -988,15 +971,50 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, 
alpm_list_t **data)
 
                /* using alpm_list_last() is ok because addtarget() adds the 
new target at the
                 * end of the tr->packages list */
-               spkg = alpm_list_last(tr->packages)->data;
+               spkg = alpm_list_last(tr_upgrade->packages)->data;
                spkg->reason = sync->newreason;
        }
-       if(_alpm_trans_prepare(tr, data) == -1) {
-               _alpm_log(PM_LOG_ERROR, _("could not prepare transaction\n"));
-               /* pm_errno is set by trans_prepare */
-               goto error;
+
+       /* fileconflict check */
+       if(!(trans->flags & PM_TRANS_FLAG_FORCE)) {
+               EVENT(trans, PM_TRANS_EVT_FILECONFLICTS_START, NULL, NULL);
+
+               _alpm_log(PM_LOG_DEBUG, "looking for file conflicts\n");
+               alpm_list_t *conflict = _alpm_db_find_fileconflicts(db_local, 
tr_upgrade,
+                                                                   
tr_upgrade->packages, tr_remove->packages);
+               if(conflict) {
+                       pm_errno = PM_ERR_FILE_CONFLICTS;
+                       if(data) {
+                               *data = conflict;
+                       } else {
+                               alpm_list_free_inner(conflict, 
(alpm_list_fn_free)_alpm_fileconflict_free);
+                               alpm_list_free(conflict);
+                       }
+                       goto error;
+               }
+
+               EVENT(trans, PM_TRANS_EVT_FILECONFLICTS_DONE, NULL, NULL);
        }
-       if(_alpm_trans_commit(tr, NULL) == -1) {
+
+       /* remove conflicting and to-be-replaced packages */
+       if(replaces) {
+               _alpm_log(PM_LOG_DEBUG, "removing conflicting and 
to-be-replaced packages\n");
+               if(_alpm_trans_prepare(tr_remove, data) == -1) {
+                       _alpm_log(PM_LOG_ERROR, _("could not prepare removal 
transaction\n"));
+                       goto error;
+               }
+               /* we want the frontend to be aware of commit details */
+               tr_remove->cb_event = trans->cb_event;
+               if(_alpm_trans_commit(tr_remove, NULL) == -1) {
+                       _alpm_log(PM_LOG_ERROR, _("could not commit removal 
transaction\n"));
+                       goto error;
+               }
+       }
+
+       /* install targets */
+       _alpm_log(PM_LOG_DEBUG, "installing packages\n");
+       /* add_prepare is not needed */
+       if(_alpm_trans_commit(tr_upgrade, NULL) == -1) {
                _alpm_log(PM_LOG_ERROR, _("could not commit transaction\n"));
                goto error;
        }
@@ -1005,9 +1023,8 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, 
alpm_list_t **data)
 error:
        FREELIST(files);
        alpm_list_free(deltas);
-       deltas = NULL;
-       _alpm_trans_free(tr);
-       tr = NULL;
+       _alpm_trans_free(tr_remove);
+       _alpm_trans_free(tr_upgrade);
        return(ret);
 }
 
diff --git a/pactest/tests/trans001.py b/pactest/tests/trans001.py
index b3d7883..b9889b1 100644
--- a/pactest/tests/trans001.py
+++ b/pactest/tests/trans001.py
@@ -18,5 +18,3 @@ self.addrule("PACMAN_RETCODE=1")
 self.addrule("!PKG_EXIST=pkg1")
 self.addrule("PKG_EXIST=pkg2")
 self.addrule("PKG_EXIST=pkg3")
-
-self.expectfailure = True
-- 
1.6.1

_______________________________________________
pacman-dev mailing list
[email protected]
http://archlinux.org/mailman/listinfo/pacman-dev

Reply via email to