Skip to contentSkip to conversation

prune: close directory earlier during loose-object directory traversal

PATCH In review Johannes Sixt 4 participants 7 messages Jun 15, 2016 2 messages not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

Johannes Sixt

author
#2 lore raw

PATCH 1/1 prune: close directory earlier during loose-object directory traversal

27e1e22d (prune: factor out loose-object directory traversal, 2014-10-16) introduced a new function for_each_loose_file_in_objdir() with a helper for_each_file_in_obj_subdir(). The latter calls callbacks for each file found during a directory traversal and finally also a callback for the directory itself.

git-prune uses the function to clean up the object directory. In particular, in the directory callback it calls rmdir(). On Windows XP, this rmdir call fails, because the directory is still open while the callback is called. Close the directory before calling the callback.

Signed-off-by Johannes Sixt <[email protected]>

---
My Windows 8.1 machine does not require this fix for some unkonwn reason. But we still cater for Windows XP users, where this change is a real improvement.

 sha1_file.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
sha1_file.c +2 -2
index 4d1b26f..5cecc68 100644
@@ -3473,12 +3473,12 @@ static int for_each_file_in_obj_subdir(int subdir_nr,
 				break;
 		}
 	}
+	closedir(dir);
+
 	strbuf_setlen(path, baselen);
-
 	if (!r && subdir_cb)
 		r = subdir_cb(subdir_nr, path->buf, data);
 
-	closedir(dir);
 	return r;
 }
Signature · 21 lines
2.3.2.245.gb5bf9d3

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
For more options, visit https://groups.google.com/d/optout.
Acked-by on v1 1/1
#3 lore raw
Quoted 10 lines · Johannes Sixt

27e1e22d (prune: factor out loose-object directory traversal, 2014-10-16) introduced a new function for_each_loose_file_in_objdir() with a helper for_each_file_in_obj_subdir(). The latter calls callbacks for each file found during a directory traversal and finally also a callback for the directory itself.

git-prune uses the function to clean up the object directory. In particular, in the directory callback it calls rmdir(). On Windows XP, this rmdir call fails, because the directory is still open while the callback is called. Close the directory before calling the callback.

Makes sense, and the patch looks good to me. Sorry for breaking things on Windows.

Acked-by Jeff King <[email protected]>

-Peff

Signature · 18 lines
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
For more options, visit https://groups.google.com/d/optout.
#4 lore raw
Quoted 13 lines · Jeff King
Quoted 10 lines · Johannes Sixt

27e1e22d (prune: factor out loose-object directory traversal, 2014-10-16) introduced a new function for_each_loose_file_in_objdir() with a helper for_each_file_in_obj_subdir(). The latter calls callbacks for each file found during a directory traversal and finally also a callback for the directory itself.

git-prune uses the function to clean up the object directory. In particular, in the directory callback it calls rmdir(). On Windows XP, this rmdir call fails, because the directory is still open while the callback is called. Close the directory before calling the callback.

Quoted 4 lines

Makes sense, and the patch looks good to me. Sorry for breaking things on Windows.

Acked-by Jeff King <[email protected]>

Sorry for reviving this old thread, but I noticed that we do not have this patch in our tree yet. I'll queue to 'pu' for now lest I forget. If I missed a good argument or concensus against the change please let me know, otherwise I'll fast track the change to 2.7 final

#5 lore raw
Quoted 18 lines · Junio C Hamano
Quoted 16 lines · Jeff King
Quoted 10 lines · Johannes Sixt

27e1e22d (prune: factor out loose-object directory traversal, 2014-10-16) introduced a new function for_each_loose_file_in_objdir() with a helper for_each_file_in_obj_subdir(). The latter calls callbacks for each file found during a directory traversal and finally also a callback for the directory itself.

git-prune uses the function to clean up the object directory. In particular, in the directory callback it calls rmdir(). On Windows XP, this rmdir call fails, because the directory is still open while the callback is called. Close the directory before calling the callback.

