Thread (1 message) 1 message, 1 author, 2016-06-15

Re: [PATCH] git-count-objects: Fix a disk-space under-estimate on Cygwin

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:47:44

Ramsay Jones [off-list ref] writes:
Signed-off-by: Ramsay Jones <redacted>
---
Could you write a concise summary in the commit log message to explain why
it is a correct fix?  Your detailed analysis after three dash lines was an
amusing read, but people who will be reading "git log" output 6 months
down the road won't have an access to it.  Something like...

    Cygwin has st_blocks in struct stat, but at least on NTFS, the field
    counts in blocks of st_blksize bytes, not in 512-byte blocks.

The Makefile already explains what NO_ST_BLOCKS_IN_STRUCT_STAT is about:

    # Define NO_ST_BLOCKS_IN_STRUCT_STAT if your platform does not have st_blocks
    # field that counts the on-disk footprint in 512-byte blocks.

so the above explanation should be enough.

Note that this is not any standard compliance.  POSIX cops out like this
in the <sys/stat.h> description:

    The unit for the st_blocks member of the stat structure is not defined
    within POSIX.1-2008. In some implementations it is 512 bytes. It may
    differ on a file system basis. There is no correlation between values
    of the st_blocks and st_blksize, and the f_bsize (from
    <sys/statvfs.h>) structure members.

IOW, a system like Cygwin that does not use 512-byte is not in violation.
quoted hunk
diff --git a/Makefile b/Makefile
index 5d5976f..5624563 100644
--- a/Makefile
+++ b/Makefile
@@ -783,6 +783,10 @@ ifeq ($(uname_O),Cygwin)
 	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
 	NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
 	OLD_ICONV = UnfortunatelyYes
+	# The st_blocks field is not in units of 512 bytes, as the code
+	# expects, which leads to an under-estimate of the disk space used.
+	# In order to use an alternate algorithm, we claim to lack st_blocks.
+	NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
This comment is redundant, as the explanation of the Makefile variable
already said the same thing.

Also it is somewhat wrong to say "claim to lack".  The Makefile variable
merely means "do not use this field".  The reason may be that the platform
lacks it, or it may have it but it does not count in 512-byte blocks.  It
does not matter---either way the field is not usable.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help