From: Jeff Hostetler <git@jeffhostetler.com>
Date: Tue, 08 Mar 2022 21:09:37 GMT
Subject: Re: [PATCH v6 16/30] compat/fsmonitor/fsm-listen-darwin: add MacOS header files for FSEvent
Message-ID: <aa6276f9-8d10-22f9-bfc0-2aa718d002e1@jeffhostetler.com>
In-Reply-To: <220307.86h78a2gcn.gmgdl@evledraar.gmail.com>



On 3/7/22 5:37 AM, Ævar Arnfjörð Bjarmason wrote:
> 
> On Tue, Mar 01 2022, Jeff Hostetler via GitGitGadget wrote:
> 
>> From: Jeff Hostetler <jeffhost@microsoft.com>
>> [...]
[...]
> 
>> +#else
>> +/*
>> + * Let Apple's headers declare `isalnum()` first, before
>> + * Git's headers override it via a constant
>> + */
> 
> 
> 
> 
>> +#include <string.h>
>> +#include <CoreFoundation/CoreFoundation.h>
>> +#include <CoreServices/CoreServices.h>
>> +#endif
> 
> In cache.h which you'rejust about to include we don't include string.h,
> but we do in git-compat-util.h, but that one includes string.h before
> doing those overrides.
> 
> This either isn't needed, or really should be some addition to
> git-compat-util.h instead. I.e. if we've missed some edge case with
> string.h and ctype.h on OSX we should handle that in git-compat-util.h
> rather than <some other file/header> needing a portability workaround.
> 
>> +
>>   #include "cache.h"
>>   #include "fsmonitor.h"
>>   #include "fsm-listen.h"
> 

You may be right here.  I commented out the <string.h> and
the <...CoreFoundation.h> lines and everything still compiled
and t7527 passed.

I'm not sure why <string.h> was added here (I inherited that
file when I took over the feature).  It may be that recent SDK
updates have eliminated the need for it.  Or it may be that it
was never necessary.  (However, the comment above it suggests
that there was a problem in the past.)

While it may not (now at least) be necessary, it's not doing
any harm, so I'd rather leave it and not interrupt things.
We can always revisit it later if we want.

Thanks,
Jeff