On Mon, Feb 08, 2010 at 02:55:49AM +0100, Guillem Jover wrote:
> On Mon, 2009-12-14 at 08:27:37 +0100, Sean Finney wrote:
> > When a merge is successful, store the common ancestor of the merge (the
> > pristine version of the conffile from the previous package version) in
> > a special location. While this may not have an immediate benefit, it poses
> > the oppurtunity for further merge resolution options in future merges.
>
> I don't think that's the only case were we want to keep the ancestor,
> this seems to miss the case discussed on the list on one of the previous
> threads involving a pristine conffile modified by the admin, then few
> pristine versions skipped with --force-confold and later on a merge
> happens.
okay, this can be added on pretty easily.
> > When the admin later chooses to install the package-shipped version of
> > a conffile, or when the package is purged, this information is removed
> > from the conffiledb as it is no longer relevant.
>
> > diff --git a/lib/dpkg/test/t-conffiledb.c b/lib/dpkg/test/t-conffiledb.c
> > index 8b07e5f..2c32322 100644
> > --- a/lib/dpkg/test/t-conffiledb.c
> > +++ b/lib/dpkg/test/t-conffiledb.c
> > @@ -40,8 +40,8 @@ void ensure_pathname_nonexisting(const char *pathname);
> > -char *conffiledbdir;
> > +char *conffiledbdir, *mergeparentdir;
>
> This one static too.
Done.
> > @@ -142,7 +150,9 @@ conffiledb_path_cversion(const struct pkginfo *pkg, const char *version,
> Abusing those functions to pass something not being a version feels a
> bit dirty, and makes them a bit cryptic.
as stated earlier, we have a disagreement on this. i *do* see these
as versions (ones seperate from the package versioning, but versions
nonetheless).
> > +void
> > +conffiledb_set_merge_parent(const struct pkginfo *pkg,
> > + const struct versionrevision *version,
> > + const char *path)
> > +{
>
> The name of the function seems a bit confusing given that it does not
> need to be restricted to just merge parents, but just pristine parents
> of the currently modified conffile.
when the other pristine parent case that you mention above is implemented,
i think it makes sense to rename this function and use it for both.
> > + tmp_fd = open(tmp_fname_buf.buf, O_CREAT|O_TRUNC|O_WRONLY, 0600);
> > + if (tmp_fd == -1)
> > + ohshite(_("open '%s' failed"), tmp_fname_buf.buf);
> > +
> > + fd_fd_copy(src_fd, tmp_fd, -1,
> > + _("recording conffiledb merge ancestor for '%.255s'"),
> > + path_quote_filename(quoted_filename, src_fname, 256));
>
> Missing fsync here.
Fixed.
> > +void
> > +conffiledb_remove_merge_parent(const struct pkginfo *pkg, const char *path)
> > +{
> > + char *pkg_dir;
> > +
> > + pkg_dir = conffiledb_path_cversion(pkg, MERGEPARENTDIR, path);
> > + ensure_pathname_nonexisting(pkg_dir);
>
> unlink() should be enough in this case.
Fixed.
So, that's the last of 'em. Look forward to hearing back from you.
sean
--
Attachment:
signature.asc
Description: Digital signature