2008/2/11 Nagy Gabor <ngaba@bibl.u-szeged.hu>:
Nagy, we've discussed this before in the past. *Please* do not just point out problems with the expectation that we will fix it. I know you are a very competent coder, and I'd appreciate you contributing solutions rather than inferring our code is shit. Saying things like "totally needless" does not start things off on the right foot with Aaron and I, who have really tried hard to clean up some questionable code. We do make mistakes.
Oh, I didn't want to say, that your code is shit (however: serious). Instead, that function is quite tricky, and not easy to understand at all (that's why I didn't provide a patch yet...: 1. I'm also quite tired/ill nowadays => I don't trust on myself now; 2. I want to do a bigger rewrite of that part)
The word "totally needless" referred to the following: there is (should be) no difference between empty and n-element list, so that pB==NULL check is not needed <- the incorrect handling of pB==NULL was a special case of this BUG.
diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c index c093705..9ef5c9f 100644 --- a/lib/libalpm/conflict.c +++ b/lib/libalpm/conflict.c @@ -279,6 +279,12 @@ static alpm_list_t *chk_filedifference(alpm_list_t *filesA, alpm_list_t *filesB) } } } + /* ensure we have completely emptied pA */ + while(pA) { + const char *strA = pA->data; + ret = alpm_list_add(ret, strdup(strA)); + pA = pA->next; + }
return(ret); }
This seems OK, but the previous code filters out directories, so this is needed here too (this was missed in the old pB==NULL handler part, too).
Ahh, you are correct. If you would have said this originally, then I would understand why this problem was not easy at first sight. :)
This patch fixes the revised upgrade040 and 041 pactests, as well as upgrade011 as attached. It doesn't fix upgrade046.py. Any thoughts for that issue? I'm not familiar enough with the whole program flow there yet.
Yes, for fixing upgrade046.py bigger efforts are needed (however, that is probably rarer issue). The problem is, when you use --force, the _alpm_db_find_fileconflicts is not called at all. And this file relocation "stuff" (skip_remove) is computed in that function; so this computation is not done at all with --force.
Yeah, interesting. We may have to rework this somehow...hmm. -Dan