Re: Recurisive directory unset

[Havoc Pennington <hp redhat com>]

Mark McLoughlin <mark skynet ie> writes: 
> 	Anyway I don't understand why you said "there's no mkdir/rmdir
> operation". Unless, I'm mistaken that's what the remove_dir backend
> vtable operation and gconf_engine_remove_dir is ?

Hmm, you're right. ;-) Apparently when talking to Glynn I was
imagining what I thought it should do instead of what it really does.

The remove_dir vtable operation is probably stupid and makes no sense,
because if you don't create directories you shouldn't have to remove
them. Also for some reason it never made it to GConfClient -
gconf_client_remove_dir() does something else entirely. And at the
moment gconf_engine_remove_dir() will warn in the presence of a
GConfClient, though that could be taken out.

Maybe we should edit xml-dir.c to delete the directory in
dir_unset_value if the dir becomes empty, and undo the deletion in
dir_set_value if required.

On the down side, this might be sort of confusing - e.g. gconf-editor
would find it tricky to keep track of directories, since they would
disappear sometimes, and there is no notification on that.
I suppose gconf-editor could just figure an empty directory was gone,
or even leave directories around to avoid user-visible oddness, though
it would then need to expect all_dirs() to return something different
from its internal state.

> 	1) Add a --recursive-delete-dir option to gconftool and fix an
> 	assertion in the xml backend. (Could you comment on the two
> 	FIXMEs I added? )

I don't think we need --recursive-delete-dir, --recursive-unset should
just be deleting directories. We could do this by adding
gconf_engine_remove_dir() to --recursive-unset, or by changing the XML
backend as described above. Hmm.

> 	2) Remove calls to POA::activate_object, which are useless on
> 	the RootPOA, since the RootPOA supports implicit activation
> 	... and you were leaking ObjectIds anyway :-)

OK, I'll take the word of the ORBit experts on this, I was just
cut-and-pasting some early-GNOME-1 boilerplate from someplace. ;-)

> 	May I commit ?

Please commit the POA fixes and XML backend fixes, but I would rather
just change --recursive-unset in gconftool, instead of adding a new
option.  Or if we make removing all keys in a dir implicitly drop the
dir, --recursive-unset will work as written (this was what I was
intending when I wrote it).

>    GHashTable* entry_cache; /* store key-value entries */
> -  GHashTable* subdir_cache; /* store subdirectories */
> +  GHashTable* subdir_cache; /* store subdirectories : FIMXE: not used? */

If it's not used let's take it out. 
 
> -  /* We should have a doc if dirty is TRUE */
> -  g_assert(d->doc != NULL);
> -

This points out a bug, which is that dir_set_value() doesn't work on
directories marked deleted.

>    d->last_access = time(NULL);
> 
>    if (d->deleted)
>      {
> +      /*
> +       * FIXME: should we recursively sync all subdirs here to be
> +       *        sure any deleted subdirs get deleted first?
> +       */
> +

See the comment in cache_delete_dir_by_pointer() on what is supposed
to happen, though what that comment describes seems a bit suspicious
if you consider that a directory can be implicitly re-added prior to
sync by doing a dir_set_value().

> +  /*
> +   * We have to do this now, because we must
> +   * ensure that directories are deleted in order.
> +   */
> +  gconf_engine_suggest_sync (conf, &err);

See, that shouldn't be needed, the XML backend is just horked.

Thanks for looking into this.

Havoc