{"thread":{"id":"55326","subject":"[PATCH 0/7] Sort lists and add static-analysis","startedAt":"2021-03-16T00:57:19Z","lastAt":"2021-03-17T22:02:15Z","messageCount":24,"participants":["Denton Liu","Eric Sunshine","Bagas Sanjaya","Ævar Arnfjörð Bjarmason","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"419223","messageId":"cover.1615856156.git.liu.denton@gmail.com","threadId":"55326","inReplyTo":null,"subject":"[PATCH 0/7] Sort lists and add static-analysis","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-03-16T00:56:19Z","receivedAt":"2021-03-16T00:57:19Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"As a follow-up to [0], sort some file lists and create a static-analysis\ncheck to ensure that those lists don't ever become un-sorted.\n\n[0]: https://lore.kernel.org/git/nycvar.QRO.7.76.6.2010081156350.50@tvgsbejvaqbjf.bet/\n\nDenton Liu (7):\n Makefile: mark 'check-builtins' as a .PHONY target\n Makefile: ASCII-sort LIB_OBJS\n builtin.h: ASCII-sort list of functions\n test-tool.h: ASCII-sort list of functions\n Makefile: add 'check-sort' target\n ci/run-static-analysis.sh: make check-builtins\n ci/run-static-analysis.sh: make check-sort\n\n Makefile | 30 ++++++++++++++++++++++++++++--\n builtin.h | 22 +++++++++++-----------\n check-sort.perl | 31 +++++++++++++++++++++++++++++++\n ci/run-static-analysis.sh | 2 +-\n t/helper/test-tool.h | 6 +++---\n 5 files changed, 74 insertions(+), 17 deletions(-)\n create mode 100755 check-sort.perl\n\n-- \n2.31.0.rc2.261.g7f71774620\n\n"},{"id":"419224","messageId":"aee5f6d1e2e123505c1e3b0b8ff952cce7de3436.1615856156.git.liu.denton@gmail.com","threadId":"55326","inReplyTo":"cover.1615856156.git.liu.denton@gmail.com","subject":"[PATCH 3/7] builtin.h: ASCII-sort list of functions","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-03-16T00:56:22Z","receivedAt":"2021-03-16T00:57:20Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"The list of builtin functions have, over time, gotten slightly out of\nASCII order. Sort this list to bring them back into order.\n\nASCII sorting was chosen over strict alphabetical order for the same\nreason as 805d9eaf5e (Makefile: ASCII-sort += lists, 2020-03-21): the\npurpose of maintaining the sorted list is to ensure line insertions are\ndeterministic. By using ASCII ordering, it is more easily mechanically\nreproducible in the future, such as by using :sort in Vim.\n\nThis patch is best viewed with `--color-moved`.\n\nSigned-off-by: Denton Liu \n---\n builtin.h | 22 +++++++++++-----------\n 1 file changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin.h b/builtin.h\nindex b6ce981b73..dd6beff6b2 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -122,16 +122,16 @@ int cmd_branch(int argc, const char **argv, const char *prefix);\n int cmd_bugreport(int argc, const char **argv, const char *prefix);\n int cmd_bundle(int argc, const char **argv, const char *prefix);\n int cmd_cat_file(int argc, const char **argv, const char *prefix);\n-int cmd_checkout(int argc, const char **argv, const char *prefix);\n-int cmd_checkout_index(int argc, const char **argv, const char *prefix);\n int cmd_check_attr(int argc, const char **argv, const char *prefix);\n int cmd_check_ignore(int argc, const char **argv, const char *prefix);\n int cmd_check_mailmap(int argc, const char **argv, const char *prefix);\n int cmd_check_ref_format(int argc, const char **argv, const char *prefix);\n+int cmd_checkout(int argc, const char **argv, const char *prefix);\n+int cmd_checkout_index(int argc, const char **argv, const char *prefix);\n int cmd_cherry(int argc, const char **argv, const char *prefix);\n int cmd_cherry_pick(int argc, const char **argv, const char *prefix);\n-int cmd_clone(int argc, const char **argv, const char *prefix);\n int cmd_clean(int argc, const char **argv, const char *prefix);\n+int cmd_clone(int argc, const char **argv, const char *prefix);\n int cmd_column(int argc, const char **argv, const char *prefix);\n int cmd_commit(int argc, const char **argv, const char *prefix);\n int cmd_commit_graph(int argc, const char **argv, const char *prefix);\n@@ -143,9 +143,9 @@ int cmd_credential_cache(int argc, const char **argv, const char *prefix);\n int cmd_credential_cache_daemon(int argc, const char **argv, const char *prefix);\n int cmd_credential_store(int argc, const char **argv, const char *prefix);\n int cmd_describe(int argc, const char **argv, const char *prefix);\n+int cmd_diff(int argc, const char **argv, const char *prefix);\n int cmd_diff_files(int argc, const char **argv, const char *prefix);\n int cmd_diff_index(int argc, const char **argv, const char *prefix);\n-int cmd_diff(int argc, const char **argv, const char *prefix);\n int cmd_diff_tree(int argc, const char **argv, const char *prefix);\n int cmd_difftool(int argc, const char **argv, const char *prefix);\n int cmd_env__helper(int argc, const char **argv, const char *prefix);\n@@ -169,16 +169,16 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix);\n int cmd_log(int argc, const char **argv, const char *prefix);\n int cmd_log_reflog(int argc, const char **argv, const char *prefix);\n int cmd_ls_files(int argc, const char **argv, const char *prefix);\n-int cmd_ls_tree(int argc, const char **argv, const char *prefix);\n int cmd_ls_remote(int argc, const char **argv, const char *prefix);\n+int cmd_ls_tree(int argc, const char **argv, const char *prefix);\n int cmd_mailinfo(int argc, const char **argv, const char *prefix);\n int cmd_mailsplit(int argc, const char **argv, const char *prefix);\n int cmd_maintenance(int argc, const char **argv, const char *prefix);\n int cmd_merge(int argc, const char **argv, const char *prefix);\n int cmd_merge_base(int argc, const char **argv, const char *prefix);\n+int cmd_merge_file(int argc, const char **argv, const char *prefix);\n int cmd_merge_index(int argc, const char **argv, const char *prefix);\n int cmd_merge_ours(int argc, const char **argv, const char *prefix);\n-int cmd_merge_file(int argc, const char **argv, const char *prefix);\n int cmd_merge_recursive(int argc, const char **argv, const char *prefix);\n int cmd_merge_tree(int argc, const char **argv, const char *prefix);\n int cmd_mktag(int argc, const char **argv, const char *prefix);\n@@ -189,6 +189,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix);\n int cmd_notes(int argc, const char **argv, const char *prefix);\n int cmd_pack_objects(int argc, const char **argv, const char *prefix);\n int cmd_pack_redundant(int argc, const char **argv, const char *prefix);\n+int cmd_pack_refs(int argc, const char **argv, const char *prefix);\n int cmd_patch_id(int argc, const char **argv, const char *prefix);\n int cmd_prune(int argc, const char **argv, const char *prefix);\n int cmd_prune_packed(int argc, const char **argv, const char *prefix);\n@@ -204,6 +205,7 @@ int cmd_remote(int argc, const char **argv, const char *prefix);\n int cmd_remote_ext(int argc, const char **argv, const char *prefix);\n int cmd_remote_fd(int argc, const char **argv, const char *prefix);\n int cmd_repack(int argc, const char **argv, const char *prefix);\n+int cmd_replace(int argc, const char **argv, const char *prefix);\n int cmd_rerere(int argc, const char **argv, const char *prefix);\n int cmd_reset(int argc, const char **argv, const char *prefix);\n int cmd_restore(int argc, const char **argv, const char *prefix);\n@@ -216,9 +218,10 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix);\n int cmd_show(int argc, const char **argv, const char *prefix);\n int cmd_show_branch(int argc, const char **argv, const char *prefix);\n int cmd_show_index(int argc, const char **argv, const char *prefix);\n+int cmd_show_ref(int argc, const char **argv, const char *prefix);\n int cmd_sparse_checkout(int argc, const char **argv, const char *prefix);\n-int cmd_status(int argc, const char **argv, const char *prefix);\n int cmd_stash(int argc, const char **argv, const char *prefix);\n+int cmd_status(int argc, const char **argv, const char *prefix);\n int cmd_stripspace(int argc, const char **argv, const char *prefix);\n int cmd_submodule__helper(int argc, const char **argv, const char *prefix);\n int cmd_switch(int argc, const char **argv, const char *prefix);\n@@ -235,14 +238,11 @@ int cmd_upload_archive_writer(int argc, const char **argv, const char *prefix);\n int cmd_upload_pack(int argc, const char **argv, const char *prefix);\n int cmd_var(int argc, const char **argv, const char *prefix);\n int cmd_verify_commit(int argc, const char **argv, const char *prefix);\n+int cmd_verify_pack(int argc, const char **argv, const char *prefix);\n int cmd_verify_tag(int argc, const char **argv, const char *prefix);\n int cmd_version(int argc, const char **argv, const char *prefix);\n int cmd_whatchanged(int argc, const char **argv, const char *prefix);\n int cmd_worktree(int argc, const char **argv, const char *prefix);\n int cmd_write_tree(int argc, const char **argv, const char *prefix);\n-int cmd_verify_pack(int argc, const char **argv, const char *prefix);\n-int cmd_show_ref(int argc, const char **argv, const char *prefix);\n-int cmd_pack_refs(int argc, const char **argv, const char *prefix);\n-int cmd_replace(int argc, const char **argv, const char *prefix);\n \n #endif\n-- \n2.31.0.rc2.261.g7f71774620\n\n"},{"id":"419225","messageId":"1d31e6aee77d374b00696597cf727d6e0d494632.1615856156.git.liu.denton@gmail.com","threadId":"55326","inReplyTo":"cover.1615856156.git.liu.denton@gmail.com","subject":"[PATCH 2/7] Makefile: ASCII-sort LIB_OBJS","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-03-16T00:56:21Z","receivedAt":"2021-03-16T00:57:20Z","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. Sort these lists back into ASCII\norder.\n\nThis patch is best viewed with `--color-moved`.\n\nSigned-off-by: Denton Liu \n---\n Makefile | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 6dbecdb606..5832aa33da 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -858,8 +858,8 @@ LIB_OBJS += date.o\n LIB_OBJS += decorate.o\n LIB_OBJS += delta-islands.o\n LIB_OBJS += diff-delta.o\n-LIB_OBJS += diff-merges.o\n LIB_OBJS += diff-lib.o\n+LIB_OBJS += diff-merges.o\n LIB_OBJS += diff-no-index.o\n LIB_OBJS += diff.o\n LIB_OBJS += diffcore-break.o\n@@ -910,8 +910,8 @@ LIB_OBJS += mailmap.o\n LIB_OBJS += match-trees.o\n LIB_OBJS += mem-pool.o\n LIB_OBJS += merge-blobs.o\n-LIB_OBJS += merge-ort.o\n LIB_OBJS += merge-ort-wrappers.o\n+LIB_OBJS += merge-ort.o\n LIB_OBJS += merge-recursive.o\n LIB_OBJS += merge.o\n LIB_OBJS += mergesort.o\n-- \n2.31.0.rc2.261.g7f71774620\n\n"},{"id":"419226","messageId":"c889973be754336005d0155ce670f16e322df9c3.1615856156.git.liu.denton@gmail.com","threadId":"55326","inReplyTo":"cover.1615856156.git.liu.denton@gmail.com","subject":"[PATCH 1/7] Makefile: mark 'check-builtins' as a .PHONY target","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-03-16T00:56:20Z","receivedAt":"2021-03-16T00:57:20Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Then convention in Git's Makefile is to mark targets which do not\nactually generate any precious files (such as static checking targets).\n.PHONY enables the target to still run, even if a file is erroneously\ncreated with the same name as the target.\n\nMark 'check-builtins' as a .PHONY target.\n\nSigned-off-by: Denton Liu \n---\n Makefile | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/Makefile b/Makefile\nindex dfb0f1000f..6dbecdb606 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3236,6 +3236,7 @@ check-docs::\n \n ### Make sure built-ins do not have dups and listed in git.c\n #\n+.PHONY: check-builtins\n check-builtins::\n \t./check-builtins.sh\n \n-- \n2.31.0.rc2.261.g7f71774620\n\n"},{"id":"419227","messageId":"5088e93d76e44de9d079b7b2296b8c810828a2f5.1615856156.git.liu.denton@gmail.com","threadId":"55326","inReplyTo":"cover.1615856156.git.liu.denton@gmail.com","subject":"[PATCH 5/7] Makefile: add 'check-sort' target","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-03-16T00:56:24Z","receivedAt":"2021-03-16T00:57:20Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"In the previous few commits, we sorted many lists into ASCII-order. In\norder to ensure that they remain that way, add the 'check-sort' target.\n\nThe check-sort.perl program ensures that consecutive lines that match\nthe same regex are sorted in ASCII-order. The 'check-sort' target runs\nthe check-sort.perl program on some files which are known to contain\nsorted lists.\n\nSigned-off-by: Denton Liu \n---\n\nNotes:\n Full disclaimer: this is the first time I've written anything in Perl.\n Please let me know if I'm doing anything unconventional :)\n\n Makefile | 25 +++++++++++++++++++++++++\n check-sort.perl | 31 +++++++++++++++++++++++++++++++\n 2 files changed, 56 insertions(+)\n create mode 100755 check-sort.perl\n\ndiff --git a/Makefile b/Makefile\nindex 5832aa33da..b23dff384d 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3240,6 +3240,31 @@ check-docs::\n check-builtins::\n \t./check-builtins.sh\n \n+.PHONY: check-sort\n+check-sort::\n+\t./check-sort.perl \\\n+\t\t'ALL_COMMANDS \\+=' \\\n+\t\t'ALL_COMMANDS_TO_INSTALL \\+=' \\\n+\t\t'BINDIR_PROGRAMS_NEED_X \\+=' \\\n+\t\t'BINDIR_PROGRAMS_NO_X \\+=' \\\n+\t\t'BUILTIN_OBJS \\+=' \\\n+\t\t'BUILT_INS \\+=' \\\n+\t\t'FUZZ_OBJS \\+=' \\\n+\t\t'GENERATED_H \\+=' \\\n+\t\t'LIB_OBJS \\+=' \\\n+\t\t'SCRIPT_LIB \\+=' \\\n+\t\t'SCRIPT_PERL \\+=' \\\n+\t\t'SCRIPT_PYTHON \\+=' \\\n+\t\t'SCRIPT_SH \\+=' \\\n+\t\t'TEST_BUILTINS_OBJS \\+=' \\\n+\t\t'TEST_PROGRAMS_NEED_X \\+=' \\\n+\t\t'THIRD_PARTY_SOURCES \\+=' \\\n+\t\t'XDIFF_OBJS \\+=' \\\n+\t\t) {\n+\tmy $matched = 0;\n+\tchomp;\n+\n+\tfor my $regex (@regexes) {\n+\t\tnext unless $_ =~ $regex;\n+\n+\t\tif ($last_regex == $regex) {\n+\t\t\tdie \"duplicate lines: '$_'\\n\" unless $last_line ne $_;\n+\t\t\tdie \"unsorted lines: '$last_line' before '$_'\\n\" unless $last_line lt $_;\n+\t\t}\n+\n+\t\t$matched = 1;\n+\t\t$last_regex = $regex;\n+\t\t$last_line = $_;\n+\t}\n+\n+\tunless ($matched) {\n+\t\t$last_regex = 0;\n+\t\t$last_line = '';\n+\t}\n+}\n-- \n2.31.0.rc2.261.g7f71774620\n\n"},{"id":"419228","messageId":"8696305c1efcea42a2227f8efaa7e1e73c71d32c.1615856156.git.liu.denton@gmail.com","threadId":"55326","inReplyTo":"cover.1615856156.git.liu.denton@gmail.com","subject":"[PATCH 6/7] ci/run-static-analysis.sh: make check-builtins","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-03-16T00:56:25Z","receivedAt":"2021-03-16T00:57:20Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"To ensure that any newly introduced builtins are in git.c command list\nand that no duplicate script files exist, run the 'check-builtins'\ntarget as part of static-analysis.\n\nSigned-off-by: Denton Liu \n---\n ci/run-static-analysis.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh\nindex 65bcebda41..1ae122fc70 100755\n--- a/ci/run-static-analysis.sh\n+++ b/ci/run-static-analysis.sh\n@@ -26,7 +26,7 @@ then\n \texit 1\n fi\n \n-make hdr-check ||\n+make hdr-check check-builtins ||\n exit 1\n \n save_good_tree\n-- \n2.31.0.rc2.261.g7f71774620\n\n"},{"id":"419229","messageId":"b817541c2754ec8fc4edc1ae774e460912e7c0a8.1615856156.git.liu.denton@gmail.com","threadId":"55326","inReplyTo":"cover.1615856156.git.liu.denton@gmail.com","subject":"[PATCH 4/7] test-tool.h: ASCII-sort list of functions","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-03-16T00:56:23Z","receivedAt":"2021-03-16T00:57:20Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"The list of test-tool functions have, over time, gotten slightly out of\nASCII order. Sort this list to bring them back into order.\n\nASCII sorting was chosen over strict alphabetical order for the same\nreason as 805d9eaf5e (Makefile: ASCII-sort += lists, 2020-03-21): the\npurpose of maintaining the sorted list is to ensure line insertions are\ndeterministic. By using ASCII ordering, it is more easily mechanically\nreproducible in the future, such as by using :sort in Vim.\n\nThis patch is best viewed with `--color-moved`.\n\nSigned-off-by: Denton Liu \n---\n t/helper/test-tool.h | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\nindex 28072c0ad5..9856e84149 100644\n--- a/t/helper/test-tool.h\n+++ b/t/helper/test-tool.h\n@@ -22,14 +22,15 @@ int cmd__example_decorate(int argc, const char **argv);\n int cmd__fast_rebase(int argc, const char **argv);\n int cmd__genrandom(int argc, const char **argv);\n int cmd__genzeros(int argc, const char **argv);\n-int cmd__hashmap(int argc, const char **argv);\n int cmd__hash_speed(int argc, const char **argv);\n+int cmd__hashmap(int argc, const char **argv);\n int cmd__index_version(int argc, const char **argv);\n int cmd__json_writer(int argc, const char **argv);\n int cmd__lazy_init_name_hash(int argc, const char **argv);\n int cmd__match_trees(int argc, const char **argv);\n int cmd__mergesort(int argc, const char **argv);\n int cmd__mktemp(int argc, const char **argv);\n+int cmd__oid_array(int argc, const char **argv);\n int cmd__oidmap(int argc, const char **argv);\n int cmd__online_cpus(int argc, const char **argv);\n int cmd__parse_options(int argc, const char **argv);\n@@ -52,7 +53,6 @@ int cmd__run_command(int argc, const char **argv);\n int cmd__scrap_cache_tree(int argc, const char **argv);\n int cmd__serve_v2(int argc, const char **argv);\n int cmd__sha1(int argc, const char **argv);\n-int cmd__oid_array(int argc, const char **argv);\n int cmd__sha256(int argc, const char **argv);\n int cmd__sigchain(int argc, const char **argv);\n int cmd__strcmp_offset(int argc, const char **argv);\n@@ -62,8 +62,8 @@ int cmd__submodule_nested_repo_config(int argc, const char **argv);\n int cmd__subprocess(int argc, const char **argv);\n int cmd__trace2(int argc, const char **argv);\n int cmd__urlmatch_normalization(int argc, const char **argv);\n-int cmd__xml_encode(int argc, const char **argv);\n int cmd__wildmatch(int argc, const char **argv);\n+int cmd__xml_encode(int argc, const char **argv);\n #ifdef GIT_WINDOWS_NATIVE\n int cmd__windows_named_pipe(int argc, const char **argv);\n #endif\n-- \n2.31.0.rc2.261.g7f71774620\n\n"},{"id":"419230","messageId":"09c993a8f78d67bcff196c5b8173a8e5f6fd805b.1615856156.git.liu.denton@gmail.com","threadId":"55326","inReplyTo":"cover.1615856156.git.liu.denton@gmail.com","subject":"[PATCH 7/7] ci/run-static-analysis.sh: make check-sort","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-03-16T00:56:26Z","receivedAt":"2021-03-16T00:57:20Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"To ensure that lists in some files remain in sorted order, run the\n'check-sort' target as part of static-analysis.\n\nSigned-off-by: Denton Liu \n---\n ci/run-static-analysis.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh\nindex 1ae122fc70..92437c09df 100755\n--- a/ci/run-static-analysis.sh\n+++ b/ci/run-static-analysis.sh\n@@ -26,7 +26,7 @@ then\n \texit 1\n fi\n \n-make hdr-check check-builtins ||\n+make hdr-check check-builtins check-sort ||\n exit 1\n \n save_good_tree\n-- \n2.31.0.rc2.261.g7f71774620\n\n"},{"id":"419271","messageId":"CAPig+cS900W=tu_=2dA4hGjRXnNDTEc=Ne4Rb=y9wy_C_4Js7w@mail.gmail.com","threadId":"55326","inReplyTo":"c889973be754336005d0155ce670f16e322df9c3.1615856156.git.liu.denton@gmail.com","subject":"Re: [PATCH 1/7] Makefile: mark 'check-builtins' as a .PHONY target","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-03-16T04:59:15Z","receivedAt":"2021-03-16T05:00:37Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Mar 15, 2021 at 8:57 PM Denton Liu wrote:\n> Then convention in Git's Makefile is to mark targets which do not\n\ns/Then/The/\n\n> actually generate any precious files (such as static checking targets).\n> .PHONY enables the target to still run, even if a file is erroneously\n> created with the same name as the target.\n>\n> Mark 'check-builtins' as a .PHONY target.\n>\n> Signed-off-by: Denton Liu \n"},{"id":"419281","messageId":"CAPig+cRM2y15cH5gLvmn5dDa=rafBL53GPua8rmjsTsmkQAkPA@mail.gmail.com","threadId":"55326","inReplyTo":"5088e93d76e44de9d079b7b2296b8c810828a2f5.1615856156.git.liu.denton@gmail.com","subject":"Re: [PATCH 5/7] Makefile: add 'check-sort' target","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-03-16T06:37:16Z","receivedAt":"2021-03-16T06:38:33Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Mar 15, 2021 at 8:57 PM Denton Liu wrote:\n> In the previous few commits, we sorted many lists into ASCII-order. In\n> order to ensure that they remain that way, add the 'check-sort' target.\n> [...]\n> Signed-off-by: Denton Liu \n> ---\n> +my @regexes = map { qr/^$_/ } @ARGV;\n> +my $last_regex = 0;\n> +my $last_line = '';\n> +while () {\n> + my $matched = 0;\n> + chomp;\n> + for my $regex (@regexes) {\n> + next unless $_ =~ $regex;\n> + if ($last_regex == $regex) {\n> + die \"duplicate lines: '$_'\\n\" unless $last_line ne $_;\n> + die \"unsorted lines: '$last_line' before '$_'\\n\" unless $last_line lt $_;\n> + }\n> + $matched = 1;\n> + $last_regex = $regex;\n> + $last_line = $_;\n> + }\n> + unless ($matched) {\n> + $last_regex = 0;\n> + $last_line = '';\n> + }\n> +}\n\nThis is, of course, endlessly bikesheddable. Here is a shorter -- and,\nat least for me, easier to understand -- way to do it:\n\n my $rc = 0;\n chomp(my @all = );\n foreach my $needle (@ARGV) {\n my @lines = grep(/^$needle/, @all);\n if (join(\"\\n\", @lines) ne join(\"\\n\", sort @lines)) {\n print \"'$needle' lines not sorted\\n\";\n $rc = 1;\n }\n }\n exit $rc;\n\nBy the way, it might be a good idea to also print the filename in\nwhich the problem occurred. Such context can be important for the\nperson trying to track down the complaint. To do so, you'd probably\nwant to pass the filename as an argument, and open and read the file\nrather than sending it only as standard-input.\n"},{"id":"419452","messageId":"YFHQ0X5m58sTQb0g@generichostname","threadId":"55326","inReplyTo":"CAPig+cRM2y15cH5gLvmn5dDa=rafBL53GPua8rmjsTsmkQAkPA@mail.gmail.com","subject":"Re: [PATCH 5/7] Makefile: add 'check-sort' target","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-03-17T09:50:09Z","receivedAt":"2021-03-17T09:51:02Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Hi Eric,\n\nOn Tue, Mar 16, 2021 at 02:37:16AM -0400, Eric Sunshine wrote:\n> On Mon, Mar 15, 2021 at 8:57 PM Denton Liu wrote:\n> > In the previous few commits, we sorted many lists into ASCII-order. In\n> > order to ensure that they remain that way, add the 'check-sort' target.\n> > [...]\n> > Signed-off-by: Denton Liu \n> > ---\n> > +my @regexes = map { qr/^$_/ } @ARGV;\n> > +my $last_regex = 0;\n> > +my $last_line = '';\n> > +while () {\n> > + my $matched = 0;\n> > + chomp;\n> > + for my $regex (@regexes) {\n> > + next unless $_ =~ $regex;\n> > + if ($last_regex == $regex) {\n> > + die \"duplicate lines: '$_'\\n\" unless $last_line ne $_;\n> > + die \"unsorted lines: '$last_line' before '$_'\\n\" unless $last_line lt $_;\n> > + }\n> > + $matched = 1;\n> > + $last_regex = $regex;\n> > + $last_line = $_;\n> > + }\n> > + unless ($matched) {\n> > + $last_regex = 0;\n> > + $last_line = '';\n> > + }\n> > +}\n> \n> This is, of course, endlessly bikesheddable. Here is a shorter -- and,\n> at least for me, easier to understand -- way to do it:\n> \n> my $rc = 0;\n> chomp(my @all = );\n> foreach my $needle (@ARGV) {\n> my @lines = grep(/^$needle/, @all);\n> if (join(\"\\n\", @lines) ne join(\"\\n\", sort @lines)) {\n> print \"'$needle' lines not sorted\\n\";\n> $rc = 1;\n> }\n> }\n> exit $rc;\n\nThat's pretty clever, thanks for showing me how it's done :)\n\nHowever, the reason I wrote it out the way that I did is because my code\nensures that consecutive lines matching the regex are sorted but if\nthere are any breaks between matching regex lines, it will consider them\nseparate blocks. Just taking all the lines fails in the case of\n`LIB_OBJS \\+=` in Makefile since we have\n\n\tLIB_OBJS += zlib.o\n\n\t[... many intervening lines ...]\n\n\tLIB_OBJS += $(COMPAT_OBJS)\n\nand that is technically unsorted. That being said, I don't really like\nmy current approach that much.\n\nI think I have two better options:\n\n\t1. Tighten up the regexes so that it excludes the\n\t $(COMPAT_OBJS). I don't want to be too strict, though,\n\t because if we end up not matching a line it might end up\n\t unsorted.\n\n\t2. Consider blank lines to be block separators and only consider\n\t it to be sorted if the text matching regexes within a block\n\t are sorted.\n\nNow that I've written that all out, I think I like option 1 more,\nalthough I could definitely be convinced to go either way.\n\n> By the way, it might be a good idea to also print the filename in\n> which the problem occurred. Such context can be important for the\n> person trying to track down the complaint. To do so, you'd probably\n> want to pass the filename as an argument, and open and read the file\n> rather than sending it only as standard-input.\n\nAgreed.\n\nThanks,\nDenton\n"},{"id":"419454","messageId":"f720a673-e477-3c77-a722-a780ff73a461@gmail.com","threadId":"55326","inReplyTo":"cover.1615856156.git.liu.denton@gmail.com","subject":"Re: [PATCH 0/7] Sort lists and add static-analysis","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-03-17T11:01:33Z","receivedAt":"2021-03-17T11:02:43Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"On 16/03/21 07.56, Denton Liu wrote:\n> As a follow-up to [0], sort some file lists and create a static-analysis\n> check to ensure that those lists don't ever become un-sorted.\n> \n> [0]: https://lore.kernel.org/git/nycvar.QRO.7.76.6.2010081156350.50@tvgsbejvaqbjf.bet/\n\nRemark for future patches that touch these file lists: please try to keep the ASCII\nsort order when adding something.\n\nBut anyways, grazie Denton.\n\n> Denton Liu (7):\n> Makefile: mark 'check-builtins' as a .PHONY target\n> Makefile: ASCII-sort LIB_OBJS\n> builtin.h: ASCII-sort list of functions\n> test-tool.h: ASCII-sort list of functions\n> Makefile: add 'check-sort' target\n> ci/run-static-analysis.sh: make check-builtins\n> ci/run-static-analysis.sh: make check-sort\n> \n> Makefile | 30 ++++++++++++++++++++++++++++--\n> builtin.h | 22 +++++++++++-----------\n> check-sort.perl | 31 +++++++++++++++++++++++++++++++\n> ci/run-static-analysis.sh | 2 +-\n> t/helper/test-tool.h | 6 +++---\n> 5 files changed, 74 insertions(+), 17 deletions(-)\n> create mode 100755 check-sort.perl\n> \n\n-- \nAn old man doll... just what I always wanted! - Clara\n"},{"id":"419456","messageId":"87mtv2dk18.fsf@evledraar.gmail.com","threadId":"55326","inReplyTo":"5088e93d76e44de9d079b7b2296b8c810828a2f5.1615856156.git.liu.denton@gmail.com","subject":"Re: [PATCH 5/7] Makefile: add 'check-sort' target","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-17T12:47:15Z","receivedAt":"2021-03-17T12:48:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Mar 16 2021, Denton Liu wrote:\n\n> In the previous few commits, we sorted many lists into ASCII-order. In\n> order to ensure that they remain that way, add the 'check-sort' target.\n>\n> The check-sort.perl program ensures that consecutive lines that match\n> the same regex are sorted in ASCII-order. The 'check-sort' target runs\n> the check-sort.perl program on some files which are known to contain\n> sorted lists.\n>\n> Signed-off-by: Denton Liu \n> ---\n>\n> Notes:\n> Full disclaimer: this is the first time I've written anything in Perl.\n> Please let me know if I'm doing anything unconventional :)\n>\n> Makefile | 25 +++++++++++++++++++++++++\n> check-sort.perl | 31 +++++++++++++++++++++++++++++++\n> 2 files changed, 56 insertions(+)\n> create mode 100755 check-sort.perl\n>\n> diff --git a/Makefile b/Makefile\n> index 5832aa33da..b23dff384d 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -3240,6 +3240,31 @@ check-docs::\n> check-builtins::\n> \t./check-builtins.sh\n> \n> +.PHONY: check-sort\n> +check-sort::\n> +\t./check-sort.perl \\\n> +\t\t'ALL_COMMANDS \\+=' \\\n> +\t\t'ALL_COMMANDS_TO_INSTALL \\+=' \\\n> +\t\t'BINDIR_PROGRAMS_NEED_X \\+=' \\\n> +\t\t'BINDIR_PROGRAMS_NO_X \\+=' \\\n> +\t\t'BUILTIN_OBJS \\+=' \\\n> +\t\t'BUILT_INS \\+=' \\\n> +\t\t'FUZZ_OBJS \\+=' \\\n> +\t\t'GENERATED_H \\+=' \\\n> +\t\t'LIB_OBJS \\+=' \\\n> +\t\t'SCRIPT_LIB \\+=' \\\n> +\t\t'SCRIPT_PERL \\+=' \\\n> +\t\t'SCRIPT_PYTHON \\+=' \\\n> +\t\t'SCRIPT_SH \\+=' \\\n> +\t\t'TEST_BUILTINS_OBJS \\+=' \\\n> +\t\t'TEST_PROGRAMS_NEED_X \\+=' \\\n> +\t\t'THIRD_PARTY_SOURCES \\+=' \\\n> +\t\t'XDIFF_OBJS \\+=' \\\n> +\t\t [...]\n> +\t./check-sort.perl '\\t\\{ \"[^\"]*\",' builtins\n '\n \n +test_expect_success 'list of builtins in git.c should be sorted' '\n + sort builtins >sorted &&\n + test_cmp sorted builtins\n +'\n +\n while read builtin\n do\n test_expect_success \"$builtin can handle -h\" '\n\nWhich just leaves:\n\n> +\t./check-sort.perl 'int cmd_[^(]*\\(' +\t./check-sort.perl 'int cmd__[^(]*\\(' Why does this part need to be a Perl script at all? We can check this in\n> the makefile itself. Make has a sort function and string comparisons,\n> e.g.:\n> \n> LIB_OBJS_SORTED =\n> LIB_OBJS_SORTED += $(sort $(LIB_OBJS))\n> ifneq (\"$(LIB_OBJS)\", \"$(LIB_OBJS_SORTED)\")\n> $(error \"please sort and de-duplicate LIB_OBJS!\")\n> endif\n> \n> This will fail/pass before/after your patches. Note that make's sort\n> isn't just a sort, it also de-deplicates (not that we're likely to have\n> that issue).\n\nI like that much better, if only because it runs quickly and\nautomatically (as opposed to much later as part of some CI process).\nReducing the length of the feedback loop makes automated checks much\nless annoying.\n\nIt doesn't indicate which lines are out of order or duplicated. We could\nprobably add some $(shell) magic to dump both sides to diff inside the\nifneq. That's getting unwieldy to use in each site, but I wonder if\nsomething like this solves that:\n\ndiff --git a/Makefile b/Makefile\nindex dfb0f1000f..3fa7893c8b 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -596,6 +596,16 @@ THIRD_PARTY_SOURCES =\n # interactive shell sessions without exporting it.\n unexport CDPATH\n \n+# usage: $(call check-sort,VAR)\n+# If this complains, then make sure the contents of VAR are ASCII-sorted.\n+define check-sort-template\n+SORTED_$1 = $$(sort $$($1))\n+ifneq ($$($1),$$(SORTED_$1))\n+$$(error \"please sort and de-duplicate $1!\")\n+endif\n+endef\n+check-sort = $(eval $(call check-sort-template,$1))\n+\n SCRIPT_SH += git-bisect.sh\n SCRIPT_SH += git-difftool--helper.sh\n SCRIPT_SH += git-filter-branch.sh\n@@ -1037,6 +1047,7 @@ LIB_OBJS += ws.o\n LIB_OBJS += wt-status.o\n LIB_OBJS += xdiff-interface.o\n LIB_OBJS += zlib.o\n+$(call check-sort,LIB_OBJS)\n \n BUILTIN_OBJS += builtin/add.o\n BUILTIN_OBJS += builtin/am.o\n\nAnd then it's just a single-liner for each block that should be checked.\nWe haven't used $(call) or $(eval) yet in our Makefile, but past\ndiscussions have reached the conclusion that they should be safe\n(they're both in GNU make 3.80, which is the oldest version worth caring\nabout).\n\nTBH, I'm a little on the fence on whether automatically checking this is\neven worth the hassle. Doing the make function above was a fun\ndiversion, but already I think this discussion has taken more time than\npeople actually spend resolving conflicts on unsorted Makefile lists.\n\n> > [...]\n> > +\t./check-sort.perl '\\t\\{ \"[^\"]*\",' \n> This last one you can IMO be done better as (or if we want to be more\n> anal, we could make git die on startup if it's not true):\n> \n> diff --git a/t/t0012-help.sh b/t/t0012-help.sh\n> index 5679e29c62..5bd2ebceca 100755\n> --- a/t/t0012-help.sh\n> +++ b/t/t0012-help.sh\n> @@ -77,6 +77,11 @@ test_expect_success 'generate builtin list' '\n> git --list-cmds=builtins >builtins\n> '\n> \n> +test_expect_success 'list of builtins in git.c should be sorted' '\n> + sort builtins >sorted &&\n> + test_cmp sorted builtins\n> +'\n\nAgain, much nicer, because it just happens as part of the test suite\n(which I hope everyone is running after making changes). If we care\nabout the order in the source code, then this check implies that we are\nnot sorting the list internally before outputting it, but that is\nprobably reasonable.\n\n> > +\t./check-sort.perl 'int cmd_[^(]*\\(' > +\t./check-sort.perl 'int cmd__[^(]*\\(' \n> As something that can't be done in the Makefile itself or that we're\n> already extracting from the tests.\n> \n> Both of those IMO would be better handled down the line by making the\n> relevant part of these files generated from the data in the Makefile, at\n> which point we'd have no need for the external perl script.\n\nAgreed. Automating away a tedious task is always better than\nautomatically checking that somebody did it right. :)\n\n-Peff\n"},{"id":"419486","messageId":"87r1kdzngs.fsf@evledraar.gmail.com","threadId":"55326","inReplyTo":"YFI9QzKMKLMXYoyz@coredump.intra.peff.net","subject":"Re: [PATCH 5/7] Makefile: add 'check-sort' target","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-17T17:42:11Z","receivedAt":"2021-03-17T17:43:20Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Mar 17 2021, Jeff King wrote:\n\n> SCRIPT_SH += git-bisect.sh\n> SCRIPT_SH += git-difftool--helper.sh\n> SCRIPT_SH += git-filter-branch.sh\n> @@ -1037,6 +1047,7 @@ LIB_OBJS += ws.o\n> LIB_OBJS += wt-status.o\n> LIB_OBJS += xdiff-interface.o\n> LIB_OBJS += zlib.o\n> +$(call check-sort,LIB_OBJS)\n> \n> BUILTIN_OBJS += builtin/add.o\n> BUILTIN_OBJS += builtin/am.o\n>\n> And then it's just a single-liner for each block that should be checked.\n> We haven't used $(call) or $(eval) yet in our Makefile, but past\n> discussions have reached the conclusion that they should be safe\n> (they're both in GNU make 3.80, which is the oldest version worth caring\n> about).\n\n...also this sort of thing can be guarded by \"ifdef DEVELOPER\" or\nsomething, which AFAICT (from trying to introduce syntax errors etc.)\nwill get parsed first, before \"make\" even tries to parse what's within\nthe ifdef.\n\nSo if we have bells & whistles in that sort of setup it can be less\nportable than the Makefile in general..\n"},{"id":"419487","messageId":"xmqqlfaly8n5.fsf@gitster.g","threadId":"55326","inReplyTo":"c889973be754336005d0155ce670f16e322df9c3.1615856156.git.liu.denton@gmail.com","subject":"Re: [PATCH 1/7] Makefile: mark 'check-builtins' as a .PHONY target","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-17T17:47:42Z","receivedAt":"2021-03-17T17:48:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu writes:\n\n> Then convention in Git's Makefile is to mark targets which do not\n> actually generate any precious files (such as static checking targets).\n\ns/Then/The/ has been pointed out, but I am not sure what \"precious\"\nmeans here. I do not think you are referring to the distinction\nbetween ordinary targets and .PRECIOUS targets here, but the use of\nthe word makes readers wonder if some interaction with the .PRECIOUS\nmarking was taken into consideration.\n\n> .PHONY enables the target to still run, even if a file is erroneously\n> created with the same name as the target.\n\nTrue. Another convention is to \"build\" such a target with\ndouble-colon rule. The existing code already follows the\nconvention, so there is nothing the patch text needs to do, but if\nwe explain the change in the log message to make the code follow\nconventions, we probably should mention it as well for completeness.\n\n\nThanks.\n\n> Mark 'check-builtins' as a .PHONY target.\n>\n> Signed-off-by: Denton Liu \n> ---\n> Makefile | 1 +\n> 1 file changed, 1 insertion(+)\n>\n> diff --git a/Makefile b/Makefile\n> index dfb0f1000f..6dbecdb606 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -3236,6 +3236,7 @@ check-docs::\n> \n> ### Make sure built-ins do not have dups and listed in git.c\n> #\n> +.PHONY: check-builtins\n> check-builtins::\n> \t./check-builtins.sh\n"},{"id":"419489","messageId":"xmqqh7l9y8gl.fsf@gitster.g","threadId":"55326","inReplyTo":"aee5f6d1e2e123505c1e3b0b8ff952cce7de3436.1615856156.git.liu.denton@gmail.com","subject":"Re: [PATCH 3/7] builtin.h: ASCII-sort list of functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-17T17:51:38Z","receivedAt":"2021-03-17T17:52:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu writes:\n\n> ASCII sorting was chosen over strict alphabetical order for the same\n> reason as 805d9eaf5e (Makefile: ASCII-sort += lists, 2020-03-21): the\n> purpose of maintaining the sorted list is to ensure line insertions are\n> deterministic. By using ASCII ordering, it is more easily mechanically\n> reproducible in the future, such as by using :sort in Vim.\n\nThis happens to work only because '(' sorts way earlier than '_'.\nOtherwise we'd probably be sorting with 'sort -t\\( -k1' or something\nlike that.\n\nI do not know if it worth mentioning, because it is unlikely that\nthe sort order between '(' and '_' would ever change ;-).\n\n> This patch is best viewed with `--color-moved`.\n>\n> Signed-off-by: Denton Liu \n> ---\n> builtin.h | 22 +++++++++++-----------\n> 1 file changed, 11 insertions(+), 11 deletions(-)\n>\n> diff --git a/builtin.h b/builtin.h\n> index b6ce981b73..dd6beff6b2 100644\n> --- a/builtin.h\n> +++ b/builtin.h\n> @@ -122,16 +122,16 @@ int cmd_branch(int argc, const char **argv, const char *prefix);\n> int cmd_bugreport(int argc, const char **argv, const char *prefix);\n> int cmd_bundle(int argc, const char **argv, const char *prefix);\n> int cmd_cat_file(int argc, const char **argv, const char *prefix);\n> -int cmd_checkout(int argc, const char **argv, const char *prefix);\n> -int cmd_checkout_index(int argc, const char **argv, const char *prefix);\n> int cmd_check_attr(int argc, const char **argv, const char *prefix);\n> int cmd_check_ignore(int argc, const char **argv, const char *prefix);\n> int cmd_check_mailmap(int argc, const char **argv, const char *prefix);\n> int cmd_check_ref_format(int argc, const char **argv, const char *prefix);\n> +int cmd_checkout(int argc, const char **argv, const char *prefix);\n> +int cmd_checkout_index(int argc, const char **argv, const char *prefix);\n> int cmd_cherry(int argc, const char **argv, const char *prefix);\n> int cmd_cherry_pick(int argc, const char **argv, const char *prefix);\n> -int cmd_clone(int argc, const char **argv, const char *prefix);\n> int cmd_clean(int argc, const char **argv, const char *prefix);\n> +int cmd_clone(int argc, const char **argv, const char *prefix);\n> int cmd_column(int argc, const char **argv, const char *prefix);\n> int cmd_commit(int argc, const char **argv, const char *prefix);\n> int cmd_commit_graph(int argc, const char **argv, const char *prefix);\n> @@ -143,9 +143,9 @@ int cmd_credential_cache(int argc, const char **argv, const char *prefix);\n> int cmd_credential_cache_daemon(int argc, const char **argv, const char *prefix);\n> int cmd_credential_store(int argc, const char **argv, const char *prefix);\n> int cmd_describe(int argc, const char **argv, const char *prefix);\n> +int cmd_diff(int argc, const char **argv, const char *prefix);\n> int cmd_diff_files(int argc, const char **argv, const char *prefix);\n> int cmd_diff_index(int argc, const char **argv, const char *prefix);\n> -int cmd_diff(int argc, const char **argv, const char *prefix);\n> int cmd_diff_tree(int argc, const char **argv, const char *prefix);\n> int cmd_difftool(int argc, const char **argv, const char *prefix);\n> int cmd_env__helper(int argc, const char **argv, const char *prefix);\n> @@ -169,16 +169,16 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix);\n> int cmd_log(int argc, const char **argv, const char *prefix);\n> int cmd_log_reflog(int argc, const char **argv, const char *prefix);\n> int cmd_ls_files(int argc, const char **argv, const char *prefix);\n> -int cmd_ls_tree(int argc, const char **argv, const char *prefix);\n> int cmd_ls_remote(int argc, const char **argv, const char *prefix);\n> +int cmd_ls_tree(int argc, const char **argv, const char *prefix);\n> int cmd_mailinfo(int argc, const char **argv, const char *prefix);\n> int cmd_mailsplit(int argc, const char **argv, const char *prefix);\n> int cmd_maintenance(int argc, const char **argv, const char *prefix);\n> int cmd_merge(int argc, const char **argv, const char *prefix);\n> int cmd_merge_base(int argc, const char **argv, const char *prefix);\n> +int cmd_merge_file(int argc, const char **argv, const char *prefix);\n> int cmd_merge_index(int argc, const char **argv, const char *prefix);\n> int cmd_merge_ours(int argc, const char **argv, const char *prefix);\n> -int cmd_merge_file(int argc, const char **argv, const char *prefix);\n> int cmd_merge_recursive(int argc, const char **argv, const char *prefix);\n> int cmd_merge_tree(int argc, const char **argv, const char *prefix);\n> int cmd_mktag(int argc, const char **argv, const char *prefix);\n> @@ -189,6 +189,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix);\n> int cmd_notes(int argc, const char **argv, const char *prefix);\n> int cmd_pack_objects(int argc, const char **argv, const char *prefix);\n> int cmd_pack_redundant(int argc, const char **argv, const char *prefix);\n> +int cmd_pack_refs(int argc, const char **argv, const char *prefix);\n> int cmd_patch_id(int argc, const char **argv, const char *prefix);\n> int cmd_prune(int argc, const char **argv, const char *prefix);\n> int cmd_prune_packed(int argc, const char **argv, const char *prefix);\n> @@ -204,6 +205,7 @@ int cmd_remote(int argc, const char **argv, const char *prefix);\n> int cmd_remote_ext(int argc, const char **argv, const char *prefix);\n> int cmd_remote_fd(int argc, const char **argv, const char *prefix);\n> int cmd_repack(int argc, const char **argv, const char *prefix);\n> +int cmd_replace(int argc, const char **argv, const char *prefix);\n> int cmd_rerere(int argc, const char **argv, const char *prefix);\n> int cmd_reset(int argc, const char **argv, const char *prefix);\n> int cmd_restore(int argc, const char **argv, const char *prefix);\n> @@ -216,9 +218,10 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix);\n> int cmd_show(int argc, const char **argv, const char *prefix);\n> int cmd_show_branch(int argc, const char **argv, const char *prefix);\n> int cmd_show_index(int argc, const char **argv, const char *prefix);\n> +int cmd_show_ref(int argc, const char **argv, const char *prefix);\n> int cmd_sparse_checkout(int argc, const char **argv, const char *prefix);\n> -int cmd_status(int argc, const char **argv, const char *prefix);\n> int cmd_stash(int argc, const char **argv, const char *prefix);\n> +int cmd_status(int argc, const char **argv, const char *prefix);\n> int cmd_stripspace(int argc, const char **argv, const char *prefix);\n> int cmd_submodule__helper(int argc, const char **argv, const char *prefix);\n> int cmd_switch(int argc, const char **argv, const char *prefix);\n> @@ -235,14 +238,11 @@ int cmd_upload_archive_writer(int argc, const char **argv, const char *prefix);\n> int cmd_upload_pack(int argc, const char **argv, const char *prefix);\n> int cmd_var(int argc, const char **argv, const char *prefix);\n> int cmd_verify_commit(int argc, const char **argv, const char *prefix);\n> +int cmd_verify_pack(int argc, const char **argv, const char *prefix);\n> int cmd_verify_tag(int argc, const char **argv, const char *prefix);\n> int cmd_version(int argc, const char **argv, const char *prefix);\n> int cmd_whatchanged(int argc, const char **argv, const char *prefix);\n> int cmd_worktree(int argc, const char **argv, const char *prefix);\n> int cmd_write_tree(int argc, const char **argv, const char *prefix);\n> -int cmd_verify_pack(int argc, const char **argv, const char *prefix);\n> -int cmd_show_ref(int argc, const char **argv, const char *prefix);\n> -int cmd_pack_refs(int argc, const char **argv, const char *prefix);\n> -int cmd_replace(int argc, const char **argv, const char *prefix);\n> \n> #endif\n"},{"id":"419491","messageId":"xmqqczvxy8bi.fsf@gitster.g","threadId":"55326","inReplyTo":"b817541c2754ec8fc4edc1ae774e460912e7c0a8.1615856156.git.liu.denton@gmail.com","subject":"Re: [PATCH 4/7] test-tool.h: ASCII-sort list of functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-17T17:54:41Z","receivedAt":"2021-03-17T17:55:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu writes:\n\n> The list of test-tool functions have, over time, gotten slightly out of\n> ASCII order. Sort this list to bring them back into order.\n>\n> ASCII sorting was chosen over strict alphabetical order for the same\n> reason as 805d9eaf5e (Makefile: ASCII-sort += lists, 2020-03-21): the\n> purpose of maintaining the sorted list is to ensure line insertions are\n> deterministic. By using ASCII ordering, it is more easily mechanically\n> reproducible in the future, such as by using :sort in Vim.\n\nLikewise. The rationale 805d9eaf (Makefile: ASCII-sort += lists,\n2020-03-21) applies better for Makefile's \"X += \" as the\nprefix before the are all common, while in the header files,\nyou have to depend on their return type being the same and '('\nsorting before '_'.\n\nNow I am inclined to say that it may be worth mentioning in the log,\nboth for this step and the previous one.\n\n\n\n> This patch is best viewed with `--color-moved`.\n>\n> Signed-off-by: Denton Liu \n> ---\n> t/helper/test-tool.h | 6 +++---\n> 1 file changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\n> index 28072c0ad5..9856e84149 100644\n> --- a/t/helper/test-tool.h\n> +++ b/t/helper/test-tool.h\n> @@ -22,14 +22,15 @@ int cmd__example_decorate(int argc, const char **argv);\n> int cmd__fast_rebase(int argc, const char **argv);\n> int cmd__genrandom(int argc, const char **argv);\n> int cmd__genzeros(int argc, const char **argv);\n> -int cmd__hashmap(int argc, const char **argv);\n> int cmd__hash_speed(int argc, const char **argv);\n> +int cmd__hashmap(int argc, const char **argv);\n> int cmd__index_version(int argc, const char **argv);\n> int cmd__json_writer(int argc, const char **argv);\n> int cmd__lazy_init_name_hash(int argc, const char **argv);\n> int cmd__match_trees(int argc, const char **argv);\n> int cmd__mergesort(int argc, const char **argv);\n> int cmd__mktemp(int argc, const char **argv);\n> +int cmd__oid_array(int argc, const char **argv);\n> int cmd__oidmap(int argc, const char **argv);\n> int cmd__online_cpus(int argc, const char **argv);\n> int cmd__parse_options(int argc, const char **argv);\n> @@ -52,7 +53,6 @@ int cmd__run_command(int argc, const char **argv);\n> int cmd__scrap_cache_tree(int argc, const char **argv);\n> int cmd__serve_v2(int argc, const char **argv);\n> int cmd__sha1(int argc, const char **argv);\n> -int cmd__oid_array(int argc, const char **argv);\n> int cmd__sha256(int argc, const char **argv);\n> int cmd__sigchain(int argc, const char **argv);\n> int cmd__strcmp_offset(int argc, const char **argv);\n> @@ -62,8 +62,8 @@ int cmd__submodule_nested_repo_config(int argc, const char **argv);\n> int cmd__subprocess(int argc, const char **argv);\n> int cmd__trace2(int argc, const char **argv);\n> int cmd__urlmatch_normalization(int argc, const char **argv);\n> -int cmd__xml_encode(int argc, const char **argv);\n> int cmd__wildmatch(int argc, const char **argv);\n> +int cmd__xml_encode(int argc, const char **argv);\n> #ifdef GIT_WINDOWS_NATIVE\n> int cmd__windows_named_pipe(int argc, const char **argv);\n> #endif\n"},{"id":"419492","messageId":"xmqq5z1py844.fsf@gitster.g","threadId":"55326","inReplyTo":"5088e93d76e44de9d079b7b2296b8c810828a2f5.1615856156.git.liu.denton@gmail.com","subject":"Re: [PATCH 5/7] Makefile: add 'check-sort' target","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-17T17:59:07Z","receivedAt":"2021-03-17T18:00:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu writes:\n\n> +\t./check-sort.perl 'int cmd_[^(]*\\(' +\t./check-sort.perl 'int cmd__[^(]*\\(' +\t./check-sort.perl '\\t\\{ \"[^\"]*\",' writes:\n\n>> +\t./check-sort.perl '\\t\\{ \"[^\"]*\",' \n> This last one you can IMO be done better as (or if we want to be more\n> anal, we could make git die on startup if it's not true):\n> \n> diff --git a/t/t0012-help.sh b/t/t0012-help.sh\n> index 5679e29c62..5bd2ebceca 100755\n> --- a/t/t0012-help.sh\n> +++ b/t/t0012-help.sh\n> @@ -77,6 +77,11 @@ test_expect_success 'generate builtin list' '\n> git --list-cmds=builtins >builtins\n> '\n> \n> +test_expect_success 'list of builtins in git.c should be sorted' '\n> + sort builtins >sorted &&\n> + test_cmp sorted builtins\n> +'\n\n\"LANG=C LC_ALL=C sort ...\"\n\nI like this 100% better than the original ;-)\n"},{"id":"419496","messageId":"xmqqwnu5wt8z.fsf@gitster.g","threadId":"55326","inReplyTo":"cover.1615856156.git.liu.denton@gmail.com","subject":"Re: [PATCH 0/7] Sort lists and add static-analysis","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-17T18:05:32Z","receivedAt":"2021-03-17T18:06:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu writes:\n\n> As a follow-up to [0], sort some file lists and create a static-analysis\n> check to ensure that those lists don't ever become un-sorted.\n>\n> [0]: https://lore.kernel.org/git/nycvar.QRO.7.76.6.2010081156350.50@tvgsbejvaqbjf.bet/\n>\n> Denton Liu (7):\n> Makefile: mark 'check-builtins' as a .PHONY target\n> Makefile: ASCII-sort LIB_OBJS\n> builtin.h: ASCII-sort list of functions\n> test-tool.h: ASCII-sort list of functions\n> Makefile: add 'check-sort' target\n> ci/run-static-analysis.sh: make check-builtins\n> ci/run-static-analysis.sh: make check-sort\n\nI'd expect that the series would be structured better if we \n\n - move #6 (static-analysis runs check-builtins) earlier, perhaps to\n the front of the series, as there is nothing to fix to make the\n check pass.\n\n - merge #7 into #5.\n\nI've also left comments on a few individual steps.\n\nThanks for working on it.\n\n> Makefile | 30 ++++++++++++++++++++++++++++--\n> builtin.h | 22 +++++++++++-----------\n> check-sort.perl | 31 +++++++++++++++++++++++++++++++\n> ci/run-static-analysis.sh | 2 +-\n> t/helper/test-tool.h | 6 +++---\n> 5 files changed, 74 insertions(+), 17 deletions(-)\n> create mode 100755 check-sort.perl\n"},{"id":"419499","messageId":"87o8fhzlw0.fsf@evledraar.gmail.com","threadId":"55326","inReplyTo":"xmqq1rcdy7zo.fsf@gitster.g","subject":"Re: [PATCH 5/7] Makefile: add 'check-sort' target","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-17T18:16:15Z","receivedAt":"2021-03-17T18:17:03Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Mar 17 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>>> +\t./check-sort.perl '\\t\\{ \"[^\"]*\",' >\n>> This last one you can IMO be done better as (or if we want to be more\n>> anal, we could make git die on startup if it's not true):\n>> \n>> diff --git a/t/t0012-help.sh b/t/t0012-help.sh\n>> index 5679e29c62..5bd2ebceca 100755\n>> --- a/t/t0012-help.sh\n>> +++ b/t/t0012-help.sh\n>> @@ -77,6 +77,11 @@ test_expect_success 'generate builtin list' '\n>> git --list-cmds=builtins >builtins\n>> '\n>> \n>> +test_expect_success 'list of builtins in git.c should be sorted' '\n>> + sort builtins >sorted &&\n>> + test_cmp sorted builtins\n>> +'\n>\n> \"LANG=C LC_ALL=C sort ...\"\n>\n> I like this 100% better than the original ;-)\n\nWe don't need to use \"LANG=C LC_ALL=C sort\", the test-lib.sh sets that\nalready, so just \"sort\" works consistently.\n\nIt's also why with GETTEXT_POISON gone we can just \"grep\" output,\ninstead of worrying that it may be in the user's locale.\n"},{"id":"419601","messageId":"CAPig+cRk-fCLFtug47w3trYQFiPzZ3bD4g11Qu4fTd3Fh6_bCQ@mail.gmail.com","threadId":"55326","inReplyTo":"YFI9QzKMKLMXYoyz@coredump.intra.peff.net","subject":"Re: [PATCH 5/7] Makefile: add 'check-sort' target","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-03-17T21:48:18Z","receivedAt":"2021-03-17T21:49:34Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Mar 17, 2021 at 1:34 PM Jeff King wrote:\n> TBH, I'm a little on the fence on whether automatically checking this is\n> even worth the hassle. Doing the make function above was a fun\n> diversion, but already I think this discussion has taken more time than\n> people actually spend resolving conflicts on unsorted Makefile lists.\n\nI had the same reaction. Like you, I jumped in for the fun diversion.\nIt allowed me to flex my Perl muscle a bit which has atrophied, but an\nout-of-order item here and there is such a minor concern, especially\nsince they don't impact correctness, that I worry that such a CI job\nwould be more hassle than it's worth. Making the feedback loop\ntighter, as discussed elsewhere in this thread, makes the idea of the\nautomated check a bit more palatable.\n"},{"id":"419602","messageId":"YFJ8KpseGSJdd3Am@coredump.intra.peff.net","threadId":"55326","inReplyTo":"CAPig+cRk-fCLFtug47w3trYQFiPzZ3bD4g11Qu4fTd3Fh6_bCQ@mail.gmail.com","subject":"Re: [PATCH 5/7] Makefile: add 'check-sort' target","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-03-17T22:01:14Z","receivedAt":"2021-03-17T22:02:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 17, 2021 at 05:48:18PM -0400, Eric Sunshine wrote:\n\n> On Wed, Mar 17, 2021 at 1:34 PM Jeff King wrote:\n> > TBH, I'm a little on the fence on whether automatically checking this is\n> > even worth the hassle. Doing the make function above was a fun\n> > diversion, but already I think this discussion has taken more time than\n> > people actually spend resolving conflicts on unsorted Makefile lists.\n> \n> I had the same reaction. Like you, I jumped in for the fun diversion.\n> It allowed me to flex my Perl muscle a bit which has atrophied, but an\n> out-of-order item here and there is such a minor concern, especially\n> since they don't impact correctness, that I worry that such a CI job\n> would be more hassle than it's worth. Making the feedback loop\n> tighter, as discussed elsewhere in this thread, makes the idea of the\n> automated check a bit more palatable.\n\nThere's an implication in what both of us said that I think is worth\ncalling out explicitly: I would not feel the same about a problem that\nimpacts the correctness of the resulting code. E.g., if the list of\nbuiltins were used with a binary search, then an unsorted list would\nproduce the wrong result. And that would be worth testing.\n\nIt just seems that the stakes here are much lower.\n\n-Peff\n"}]}