On Fri, 15 Feb 2013 15:06:17 +1000
Allan McRae <[email protected]> wrote:

> On 12/02/13 22:23, Allan McRae wrote:
> > On 12/02/13 22:05, Allan McRae wrote:
> >> Pacman currently bails when trying to extract a file over a directory
> >> when using --force.  Instead of ignoring all conflict, perform the
> >> check and skip any file-file conflicts. Conflicts between directories
> >> and files are still flagged and cause the transation to abort.
> >>
> >> As a bonus, we now know about files changing packages when using
> >> --force, so we can skip removing them fixing upgrade046.
> >>
> >> Signed-off-by: Allan McRae <[email protected]>
> >> ---
> >>  doc/pacman.8.txt                |  2 ++
> >>  lib/libalpm/conflict.c          | 26 ++++++++++++++++++++++++--
> >>  lib/libalpm/sync.c              |  2 +-
> >>  src/pacman/sync.c               |  3 +++
> >>  test/pacman/tests/upgrade046.py |  2 --
> >>  5 files changed, 30 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt
> >> index 358d506..33a9421 100644
> >> --- a/doc/pacman.8.txt
> >> +++ b/doc/pacman.8.txt
> >> @@ -205,6 +205,8 @@ Upgrade Options (apply to '-S' and '-U')[[UO]]
> >>    Bypass file conflict checks and overwrite conflicting files. If the
> >>    package that is about to be installed contains files that are already
> >>    installed, this option will cause all those files to be overwritten.
> >> +  Using '--force' will not allow overwriting a directory with a file or
> >> +  installing packages with conflicting files and directories.
> >>    This option should be used with care, ideally not at all.
> >>  
> >>  *\--asdeps*::
> >> diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c
> >> index 610e5ad..6195c9c 100644
> >> --- a/lib/libalpm/conflict.c
> >> +++ b/lib/libalpm/conflict.c
> >> @@ -439,8 +439,10 @@ alpm_list_t 
> >> *_alpm_db_find_fileconflicts(alpm_handle_t *handle,
> >>                    alpm_pkg_t *p2 = j->data;
> >>                    _alpm_filelist_resolve(handle, alpm_pkg_get_files(p2));
> >>  
> >> -                  common_files = 
> >> _alpm_filelist_intersection(alpm_pkg_get_files(p1),
> >> -                                  alpm_pkg_get_files(p2));
> >> +                  alpm_filelist_t *p1_files = alpm_pkg_get_files(p1);
> >> +                  alpm_filelist_t *p2_files = alpm_pkg_get_files(p2);
> >> +
> >> +                  common_files = _alpm_filelist_intersection(p1_files, 
> >> p2_files);
> >>  
> >>                    if(common_files) {
> >>                            alpm_list_t *k;
> >> @@ -448,6 +450,18 @@ alpm_list_t 
> >> *_alpm_db_find_fileconflicts(alpm_handle_t *handle,
> >>                            for(k = common_files; k; k = k->next) {
> >>                                    alpm_file_t *file = k->data;
> >>                                    snprintf(path, PATH_MAX, "%s%s", 
> >> handle->root, file->name);
> >> +
> >> +                                  /* can skip file-file conflicts when 
> >> forced */
> 
> Clarified comment:
> 
> /* can skip file-file conflicts when forced
>  * the trailing "/" has been stripped from directories when
>  * calculating the intersection, so only files will succeed in
>  * alpm_filelist_contains() */
> 

It strips the trailing '/' for the comparison, but the files returned
include it.  Since dir-dir intersections are not returned, just
checking p2_files would be enough to ensure that the path is a file in
both lists. 

> >> +                                  if((handle->trans->flags & 
> >> ALPM_TRANS_FLAG_FORCE) &&
> >> +                                                  
> >> alpm_filelist_contains(p1_files, file->name) &&
> >> +                                                  
> >> alpm_filelist_contains(p2_files, file->name)) {
> >> +                                          _alpm_log(handle, 
> >> ALPM_LOG_DEBUG,
> >> +                                                  "%s exists in both '%s' 
> >> and '%s'\n", file->name,
> >> +                                                  p1->name, p2->name);
> >> +                                          _alpm_log(handle, 
> >> ALPM_LOG_DEBUG,
> >> +                                                  "file-file conflict 
> >> being forced\n");
> >> +                                          continue;
> >> +                                  }
> >>                                    conflicts = add_fileconflict(handle, 
> >> conflicts, path, p1, p2);
> >>                                    if(handle->pm_errno == ALPM_ERR_MEMORY) 
> >> {
> >>                                            FREELIST(conflicts);
> >> @@ -601,6 +615,14 @@ alpm_list_t 
> >> *_alpm_db_find_fileconflicts(alpm_handle_t *handle,
> >>                            }
> >>                    }
> >>  
> >> +                  /* skip file-file conflicts when being forced */
> >> +                  if((handle->trans->flags & ALPM_TRANS_FLAG_FORCE) &&
> >> +                                  !S_ISDIR(file->mode) && 
> >> !S_ISDIR(lsbuf.st_mode)) {
> > 
> > Overly strict...  it is fine overwritting a file on the filesystem with
> > a directory, just no a directory with a file:
> > 
> > +                                       !S_ISDIR(lsbuf.st_mode)) {
> > 
> >> +                          _alpm_log(handle, ALPM_LOG_DEBUG,
> >> +                                                  "file-file conflict 
> >> being force\n");
> > 
> > +                                                       "conflict with
> > file on filesystem being forced\n");
> > 
> > 
> >> +                          resolved_conflict = 1;
> >> +                  }
> >> +
> >>                    if(!resolved_conflict) {
> >>                            conflicts = add_fileconflict(handle, conflicts, 
> >> path, p1, NULL);
> >>                            if(handle->pm_errno == ALPM_ERR_MEMORY) {
> >> diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
> >> index be55500..15e6957 100644
> >> --- a/lib/libalpm/sync.c
> >> +++ b/lib/libalpm/sync.c
> >> @@ -1241,7 +1241,7 @@ int _alpm_sync_commit(alpm_handle_t *handle, 
> >> alpm_list_t **data)
> >>    trans->state = STATE_COMMITING;
> >>  
> >>    /* fileconflict check */
> >> -  if(!(trans->flags & (ALPM_TRANS_FLAG_FORCE|ALPM_TRANS_FLAG_DBONLY))) {
> >> +  if(!(trans->flags & ALPM_TRANS_FLAG_DBONLY)) {
> >>            EVENT(handle, ALPM_EVENT_FILECONFLICTS_START, NULL, NULL);
> >>  
> >>            _alpm_log(handle, ALPM_LOG_DEBUG, "looking for file 
> >> conflicts\n");
> >> diff --git a/src/pacman/sync.c b/src/pacman/sync.c
> >> index 417773d..924cdf5 100644
> >> --- a/src/pacman/sync.c
> >> +++ b/src/pacman/sync.c
> >> @@ -881,6 +881,9 @@ int sync_prepare_execute(void)
> >>                    alpm_strerror(err));
> >>            switch(err) {
> >>                    case ALPM_ERR_FILE_CONFLICTS:
> >> +                          if(config->flags & ALPM_TRANS_FLAG_FORCE) {
> >> +                                  printf(_("unable to %s directory-file 
> >> conflicts\n"), "--force");
> >> +                          }
> >>                            for(i = data; i; i = alpm_list_next(i)) {
> >>                                    alpm_fileconflict_t *conflict = i->data;
> >>                                    switch(conflict->type) {
> >> diff --git a/test/pacman/tests/upgrade046.py 
> >> b/test/pacman/tests/upgrade046.py
> >> index a02a713..d9ac66b 100644
> >> --- a/test/pacman/tests/upgrade046.py
> >> +++ b/test/pacman/tests/upgrade046.py
> >> @@ -29,5 +29,3 @@
> >>  self.addrule("FILE_MODIFIED=bin/foobar")
> >>  self.addrule("FILE_EXIST=usr/share/file")
> >>  self.addrule("FILE_MODIFIED=usr/share/file")
> >> -
> >> -self.expectfailure = True
> >>
> > 
> > 
> > 
> > 
> 
> 


Reply via email to