reimplement commit hooks mechanism

* do NOT reinvent the wheel (directory traversal, sorting, script execution, etc.) when the functionality is already provided by the system.
* eliminate hard-coded hook directory in various feastures. everyone should obtain the directory path from the config backend.

6 files changed, 74 insertions, 33 deletions

diff --git a/Makefile.am b/Makefile.am
index 78c1a66..8bcd723 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3,7 +3,6 @@ SUBDIRS = . perl_dmod
 share_perl5dir = /opt/vyatta/share/perl5/Vyatta
 completiondir	 = /etc/bash_completion.d
 initddir	 = /etc/init.d
-commit_run_dir = /etc/commit
 defaultdir = /etc/default
 etc_shell_leveldir = $(sysconfdir)/shell/level
 dhcphookdir = /etc/dhcp3/dhclient-exit-hooks.d
@@ -118,7 +117,6 @@ cpiop = find  . ! -regex '\(.*~\|.*\.bak\|.*\.swp\|.*\#.*\#\)' -print0 | \
 
 install-exec-hook:
 	mkdir -p $(DESTDIR)$(cfgdir)
-	mkdir -p $(DESTDIR)$(commit_run_dir)
 	mkdir -p $(DESTDIR)$(etc_shell_leveldir)
 	cd etc/shell/level; $(cpiop) $(DESTDIR)$(etc_shell_leveldir)
 	cd $(DESTDIR)$(sbindir); \
diff --git a/debian/vyatta-cfg.postinst.in b/debian/vyatta-cfg.postinst.in
index 5585e2a..7ba4332 100644
--- a/debian/vyatta-cfg.postinst.in
+++ b/debian/vyatta-cfg.postinst.in
@@ -33,6 +33,14 @@ for bin in my_cli_bin my_cli_shell_api; do
   setcap cap_sys_admin=pe $sbindir/$bin
 done
 
+# commit hooks
+for hook in PreCommit PostCommit; do
+  d=$(cli-shell-api get${hook}HookDir)
+  if [ -n "$d" ] && [ ! -e "$d" ]; then
+    mkdir -p "$d"
+  fi
+done
+
 # handle renamed file (not automatically deleted since it was "conffile")
 rm -f /etc/bash_completion.d/20vyatta-cfg
 
diff --git a/etc/bash_completion.d/vyatta-cfg b/etc/bash_completion.d/vyatta-cfg
index 0a62267..dfd73ac 100755
--- a/etc/bash_completion.d/vyatta-cfg
+++ b/etc/bash_completion.d/vyatta-cfg
@@ -123,11 +123,12 @@ commit ()
       args[${#args[@]}]="$arg"    
     fi
   done
-  args+=("-C '$comment'")
 
+  export COMMIT_COMMENT="$comment"
   export COMMIT_VIA=cli
   /opt/vyatta/sbin/my_commit "${args[@]}"
   unset COMMIT_VIA
+  unset COMMIT_COMMENT
 }
 
 commit-confirm ()
diff --git a/src/cli_shell_api.cpp b/src/cli_shell_api.cpp
index 6e8f384..0caccb9 100644
--- a/src/cli_shell_api.cpp
+++ b/src/cli_shell_api.cpp
@@ -486,6 +486,26 @@ _cf_process_args(Cstore& cstore, const Cpath& args, Cpath& path)
   return root;
 }
 
