[pacman-dev] [PATCH] Perform limited conflict checking with --force

[Allan McRae]

On 16/02/13 10:38, Allan McRae wrote:
> On 16/02/13 05:42, Andrew Gregory wrote:
>> On Fri, 15 Feb 2013 15:06:17 +1000
>> Allan McRae <allan at archlinux.org> 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 <allan at archlinux.org>
>>>>> ---
>>>>>  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. 
>>
> 
> So...  that comment is wrong, but we still need to check both of them.
> We want to detect file-file conflicts for skipping, and not dir-file
> conflicts.  So, the file string must be in both packages (whether or not
> the trailing "/" is there - which is somewhat random...).

And now I understand what is being pointed out.  The value from the
first package is always returned, so not point checking there...