Re: [PATCH 6/7] Implement 'm' option for conffile merging during conflict resolution

[Guillem Jover <guillem@debian.org>]

Hi!

On Mon, 2009-12-14 at 08:27:36 +0100, Sean Finney wrote:
> The conflict resolution prompt has been updated with a new option for
> automatic merging:
> 
>           M     : attempt to automatically merge the differences
> 
> This functions by performing a 3-way diff using the new conffile, the
> currently-installed conffile, and the pristine version of the conffile
> from the currently installed version of the package (which is retreived
> from the conffile database). The merge output is generated alongside the
> conffile, in the form <path>.dpkg-merge.
> 
> On a successful merge, the merge output replaces the conffile, and dpkg
> continues as if the admin had selected to keep the old conffile. On merge
> failure the admin is informed of the failure and the the conflict prompt
> is redisplayed. In such an event the merge output will still be present
> in case it is useful for merge resolution.

I don't think blindly accepting a successful automerge is a safe
option, we probably want to give the user the opportunity to inspect
the result first.

> This commit also contains updates to the conffiledb unit tests, covering
> the new merge functionality.

> diff --git a/src/conffiledb.c b/src/conffiledb.c
> index 2dec3b5..baaf7ba 100644
> --- a/src/conffiledb.c
> +++ b/src/conffiledb.c
> @@ -25,17 +25,19 @@
>  #include <unistd.h>
>  #include <sys/types.h>
>  #include <sys/stat.h>
> -#include <sys/wait.h>
>  #include <fcntl.h>
>  #include <stdlib.h>
>  #include <errno.h>
> +#include <sys/wait.h>

Unneeded change.

> +int
> +conffiledb_automerge(const struct pkginfo *pkg, const char *path)
> +{
[...]
> +	/* create a file for the merge results */
> +	varbufprintf(&merge_output_fname, "%s%s", path, DPKGMERGEEXT);
> +	merge_fd = open(merge_output_fname.buf, O_CREAT|O_TRUNC|O_WRONLY, 0600);
> +	if (merge_fd == -1)
> +		ohshite(_("open '%s' failed"), merge_output_fname.buf);

> +	/* now copy the piped output into the merge results file */
> +	if (close(merge_pipe[1]) == -1)
> +		ohshite(_("close on pipe failed"));
> +	fd_fd_copy(merge_pipe[0], merge_fd, -1, _("conffiledb merge output"));

Missing fsync here.

> +	if (close(merge_fd) == -1)
> +		ohshite(_("close '%s' failed"), merge_output_fname.buf);

> @@ -714,6 +716,18 @@ promptconfaction(struct pkginfo *pkg, const char *cfgfile,
>  			free(prev_conffile);
>  		}
>  
> +		if (cc == 'm') {
> +			fprintf(stderr, _("attempting automatic merge of %s: "), cfgfile);
> +			if (conffiledb_automerge(pkg, cfgfile) == 0) {
> +				fprintf(stderr, _("success.\n"));
> +				/* so let's just pretend they said "keep my
> +				 * version" ;) */
> +				cc = 'n';
> +			} else {
> +				fprintf(stderr, _("failed.\n"));
> +			}
> +		}

So if we don't pretend to have said “keep it”, we might still want to
keep the state for that result for the next iteration. And probably it
makes sense to add a new --force-confmerge or similar to automatically
accept successful merges for the adventurous.

regards,
guillem