+// output the "pre-commit hook dir"
+static void
+getPreCommitHookDir(Cstore& cstore, const Cpath& args)
+{
+  const char *d = commit::getCommitHookDir(commit::PRE_COMMIT);
+  if (d) {
+    printf("%s", d);
+  }
+}
+
+// output the "post-commit hook dir"
+static void
+getPostCommitHookDir(Cstore& cstore, const Cpath& args)
+{
+  const char *d = commit::getCommitHookDir(commit::POST_COMMIT);
+  if (d) {
+    printf("%s", d);
+  }
+}
+
 /* the following "cf" functions form the "config file" shell API, which
  * allows shell scripts to "query" the "config" represented by a config
  * file in a way similar to how they query the active/working config.
@@ -574,6 +594,9 @@ static OpT ops[] = {
   OP(showConfig, -1, NULL, -1, NULL, true),
   OP(loadFile, 1, "Must specify config file", -1, NULL, false),
 
+  OP(getPreCommitHookDir, 0, "No argument expected", -1, NULL, false),
+  OP(getPostCommitHookDir, 0, "No argument expected", -1, NULL, false),
+
   OP(cfExists, -1, NULL, 2, "Must specify config file and path", false),
   OP(cfReturnValue, -1, NULL, 2, "Must specify config file and path", false),
   OP(cfReturnValues, -1, NULL, 2, "Must specify config file and path", false),
diff --git a/src/commit/commit-algorithm.cpp b/src/commit/commit-algorithm.cpp
index ad775df..80567f0 100644
--- a/src/commit/commit-algorithm.cpp
+++ b/src/commit/commit-algorithm.cpp
@@ -26,7 +26,13 @@ using namespace commit;
 using namespace std;
 
 
-////// static functions
+////// static
+static const char *commit_hook_dirs[3] = {
+  "/etc/commit/pre-hooks.d",
+  "/etc/commit/post-hooks.d",
+  NULL
+};
+
 static void
 _set_node_commit_state(CfgNode& node, CommitState s, bool recursive)
 {
@@ -742,6 +748,16 @@ _get_commit_other_node(CfgNode *cfg1, CfgNode *cfg2, const Cpath& cur_path)
   return cn;
 }
 
+static void
+_execute_hooks(CommitHook hook)
+{
+  string cmd = "/bin/run-parts --regex='^[a-zA-Z0-9._-]+$' -- '";
+  cmd += getCommitHookDir(hook);
+  cmd += "'";
+  // not checking return status
+  system(cmd.c_str());
+}
+
 
 ////// class CommitData
 CommitData::CommitData()
@@ -1043,6 +1059,15 @@ PrioNode::setSubtreeSuccess()
 
 
 ////// exported functions
+const char *
+commit::getCommitHookDir(CommitHook hook)
+{
+  if (hook > LAST) {
+    return NULL;
+  }
+  return commit_hook_dirs[hook];
+}
+
 CfgNode *
 commit::getCommitTree(CfgNode *cfg1, CfgNode *cfg2, const Cpath& cur_path)
 {
@@ -1154,6 +1179,7 @@ commit::doCommit(Cstore& cs, CfgNode& cfg1, CfgNode& cfg2)
     return true;
   }
 
+  _execute_hooks(PRE_COMMIT);
   set_in_commit(true);
 
   PrioNode proot(root); // proot corresponds to root
@@ -1186,9 +1212,11 @@ commit::doCommit(Cstore& cs, CfgNode& cfg1, CfgNode& cfg2)
     pq.pop();
   }
   bool ret = true;
+  const char *cst = "SUCCESS";
   if (f > 0) {
     OUTPUT_USER("Commit failed\n");
     ret = false;
+    cst = ((s > 0) ? "PARTIAL" : "FAILURE");
   }
 
   if (!cs.commitConfig(proot)) {
@@ -1204,6 +1232,11 @@ commit::doCommit(Cstore& cs, CfgNode& cfg1, CfgNode& cfg2)
   if (ret) {
     ret = cs.markSessionUnsaved();
   }
+
+  setenv("COMMIT_STATUS", cst, 1);
+  _execute_hooks(POST_COMMIT);
+  unsetenv("COMMIT_STATUS");
+
   return ret;
 }
 
diff --git a/src/commit/commit-algorithm.hpp b/src/commit/commit-algorithm.hpp
index 3bc89a7..fca04cb 100644
--- a/src/commit/commit-algorithm.hpp
+++ b/src/commit/commit-algorithm.hpp
@@ -50,6 +50,12 @@ enum CommitTreeTraversalOrder {
   POST_ORDER
 };
 
+enum CommitHook {
+  PRE_COMMIT,
+  POST_COMMIT,
+  LAST
+};
+
 class CommitData {
 public:
   CommitData();
@@ -120,35 +126,6 @@ public:
   void setSubtreeFailure();
   void setSubtreeSuccess();
 
-#if 0
-  // XXX testing
-  void print(size_t lvl) {
-#define INDENT for(size_t xyz = 0; xyz < lvl; xyz++) { printf("  "); };
-    INDENT;
-    if (_node) {
-      printf("[%s]\n", _node->getCommitPath().to_string().c_str());
-    } else {
-      printf("[---]\n");
-    }
-    INDENT; printf("|cp[%s]\n",
-                   (_cfg_parent
-                    ? _cfg_parent->getCommitPath().to_string().c_str()
-                      : NULL));
-    if (_node) {
-      INDENT; printf("======\n");
-      _node->rprint(lvl);
-      INDENT; printf("======\n");
-    }
-#undef INDENT
-  }
-  void rprint(size_t lvl) {
-    print(lvl);
-    for (size_t i = 0; i < numChildNodes(); i++) {
-      childAt(i)->rprint(lvl + 1);
-    }
-  }
-#endif
-
 private:
   CfgNode *_node;
   CfgNode *_cfg_parent;
@@ -184,6 +161,7 @@ typedef std::pair<CommitState, std::tr1::shared_ptr<Cpath> >
 typedef std::vector<CommittedPathT> CommittedPathListT;
 
 // exported functions
+const char *getCommitHookDir(CommitHook hook);
 CfgNode *getCommitTree(CfgNode *cfg1, CfgNode *cfg2, const Cpath& cur_path);
 bool isCommitPathEffective(Cstore& cs, const Cpath& pcomps,
                            std::tr1::shared_ptr<Ctemplate> def,