[pacman-dev] [PATCH] Remove trans->targets

[Xavier]

On Mon, Mar 10, 2008 at 07:07:04PM -0500, Dan McGee wrote:
> Your commit message (after I wrapped at 76 chars):
> 
> Remove trans->targets
> 
> Its implementation was quite broken:
> * add_loadtarget() might have silently filtered out some targets (replacing
>   older version...), while ->target was recorded by trans_loadtarget =>
>   confused add_commit due to list_count(trans->targets)
> * this was used in sync.c to determine whether a target is implicit or not;
>   wrong: 'repo/pkg' [now we give warning always; imho this is acceptable,
>   before this patch we silently removed user confirmed replacements too]
> * now remove001.py fails (imho this is the natural behavior)
> 
> The patch looks fine, but a few comments. I struggle to get through
> your commit message. Code uses (), [], -> and ==>. English should not.
> I know it is not your first language, but can you please step away
> from the symbols when you write up your message? Example:

I agree that the form is a bit confusing :)

> 
> Remove trans->targets
> 
> The implementation was quite broken:
> * add_loadtarget() could possibly silently filter out some targets
> when it replaced an older version. <not even sure what the rest meant>
> When target was recorded by trans_loadtarget(), add_commit() was
> mislead due to the incorrect list_count(trans->targets) call.

Well, add_loadtarget silently skipped the target, which means it didn't add
it to trans->packages, but trans_loadtarget still added it to trans->targets.
That means that trans->targets and trans->packages were not in sync anymore.
So add_commit should use list_count(trans->packages) rather than
list_count(trans->targets).

> * The target list was used in sync.c to determine whether a target was
> implicit or not. This was incorrect behavior 

The wrong behavior wasn't clearly explained, only by the 'repo/pkg'.
It seems that I indeed overlooked something. That usage of the targets list
wasn't corect, because I forgot about the 'repo/pkg' syntax that could be used
in the targets list, and didn't take this into account.

> and we could silently
> remove user-confirmed replacements as well.


> * remove001.py was an odd test in that it installed the same package 5
> times, this behavior is not necessary if the library does its job
> correctly so should be changed.
> 
> I REALLY appreciate your work, but I would appreciate it even more if
> you could just simplify your commit writeup a bit.
> 
> -Dan
> 
> _______________________________________________
> pacman-dev mailing list
> pacman-dev at archlinux.org
> http://archlinux.org/mailman/listinfo/pacman-dev