Re: [PATCH v2 3/5] gc --auto: exclude base pack if not enough mem to "repack -ad"

[Johannes Schindelin]

Hi Duy,

On Tue, 6 Mar 2018, Nguyễn Thái Ngọc Duy wrote:

> diff --git a/builtin/gc.c b/builtin/gc.c
> index 77fa720bd0..273657ddf4 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -187,7 +211,101 @@ static int too_many_packs(void)
>       return gc_auto_pack_limit < cnt;
>  }
>  
> -static void add_repack_all_option(void)
> +static inline unsigned long total_ram(void)
> +{
> +     unsigned long default_ram = 4;
> +#ifdef HAVE_SYSINFO
> +     struct sysinfo si;
> +
> +     if (!sysinfo(&si))
> +             return si.totalram;
> +#elif defined(HAVE_BSD_SYSCTL) && defined(HW_MEMSIZE)
> +     int64_t physical_memory;
> +     int mib[2];
> +     size_t length;
> +
> +     mib[0] = CTL_HW;
> +     mib[1] = HW_MEMSIZE;
> +     length = sizeof(int64_t);
> +     if (!sysctl(mib, 2, &physical_memory, &length, NULL, 0))
> +             return physical_memory;
> +#elif defined(GIT_WINDOWS_NATIVE)
> +     MEMORYSTATUSEX memInfo;
> +
> +     memInfo.dwLength = sizeof(MEMORYSTATUSEX);
> +     if (GlobalMemoryStatusEx(&memInfo))
> +             return memInfo;ullTotalPhys;

This fails to compile:

builtin/gc.c: In function 'total_ram':
builtin/gc.c:235:10: error: incompatible types when returning type 
'MEMORYSTATUSEX {aka struct _MEMORYSTATUSEX}' but 'long unsigned int' was 
expected
           return memInfo;ullTotalPhys;
                  ^~~~~~~
builtin/gc.c:234:2: error: this 'if' clause does not guard... 
[-Werror=misleading-indentation]
          if (GlobalMemoryStatusEx(&memInfo))
          ^~
builtin/gc.c:235:18: note: ...this statement, but the latter is misleadingly 
indented as if it were guarded by the 'if'
           return memInfo;ullTotalPhys;
                          ^~~~~~~~~~~~
builtin/gc.c:235:18: error: 'ullTotalPhys' undeclared (first use in this 
function)
builtin/gc.c:235:18: note: each undeclared identifier is reported only once for 
each function it appears in

I suspect that the first semicolon wanted to be a period instead. At least
it fixes the build here (that's all I can test, I'm at GitMerge and miss a
very interesting discussion about the serialized commit graph to write
this):

-- snip --
diff --git a/builtin/gc.c b/builtin/gc.c
index 4f46a99ab54..9c12f1ee9af 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -232,7 +232,7 @@ static inline unsigned long total_ram(void)
 
        memInfo.dwLength = sizeof(MEMORYSTATUSEX);
        if (GlobalMemoryStatusEx(&memInfo))
-               return memInfo;ullTotalPhys;
+               return memInfo.ullTotalPhys;
 #else
        fprintf(stderr, _("unrecognized platform, assuming %lu GB RAM\n"),
                default_ram);

-- snap --

Junio, may I ask you to put this into a SQUASH??? commit so that the
Windows build no longer fails?

Thanks,
Dscho