augeas/augeas-0.7.2-09-5ee81630.patch

From 5ee8163051be8214507c13c86171ac90ca7cb91f Mon Sep 17 00:00:00 2001
From: David Lutterkort <lutter@redhat.com>
Date: Tue, 29 Jun 2010 15:32:44 -0700
Subject: [PATCH 9/9] Avoid unnecessary file parsing when reloading the tree

We used to reparse every file we knew about upon aug_load. Now, we only
reparse files if the file has changed on disk.

We test a few scenarios to make sure aug_load retains its behavior of
obliterating the tree and filling it with the latest from disk. This
includes throwing away unsaved changes or trees that have been deleted.
---
 src/augeas.c      |   72 ++++++++++++++++++++++++++-
 src/transform.c   |   78 +++++++++++++++++++++++++++--
 tests/cutest.c    |   17 ++++++
 tests/cutest.h    |    4 ++
 tests/test-load.c |  142 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xpath.tests |    1 +
 6 files changed, 307 insertions(+), 7 deletions(-)

diff --git a/src/augeas.c b/src/augeas.c
index 744b699..45c2207 100644
--- a/src/augeas.c
+++ b/src/augeas.c
@@ -451,6 +451,51 @@ void tree_unlink_children(struct augeas *aug, struct tree *tree) {
         tree_unlink(tree->children);
 }
 
