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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 7, 2018, 10:48 UTC
Message-ID: <nycvar.QRO.7.76.6.1803071140130.20700@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz>
In-Reply-To: <20180306104158.6541-4-pclouds@gmail.com>

Hi Duy,

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

Show 33 quoted lines

> 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