Quoth Jani Nikula on Dec 09 at 1:55 pm: > On Thu, 8 Dec 2011 18:34:29 -0500, Austin Clements <amdragon@MIT.EDU> wrote: > > Quoth Jani Nikula on Dec 09 at 12:48 am: > > > + /* Check access before fork() for speed and simplicity of error handling. */ > > > + if (access (hook_path, X_OK) == -1) { > > > + /* Ignore ENOENT. It's okay not to have a hook, hook dir, or even > > > + * notmuch dir. Dangling symbolic links also result in ENOENT, but > > > + * we'll ignore that too for simplicity. */ > > > + if (errno != ENOENT) { > > > + fprintf (stderr, "Error: %s hook access failed: %s\n", hook, > > > + strerror (errno)); > > > + status = 1; > > > + } > > > > Is it the intent that a present but non-executable hook (errno == > > EACCES) will print the above error message and return with a failure? > > I'm pretty sure this differs from the behavior of git hooks. > > It differs from git, and it is intentional. Git bails out with success > status, without even a warning, for *all* access() failures. That may be > fine for git (which generally expects the user to know what he's doing) > but I'd argue notmuch should let the user know something is wrong. > > Also for EACCES, IMHO failing is more useful to the user than silently > ignoring. If the hook exists, but isn't executable, I think it's way > more likely that the user forgot to chmod +x than intentionally dropped > x so the hook would not be run. (And I think we agreed on IRC that in > the future, sample hooks would be named hook.sample and have executable > bit set.) > > Anyway, this is my opinion; it's not a big deal to change if there are > compelling reasons to ignore EACCES that I didn't think of. Consider me convinced.