+static void tree_mark_files(struct tree *tree) {
+    if (tree_child(tree, "path") != NULL) {
+        tree_mark_dirty(tree);
+    } else {
+        list_for_each(c, tree->children) {
+            tree_mark_files(c);
+        }
+    }
+}
+
+static void tree_rm_dirty_files(struct augeas *aug, struct tree *tree) {
+    struct tree *p;
+
+    if (!tree->dirty)
+        return;
+
+    if ((p = tree_child(tree, "path")) != NULL) {
+        aug_rm(aug, p->value);
+        tree_unlink(tree);
+    } else {
+        struct tree *c = tree->children;
+        while (c != NULL) {
+            struct tree *next = c->next;
+            tree_rm_dirty_files(aug, c);
+            c = next;
+        }
+    }
+}
+
+static void tree_rm_dirty_leaves(struct augeas *aug, struct tree *tree,
+                                 struct tree *protect) {
+    if (! tree->dirty)
+        return;
+
+    struct tree *c = tree->children;
+    while (c != NULL) {
+        struct tree *next = c->next;
+        tree_rm_dirty_leaves(aug, c, protect);
+        c = next;
+    }
+
+    if (tree != protect && tree->children == NULL)
+        tree_unlink(tree);
+}
+
 int aug_load(struct augeas *aug) {
     struct tree *meta = tree_child_cr(aug->origin, s_augeas);
     struct tree *meta_files = tree_child_cr(meta, s_files);
@@ -462,13 +507,36 @@ int aug_load(struct augeas *aug) {
 
     ERR_NOMEM(load == NULL, aug);
 
-    tree_unlink_children(aug, meta_files);
-    tree_unlink_children(aug, files);
+    /* To avoid unnecessary loads of files, we reload an existing file in
+     * several steps:
+     * (1) mark all file nodes under /augeas/files as dirty (and only those)
+     * (2) process all files matched by a lens; we check (in
+     *     transform_load) if the file has been modified. If it has, we
+     *     reparse it. Either way, we clear the dirty flag. We also need to
+     *     reread the file if part or all of it has been modified in the
+     *     tree but not been saved yet
+     * (3) remove all files from the tree that still have a dirty entry
+     *     under /augeas/files. Those files are not processed by any lens
+     *     anymore
+     * (4) Remove entries from /augeas/files and /files that correspond
+     *     to directories without any files of interest
+     */
+    tree_clean(meta_files);
+    tree_mark_files(meta_files);
 
     list_for_each(xfm, load->children) {
         if (transform_validate(aug, xfm) == 0)
             transform_load(aug, xfm);
     }
+
+    /* This makes it possible to spot 'directories' that are now empty
+     * because we removed their file contents */
+    tree_clean(files);
+
+    tree_rm_dirty_files(aug, meta_files);
+    tree_rm_dirty_leaves(aug, meta_files, meta_files);
+    tree_rm_dirty_leaves(aug, files, files);
+
     tree_clean(aug->origin);
 
     list_for_each(v, vars->children) {
diff --git a/src/transform.c b/src/transform.c
index 0c56034..00552da 100644
--- a/src/transform.c
+++ b/src/transform.c
@@ -49,6 +49,7 @@ static const int glob_flags = GLOB_NOSORT;
 /* Loaded files are tracked underneath METATREE. When a file with name
  * FNAME is loaded, certain entries are made under METATREE / FNAME:
  *   path      : path where tree for FNAME is put
+ *   mtime     : time of last modification of the file as reported by stat(2)
  *   lens/info : information about where the applied lens was loaded from
  *   lens/id   : unique hexadecimal id of the lens
  *   error     : indication of errors during processing FNAME, or NULL
@@ -60,6 +61,7 @@ static const int glob_flags = GLOB_NOSORT;
 static const char *const s_path = "path";
 static const char *const s_lens = "lens";
 static const char *const s_info = "info";
+static const char *const s_mtime = "mtime";
 
 static const char *const s_error = "error";
 /* These are all put underneath "error" */
@@ -111,6 +113,59 @@ static bool is_regular_file(const char *path) {
     return S_ISREG(st.st_mode);
 }
 
+static char *mtime_as_string(struct augeas *aug, const char *fname) {
+    int r;
+    struct stat st;
+    char *result = NULL;
+
+    r = stat(fname, &st);
+    if (r < 0) {
+        /* If we fail to stat, silently ignore the error
+         * and report an impossible mtime */
+        result = strdup("0");
+        ERR_NOMEM(result == NULL, aug);
+    } else {
+        r = xasprintf(&result, "%ld", (long) st.st_mtime);
+        ERR_NOMEM(r < 0, aug);
+    }
+    return result;
+ error:
+    FREE(result);
+    return NULL;
+}
+
+static bool file_current(struct augeas *aug, const char *fname,
+                         struct tree *finfo) {
+    struct tree *mtime = tree_child(finfo, s_mtime);
+    struct tree *file = NULL, *path = NULL;
+    int r;
+    struct stat st;
+    int64_t mtime_i;
+
+    if (mtime == NULL || mtime->value == NULL)
+        return false;
+
+    r = xstrtoint64(mtime->value, 10, &mtime_i);
+    if (r < 0) {
+        /* Ignore silently and err on the side of caution */
+        return false;
+    }
+
+    r = stat(fname, &st);
+    if (r < 0)
+        return false;
+
+    if (mtime_i != (int64_t) st.st_mtime)
+        return false;
+
+    path = tree_child(finfo, s_path);
+    if (path == NULL)
+        return false;
+
+    file = tree_find(aug, path->value);
+    return (file != NULL && ! file->dirty);
+}
+
 static int filter_generate(struct tree *xfm, const char *root,
                            int *nmatches, char ***matches) {
     glob_t globbuf;
@@ -326,7 +381,8 @@ static int store_error(struct augeas *aug,
  * Returns 0 on success, -1 on error
  */
 static int add_file_info(struct augeas *aug, const char *node,
-                         struct lens *lens, const char *lens_name) {
+                         struct lens *lens, const char *lens_name,
+                         const char *filename) {
     struct tree *file, *tree;
     char *tmp = NULL;
     int r;
@@ -348,6 +404,13 @@ static int add_file_info(struct augeas *aug, const char *node,
     r = tree_set_value(tree, node);
     ERR_NOMEM(r < 0, aug);
 
+    /* Set 'mtime' */
+    tmp = mtime_as_string(aug, filename);
+    ERR_BAIL(aug);
+    tree = tree_child_cr(file, s_mtime);
+    ERR_NOMEM(tree == NULL, aug);
+    tree_store_value(tree, &tmp);
+
     /* Set 'lens/info' */
     tmp = format_info(lens->info);
     ERR_NOMEM(tmp == NULL, aug);
@@ -362,6 +425,8 @@ static int add_file_info(struct augeas *aug, const char *node,
     r = tree_set_value(tree, lens_name);
     ERR_NOMEM(r < 0, aug);
 
+    tree_clean(file);
+
     result = 0;
  error:
     free(path);
@@ -404,7 +469,7 @@ static int load_file(struct augeas *aug, struct lens *lens,
     path = file_name_path(aug, filename);
     ERR_NOMEM(path == NULL, aug);
 
-    r = add_file_info(aug, path, lens, lens_name);
+    r = add_file_info(aug, path, lens, lens_name, filename);
     if (r < 0)
         goto done;
 
@@ -602,7 +667,8 @@ int transform_load(struct augeas *aug, struct tree *xfm) {
     for (int i=0; i < nmatches; i++) {
         const char *filename = matches[i] + strlen(aug->root) - 1;
         struct tree *finfo = file_info(aug, filename);
-        if (finfo != NULL && tree_child(finfo, s_lens) != NULL) {
+        if (finfo != NULL && !finfo->dirty &&
+            tree_child(finfo, s_lens) != NULL) {
             const char *s = xfm_lens_name(finfo);
             char *fpath = file_name_path(aug, matches[i]);
             transform_file_error(aug, "mxfm_load", filename,
@@ -610,9 +676,11 @@ int transform_load(struct augeas *aug, struct tree *xfm) {
                                  s, lens_name);
             aug_rm(aug, fpath);
             free(fpath);
-        } else {
+        } else if (!file_current(aug, matches[i], finfo)) {
             load_file(aug, lens, lens_name, matches[i]);
         }
+        if (finfo != NULL)
+            finfo->dirty = 0;
         FREE(matches[i]);
     }
     lens_release(lens);
@@ -945,7 +1013,7 @@ int transform_save(struct augeas *aug, struct tree *xfm,
     result = 1;
 
  done:
-    r = add_file_info(aug, path, lens, lens_name);
+    r = add_file_info(aug, path, lens, lens_name, filename);
     if (r < 0) {
         err_status = "file_info";
         result = -1;
diff --git a/tests/cutest.c b/tests/cutest.c
index 120e31c..08dbfd7 100644
--- a/tests/cutest.c
+++ b/tests/cutest.c
@@ -138,6 +138,23 @@ void CuAssertStrEquals_LineMsg(CuTest* tc, const char* file, int line,
 	CuFailInternal(tc, file, line, string);
 }
 
+void CuAssertStrNotEqual_LineMsg(CuTest* tc, const char* file, int line,
+                                 const char* message,
+                                 const char* expected, const char* actual) {
+	char *string = NULL;
+
+    if (expected != NULL && actual != NULL && strcmp(expected, actual) != 0)
+        return;
+
+	if (message != NULL) {
+        asprintf_or_die(&string, "%s: expected <%s> but was <%s>", message,
+                        expected, actual);
+	} else {
+        asprintf_or_die(&string, "expected <%s> but was <%s>", expected, actual);
+    }
+	CuFailInternal(tc, file, line, string);
+}
+
 void CuAssertIntEquals_LineMsg(CuTest* tc, const char* file, int line,
                                const char* message,
                                int expected, int actual) {
diff --git a/tests/cutest.h b/tests/cutest.h
index a667e50..616fb1f 100644
--- a/tests/cutest.h
+++ b/tests/cutest.h
@@ -61,6 +61,9 @@ void CuAssert_Line(CuTest* tc, const char* file, int line, const char* message,
 void CuAssertStrEquals_LineMsg(CuTest* tc,
 	const char* file, int line, const char* message,
 	const char* expected, const char* actual);
+void CuAssertStrNotEqual_LineMsg(CuTest* tc,
+	const char* file, int line, const char* message,
+	const char* expected, const char* actual);
 void CuAssertIntEquals_LineMsg(CuTest* tc,
 	const char* file, int line, const char* message,
 	int expected, int actual);
@@ -82,6 +85,7 @@ void CuAssertPtrNotEqual_LineMsg(CuTest* tc,
 
 #define CuAssertStrEquals(tc,ex,ac)           CuAssertStrEquals_LineMsg((tc),__FILE__,__LINE__,NULL,(ex),(ac))
 #define CuAssertStrEquals_Msg(tc,ms,ex,ac)    CuAssertStrEquals_LineMsg((tc),__FILE__,__LINE__,(ms),(ex),(ac))
+#define CuAssertStrNotEqual(tc,ex,ac)         CuAssertStrNotEqual_LineMsg((tc),__FILE__,__LINE__,NULL,(ex),(ac))
 #define CuAssertIntEquals(tc,ex,ac)           CuAssertIntEquals_LineMsg((tc),__FILE__,__LINE__,NULL,(ex),(ac))
 #define CuAssertIntEquals_Msg(tc,ms,ex,ac)    CuAssertIntEquals_LineMsg((tc),__FILE__,__LINE__,(ms),(ex),(ac))
 #define CuAssertDblEquals(tc,ex,ac,dl)        CuAssertDblEquals_LineMsg((tc),__FILE__,__LINE__,NULL,(ex),(ac),(dl))
diff --git a/tests/test-load.c b/tests/test-load.c
index a8107a1..fe92305 100644
--- a/tests/test-load.c
+++ b/tests/test-load.c
@@ -70,6 +70,8 @@ static struct augeas *setup_writable_hosts(CuTest *tc) {
     r = aug_set(aug, "/augeas/load/Hosts/incl", "/etc/hosts");
     CuAssertRetSuccess(tc, r);
 
+    free(build_root);
+    free(etcdir);
     return aug;
 }
 
@@ -262,6 +264,142 @@ static void testDefvarExpr(CuTest *tc) {
     aug_close(aug);
 }
 
+static void testReloadChanged(CuTest *tc) {
+    FILE *fp;
+    augeas *aug = NULL;
+    const char *build_root, *mtime2, *s;
+    char *mtime1;
+    char *hosts = NULL;
+    int r;
+
+    aug = setup_writable_hosts(tc);
+
+    r = aug_load(aug);
+    CuAssertRetSuccess(tc, r);
+
+    r = aug_get(aug, "/augeas/root", &build_root);
+    CuAssertIntEquals(tc, 1, r);
+
+    r = aug_get(aug, "/augeas/files/etc/hosts/mtime", &s);
+    CuAssertIntEquals(tc, 1, r);
+    mtime1 = strdup(s);
+    CuAssertPtrNotNull(tc, mtime1);
+
+    /* Tickle /etc/hosts behind augeas' back */
+    r = asprintf(&hosts, "%setc/hosts", build_root);
+    CuAssertPositive(tc, r);
+
+    fp = fopen(hosts, "a");
+    CuAssertPtrNotNull(tc, fp);
+
+    r = fprintf(fp, "192.168.0.1 other.example.com\n");
+    CuAssertTrue(tc, r > 0);
+
+    r = fclose(fp);
+    CuAssertRetSuccess(tc, r);
+
+    /* Unsaved changes are discarded */
+    r = aug_set(aug, "/files/etc/hosts/1/ipaddr", "127.0.0.2");
+    CuAssertRetSuccess(tc, r);
+
+    /* Check that we really did load the right file*/
+    r = aug_load(aug);
+    CuAssertRetSuccess(tc, r);
+
+    r = aug_get(aug, "/augeas/files/etc/hosts/mtime", &mtime2);
+    CuAssertIntEquals(tc, 1, r);
+    CuAssertStrNotEqual(tc, mtime1, mtime2);
+
+    r = aug_match(aug, "/files/etc/hosts/*[ipaddr = '192.168.0.1']", NULL);
+    CuAssertIntEquals(tc, 1, r);
+
+    r = aug_match(aug, "/files/etc/hosts/1[ipaddr = '127.0.0.1']", NULL);
+    CuAssertIntEquals(tc, 1, r);
+
+    free(mtime1);
+    free(hosts);
+    aug_close(aug);
+}
+
+static void testReloadDirty(CuTest *tc) {
+    augeas *aug = NULL;
+    int r;
+
+    aug = setup_writable_hosts(tc);
+
+    r = aug_load(aug);
+    CuAssertRetSuccess(tc, r);
+
+    /* Unsaved changes are discarded */
+    r = aug_set(aug, "/files/etc/hosts/1/ipaddr", "127.0.0.2");
+    CuAssertRetSuccess(tc, r);
+
+    r = aug_load(aug);
+    CuAssertRetSuccess(tc, r);
+
+    r = aug_match(aug, "/files/etc/hosts/1[ipaddr = '127.0.0.1']", NULL);
+    CuAssertIntEquals(tc, 1, r);
+
+    aug_close(aug);
+}
+
+static void testReloadDeleted(CuTest *tc) {
+    augeas *aug = NULL;
+    int r;
+
+    aug = setup_writable_hosts(tc);
+
+    r = aug_load(aug);
+    CuAssertRetSuccess(tc, r);
+
+    /* A missing file causes a reload */
+    r = aug_rm(aug, "/files/etc/hosts");
+    CuAssertPositive(tc, r);
+
+    r = aug_load(aug);
+    CuAssertRetSuccess(tc, r);
+
+    r = aug_match(aug, "/files/etc/hosts/1[ipaddr = '127.0.0.1']", NULL);
+    CuAssertIntEquals(tc, 1, r);
+
+    /* A missing entry in a file causes a reload */
+    r = aug_rm(aug, "/files/etc/hosts/1/ipaddr");
+    CuAssertPositive(tc, r);
+
+    r = aug_load(aug);
+    CuAssertRetSuccess(tc, r);
+
+    r = aug_match(aug, "/files/etc/hosts/1[ipaddr = '127.0.0.1']", NULL);
+    CuAssertIntEquals(tc, 1, r);
+
+    aug_close(aug);
+}
+
+static void testReloadDeletedMeta(CuTest *tc) {
+    augeas *aug = NULL;
+    int r;
+
+    aug = setup_writable_hosts(tc);
+
+    r = aug_load(aug);
+    CuAssertRetSuccess(tc, r);
+
+    /* Unsaved changes are discarded */
+    r = aug_rm(aug, "/augeas/files/etc/hosts");
+    CuAssertPositive(tc, r);
+
+    r = aug_set(aug, "/files/etc/hosts/1/ipaddr", "127.0.0.2");
+    CuAssertRetSuccess(tc, r);
+
+    r = aug_load(aug);
+    CuAssertRetSuccess(tc, r);
+
+    r = aug_match(aug, "/files/etc/hosts/1[ipaddr = '127.0.0.1']", NULL);
+    CuAssertIntEquals(tc, 1, r);
+
+    aug_close(aug);
+}
+
 int main(void) {
     char *output = NULL;
     CuSuite* suite = CuSuiteNew();
@@ -274,6 +412,10 @@ int main(void) {
     SUITE_ADD_TEST(suite, testLoadSave);
     SUITE_ADD_TEST(suite, testLoadDefined);
     SUITE_ADD_TEST(suite, testDefvarExpr);
+    SUITE_ADD_TEST(suite, testReloadChanged);
+    SUITE_ADD_TEST(suite, testReloadDirty);
+    SUITE_ADD_TEST(suite, testReloadDeleted);
+    SUITE_ADD_TEST(suite, testReloadDeletedMeta);
 
     abs_top_srcdir = getenv("abs_top_srcdir");
     if (abs_top_srcdir == NULL)
diff --git a/tests/xpath.tests b/tests/xpath.tests
index f2575a3..b16792a 100644
--- a/tests/xpath.tests
+++ b/tests/xpath.tests
@@ -148,6 +148,7 @@ test ipaddr-sibling //*[../ipaddr]
 
 test lircd-ancestor //*[ancestor::kudzu][label() != '#comment']
      /augeas/files/etc/sysconfig/kudzu/path = /files/etc/sysconfig/kudzu
+     /augeas/files/etc/sysconfig/kudzu/mtime = ...
      /augeas/files/etc/sysconfig/kudzu/lens = @Shellvars
      /augeas/files/etc/sysconfig/kudzu/lens/info = ...
      /files/etc/sysconfig/kudzu/SAFE = no
-- 
1.6.6.1