Re: [PATCH 2/6] path: Make the 'get_st_mode_bits' symbol a file static

[Ramsay Jones <ramsay@ramsay1.demon.co.uk>]

Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
> 
>> On MinGW, sparse issues an "'get_st_mode_bits' not declared. Should
>> it be static?" warning. The MinGW and MSVC builds do not see the
>> declaration of this function, within git-compat-util.h, due to it's
>> placement within an preprocessor conditional. (So, one solution would
>> be to simply move the declaration to the top level of the header.)
> 
> Well, the idea was that the user of this function in path.c will
> call get_st_mode_bits(), and whatever platform that provides a
> replacement implementation would do
> 
> 	#define get_st_mode_bits(a,b) cygwin_get_st_mode_bits((a),(b))
> 
> so that the calling site in path.c will end up calling that
> replacement implementation.  So if anything get_st_mode_bits()
> declaration may want to go at the _end_ (not top) after including
> all the compatibility crufts.
> 
> We could make the declaration static to path.c, but then nobody
> other than path.c would be able to make use of it in the future,
> and we'll have the same discussion when somebody wants to hoist the
> declaration to git-compat-util.h, no?

I don't have a problem with keeping the declaration in git-compat-util.h
(after moving it so that the MinGW and MSVC builds can see it, of course)
if you would rather do that.

However, I'm always a little wary when I hear someone say "this may be
useful to others in the future, so lets do X to make it easier ...".
I have noticed that, much more often than not, that future user never
does materialise ... ;-)

Having said that ...

Back in 2011, when I was trying to fix t7606-merge-custom.sh (see
commit b8a9733377, "help.c: Fix detection of custom merge strategy
on cygwin", 16-06-2011), I had a patch that looked very similar to
the solution Torsten has arrived at here! (The main difference was
that I called stat() rather than lstat() to get the mode bits).

I decided to go with the simpler solution in commit b8a9733377.

ATB,
Ramsay Jones