git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
Nov 24, 2009, 22:08 UTC
Message-ID
<7vskc3bmob.fsf@alter.siamese.dyndns.org>
In-Reply-To
<4B0C3D0D.60309@ramsay1.demon.co.uk>
Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
Show 26 quoted lines
> Err... no :-D
>
> Note that my suggested addition to your patch is in the core.filemode == false
> code path, and so does not affect the "disk-space under-estimate" problem at all.
>
> [To be clear: the "disk-space under-estimate" problem only happens when
> core.filemode == true and the regular cygwin lstat()/stat() functions are used.
> When core.filemode == false, the code in compat/cygwin.c (namely cygwin_lstat()
> and cygwin_stat()) will (most likely) be called instead. These functions use
> WIN32 api calls to implement equivalent, but presumably faster, versions of the
> stat functions]
>
>> You are forcing st_blksize to 512 but still return the same old st_blocks;
>> I do not understand what that would achieve.
>
> Well, as I said, I haven't tested your patch, or my suggested addition, so I could
> well be wrong... but what I aimed to achieve was to:
>
>     - avoid "undefined behaviour" in on_disk_bytes(), since the value in
>       st_blksize would otherwise be undefined (ie whatever happened to be
>       on the stack-frame of the count_objects() function).
>     - initialize the st_blksize field with a value consistent with the
>       st_blocks field, which is derived from the st_size field, as the
>       number of 512-byte blocks. (see the context line just before the
>       + line in the above diff, along with the size_to_blocks macro)
>     - return the same answer from this code as before.

Ah, sorry, so then I misread your comment. size_to_blocks() in compat/cygwin.c counts blocks in 512 (I just checked) and you are applying the reverse.

But you are right. If the emulation used on cygwin is _not_ doing the "blocks * blksize is close to size" thing that is not POSIX but you saw in your experiment on NTFS, and if we need your follow-up patch to make it do so, there is no point in using my patch.

> Note that the answer returned from this core.filemode == false code path
> is different to the core.filemode == true code path. Which is why I *slightly*
> prefer my original patch.

Thanks for a clarification. I've already queued both to 'next', but I will revert mine and make your patch graduate to 'master'.

Previous: Ramsay Jones
Message 7 of 7 in “git-count-objects: Fix a disk-space under-estimate on Cygwin”
  1. git-count-objects: Fix a disk-space under-estimate on CygwinRamsay Jones, Nov 19, 2009
  2. Junio C HamanoNov 20, 2009
  3. Junio C HamanoNov 20, 2009
  4. Ramsay JonesNov 21, 2009
  5. Junio C HamanoNov 22, 2009
  6. Ramsay JonesNov 24, 2009
  7. Junio C HamanoNov 24, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.