Re: [PATCH 02/14] tempfile: a new module for handling temporary files

On 06/10/2015 07:36 PM, Junio C Hamano wrote:

Michael Haggerty [off-list ref] writes:

quoted

diff --git a/builtin/add.c b/builtin/add.c
index df5135b..aaa9ce4 100644
--- a/builtin/add.c
+++ b/builtin/add.c

@@ -5,6 +5,7 @@
  */
 #include "cache.h"
 #include "builtin.h"
+#include "tempfile.h"
 #include "lockfile.h"
 #include "dir.h"
 #include "pathspec.h"

It is a bit sad that all users of lockfile.h has to include
tempfile.h; even when trying to find out something as basic as the
name of the file on which the lock is held, they need to look at
lk->tempfile.filename and that requires inclusion of tempfile.h

It is a good idea to have tempfile as a separate module, as it
allows new callers to use the same "clean-on-exit" infrastructure on
things that are not locks, i.e. they can include tempfile.h without
having to include lockfile.h, but I have to wonder if it is better
to include tempfile.h from inside lockfile.h (which is alrady done)
and allow users of lockfile API to assume that inclusion will always
stay there.  After all, if they are taking locks, they already know
lk->tempfile is the mechanism through which they need to learn about
various aspects of the underlying files.

Hmmm, currently lockfile.h doesn't include tempfile.h. But I think it is
a good idea for it to do so. (I would have done it already but I thought
it was against project policy.)

I will make this change in v2.

[...]

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu