Nagy, would that patch work for you?

Wow, you are very active nowadays ;-)

Well, the patch is much simpler than expected. With -R it seems perfect.

1. If I understand correctly, this won't work with -S replaces, right? Because sync_commit doesn't pass callback functions to the created remove transaction. But this can be changed easily.
2. And maybe do a trans != REMOVEUPGRADE check for safety (now we skip the prepare part of REMOVEUPGRADE, so this is not a real problem).
3. My first thought was after reading your simple (=> nice) patch: Couldn't we implement this in front-end? (Do this check just before remove_commit in front-end). Of course after moving the whole holdpkg stuff there?
Pro:
+ simpler, cleaner back-end
Contras:
- in backend, we can do more sophisticated stuffs with holdpkg than in front-end, if needed (for example: ask confirmation before pulling -Rs selected holdpkg _dependency_)
- in front-end we should implement this twice: for -R and for -S %REPLACES% (if we want that)

1. Well, in my opinion, replacing is similar to upgrading in the sense that the functionality is preserved. So HoldPkg is not so important here.
2. Indeed, I saw it was skipped so I didn't add this check, but I agree it would be safer.
3. As you said, the current patch is very simple. Moving this to frontend would be a bit more work, and as you said, there are both advantages and inconvenient, so it is not clearly better. So I will propose this patch to Dan as it is. You are welcome to write a patch to move it to frontend if you like it better :)

To sum up, the only thing I am going to change to the patch is 2. (adding REMOVEUPGRADE check).
Thanks for the comments.

Well, I changed my mind, I think the code is clear enough as it is :

/* skip all checks if we are doing this removal as part of an upgrade */
if(trans->type == PM_TRANS_TYPE_REMOVEUPGRADE) {
return(0);
}

So checking this again as the end of the remove_prepare function looks weird, and it adds another level to the code, which I dislike :)

1. Well, this is the harder to implement;-) Without modification your patch will automatically refuse to replace a holdpkg package. Btw, I agree that replacement is similar to upgrade, so holdpkg handling is not important here, indeed.
3. But this just gave me one more reason for implement HoldPkg in front-end, it eliminated my second contra. Probably I will write my front-end version based on your patch. I am pretty sure now, that holdpkg shouldn't belong to libalpm.

And if we prefer simplicity (ie. we don't implement sophisticated holdpkg stuffs, like in my first contra), the --holdpkg option (the 1. point in my request) is useless.

"Well, I changed my mind, I think the code is clear enough as it is :"
Indeed, I've missed that. Thx.

Hmm ok then, I will wait for your frontend version :)

Hm. I've just realised, that -S removes can happen due to conflict resolving. This is an edge case (conflict resolving + holdpkg), but I should have thought about that earlier :-/

Don't worry about this case either. Usually when a package conflicts with another, it is because it provides the same thing (even the same files!).
So no problem.

That's true, but I don't like these assumptions. Theoretically that case should be handled too.

1. Well, during this discussion I convinced myself about the fact that _atm_ HoldPkg is just a spam in alpm. If the simple "just before trans_commit check" is enough, I vote for keeping my ML patch, and implement -S removes check later. (I suspect that with universal transaction we can share this holdpkg check codepart easily between -S and -R.)

2. On the other side, I always kept holdpkg analogous to ignorepkg, that's why I was suprised when you showed me your cool small patch. To be more concrete, I like ignorepkg because I can _affect_ some libalpm mechanisms, see resolvedeps for example. With (an "advanced") holdpkg user could affect -Rs, which would mean: select all unused dependencies except holdpkg (that's why I first thought about --holdpkg option). In this case holdpkg-stuff asks confirmation for "adding holdpkg to transaction" not confirmation for "continue" (so HoldPkg acts like a filter similar to ignorepkg). Ignorepkg and holdpkg represent some kind of "hidden" packages in this interpretation.

So the question is, whether it is worth thinking about 2 or no? (In future?) If no, ie. we choose the simple holdpkg (pre-commit check), I can confirm my ML-patch.

1. Ok

2. I see your point and it is interesting, but in my opinion it is not that useful. I never felt the need for that, and never saw any user requesting it.

Status of this?
The patch is applied in my git repo.