Makes sense, and the patch looks good to me. Sorry for breaking things on Windows.

Quoted 6 lines
Quoted 1 line
Acked-by Jeff King <[email protected]>

Sorry for reviving this old thread, but I noticed that we do not have this patch in our tree yet. I'll queue to 'pu' for now lest I forget. If I missed a good argument or concensus against the change please let me know, otherwise I'll fast track the change to 2.7 final

Ah, thanks for doing that. I noticed it when picking through "git branch --no-merged pu" of your workspace a few weeks ago, but forgot to follow up. I certainly have no objections.

-Peff

Signature · 18 lines
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
For more options, visit https://groups.google.com/d/optout.
#6 lore raw

Hi Junio & Peff,

Quoted 22 lines · Jeff King
Quoted 20 lines · Junio C Hamano
Quoted 17 lines · Jeff King
Quoted 10 lines · Johannes Sixt

27e1e22d (prune: factor out loose-object directory traversal, 2014-10-16) introduced a new function for_each_loose_file_in_objdir() with a helper for_each_file_in_obj_subdir(). The latter calls callbacks for each file found during a directory traversal and finally also a callback for the directory itself.

git-prune uses the function to clean up the object directory. In particular, in the directory callback it calls rmdir(). On Windows XP, this rmdir call fails, because the directory is still open while the callback is called. Close the directory before calling the callback.

Makes sense, and the patch looks good to me. Sorry for breaking things on Windows.

Acked-by Jeff King <[email protected]>
Quoted 8 lines
Quoted 4 lines

Sorry for reviving this old thread, but I noticed that we do not have this patch in our tree yet. I'll queue to 'pu' for now lest I forget. If I missed a good argument or concensus against the change please let me know, otherwise I'll fast track the change to 2.7 final

Ah, thanks for doing that. I noticed it when picking through "git branch --no-merged pu" of your workspace a few weeks ago, but forgot to follow up. I certainly have no objections.

Git for Windows carries this patch since Git for Windows v2.5.0. So: no objection from my side, either.

Ciao,
Dscho

Signature · 18 lines
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
For more options, visit https://groups.google.com/d/optout.
#7 lore raw
Quoted 11 lines · Johannes Schindelin
Quoted 8 lines
Quoted 4 lines

Sorry for reviving this old thread, but I noticed that we do not have this patch in our tree yet. I'll queue to 'pu' for now lest I forget. If I missed a good argument or concensus against the change please let me know, otherwise I'll fast track the change to 2.7 final

Ah, thanks for doing that. I noticed it when picking through "git branch --no-merged pu" of your workspace a few weeks ago, but forgot to follow up. I certainly have no objections.

Git for Windows carries this patch since Git for Windows v2.5.0. So: no objection from my side, either.

Thanks!

Signature · 18 lines
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
For more options, visit https://groups.google.com/d/optout.
#8 lore raw
In reply to Johannes Sixt

Hi,

Quoted 11 lines · Johannes Sixt

27e1e22d (prune: factor out loose-object directory traversal, 2014-10-16) introduced a new function for_each_loose_file_in_objdir() with a helper for_each_file_in_obj_subdir(). The latter calls callbacks for each file found during a directory traversal and finally also a callback for the directory itself.

git-prune uses the function to clean up the object directory. In particular, in the directory callback it calls rmdir(). On Windows XP, this rmdir call fails, because the directory is still open while the callback is called. Close the directory before calling the callback.

Quoted 5 lines
Signed-off-by Johannes Sixt <[email protected]>

---
My Windows 8.1 machine does not require this fix for some unkonwn reason. But we still cater for Windows XP users, where this change is a real improvement.

I believe that we have a concrete bug report for that:

    https://github.com/git-for-windows/git/issues/231

Ciao,
Johannes

Signature · 18 lines
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
For more options, visit https://groups.google.com/d/optout.