[v1,1/1] t5601-99: Enable colliding file detection for MINGW
diff mbox series

Message ID 20181122201640.78495-1-carenas@gmail.com
State New
Headers show
Series
  • [v1,1/1] t5601-99: Enable colliding file detection for MINGW
Related show

Commit Message

Carlo Marcelo Arenas Belón Nov. 22, 2018, 8:16 p.m. UTC
Which FS was this tested on?, is Git LFS I keep hearing about also considered
a "filesystem" for git?

Could you also test with the following applied on top?

Carlo
-- >8 --
Subject: [PATCH] entry: remove windows fallback to inode checking

this test is really FS specific, so is better to avoid any compiled
assumptions about the platform and let the user drive the fallback
through core.checkStat instead

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 entry.c | 4 ----
 1 file changed, 4 deletions(-)

Comments

Johannes Schindelin Nov. 23, 2018, 11:24 a.m. UTC | #1
Hi Carlo,

On Thu, 22 Nov 2018, Carlo Marcelo Arenas Belón wrote:

> Subject: [PATCH] entry: remove windows fallback to inode checking
> 
> this test is really FS specific, so is better to avoid any compiled
> assumptions about the platform and let the user drive the fallback
> through core.checkStat instead
> 
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
>  entry.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/entry.c b/entry.c
> index 0a3c451f5f..5ae74856e6 100644
> --- a/entry.c
> +++ b/entry.c
> @@ -404,10 +404,6 @@ static void mark_colliding_entries(const struct checkout *state,
>  {
>  	int i, trust_ino = check_stat;
>  
> -#if defined(GIT_WINDOWS_NATIVE) || defined(__CYGWIN__)
> -	trust_ino = 0;
> -#endif
> -

No, we cannot drop this. You may not see it in git.git's source code, but
in Git for Windows' patches, we have an experimental feature (which had
seemed to stabilize, but Ben Peart is currently doing wonders with it,
improving the performance substantially) for accelerating the file
metadata enumeration in a noticeable manner. The only way we can do that
is by *not* insisting on a correct inode.

Besides, IIRC even our regular stat() now "fails" to fill the inode field.

So no, we cannot do that. We can probably drop the `||
defined(__CYGINW__)` part (Cygwin even generates a fake inode for FAT,
where no equivalent is available, by hashing the full normalized path).
But you cannot drop the `GIT_WINDOWS_NATIVE` part.

Ciao,
Johannes

>  	ce->ce_flags |= CE_MATCHED;
>  
>  	for (i = 0; i < state->istate->cache_nr; i++) {
> -- 
> 2.20.0.rc1
> 
>

Patch
diff mbox series

diff --git a/entry.c b/entry.c
index 0a3c451f5f..5ae74856e6 100644
--- a/entry.c
+++ b/entry.c
@@ -404,10 +404,6 @@  static void mark_colliding_entries(const struct checkout *state,
 {
 	int i, trust_ino = check_stat;
 
-#if defined(GIT_WINDOWS_NATIVE) || defined(__CYGWIN__)
-	trust_ino = 0;
-#endif
-
 	ce->ce_flags |= CE_MATCHED;
 
 	for (i = 0; i < state->istate->cache_nr; i++) {