Re: [PATCH 2/6] fsmonitor: Add dir.h include, for untracked_cache_invalidate_path

[Junio C Hamano]

Alex Vandiver  writes:

>> But I am not sure if this is a right direction to go in.  If a .C
>> user of fsmonitor needs (does not need) things from dir.h, that file
>> can (does not need to) include dir.h itself.
>
> Hm; I was patterning based on existing .h files, which don't seem shy
> about pulling in other .h files.

IIUC, existing X.h do pull in Y.h when X.h uses a structure or a
typedef defined in Y.h (but using pointer to such a structure or a
type does not count) defined in Y.h; in such a case, a user of X.h
that wants to use what is defined in X.h would not be able to use
it without somehow knowing the shape of such a structure or type and
would be forced to pull in Y.h itself.  "static inline" falls into
the same category---as it stands, anybody that includes fsmonitor.h
and wants to use one of these static inline functions would need to
have definitions from dir.h, which I agree is wrong and I understand
that you want to include dir.h there.

Re: [PATCH 2/6] fsmonitor: Add dir.h include, for untracked_cache_invalidate_path

[Alex Vandiver]

On Tue, 19 Dec 2017, Junio C Hamano wrote:
> Alex Vandiver <ale...@dropbox.com> writes:
> 
> > Subject: Re: [PATCH 2/6] fsmonitor: Add dir.h include, for 
> > untracked_cache_invalidate_path
> 
> Perhaps
> 
> "Subject: fsmonitor.h: include dir.h"

Certainly more concise.

> But I am not sure if this is a right direction to go in.  If a .C
> user of fsmonitor needs (does not need) things from dir.h, that file
> can (does not need to) include dir.h itself.

Hm; I was patterning based on existing .h files, which don't seem shy
about pulling in other .h files.

> I think this header does excessive "static inline" as premature
> optimization, so a better "fix" to your perceived problem may be to
> make them not "static inline".

Yeah, quite possibly.  Ben, do you recall your rationale for inlining
them in 6a6da08f6 ("fsmonitor: teach git to optionally utilize a file
system monitor to speed up detecting new or changed files.",
2017-09-22) ?

 - Alex

Re: [PATCH 2/6] fsmonitor: Add dir.h include, for untracked_cache_invalidate_path

[Junio C Hamano]

Alex Vandiver <ale...@dropbox.com> writes:

> Subject: Re: [PATCH 2/6] fsmonitor: Add dir.h include, for 
> untracked_cache_invalidate_path

Perhaps

"Subject: fsmonitor.h: include dir.h"

But I am not sure if this is a right direction to go in.  If a .C
user of fsmonitor needs (does not need) things from dir.h, that file
can (does not need to) include dir.h itself.

I think this header does excessive "static inline" as premature
optimization, so a better "fix" to your perceived problem may be to
make them not "static inline".

> This missing include is currently hidden by dint of the fact that
> dir.h is already included by all things that currently include
> fsmonitor.h
>
> Signed-off-by: Alex Vandiver <ale...@dropbox.com>
> ---


>  fsmonitor.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/fsmonitor.h b/fsmonitor.h
> index cd3cc0ccf..5f68ca4d2 100644
> --- a/fsmonitor.h
> +++ b/fsmonitor.h
> @@ -1,5 +1,6 @@
>  #ifndef FSMONITOR_H
>  #define FSMONITOR_H
> +#include "dir.h"
>  
>  extern struct trace_key trace_fsmonitor;