[pacman-dev] pacman doesn't remove lockfile after -Sy failure

[Dan McGee]

On 10/3/07, Xavier <shiningxc at gmail.com> wrote:
> On Wed, Oct 03, 2007 at 07:54:26PM +0200, Nagy Gabor wrote:
> > Hi!
> > I said everything in the subject.
> > You can test with
> > /etc/rc.d/network stop
> > pacman -Sy
> > pacman -Sy
> >
>
> I think this has been introduced by :
> http://projects.archlinux.org/git/?p=pacman.git;a=commitdiff;h=db9e10f14215ab6453cd663a62cae5bdfac3d61b#patch2
>
> See, when -Sy fails, alpm_trans_release is not called, only alpm_release in
> src/pacman/pacman.c , cleanup function.
>
> Previously, alpm_release would still call alpm_trans_release when needed. But
> it doesn't anymore.
> However, this is not necessarily a problem.
> When Sy fails, it could go to the cleanup part instead of returning
> directly, so that alpm_trans_release is called.
>
> Here is a patch, not based on git though, but on toofishes working branch,
> which reverted the other broken commit.
>
>
> >From b5a0f808e43c9145c1612ea7ecf8f2dce38f1936 Mon Sep 17 00:00:00 2001
> From: Chantry Xavier <shiningxc at gmail.com>
> Date: Wed, 3 Oct 2007 22:13:32 +0200
> Subject: [PATCH] pacman/sync.c : exit cleanly when synchronizing fails.
>
> Signed-off-by: Chantry Xavier <shiningxc at gmail.com>
> ---
>  src/pacman/sync.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/src/pacman/sync.c b/src/pacman/sync.c
> index 11941eb..02b8d02 100644
> --- a/src/pacman/sync.c
> +++ b/src/pacman/sync.c
> @@ -394,6 +394,7 @@ int pacman_sync(alpm_list_t *targets)
>  {
>         int retval = 0;
>         alpm_list_t *sync_dbs = NULL;
> +       alpm_list_t *data = NULL;
>
>         /* clean the cache */
>         if(config->op_s_clean) {
> @@ -454,7 +455,8 @@ int pacman_sync(alpm_list_t *targets)
>                 alpm_logaction("synchronizing package lists");
>                 if(!sync_synctree(config->op_s_sync, sync_dbs)) {
>                         fprintf(stderr, _("error: failed to synchronize any databases\n"));
> -                       return(1);
> +                       retval = 1;
> +                       goto cleanup;
>                 }
>         }
>
> @@ -585,7 +587,6 @@ int pacman_sync(alpm_list_t *targets)
>         }
>
>         /* Step 2: "compute" the transaction based on targets and flags */
> -       alpm_list_t *data;
>         if(alpm_trans_prepare(&data) == -1) {
>                 fprintf(stderr, _("error: failed to prepare transaction (%s)\n"),
>                         alpm_strerrorlast());
> --
> 1.5.3.3

Lost track of this...Xavier, can you give me an update? Do we still
need to merge something like this?

-Dan