{"thread":{"id":"54370","subject":"[PATCH] Makefile: ASCII-sort += lists","startedAt":"2020-10-08T07:39:34Z","lastAt":"2020-10-09T16:12:36Z","messageCount":9,"participants":["Denton Liu","Johannes Schindelin","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"407124","messageId":"f0f1ef1f677133eabd1bce00c6cdbbcc6477f00b.1602142738.git.liu.denton@gmail.com","threadId":"54370","inReplyTo":null,"subject":"[PATCH] Makefile: ASCII-sort += lists","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2020-10-08T07:39:26Z","receivedAt":"2020-10-08T07:39:34Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"In 805d9eaf5e (Makefile: ASCII-sort += lists, 2020-03-21), the += lists\nin the Makefile were sorted into ASCII order. Since then, more out of\norder elements have been introduced. Resort these lists back into ASCII\norder.\n\nThis patch is best viewed with `--color-moved`.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n Makefile | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 5311b1d2c4..95571ee3fc 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -820,8 +820,8 @@ TEST_SHELL_PATH = $(SHELL_PATH)\n LIB_FILE = libgit.a\n XDIFF_LIB = xdiff/lib.a\n \n-GENERATED_H += config-list.h\n GENERATED_H += command-list.h\n+GENERATED_H += config-list.h\n \n LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \\\n \t$(FIND) . \\\n@@ -998,9 +998,9 @@ LIB_OBJS += sigchain.o\n LIB_OBJS += split-index.o\n LIB_OBJS += stable-qsort.o\n LIB_OBJS += strbuf.o\n-LIB_OBJS += strvec.o\n LIB_OBJS += streaming.o\n LIB_OBJS += string-list.o\n+LIB_OBJS += strvec.o\n LIB_OBJS += sub-process.o\n LIB_OBJS += submodule-config.o\n LIB_OBJS += submodule.o\n@@ -1066,15 +1066,15 @@ BUILTIN_OBJS += builtin/checkout-index.o\n BUILTIN_OBJS += builtin/checkout.o\n BUILTIN_OBJS += builtin/clean.o\n BUILTIN_OBJS += builtin/clone.o\n-BUILTIN_OBJS += builtin/credential-cache.o\n-BUILTIN_OBJS += builtin/credential-cache--daemon.o\n-BUILTIN_OBJS += builtin/credential-store.o\n BUILTIN_OBJS += builtin/column.o\n BUILTIN_OBJS += builtin/commit-graph.o\n BUILTIN_OBJS += builtin/commit-tree.o\n BUILTIN_OBJS += builtin/commit.o\n BUILTIN_OBJS += builtin/config.o\n BUILTIN_OBJS += builtin/count-objects.o\n+BUILTIN_OBJS += builtin/credential-cache--daemon.o\n+BUILTIN_OBJS += builtin/credential-cache.o\n+BUILTIN_OBJS += builtin/credential-store.o\n BUILTIN_OBJS += builtin/credential.o\n BUILTIN_OBJS += builtin/describe.o\n BUILTIN_OBJS += builtin/diff-files.o\n-- \n2.29.0.rc0.261.g7178c9af9c\n\n"},{"id":"407140","messageId":"nycvar.QRO.7.76.6.2010081156350.50@tvgsbejvaqbjf.bet","threadId":"54370","inReplyTo":"f0f1ef1f677133eabd1bce00c6cdbbcc6477f00b.1602142738.git.liu.denton@gmail.com","subject":"Re: [PATCH] Makefile: ASCII-sort += lists","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-10-08T10:06:47Z","receivedAt":"2020-10-08T10:06:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Denton,\n\nOn Thu, 8 Oct 2020, Denton Liu wrote:\n\n> In 805d9eaf5e (Makefile: ASCII-sort += lists, 2020-03-21), the += lists\n> in the Makefile were sorted into ASCII order. Since then, more out of\n> order elements have been introduced. Resort these lists back into ASCII\n> order.\n\nPersonally, I would write \"Re-sort\" or even \"Sort again\", so that readers\nsuch as myself do not stumble over the verb \"resort\" (as in \"We resort to\ndesperate measures\").\n\nAlso, this strikes me as yet another task that is so automatable that we\nshould really avoid bothering humans with it. I gave it a quick whirl, and\nthis Perl script seems to do the job for me:\n\n\t$key = '';\n\t@to_sort = ();\n\n\tsub flush_sorted {\n\t\tif ($#to_sort >= 0) {\n\t\t\tprint join('', sort @to_sort);\n\t\t\t@to_sort = ();\n\t\t}\n\t}\n\n\twhile (<>) {\n\t\tif (/^(\\S+) \\+=/) {\n\t\t\tif ($key ne $1) {\n\t\t\t\tflush_sorted;\n\t\t\t\t$key = $1;\n\t\t\t}\n\t\t\tpush @to_sort, $_;\n\t\t} else {\n\t\t\tflush_sorted;\n\t\t\tprint $_;\n\t\t}\n\t}\n\tflush_sorted;\n\nIt is not the most elegant Perl script I ever wrote, but it does the job\nfor me. And we could probably adapt and use it for other instances where\nwe want to keep things sorted (think `commands[]` in `git.c` and the\n`cmd_*()` declarations in `builtin.h`, for example) and hook it up in\n`ci/run-static-analysis.sh` for added benefit.\n\nMy little script also finds this:\n\n-- snip --\n@@ -1231,8 +1231,8 @@ space := $(empty) $(empty)\n\n ifdef SANITIZE\n SANITIZERS := $(foreach flag,$(subst $(comma),$(space),$(SANITIZE)),$(flag))\n-BASIC_CFLAGS += -fsanitize=$(SANITIZE) -fno-sanitize-recover=$(SANITIZE)\n BASIC_CFLAGS += -fno-omit-frame-pointer\n+BASIC_CFLAGS += -fsanitize=$(SANITIZE) -fno-sanitize-recover=$(SANITIZE)\n ifneq ($(filter undefined,$(SANITIZERS)),)\n BASIC_CFLAGS += -DSHA1DC_FORCE_ALIGNED_ACCESS\n endif\n-- snap --\n\nI am not _so_ sure that we want to order `BASIC_CFLAGS`, but then, it does\nnot hurt, does it?\n\nCiao,\nDscho\n\n>\n> This patch is best viewed with `--color-moved`.\n>\n> Signed-off-by: Denton Liu <liu.denton@gmail.com>\n> ---\n>  Makefile | 10 +++++-----\n>  1 file changed, 5 insertions(+), 5 deletions(-)\n>\n> diff --git a/Makefile b/Makefile\n> index 5311b1d2c4..95571ee3fc 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -820,8 +820,8 @@ TEST_SHELL_PATH = $(SHELL_PATH)\n>  LIB_FILE = libgit.a\n>  XDIFF_LIB = xdiff/lib.a\n>\n> -GENERATED_H += config-list.h\n>  GENERATED_H += command-list.h\n> +GENERATED_H += config-list.h\n>\n>  LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \\\n>  \t$(FIND) . \\\n> @@ -998,9 +998,9 @@ LIB_OBJS += sigchain.o\n>  LIB_OBJS += split-index.o\n>  LIB_OBJS += stable-qsort.o\n>  LIB_OBJS += strbuf.o\n> -LIB_OBJS += strvec.o\n>  LIB_OBJS += streaming.o\n>  LIB_OBJS += string-list.o\n> +LIB_OBJS += strvec.o\n>  LIB_OBJS += sub-process.o\n>  LIB_OBJS += submodule-config.o\n>  LIB_OBJS += submodule.o\n> @@ -1066,15 +1066,15 @@ BUILTIN_OBJS += builtin/checkout-index.o\n>  BUILTIN_OBJS += builtin/checkout.o\n>  BUILTIN_OBJS += builtin/clean.o\n>  BUILTIN_OBJS += builtin/clone.o\n> -BUILTIN_OBJS += builtin/credential-cache.o\n> -BUILTIN_OBJS += builtin/credential-cache--daemon.o\n> -BUILTIN_OBJS += builtin/credential-store.o\n>  BUILTIN_OBJS += builtin/column.o\n>  BUILTIN_OBJS += builtin/commit-graph.o\n>  BUILTIN_OBJS += builtin/commit-tree.o\n>  BUILTIN_OBJS += builtin/commit.o\n>  BUILTIN_OBJS += builtin/config.o\n>  BUILTIN_OBJS += builtin/count-objects.o\n> +BUILTIN_OBJS += builtin/credential-cache--daemon.o\n> +BUILTIN_OBJS += builtin/credential-cache.o\n> +BUILTIN_OBJS += builtin/credential-store.o\n>  BUILTIN_OBJS += builtin/credential.o\n>  BUILTIN_OBJS += builtin/describe.o\n>  BUILTIN_OBJS += builtin/diff-files.o\n> --\n> 2.29.0.rc0.261.g7178c9af9c\n>\n>\n"},{"id":"407161","messageId":"20201008160350.GB2823778@coredump.intra.peff.net","threadId":"54370","inReplyTo":"f0f1ef1f677133eabd1bce00c6cdbbcc6477f00b.1602142738.git.liu.denton@gmail.com","subject":"Re: [PATCH] Makefile: ASCII-sort += lists","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-10-08T16:03:50Z","receivedAt":"2020-10-08T16:03:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 08, 2020 at 12:39:26AM -0700, Denton Liu wrote:\n\n>  LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \\\n>  \t$(FIND) . \\\n> @@ -998,9 +998,9 @@ LIB_OBJS += sigchain.o\n>  LIB_OBJS += split-index.o\n>  LIB_OBJS += stable-qsort.o\n>  LIB_OBJS += strbuf.o\n> -LIB_OBJS += strvec.o\n>  LIB_OBJS += streaming.o\n>  LIB_OBJS += string-list.o\n> +LIB_OBJS += strvec.o\n>  LIB_OBJS += sub-process.o\n>  LIB_OBJS += submodule-config.o\n>  LIB_OBJS += submodule.o\n> @@ -1066,15 +1066,15 @@ BUILTIN_OBJS += builtin/checkout-index.o\n>  BUILTIN_OBJS += builtin/checkout.o\n>  BUILTIN_OBJS += builtin/clean.o\n>  BUILTIN_OBJS += builtin/clone.o\n> -BUILTIN_OBJS += builtin/credential-cache.o\n> -BUILTIN_OBJS += builtin/credential-cache--daemon.o\n> -BUILTIN_OBJS += builtin/credential-store.o\n>  BUILTIN_OBJS += builtin/column.o\n>  BUILTIN_OBJS += builtin/commit-graph.o\n>  BUILTIN_OBJS += builtin/commit-tree.o\n>  BUILTIN_OBJS += builtin/commit.o\n>  BUILTIN_OBJS += builtin/config.o\n>  BUILTIN_OBJS += builtin/count-objects.o\n> +BUILTIN_OBJS += builtin/credential-cache--daemon.o\n> +BUILTIN_OBJS += builtin/credential-cache.o\n> +BUILTIN_OBJS += builtin/credential-store.o\n>  BUILTIN_OBJS += builtin/credential.o\n>  BUILTIN_OBJS += builtin/describe.o\n>  BUILTIN_OBJS += builtin/diff-files.o\n\nWow. Both of these hunks are from me, and I clearly _tried_ to put them\nin the right place. I think...maybe I'm just bad at alphabetizing?\n\nCuriously, the only subtle part (ascii \"-\" versus \".\") was wrong in the\noriginal spot I moved it from, so I won't blame myself for that. :)\n\nAnyway, the whole patch looks good to me.\n\n-Peff\n"},{"id":"407162","messageId":"20201008160816.GC2823778@coredump.intra.peff.net","threadId":"54370","inReplyTo":"nycvar.QRO.7.76.6.2010081156350.50@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] Makefile: ASCII-sort += lists","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-10-08T16:08:16Z","receivedAt":"2020-10-08T16:08:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 08, 2020 at 12:06:47PM +0200, Johannes Schindelin wrote:\n\n> My little script also finds this:\n> \n> -- snip --\n> @@ -1231,8 +1231,8 @@ space := $(empty) $(empty)\n> \n>  ifdef SANITIZE\n>  SANITIZERS := $(foreach flag,$(subst $(comma),$(space),$(SANITIZE)),$(flag))\n> -BASIC_CFLAGS += -fsanitize=$(SANITIZE) -fno-sanitize-recover=$(SANITIZE)\n>  BASIC_CFLAGS += -fno-omit-frame-pointer\n> +BASIC_CFLAGS += -fsanitize=$(SANITIZE) -fno-sanitize-recover=$(SANITIZE)\n>  ifneq ($(filter undefined,$(SANITIZERS)),)\n>  BASIC_CFLAGS += -DSHA1DC_FORCE_ALIGNED_ACCESS\n>  endif\n> -- snap --\n> \n> I am not _so_ sure that we want to order `BASIC_CFLAGS`, but then, it does\n> not hurt, does it?\n\nI agree it would not be wrong to reorder here from the compiler's\nperspective, but:\n\n  - the current ordering is not arbitrary; the intent was to show that\n    we are enabling -fsanitize, and then follow it up with any other\n    related options (first any that apply to all sanitizers, of which\n    there is only one, and then any sanitizer-specific ones). The patch\n    above splits that logic apart.\n\n  - I'd worry that there are cases in which order _does_ matter to the\n    compiler. I'm not sure if anything that goes in CFLAGS might\n    qualify, but certainly order can matter for other parts of the\n    command-line (e.g., static library order).\n\n    So it might be setting us up for confusion later.\n\n-Peff\n"},{"id":"407171","messageId":"xmqq5z7ky5wc.fsf@gitster.c.googlers.com","threadId":"54370","inReplyTo":"f0f1ef1f677133eabd1bce00c6cdbbcc6477f00b.1602142738.git.liu.denton@gmail.com","subject":"Re: [PATCH] Makefile: ASCII-sort += lists","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-10-08T17:38:43Z","receivedAt":"2020-10-08T17:38:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu <liu.denton@gmail.com> writes:\n\n> In 805d9eaf5e (Makefile: ASCII-sort += lists, 2020-03-21), the += lists\n> in the Makefile were sorted into ASCII order. Since then, more out of\n> order elements have been introduced. Resort these lists back into ASCII\n> order.\n\nOK.  I tend to agree with the comment on Resort (by Dscho??  Sorry I\nforgot), so I'll locally do s/Resort/Sort/ while applying.\n\nBy the way, I have a mixed feelings about a patch like this very\nclose to prerelease freeze.  It is a good timing because not much\n_ought_ to be changing, and a small mechanical code churn like this\none whose correctness is so obvious is easy to accept without risk\nof breaking things.  But at the same time, it is distracting when we\nwould rather see contributors' and reviewers' time spent on double\nchecking that the changes made during this cycle did not introduce\nregressions.\n\nWill queue.  Thanks.\n"},{"id":"407191","messageId":"20201009014528.GA1314@generichostname","threadId":"54370","inReplyTo":"nycvar.QRO.7.76.6.2010081156350.50@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] Makefile: ASCII-sort += lists","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2020-10-09T01:45:28Z","receivedAt":"2020-10-09T01:45:35Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Hi Dscho,\n\nOn Thu, Oct 08, 2020 at 12:06:47PM +0200, Johannes Schindelin wrote:\n> Also, this strikes me as yet another task that is so automatable that we\n> should really avoid bothering humans with it.\n\nYep, I found these changes via a similar-looking Python script. I like\nthe Perl version, though, since it gives a path for inclusion so that we\ncan automate this task.\n\n> I gave it a quick whirl, and\n> this Perl script seems to do the job for me:\n> \n> \t$key = '';\n> \t@to_sort = ();\n> \n> \tsub flush_sorted {\n> \t\tif ($#to_sort >= 0) {\n> \t\t\tprint join('', sort @to_sort);\n> \t\t\t@to_sort = ();\n> \t\t}\n> \t}\n> \n> \twhile (<>) {\n> \t\tif (/^(\\S+) \\+=/) {\n> \t\t\tif ($key ne $1) {\n> \t\t\t\tflush_sorted;\n> \t\t\t\t$key = $1;\n> \t\t\t}\n> \t\t\tpush @to_sort, $_;\n> \t\t} else {\n> \t\t\tflush_sorted;\n> \t\t\tprint $_;\n> \t\t}\n> \t}\n> \tflush_sorted;\n> \n> It is not the most elegant Perl script I ever wrote, but it does the job\n> for me. And we could probably adapt and use it for other instances where\n> we want to keep things sorted (think `commands[]` in `git.c` and the\n> `cmd_*()` declarations in `builtin.h`, for example) and hook it up in\n> `ci/run-static-analysis.sh` for added benefit.\n> \n> My little script also finds this:\n> \n> -- snip --\n> @@ -1231,8 +1231,8 @@ space := $(empty) $(empty)\n> \n>  ifdef SANITIZE\n>  SANITIZERS := $(foreach flag,$(subst $(comma),$(space),$(SANITIZE)),$(flag))\n> -BASIC_CFLAGS += -fsanitize=$(SANITIZE) -fno-sanitize-recover=$(SANITIZE)\n>  BASIC_CFLAGS += -fno-omit-frame-pointer\n> +BASIC_CFLAGS += -fsanitize=$(SANITIZE) -fno-sanitize-recover=$(SANITIZE)\n>  ifneq ($(filter undefined,$(SANITIZERS)),)\n>  BASIC_CFLAGS += -DSHA1DC_FORCE_ALIGNED_ACCESS\n>  endif\n> -- snap --\n\nI opted to exclude this hunk because it didn't seem like a list that\nshould be sorted. Perhaps if we include this in the static-analysis\nscript, we could define a whitelist of lists that we want to keep\nsorted?\n\nThanks,\nDenton\n"},{"id":"407208","messageId":"nycvar.QRO.7.76.6.2010091246420.50@tvgsbejvaqbjf.bet","threadId":"54370","inReplyTo":"20201008160816.GC2823778@coredump.intra.peff.net","subject":"Re: [PATCH] Makefile: ASCII-sort += lists","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-10-09T10:49:15Z","receivedAt":"2020-10-09T13:07:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Thu, 8 Oct 2020, Jeff King wrote:\n\n> On Thu, Oct 08, 2020 at 12:06:47PM +0200, Johannes Schindelin wrote:\n>\n> > My little script also finds this:\n> >\n> > -- snip --\n> > @@ -1231,8 +1231,8 @@ space := $(empty) $(empty)\n> >\n> >  ifdef SANITIZE\n> >  SANITIZERS := $(foreach flag,$(subst $(comma),$(space),$(SANITIZE)),$(flag))\n> > -BASIC_CFLAGS += -fsanitize=$(SANITIZE) -fno-sanitize-recover=$(SANITIZE)\n> >  BASIC_CFLAGS += -fno-omit-frame-pointer\n> > +BASIC_CFLAGS += -fsanitize=$(SANITIZE) -fno-sanitize-recover=$(SANITIZE)\n> >  ifneq ($(filter undefined,$(SANITIZERS)),)\n> >  BASIC_CFLAGS += -DSHA1DC_FORCE_ALIGNED_ACCESS\n> >  endif\n> > -- snap --\n> >\n> > I am not _so_ sure that we want to order `BASIC_CFLAGS`, but then, it does\n> > not hurt, does it?\n>\n> I agree it would not be wrong to reorder here from the compiler's\n> perspective, but:\n>\n>   - the current ordering is not arbitrary; the intent was to show that\n>     we are enabling -fsanitize, and then follow it up with any other\n>     related options (first any that apply to all sanitizers, of which\n>     there is only one, and then any sanitizer-specific ones). The patch\n>     above splits that logic apart.\n>\n>   - I'd worry that there are cases in which order _does_ matter to the\n>     compiler. I'm not sure if anything that goes in CFLAGS might\n>     qualify, but certainly order can matter for other parts of the\n>     command-line (e.g., static library order).\n>\n>     So it might be setting us up for confusion later.\n\nFair enough. It's easy to exclude `.*_CFLAGS` via a negative look-behind:\n\n\t$key = '';\n\t@to_sort = ();\n\twhile (<>) {\n\t\tif ($#to_sort >= 0) {\n\t\t\tif (/^$key \\+=/) {\n\t\t\t\tpush @to_sort, $_;\n\t\t\t\tnext;\n\t\t\t}\n\t\t\tprint join('', sort @to_sort);\n\t\t\t@to_sort = ();\n\t\t}\n\t\tif (/^(\\S+(?<!_CFLAGS)) \\+=/) {\n\t\t\t$key = $1;\n\t\t\tpush @to_sort, $_;\n\t\t} else {\n\t\t\tprint $_;\n\t\t}\n\t}\n\n\tif ($#to_sort >= 0) {\n\t\tprint join('', sort @to_sort);\n\t}\n\nCiao,\nDscho\n"},{"id":"407209","messageId":"nycvar.QRO.7.76.6.2010091250200.50@tvgsbejvaqbjf.bet","threadId":"54370","inReplyTo":"20201009014528.GA1314@generichostname","subject":"Re: [PATCH] Makefile: ASCII-sort += lists","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-10-09T10:51:11Z","receivedAt":"2020-10-09T13:08:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Denton,\n\nOn Thu, 8 Oct 2020, Denton Liu wrote:\n\n> On Thu, Oct 08, 2020 at 12:06:47PM +0200, Johannes Schindelin wrote:\n> > Also, this strikes me as yet another task that is so automatable that we\n> > should really avoid bothering humans with it.\n>\n> Yep, I found these changes via a similar-looking Python script. I like\n> the Perl version, though, since it gives a path for inclusion so that we\n> can automate this task.\n\nHeh, when it comes to string processing, I always reach for Perl.\n\n> > I gave it a quick whirl, and\n> > this Perl script seems to do the job for me:\n> >\n> > \t$key = '';\n> > \t@to_sort = ();\n> >\n> > \tsub flush_sorted {\n> > \t\tif ($#to_sort >= 0) {\n> > \t\t\tprint join('', sort @to_sort);\n> > \t\t\t@to_sort = ();\n> > \t\t}\n> > \t}\n> >\n> > \twhile (<>) {\n> > \t\tif (/^(\\S+) \\+=/) {\n> > \t\t\tif ($key ne $1) {\n> > \t\t\t\tflush_sorted;\n> > \t\t\t\t$key = $1;\n> > \t\t\t}\n> > \t\t\tpush @to_sort, $_;\n> > \t\t} else {\n> > \t\t\tflush_sorted;\n> > \t\t\tprint $_;\n> > \t\t}\n> > \t}\n> > \tflush_sorted;\n> >\n> > It is not the most elegant Perl script I ever wrote, but it does the job\n> > for me. And we could probably adapt and use it for other instances where\n> > we want to keep things sorted (think `commands[]` in `git.c` and the\n> > `cmd_*()` declarations in `builtin.h`, for example) and hook it up in\n> > `ci/run-static-analysis.sh` for added benefit.\n> >\n> > My little script also finds this:\n> >\n> > -- snip --\n> > @@ -1231,8 +1231,8 @@ space := $(empty) $(empty)\n> >\n> >  ifdef SANITIZE\n> >  SANITIZERS := $(foreach flag,$(subst $(comma),$(space),$(SANITIZE)),$(flag))\n> > -BASIC_CFLAGS += -fsanitize=$(SANITIZE) -fno-sanitize-recover=$(SANITIZE)\n> >  BASIC_CFLAGS += -fno-omit-frame-pointer\n> > +BASIC_CFLAGS += -fsanitize=$(SANITIZE) -fno-sanitize-recover=$(SANITIZE)\n> >  ifneq ($(filter undefined,$(SANITIZERS)),)\n> >  BASIC_CFLAGS += -DSHA1DC_FORCE_ALIGNED_ACCESS\n> >  endif\n> > -- snap --\n>\n> I opted to exclude this hunk because it didn't seem like a list that\n> should be sorted. Perhaps if we include this in the static-analysis\n> script, we could define a whitelist of lists that we want to keep\n> sorted?\n\nI agree, modulo s/whitelist/allow list/.\n\nThanks,\nDscho\n"},{"id":"407216","messageId":"xmqqy2kftm36.fsf@gitster.c.googlers.com","threadId":"54370","inReplyTo":"nycvar.QRO.7.76.6.2010091250200.50@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] Makefile: ASCII-sort += lists","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-10-09T16:12:29Z","receivedAt":"2020-10-09T16:12:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> > -BASIC_CFLAGS += -fsanitize=$(SANITIZE) -fno-sanitize-recover=$(SANITIZE)\n>> >  BASIC_CFLAGS += -fno-omit-frame-pointer\n>> > +BASIC_CFLAGS += -fsanitize=$(SANITIZE) -fno-sanitize-recover=$(SANITIZE)\n>> >  ifneq ($(filter undefined,$(SANITIZERS)),)\n>> >  BASIC_CFLAGS += -DSHA1DC_FORCE_ALIGNED_ACCESS\n>> >  endif\n>> > -- snap --\n>>\n>> I opted to exclude this hunk because it didn't seem like a list that\n>> should be sorted. Perhaps if we include this in the static-analysis\n>> script, we could define a whitelist of lists that we want to keep\n>> sorted?\n>\n> I agree, modulo s/whitelist/allow list/.\n\nIf we were to do this, I agree that explicitly enumerating \"lists\nwhose elements must be sorted\" would be a much better approach than\ndeclaring that our lists by default must be sorted and have a list\nof \"lists whose elements are sorted in an order that has meaning,\nnot just by codepoints\".\n\nBut I somehow find the use of allow-list (as a concept [*1*])\nawkward in this context.  Technically, a list of things whose\nsortedness we care about may be \"allowed to be automatically\nmodified\", and the remainder would be \"forbidden from getting\ntouched\".  But both are quite awkward way to think about them.\n\nIt would become even more awkward if the list is going to be used in\na make target whose name has \"check\" in it, in which case the target\nwould only point out problem so that the user can fix, and at that\npoint, the said list would become a list of things that are \"allowed\nto be checked\".\n\n\n[Footnote]\n\n*1* ... not the phrase---I do not care in what color the allowed\n    things are painted.\n"}]}