[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

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



On Sun, Feb 07, 2010 at 08:51:27PM +0100, Guillem Jover wrote:
> > 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.

i agree, but i didn't want to make substantial modifications to the
general ui / flow of things without having a discussion here first.

so... could you propose what it ought to do / prompt after a successful
and/or failed merge?

> > 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.

huh... not sure how that happened.  fixed.

> > +int
> > +conffiledb_automerge(const struct pkginfo *pkg, const char *path)
> > +{
> [...]
> > +	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.

Added.

> > +		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.

indeed.


	sean

Attachment: signature.asc
Description: Digital signature


Reply to: