Re: [PATCH] Makefile: ASCII-sort += lists

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Oct 8, 2020, 10:06 UTC
Message-ID: <nycvar.QRO.7.76.6.2010081156350.50@tvgsbejvaqbjf.bet>
In-Reply-To: <f0f1ef1f677133eabd1bce00c6cdbbcc6477f00b.1602142738.git.liu.denton@gmail.com>

Hi Denton,

On Thu, 8 Oct 2020, Denton Liu wrote:

> In 805d9eaf5e (Makefile: ASCII-sort += lists, 2020-03-21), the += lists
> in the Makefile were sorted into ASCII order. Since then, more out of
> order elements have been introduced. Resort these lists back into ASCII
> order.

Personally, I would write "Re-sort" or even "Sort again", so that readers such as myself do not stumble over the verb "resort" (as in "We resort to desperate measures").

Also, this strikes me as yet another task that is so automatable that we should really avoid bothering humans with it. I gave it a quick whirl, and this Perl script seems to do the job for me:

	$key = '';
	@to_sort = ();

	sub flush_sorted {
		if ($#to_sort >= 0) {
			print join('', sort @to_sort);
			@to_sort = ();
		}
	}

	while (<>) {
		if (/^(\S+) \+=/) {
			if ($key ne $1) {
				flush_sorted;
				$key = $1;
			}
			push @to_sort, $_;
		} else {
			flush_sorted;
			print $_;
		}
	}
	flush_sorted;

It is not the most elegant Perl script I ever wrote, but it does the job for me. And we could probably adapt and use it for other instances where we want to keep things sorted (think `commands[]` in `git.c` and the `cmd_*()` declarations in `builtin.h`, for example) and hook it up in `ci/run-static-analysis.sh` for added benefit.

My little script also finds this:

-- snip --

@@ -1231,8 +1231,8 @@ space := $(empty) $(empty)

 ifdef SANITIZE
 SANITIZERS := $(foreach flag,$(subst $(comma),$(space),$(SANITIZE)),$(flag))
-BASIC_CFLAGS += -fsanitize=$(SANITIZE) -fno-sanitize-recover=$(SANITIZE)
 BASIC_CFLAGS += -fno-omit-frame-pointer
+BASIC_CFLAGS += -fsanitize=$(SANITIZE) -fno-sanitize-recover=$(SANITIZE)
 ifneq ($(filter undefined,$(SANITIZERS)),)
 BASIC_CFLAGS += -DSHA1DC_FORCE_ALIGNED_ACCESS
 endif
-- snap --

I am not _so_ sure that we want to order `BASIC_CFLAGS`, but then, it does
not hurt, does it?

Ciao,
Dscho

>
> This patch is best viewed with `--color-moved`.
>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
>  Makefile | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 5311b1d2c4..95571ee3fc 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -820,8 +820,8 @@ TEST_SHELL_PATH = $(SHELL_PATH)
>  LIB_FILE = libgit.a
>  XDIFF_LIB = xdiff/lib.a
>
> -GENERATED_H += config-list.h
>  GENERATED_H += command-list.h
> +GENERATED_H += config-list.h
>
>  LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \
>  	$(FIND) . \
> @@ -998,9 +998,9 @@ LIB_OBJS += sigchain.o
>  LIB_OBJS += split-index.o
>  LIB_OBJS += stable-qsort.o
>  LIB_OBJS += strbuf.o
> -LIB_OBJS += strvec.o
>  LIB_OBJS += streaming.o
>  LIB_OBJS += string-list.o
> +LIB_OBJS += strvec.o
>  LIB_OBJS += sub-process.o
>  LIB_OBJS += submodule-config.o
>  LIB_OBJS += submodule.o
> @@ -1066,15 +1066,15 @@ BUILTIN_OBJS += builtin/checkout-index.o
>  BUILTIN_OBJS += builtin/checkout.o
>  BUILTIN_OBJS += builtin/clean.o
>  BUILTIN_OBJS += builtin/clone.o
> -BUILTIN_OBJS += builtin/credential-cache.o
> -BUILTIN_OBJS += builtin/credential-cache--daemon.o
> -BUILTIN_OBJS += builtin/credential-store.o
>  BUILTIN_OBJS += builtin/column.o
>  BUILTIN_OBJS += builtin/commit-graph.o
>  BUILTIN_OBJS += builtin/commit-tree.o
>  BUILTIN_OBJS += builtin/commit.o
>  BUILTIN_OBJS += builtin/config.o
>  BUILTIN_OBJS += builtin/count-objects.o
> +BUILTIN_OBJS += builtin/credential-cache--daemon.o
> +BUILTIN_OBJS += builtin/credential-cache.o
> +BUILTIN_OBJS += builtin/credential-store.o
>  BUILTIN_OBJS += builtin/credential.o
>  BUILTIN_OBJS += builtin/describe.o
>  BUILTIN_OBJS += builtin/diff-files.o
> --
> 2.29.0.rc0.261.g7178c9af9c
>
>