{"thread":{"id":"56000","subject":"Why the Makefile is so eager to re-build & re-link","startedAt":"2021-06-24T13:33:34Z","lastAt":"2021-11-08T19:18:43Z","messageCount":87,"participants":["Ævar Arnfjörð Bjarmason","Jeff King","Johannes Sixt","Øystein Walle","Felipe Contreras","Eric Sunshine","Junio C Hamano","Taylor Blau"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"428373","messageId":"874kdn1j6i.fsf@evledraar.gmail.com","threadId":"56000","inReplyTo":null,"subject":"Why the Makefile is so eager to re-build & re-link","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-24T13:16:48Z","receivedAt":"2021-06-24T13:33:34Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This is probably all stuff that's been on list-before / known by\nsome/all people in the CC list, but in case not: I looked a bit into why\nwe'e so frequently re-linking and re compiling things these days,\nslowing down e.g. \"git rebase --exec='make ...'\".\n\nThese are all fixable issues, I haven't worked on them, just some notes\nin case anyone has better ideas:\n\n * version.c: The biggest offender, every time you move to a new commit\n we have a FORCE on GIT-VERSION-FILE, if it changes (which it will, if\n you're in a git checkout), we in turn re-compile version.o, link\n libgit to contain it, and then need to re-link everything that uses\n libgit.\n\n Some of this can be micro-optimized by moving that out of libgit,\n e.g. we re-link git-shell, which doesn't ever report the version, we\n do the same for all the perl scripts, just because obscure codepaths\n of them that could shell out to \"git version\" want to report it, and\n we embed it in the generated code.\n\n But I think the best approach here is to piggy-back on\n SKIP_DASHED_BUILT_INS, if you have that enabled then only \"git\" (and\n git-upload-pack etc.) need to be aware of the version, which they can\n then set in the environment for the others.\n\n This also applies to e.g. the git-http* stuff, which has a user agent\n derived from the version.\n\n * {command,config}-list.h (and in-flight, my hook-list.h): Every time\n you touch a Documentation/git-*.txt we need to re-generate these, and\n since their mtime changes we re-compile and re-link all the way up to\n libgit and our other tools.\n\n I think the best solution here is to make the generate-*.sh\n shellscripts faster (just one takes ~300ms of nested shellscripting,\n just to grep out the first few lines of every git-*.txt, in e.g. Perl\n or a smarter awk script this would be <5ms).\n\n Then we make those FORCE, but most of the time the config or command\n summary (or list of hooks) doesn't change, so we don't need to\n replace the file.\n\nPerhaps even better would be to piggy-back on the RUNTIME_PREFIX\nsupport, and simply drop in generated plain-text files, so in your build\ncheckout the list of hooks, commands etc. would be parsed instead of\ncompiled in. Then we wouldn't need to re-build or re-link anything for\nthe version or this other data.\n\nWe could/should still have some facility to compile those in for\n\"install\", see also [1] for some concerns / ideas for other similar\nthings that could use that.\n\n1. https://lore.kernel.org/git/87czvoowg2.fsf@evledraar.gmail.com/\n\n"},{"id":"428381","messageId":"YNSh0CskelTwuZq0@coredump.intra.peff.net","threadId":"56000","inReplyTo":"874kdn1j6i.fsf@evledraar.gmail.com","subject":"Re: Why the Makefile is so eager to re-build & re-link","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-06-24T15:16:32Z","receivedAt":"2021-06-24T15:16:42Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 24, 2021 at 03:16:48PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> This is probably all stuff that's been on list-before / known by\n> some/all people in the CC list, but in case not: I looked a bit into why\n> we'e so frequently re-linking and re compiling things these days,\n> slowing down e.g. \"git rebase --exec='make ...'\".\n> \n> These are all fixable issues, I haven't worked on them, just some notes\n> in case anyone has better ideas:\n\nFrom a quick skim I didn't see anything wrong in your analysis or\nsuggestions. I do kind of wonder if we are hitting a point of\ndiminishing returns here. \"make -j16\" on my system takes ~175ms for a\nnoop, and ~650ms if I have to regenerate version.h (it's something like\n2s total of CPU, but I have 8 cores).\n\nI know I've probably got a nicer machine than many other folks. But at\nsome point correctness and avoiding complexity in the Makefile become a\nwin over shaving off a second from compile times. You'd probably find\nlower hanging fruit in the test suite which could shave off tens of\nseconds. :)\n\n> * {command,config}-list.h (and in-flight, my hook-list.h): Every time\n> you touch a Documentation/git-*.txt we need to re-generate these, and\n> since their mtime changes we re-compile and re-link all the way up to\n> libgit and our other tools.\n> \n> I think the best solution here is to make the generate-*.sh\n> shellscripts faster (just one takes ~300ms of nested shellscripting,\n> just to grep out the first few lines of every git-*.txt, in e.g. Perl\n> or a smarter awk script this would be <5ms).\n\nYeah, I think Eric mentioned he had looked into doing this in perl, but\nwe weren't entirely happy with the dependency. Here's another really odd\nthing I noticed:\n\n $ time sh ./generate-cmdlist.sh command-list.txt >one\n real\t0m1.323s\n user\t0m1.531s\n sys\t0m0.834s\n\n $ time sh -x ./generate-cmdlist.sh command-list.txt >two\n [a bunch of trace output]\n real\t0m0.513s\n user\t0m0.754s\n sys\t0m0.168s\n\n $ cmp one two\n [no output]\n\nEr, what? Running with \"-x\" makes it almost 3 times faster to generate\nthe same output? I'd have said this is an anomaly, but it's repeatable\n(and swapping the order produces the same output, so it's not some weird\npriming thing). And then to top it all off, redirecting the trace is\nslow again:\n\n $ time sh -x ./generate-cmdlist.sh command-list.txt >two 2>/dev/null\n real\t0m1.363s\n user\t0m1.538s\n sys\t0m0.902s\n\nA little mini-mystery that I think I may leave unsolved for now.\n\n> Then we make those FORCE, but most of the time the config or command\n> summary (or list of hooks) doesn't change, so we don't need to\n> replace the file.\n\nYes, possibly we could use the \"if it hasn't changed, don't update the\nfile\" trick to avoid cascading updates.\n\n> Perhaps even better would be to piggy-back on the RUNTIME_PREFIX\n> support, and simply drop in generated plain-text files, so in your build\n> checkout the list of hooks, commands etc. would be parsed instead of\n> compiled in. Then we wouldn't need to re-build or re-link anything for\n> the version or this other data.\n\nYeah, that would work. I worry a bit that the value of something like\n\"version.h\" is lost with a runtime file, though. The point is to bake it\ninto the binary so you can't accidentally get the wrong value (say, from\nrunning \"./git\" from the build directory, which looks at the runtime\nfile where the binary _would_ be installed, except you haven't run \"make\ninstall\" yet).\n\nFor cmdlist stuff it could be a good match, though.\n\n-Peff\n"},{"id":"428387","messageId":"87y2azyzer.fsf@evledraar.gmail.com","threadId":"56000","inReplyTo":"YNSh0CskelTwuZq0@coredump.intra.peff.net","subject":"Re: Why the Makefile is so eager to re-build & re-link","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-24T15:28:11Z","receivedAt":"2021-06-24T16:56:34Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Jun 24 2021, Jeff King wrote:\n\n> On Thu, Jun 24, 2021 at 03:16:48PM +0200, Ævar Arnfjörð Bjarmason wrote:\n>\n>> This is probably all stuff that's been on list-before / known by\n>> some/all people in the CC list, but in case not: I looked a bit into why\n>> we'e so frequently re-linking and re compiling things these days,\n>> slowing down e.g. \"git rebase --exec='make ...'\".\n>> \n>> These are all fixable issues, I haven't worked on them, just some notes\n>> in case anyone has better ideas:\n>\n> From a quick skim I didn't see anything wrong in your analysis or\n> suggestions. I do kind of wonder if we are hitting a point of\n> diminishing returns here. \"make -j16\" on my system takes ~175ms for a\n> noop, and ~650ms if I have to regenerate version.h (it's something like\n> 2s total of CPU, but I have 8 cores).\n>\n> I know I've probably got a nicer machine than many other folks. But at\n> some point correctness and avoiding complexity in the Makefile become a\n> win over shaving off a second from compile times. You'd probably find\n> lower hanging fruit in the test suite which could shave off tens of\n> seconds. :)\n\nIt's mainly annoying when e.g. doing a rebase of an N patch series,\nthose ~700ms v.s. ~200ms add up quickly.\n\n>> * {command,config}-list.h (and in-flight, my hook-list.h): Every time\n>> you touch a Documentation/git-*.txt we need to re-generate these, and\n>> since their mtime changes we re-compile and re-link all the way up to\n>> libgit and our other tools.\n>> \n>> I think the best solution here is to make the generate-*.sh\n>> shellscripts faster (just one takes ~300ms of nested shellscripting,\n>> just to grep out the first few lines of every git-*.txt, in e.g. Perl\n>> or a smarter awk script this would be <5ms).\n>\n> Yeah, I think Eric mentioned he had looked into doing this in perl, but\n> we weren't entirely happy with the dependency. Here's another really odd\n> thing I noticed:\n>\n> $ time sh ./generate-cmdlist.sh command-list.txt >one\n> real\t0m1.323s\n> user\t0m1.531s\n> sys\t0m0.834s\n>\n> $ time sh -x ./generate-cmdlist.sh command-list.txt >two\n> [a bunch of trace output]\n> real\t0m0.513s\n> user\t0m0.754s\n> sys\t0m0.168s\n>\n> $ cmp one two\n> [no output]\n>\n> Er, what? Running with \"-x\" makes it almost 3 times faster to generate\n> the same output? I'd have said this is an anomaly, but it's repeatable\n> (and swapping the order produces the same output, so it's not some weird\n> priming thing). And then to top it all off, redirecting the trace is\n> slow again:\n>\n> $ time sh -x ./generate-cmdlist.sh command-list.txt >two 2>/dev/null\n> real\t0m1.363s\n> user\t0m1.538s\n> sys\t0m0.902s\n>\n> A little mini-mystery that I think I may leave unsolved for now.\n\nSounds interesting if true, I haven't looked into it.\n\n>> Then we make those FORCE, but most of the time the config or command\n>> summary (or list of hooks) doesn't change, so we don't need to\n>> replace the file.\n>\n> Yes, possibly we could use the \"if it hasn't changed, don't update the\n> file\" trick to avoid cascading updates.\n\nThe problem is also that you can only do it at the lowest level, or\nyou'll get into a dead-end of something else depending on the FORCE\ntarget continually re-making it, even though the target itself decided\nthere was nothing to do based on a cmp(1).\n\n>> Perhaps even better would be to piggy-back on the RUNTIME_PREFIX\n>> support, and simply drop in generated plain-text files, so in your build\n>> checkout the list of hooks, commands etc. would be parsed instead of\n>> compiled in. Then we wouldn't need to re-build or re-link anything for\n>> the version or this other data.\n>\n> Yeah, that would work. I worry a bit that the value of something like\n> \"version.h\" is lost with a runtime file, though. The point is to bake it\n> into the binary so you can't accidentally get the wrong value (say, from\n> running \"./git\" from the build directory, which looks at the runtime\n> file where the binary _would_ be installed, except you haven't run \"make\n> install\" yet).\n\nI think all of those concerns are covered under RUNTIME_PREFIX, it\ndiscovers files relative to git whether you have it installed or not.\n\nI still haven't looked into why I sometimes need --exec-path=$PWD in the\nbuild checkout, and sometimes not though...\n"},{"id":"428432","messageId":"fb23a23e-13be-14a8-4fbe-5ca2b4bcdb52@kdbg.org","threadId":"56000","inReplyTo":"YNSh0CskelTwuZq0@coredump.intra.peff.net","subject":"Re: Why the Makefile is so eager to re-build & re-link","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2021-06-24T21:30:19Z","receivedAt":"2021-06-24T21:30:25Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 24.06.21 um 17:16 schrieb Jeff King:\n> On Thu, Jun 24, 2021 at 03:16:48PM +0200, Ævar Arnfjörð Bjarmason wrote:\n>> * {command,config}-list.h (and in-flight, my hook-list.h): Every time\n>> you touch a Documentation/git-*.txt we need to re-generate these, and\n>> since their mtime changes we re-compile and re-link all the way up to\n>> libgit and our other tools.\n>>\n>> I think the best solution here is to make the generate-*.sh\n>> shellscripts faster (just one takes ~300ms of nested shellscripting,\n>> just to grep out the first few lines of every git-*.txt, in e.g. Perl\n>> or a smarter awk script this would be <5ms).\n> \n> Yeah, I think Eric mentioned he had looked into doing this in perl, but\n> we weren't entirely happy with the dependency. Here's another really odd\n> thing I noticed:\n> \n> $ time sh ./generate-cmdlist.sh command-list.txt >one\n> real\t0m1.323s\n> user\t0m1.531s\n> sys\t0m0.834s\n> \n> $ time sh -x ./generate-cmdlist.sh command-list.txt >two\n> [a bunch of trace output]\n> real\t0m0.513s\n> user\t0m0.754s\n> sys\t0m0.168s\n> \n> $ cmp one two\n> [no output]\n> \n> Er, what? Running with \"-x\" makes it almost 3 times faster to generate\n> the same output? I'd have said this is an anomaly, but it's repeatable\n> (and swapping the order produces the same output, so it's not some weird\n> priming thing). And then to top it all off, redirecting the trace is\n> slow again:\n> \n> $ time sh -x ./generate-cmdlist.sh command-list.txt >two 2>/dev/null\n> real\t0m1.363s\n> user\t0m1.538s\n> sys\t0m0.902s\n> \n> A little mini-mystery that I think I may leave unsolved for now.\n\nStrange, really. Reminds me of the case that the `read` built-in must\nread input byte by byte if stdin is not connected to a (searchable) file.\n\nI have two patches that speed up generate-cmdlist.sh a bit:\n\ngenerate-cmdlist.sh: replace for loop by printf's auto-repeat feature\n(https://github.com/j6t/git/commit/b6d05f653bede727bc001f299b57969f62d3bc03)\ngenerate-cmdlist.sh: spawn fewer processes\n(https://github.com/j6t/git/commit/fd8721ee8fae06d7b584fa5166f32bf5521ca304)\n\nthat I can submit (give me some time, though) or interested parties\ncould pick up.\n\n-- Hannes\n"},{"id":"428441","messageId":"20210624233516.23564-1-oystwa@gmail.com","threadId":"56000","inReplyTo":"874kdn1j6i.fsf@evledraar.gmail.com","subject":"Re: Why the Makefile is so eager to re-build & re-link","fromName":"Øystein Walle","fromEmail":"oystwa@gmail.com","sentAt":"2021-06-24T23:35:16Z","receivedAt":"2021-06-24T23:35:26Z","isPatch":false,"sender":{"key":"oystwa@gmail.com","avatar":"https://avatars.githubusercontent.com/u/794585?v=4"},"body":"Hi, Ævar\n\n> {command,config}-list.h (and in-flight, my hook-list.h): Every time\n> you touch a Documentation/git-*.txt we need to re-generate these, and\n> since their mtime changes we re-compile and re-link all the way up to\n> libgit and our other tools.\n> \n> I think the best solution here is to make the generate-*.sh\n> shellscripts faster (just one takes ~300ms of nested shellscripting,\n> just to grep out the first few lines of every git-*.txt, in e.g. Perl\n> or a smarter awk script this would be <5ms).\n> \n> Then we make those FORCE, but most of the time the config or command\n> summary (or list of hooks) doesn't change, so we don't need to\n> replace the file.\n\nOne possible technique to fix this is to generate to a temporary file, and copy\nthe temporary file over the real file if it's actually different. I see the\nMakefile already does create a temporary file, so how about something like\nthis:\n\ndiff --git a/Makefile b/Makefile\nindex 93664d6714..9b2f081a2a 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2216,7 +2216,8 @@ command-list.h: generate-cmdlist.sh command-list.txt\n command-list.h: $(wildcard Documentation/git*.txt)\n \t$(QUIET_GEN)$(SHELL_PATH) ./generate-cmdlist.sh \\\n \t\t$(patsubst %,--exclude-program %,$(EXCLUDED_PROGRAMS)) \\\n-\t\tcommand-list.txt >$@+ && mv $@+ $@\n+\t\tcommand-list.txt >$@+ && \\\n+\t\tif ! cmp -s $@ $@+; then mv $@+ $@; fi\n \n SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\\\n \t$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\\\n\n\nThis seems to work fine from my basic testing. I think it can even be written\nmore terse as `&& ! cmp ... &&` but that looks a bit weird to me. In any case\nit looks like it can easily be added the other relevant places in the Makefile\ntoo.\n\nØsse\n"},{"id":"428442","messageId":"20210624233918.23699-1-oystwa@gmail.com","threadId":"56000","inReplyTo":"20210624233516.23564-1-oystwa@gmail.com","subject":"Re: Why the Makefile is so eager to re-build & re-link","fromName":"Øystein Walle","fromEmail":"oystwa@gmail.com","sentAt":"2021-06-24T23:39:18Z","receivedAt":"2021-06-24T23:39:29Z","isPatch":false,"sender":{"key":"oystwa@gmail.com","avatar":"https://avatars.githubusercontent.com/u/794585?v=4"},"body":"Oops, I missed that you had already discussed this idea. Sorry about about\nthat.\n\nØsse\n"},{"id":"428449","messageId":"87wnqiyejg.fsf@evledraar.gmail.com","threadId":"56000","inReplyTo":"20210624233516.23564-1-oystwa@gmail.com","subject":"Re: Why the Makefile is so eager to re-build & re-link","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-25T00:11:57Z","receivedAt":"2021-06-25T00:27:27Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Jun 25 2021, Øystein Walle wrote:\n\n> Hi, Ævar\n>\n>> {command,config}-list.h (and in-flight, my hook-list.h): Every time\n>> you touch a Documentation/git-*.txt we need to re-generate these, and\n>> since their mtime changes we re-compile and re-link all the way up to\n>> libgit and our other tools.\n>> \n>> I think the best solution here is to make the generate-*.sh\n>> shellscripts faster (just one takes ~300ms of nested shellscripting,\n>> just to grep out the first few lines of every git-*.txt, in e.g. Perl\n>> or a smarter awk script this would be <5ms).\n>> \n>> Then we make those FORCE, but most of the time the config or command\n>> summary (or list of hooks) doesn't change, so we don't need to\n>> replace the file.\n>\n> One possible technique to fix this is to generate to a temporary file, and copy\n> the temporary file over the real file if it's actually different. I see the\n> Makefile already does create a temporary file, so how about something like\n> this:\n>\n> diff --git a/Makefile b/Makefile\n> index 93664d6714..9b2f081a2a 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -2216,7 +2216,8 @@ command-list.h: generate-cmdlist.sh command-list.txt\n> command-list.h: $(wildcard Documentation/git*.txt)\n> \t$(QUIET_GEN)$(SHELL_PATH) ./generate-cmdlist.sh \\\n> \t\t$(patsubst %,--exclude-program %,$(EXCLUDED_PROGRAMS)) \\\n> -\t\tcommand-list.txt >$@+ && mv $@+ $@\n> +\t\tcommand-list.txt >$@+ && \\\n> +\t\tif ! cmp -s $@ $@+; then mv $@+ $@; fi\n> \n> SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\\\n> \t$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\\\n>\n>\n> This seems to work fine from my basic testing. I think it can even be written\n> more terse as `&& ! cmp ... &&` but that looks a bit weird to me. In any case\n> it looks like it can easily be added the other relevant places in the Makefile\n> too.\n\nI see your downthread \"oops\", no worries. Just to elaborate slightly on\nthe comment I had in [1] (and as you've doubtless discovered) then yes,\nthis works the first time:\n \n $ touch Documentation/git-bisect.txt\n $ make -k -j $(nproc) git\n GEN command-list.h\n\nWithout your change we'd have also had to recompile \"git\", but the\nproblem is that if you run and re-run that we'll keep making\ncommand-list.h again and again, which is \"make\" getting stuck on,\nbecause:\n\n $ make -d -k -j $(nproc) git 2>&1 | grep newer.*command-list.h\n Prerequisite 'Documentation/git-bisect.txt' is newer than target 'command-list.h'.\n\nHence the \"lowest level\" comment in [1], i.e. you can only use this\n\"cmp\" trick for things that don't themselves have dependencies.\n\nWhich at the end of the day is why make is both wonderful, and why it\nsucks.\n\nIt sometimes gets hard to force your \"circular\" dependency graph into\nthe \"square peg\" of files and their mtimes, but it's also great for\nsimplicity that everyone can't write their own custom \"this is what it\nmeans for my dependencies to have changed\" function (which in practice\nwould break way more in the hands of most programmers, including yours\ntruly, than files & their mtimes).\n\nE.g. in this case we don't *actually* depend on Documentation/git*.txt\nfor the purposes of re-making things all the way up the graph, we depend\non the tiny bit of data we extract from those files, basically just the\nNAME section at the very beginning.\n\nThe only way to represent that information in a way that \"make\"\nunderstands would be to add another intermediate step where we extract\nexactly that information out into intermediate files with a script, and\nthen in turn depend on those files.\n\nWhich actually, might not be such a bad idea as a solution to this\nparticular problem.\n\nI.e. just to have individual generated versions of\ngit-bisect.txt.just-what-command-list.h-needs. We do something vaguely\nsimilar with Documentation/build-docdep.perl, which is another glorious\nhack of trying to get make's dependency graph to behave as we'd like.\n\nAs a practical matter it wouldn't get you very far except for doc-only\nchanges, since you'd still have the version.c problem, which also\nchanges on every (code) commit.\n\n1. http://lore.kernel.org/git/87y2azyzer.fsf@evledraar.gmail.com\n"},{"id":"428457","messageId":"87r1gqxqxn.fsf@evledraar.gmail.com","threadId":"56000","inReplyTo":"fb23a23e-13be-14a8-4fbe-5ca2b4bcdb52@kdbg.org","subject":"Re: Why the Makefile is so eager to re-build & re-link","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-25T08:34:20Z","receivedAt":"2021-06-25T08:57:14Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Jun 24 2021, Johannes Sixt wrote:\n\n> Am 24.06.21 um 17:16 schrieb Jeff King:\n>> On Thu, Jun 24, 2021 at 03:16:48PM +0200, Ævar Arnfjörð Bjarmason wrote:\n>>> * {command,config}-list.h (and in-flight, my hook-list.h): Every time\n>>> you touch a Documentation/git-*.txt we need to re-generate these, and\n>>> since their mtime changes we re-compile and re-link all the way up to\n>>> libgit and our other tools.\n>>>\n>>> I think the best solution here is to make the generate-*.sh\n>>> shellscripts faster (just one takes ~300ms of nested shellscripting,\n>>> just to grep out the first few lines of every git-*.txt, in e.g. Perl\n>>> or a smarter awk script this would be <5ms).\n>> \n>> Yeah, I think Eric mentioned he had looked into doing this in perl, but\n>> we weren't entirely happy with the dependency. Here's another really odd\n>> thing I noticed:\n>> \n>> $ time sh ./generate-cmdlist.sh command-list.txt >one\n>> real\t0m1.323s\n>> user\t0m1.531s\n>> sys\t0m0.834s\n>> \n>> $ time sh -x ./generate-cmdlist.sh command-list.txt >two\n>> [a bunch of trace output]\n>> real\t0m0.513s\n>> user\t0m0.754s\n>> sys\t0m0.168s\n>> \n>> $ cmp one two\n>> [no output]\n>> \n>> Er, what? Running with \"-x\" makes it almost 3 times faster to generate\n>> the same output? I'd have said this is an anomaly, but it's repeatable\n>> (and swapping the order produces the same output, so it's not some weird\n>> priming thing). And then to top it all off, redirecting the trace is\n>> slow again:\n>> \n>> $ time sh -x ./generate-cmdlist.sh command-list.txt >two 2>/dev/null\n>> real\t0m1.363s\n>> user\t0m1.538s\n>> sys\t0m0.902s\n>> \n>> A little mini-mystery that I think I may leave unsolved for now.\n>\n> Strange, really. Reminds me of the case that the `read` built-in must\n> read input byte by byte if stdin is not connected to a (searchable) file.\n>\n> I have two patches that speed up generate-cmdlist.sh a bit:\n>\n> generate-cmdlist.sh: replace for loop by printf's auto-repeat feature\n> (https://github.com/j6t/git/commit/b6d05f653bede727bc001f299b57969f62d3bc03)\n> generate-cmdlist.sh: spawn fewer processes\n> (https://github.com/j6t/git/commit/fd8721ee8fae06d7b584fa5166f32bf5521ca304)\n>\n> that I can submit (give me some time, though) or interested parties\n> could pick up.\n\nInteresting, but I think rather than micro-optimizing the O(n) loop it\nmakes more sense to turn it into a series of O(1) in -j parallel,\ni.e. actually use the make dependency graph for this as I suggested in:\nhttps://lore.kernel.org/git/87wnqiyejg.fsf@evledraar.gmail.com/\n\nSomething like the hacky throwaway patch that follows. Now when you\ntouch a file in Documentation/git-*.txt you re-make just that file\nchain, which gets assembled into the command-list.h:\n\t\n\t$ touch Documentation/git-add.txt; time make -k -j $(nproc) git 2>&1\n\tGIT_VERSION = 2.32.0.94.g87ef2a6b7ed.dirty\n\t GEN build/Documentation/git-add.txt.cmdlist.in\n\t CC version.o\n\t GEN build/Documentation/git-add.txt.cmdlist\n\t GEN command-list.h\n\t CC help.o\n\t AR libgit.a\n\t LINK git\n\nThose build/* files are, respectively, the relevant line corresponding\nto the *.txt file from command-list.txt:\n\n $ cat build/Documentation/git-add.txt.cmdlist.in\n git-add mainporcelain worktree\n\nAnd then the line we want in command-list.h at the end:\n\n $ cat build/Documentation/git-add.txt.cmdlist\n { \"git-add\", N_(\"Add file contents to the index\"), 0 | CAT_mainporcelain | CAT_worktree },\n\nThe build process in then just a matter of keeping those files\nup-to-date, and having \"make\" create the command-list.h is just a matter\nof:\n\n cat build/Documentation/*.cmdlist\n\nWell, with the categories still being an O(n) affair, but the cost of\ngenerating those is trivial.\n\nI think that aside from optimizing for speed it just makes things\nclearer, if you modify e.g. git-add.txt you see a clear chain of output\nfrom make about how we generate output from that, and then make the\ncommand-list.h.\n\nNow we'd just show a mysterious command-list.h entry. Whenever you have\n\"make\" create one file from many it becomes hard to see at a glance\nwhat's going on.\n\nI'd be curious to see what how that performs e.g. on Windows, on Linux I\nget e.g. (warm cache):\n\t\n\t$ touch Documentation/git-a*.txt; time make -k -j $(nproc) command-list.h 2>&1\n\t GEN build/Documentation/git-apply.txt.cmdlist.in\n\t GEN build/Documentation/git-annotate.txt.cmdlist.in\n\t GEN build/Documentation/git-am.txt.cmdlist.in\n\t GEN build/Documentation/git-add.txt.cmdlist.in\n\t GEN build/Documentation/git-archimport.txt.cmdlist.in\n\t GEN build/Documentation/git-archive.txt.cmdlist.in\n\t GEN build/Documentation/git-apply.txt.cmdlist\n\t GEN build/Documentation/git-annotate.txt.cmdlist\n\t GEN build/Documentation/git-am.txt.cmdlist\n\t GEN build/Documentation/git-add.txt.cmdlist\n\t GEN build/Documentation/git-archimport.txt.cmdlist\n\t GEN build/Documentation/git-archive.txt.cmdlist\n\t GEN command-list.h\n\t\n\treal 0m0.214s\n\tuser 0m0.196s\n\tsys 0m0.071s\n\t\nDoing the same on master takes around 600ms for me at best:\n\t\n\t$ touch Documentation/git-a*.txt; time make -k -j $(nproc) command-list.h 2>&1\n\t GEN command-list.h\n\t\n\treal 0m0.611s\n\tuser 0m0.756s\n\tsys 0m0.112s\n\nIt's even faster when I have to make all of them (my -j is = 8), or\naround 450ms after a \"touch Documentation/git-*.txt\".\n\nWe have ~170 lines we process in command-list.txt, I'd think on Windows\nyou'd get much better results, instead of optimizing those number of\n\"sort | uniq\" to the same number of \"sort -u\" the common case is just\nrunning 1-5 of those, as that's all the *.txt files you changed, then\njust \"cat-ing\" the full set.\n\ndiff --git a/.gitignore b/.gitignore\nindex 311841f9bed..9b365395496 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -13,6 +13,7 @@\n /GIT-USER-AGENT\n /GIT-VERSION-FILE\n /bin-wrappers/\n+/build/\n /git\n /git-add\n /git-add--interactive\ndiff --git a/Makefile b/Makefile\nindex c3565fc0f8f..5e845bd0f69 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2231,12 +2231,30 @@ config-list.h: Documentation/*config.txt Documentation/config/*.txt\n \t$(QUIET_GEN)$(SHELL_PATH) ./generate-configlist.sh \\\n \t\t>$@+ && mv $@+ $@\n \n-command-list.h: generate-cmdlist.sh command-list.txt\n+build/Documentation:\n+\t$(QUIET_GEN)mkdir -p build/Documentation\n+.PRECIOUS: build/Documentation/git%.txt.cmdlist.in\n+build/Documentation/git%.txt.cmdlist.in: Documentation/git%.txt\n+\t$(QUIET_GEN)if ! grep -w $(patsubst build/Documentation/%.txt.cmdlist.in,%,$@) command-list.txt >$@; \\\n+\tthen \\\n+\t\t# For e.g. git-init-db, which has a *.txt file, but no \\\n+\t\t# command-list.h entry \\\n+\t\t>$@; \\\n+\tfi\n+build/Documentation/git%.txt.cmdlist: build/Documentation/git%.txt.cmdlist.in\n+\t$(QUIET_GEN)./generate-cmdlist.sh --tail $< >$@\n \n-command-list.h: $(wildcard Documentation/git*.txt)\n-\t$(QUIET_GEN)$(SHELL_PATH) ./generate-cmdlist.sh \\\n+COMMAND_LIST_H_FILES = $(filter-out Documentation/git.txt,$(wildcard Documentation/git*.txt))\n+\n+COMMAND_LIST_GEN = $(patsubst Documentation/%.txt,build/Documentation/%.txt.cmdlist,$(COMMAND_LIST_H_FILES))\n+command-list.h: build/Documentation generate-cmdlist.sh command-list.txt $(COMMAND_LIST_GEN)\n+\t$(QUIET_GEN)$(SHELL_PATH) ./generate-cmdlist.sh --header \\\n \t\t$(patsubst %,--exclude-program %,$(EXCLUDED_PROGRAMS)) \\\n-\t\tcommand-list.txt >$@+ && mv $@+ $@\n+\t\tcommand-list.txt >$@+ && \\\n+\techo \"static struct cmdname_help command_list[] = {\" >>$@+ && \\\n+\tcat build/Documentation/*.txt.cmdlist >>$@+ && \\\n+\techo \"};\" >>$@+ && \\\n+\tmv $@+ $@\n \n SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\\\n \t$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\\\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex 9dbbb08e70a..f80266d5138 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -1,5 +1,8 @@\n #!/bin/sh\n \n+mode=$1\n+shift\n+\n die () {\n \techo \"$@\" >&2\n \texit 1\n@@ -61,8 +64,6 @@ define_category_names () {\n }\n \n print_command_list () {\n-\techo \"static struct cmdname_help command_list[] = {\"\n-\n \tcommand_list \"$1\" |\n \twhile read cmd rest\n \tdo\n@@ -73,6 +74,9 @@ print_command_list () {\n \t\tdone\n \t\techo \" },\"\n \tdone\n+}\n+\n+end_print_command_list () {\n \techo \"};\"\n }\n \n@@ -84,6 +88,12 @@ do\n \tshift\n done\n \n+if test \"$mode\" = \"--tail\"\n+then\n+\tprint_command_list \"$1\"\n+\texit 0\n+fi\n+\n echo \"/* Automatically generated by generate-cmdlist.sh */\n struct cmdname_help {\n \tconst char *name;\n@@ -94,5 +104,3 @@ struct cmdname_help {\n define_categories \"$1\"\n echo\n define_category_names \"$1\"\n-echo\n-print_command_list \"$1\"\n"},{"id":"428458","messageId":"87o8buxqkv.fsf@evledraar.gmail.com","threadId":"56000","inReplyTo":"87r1gqxqxn.fsf@evledraar.gmail.com","subject":"Re: Why the Makefile is so eager to re-build & re-link","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-25T09:01:18Z","receivedAt":"2021-06-25T09:04:57Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Jun 25 2021, Ævar Arnfjörð Bjarmason wrote:\n\nJust one small thing I forgot to note:\n\n> On Thu, Jun 24 2021, Johannes Sixt wrote:\n>> I have two patches that speed up generate-cmdlist.sh a bit:\n> [...]\n> Interesting, but I think rather than micro-optimizing the O(n) loop it\n> makes more sense to turn it into a series of O(1) in -j parallel,\n> i.e. actually use the make dependency graph for this as I suggested in:\n> https://lore.kernel.org/git/87wnqiyejg.fsf@evledraar.gmail.com/\n> [...]\n> diff --git a/.gitignore b/.gitignore\n> index 311841f9bed..9b365395496 100644\n> --- a/.gitignore\n> +++ b/.gitignore\n> @@ -13,6 +13,7 @@\n> /GIT-USER-AGENT\n> /GIT-VERSION-FILE\n> /bin-wrappers/\n> +/build/\n> /git\n> /git-add\n> /git-add--interactive\n> diff --git a/Makefile b/Makefile\n> index c3565fc0f8f..5e845bd0f69 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -2231,12 +2231,30 @@ config-list.h: Documentation/*config.txt Documentation/config/*.txt\n> \t$(QUIET_GEN)$(SHELL_PATH) ./generate-configlist.sh \\\n> \t\t>$@+ && mv $@+ $@\n> \n> -command-list.h: generate-cmdlist.sh command-list.txt\n> +build/Documentation:\n> +\t$(QUIET_GEN)mkdir -p build/Documentation\n> +.PRECIOUS: build/Documentation/git%.txt.cmdlist.in\n> +build/Documentation/git%.txt.cmdlist.in: Documentation/git%.txt\n> +\t$(QUIET_GEN)if ! grep -w $(patsubst build/Documentation/%.txt.cmdlist.in,%,$@) command-list.txt >$@; \\\n> +\tthen \\\n> +\t\t# For e.g. git-init-db, which has a *.txt file, but no \\\n> +\t\t# command-list.h entry \\\n> +\t\t>$@; \\\n> +\tfi\n> +build/Documentation/git%.txt.cmdlist: build/Documentation/git%.txt.cmdlist.in\n> +\t$(QUIET_GEN)./generate-cmdlist.sh --tail $< >$@\n> \n> -command-list.h: $(wildcard Documentation/git*.txt)\n> -\t$(QUIET_GEN)$(SHELL_PATH) ./generate-cmdlist.sh \\\n> +COMMAND_LIST_H_FILES = $(filter-out Documentation/git.txt,$(wildcard Documentation/git*.txt))\n> +\n> +COMMAND_LIST_GEN = $(patsubst Documentation/%.txt,build/Documentation/%.txt.cmdlist,$(COMMAND_LIST_H_FILES))\n> +command-list.h: build/Documentation generate-cmdlist.sh command-list.txt $(COMMAND_LIST_GEN)\n> +\t$(QUIET_GEN)$(SHELL_PATH) ./generate-cmdlist.sh --header \\\n> \t\t$(patsubst %,--exclude-program %,$(EXCLUDED_PROGRAMS)) \\\n\nI left this --exclude-program code (added in 724d63569fe (help -a: do\nnot list commands that are excluded from the build, 2019-04-18)) in just\nbecause I couldn't be bothered to rewrite it for this demo. But of\ncourse that's also something that becomes much easier once we do this in\na Makefile-native way, we'd just $(filter) that list out ourselves, so\nno need to have some shellscript deal with it.\n\nThat's also true on \"master\" right now, 724d63569fe doesn't discuss why\nit went for the shellscript approach. I suspect it's just that the\nauthor thought it was easier than dealing with some combination of\n$(filter) and (a possibly nested) $(pathsubst).\n\n> -\t\tcommand-list.txt >$@+ && mv $@+ $@\n> +\t\tcommand-list.txt >$@+ && \\\n> +\techo \"static struct cmdname_help command_list[] = {\" >>$@+ && \\\n> +\tcat build/Documentation/*.txt.cmdlist >>$@+ && \\\n> +\techo \"};\" >>$@+ && \\\n> +\tmv $@+ $@\n"},{"id":"428504","messageId":"60d647f8b427c_cc8d20877@natae.notmuch","threadId":"56000","inReplyTo":"YNSh0CskelTwuZq0@coredump.intra.peff.net","subject":"Re: Why the Makefile is so eager to re-build & re-link","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-25T21:17:44Z","receivedAt":"2021-06-25T21:17:48Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Jeff King wrote:\n> On Thu, Jun 24, 2021 at 03:16:48PM +0200, Ævar Arnfjörð Bjarmason wrote:\n> \n> > This is probably all stuff that's been on list-before / known by\n> > some/all people in the CC list, but in case not: I looked a bit into why\n> > we'e so frequently re-linking and re compiling things these days,\n> > slowing down e.g. \"git rebase --exec='make ...'\".\n> > \n> > These are all fixable issues, I haven't worked on them, just some notes\n> > in case anyone has better ideas:\n> \n> From a quick skim I didn't see anything wrong in your analysis or\n> suggestions. I do kind of wonder if we are hitting a point of\n> diminishing returns here. \"make -j16\" on my system takes ~175ms for a\n> noop, and ~650ms if I have to regenerate version.h (it's something like\n> 2s total of CPU, but I have 8 cores).\n> \n> I know I've probably got a nicer machine than many other folks. But at\n> some point correctness and avoiding complexity in the Makefile become a\n> win over shaving off a second from compile times. You'd probably find\n> lower hanging fruit in the test suite which could shave off tens of\n> seconds. :)\n\nThat's not a good enough reason to not try to improve something.\nIn fact, it's a known fallacy called the fallacy of relative privation [1].\n\nAlso, your definition of \"correctness\" is not necessarily shared by\neveryone.\n\nI for one don't see what I'm gaining by running tests with\nGIT_VERSION = 2.32.0.97.g949e814b27, and then 2.32.0.98.gcfb60a24d6.\n\nWhat's wrong with GIT_VERSION = 2.33-dev?\n\nSure, I understand that some people might want to have a precise\nversion, but when I'm doing development I don't. Perhaps set a static\nversion when DEVELOPER=1?\n\n[1] https://rationalwiki.org/wiki/Not_as_bad_as\n\n-- \nFelipe Contreras"},{"id":"428705","messageId":"YNqBtrXzUlJiuc7y@coredump.intra.peff.net","threadId":"56000","inReplyTo":"87r1gqxqxn.fsf@evledraar.gmail.com","subject":"Re: Why the Makefile is so eager to re-build & re-link","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-06-29T02:13:10Z","receivedAt":"2021-06-29T02:13:14Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 25, 2021 at 10:34:20AM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> Interesting, but I think rather than micro-optimizing the O(n) loop it\n> makes more sense to turn it into a series of O(1) in -j parallel,\n> i.e. actually use the make dependency graph for this as I suggested in:\n> https://lore.kernel.org/git/87wnqiyejg.fsf@evledraar.gmail.com/\n\nI have mixed feelings on that. I do like the general notion of breaking\napart tasks and feeding the dependencies to \"make\", because that lets it\ndo a better job of parallelizing or avoiding already-done work. But\nthere's a cost to running any job, so eventually you get to a unit of\nwork that's so small the overhead dominates.\n\nFor instance, starting from a built Git but dirtying all doc files with\n\"touch Documentation/*.txt\", running \"time make -j16\" yields:\n\n real\t0m1.749s\n user\t0m2.963s\n sys\t0m1.146s\n\nWith your patch to break it apart into many jobs, the same operation\ngives:\n\n real\t0m0.762s\n user\t0m3.054s\n sys\t0m0.600s\n\nSo that took fewer wall-clock seconds, but we actually spent more CPU.\nOn a system with fewer cores, it would probably be a loss in both\nplaces.\n\nNow maybe that's a good tradeoff, especially because the common case\n(aside from a build-from-scratch, which will spend loads more time\nactually compiling anyway) is that only a handful of files would be\nupdated.\n\nBut if we can just make the actual operation faster, then even O(n)\nrepeated work might be a win in all cases, because it's avoiding the\noverhead of extra jobs.\n\nI dunno. I think there's a formula here that depends on the overhead of\na job versus the time to handle a single file in the script, coupled\nwith the expected number of changed files for any run. I'm not sure of\nthe exact values of those numbers in this case, but am mostly pointing\nout that it's a tradeoff and not always a pure win. :)\n\n> Something like the hacky throwaway patch that follows. Now when you\n> touch a file in Documentation/git-*.txt you re-make just that file\n> chain, which gets assembled into the command-list.h:\n\nI know you said this was throwaway, but in case you do pursue it\nfurther, my first run hit:\n\n $ time make\n GIT_VERSION = 2.32.0.94.gaa5e6f14dd\n * new prefix flags\n GEN build/Documentation\n GEN build/Documentation/git-add.txt.cmdlist.in\n /bin/sh: 1: cannot create build/Documentation/git-add.txt.cmdlist.in: Directory nonexistent\n /bin/sh: 5: cannot create build/Documentation/git-add.txt.cmdlist.in: Directory nonexistent\n GEN build/Documentation/git-am.txt.cmdlist.in\n /bin/sh: 1: cannot create build/Documentation/git-am.txt.cmdlist.in: Directory nonexistent\n /bin/sh: 5: cannot create build/Documentation/git-am.txt.cmdlist.in: Directory nonexistent\n\nSo I'd guess there's some race with creating the build/Documentation\ndirectory (a subsequent run worked fine).\n\n-Peff\n"},{"id":"428715","messageId":"CAPig+cSzKoOzU-zPOZqfNpPYBFpcWqvDP3mwLvAn5WkiNW0UMw@mail.gmail.com","threadId":"56000","inReplyTo":"YNSh0CskelTwuZq0@coredump.intra.peff.net","subject":"Re: Why the Makefile is so eager to re-build & re-link","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-06-29T05:04:53Z","receivedAt":"2021-06-29T05:05:06Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Jun 24, 2021 at 11:16 AM Jeff King wrote:\n> On Thu, Jun 24, 2021 at 03:16:48PM +0200, Ævar Arnfjörð Bjarmason wrote:\n> > I think the best solution here is to make the generate-*.sh\n> > shellscripts faster (just one takes ~300ms of nested shellscripting,\n> > just to grep out the first few lines of every git-*.txt, in e.g. Perl\n> > or a smarter awk script this would be <5ms).\n>\n> Yeah, I think Eric mentioned he had looked into doing this in perl, but\n> we weren't entirely happy with the dependency. [...]\n\nFor what it's worth, the original `generate-cmdlist` was a shell\nscript which I rewrote[1] in `awk` to extend the functionality, but\nJunio felt uncomfortable[2] about making `awk` a build dependency, so\nI rewrote[3] it again in `perl`. However, the `perl` version didn't\nlast long since we got a report[4] that Git would no longer build in\nthe FreeBSD ports tree, so I rewrote[5] it a final time in shell, thus\ncoming full circle (but with extended functionality).\n\n[1]: https://lore.kernel.org/git/1431976697-26288-4-git-send-email-sebastien.guimmara@gmail.com/\n[2]: https://lore.kernel.org/git/xmqqr3qda7kx.fsf@gitster.dls.corp.google.com/\n[3]: https://lore.kernel.org/git/1432149781-24596-4-git-send-email-sebastien.guimmara@gmail.com/\n[4]: https://lore.kernel.org/git/loom.20150814T171757-901@post.gmane.org/\n[5]: https://lore.kernel.org/git/1440365469-9928-1-git-send-email-sunshine@sunshineco.com/\n"},{"id":"429034","messageId":"patch-1.1-911881ce19f-20210702T115617Z-avarab@gmail.com","threadId":"56000","inReplyTo":"874kdn1j6i.fsf@evledraar.gmail.com","subject":"[PATCH] Documentation/Makefile: don't re-build on 'git version' changes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-07-02T11:58:47Z","receivedAt":"2021-07-02T11:59:01Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Since 9a71722b4df (Doc: auto-detect changed build flags, 2019-03-17)\nwe've been eagerly re-building the documentation whenever the output\nof \"git version\" (via the GIT-VERSION file) changed. This was never\nthe intention, and was a regression on what we intended in\n7b8a74f39cb (Documentation: Replace @@GIT_VERSION@@ in documentation,\n2007-03-25).\n\nSo let's add an ASCIIDOC_MANVERSION variable that we exclude from\nASCIIDOC_COMMON. The change in 9a71722b4df was only intending to catch\ncases where we e.g. switched between asciidoc and asciidoctor, not to\nundo the logic in 7b8a74f39cb and force a re-build every time our HEAD\nchanged in the repository.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n\nAs a follow-up to\nhttps://lore.kernel.org/git/874kdn1j6i.fsf@evledraar.gmail.com/ I cut\n\"make man\" out of my \"rebase -x\" invocations, I could swear it didn't\nused to take so long. Turns out it didn't, and that its eagerness is a\nrecent-ish regression.\n\nThis is what we used to do before v2.22.0, so I'm not too worried\nabout the edge case discussed in the comment here. I think an\nimprovement on this might be to e.g. force all the flags with a \"make\ndist\" or one of the install targets.\n\nIn practice I don't think there's many/any people who build releases\nthat matter to anyone out of the checkout they've been using for their\nown development.\n\n Documentation/Makefile | 17 ++++++++++++++---\n 1 file changed, 14 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex f5605b7767f..6b3f0bb6c8b 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -137,11 +137,12 @@ ASCIIDOC_HTML = xhtml11\n ASCIIDOC_DOCBOOK = docbook\n ASCIIDOC_CONF = -f asciidoc.conf\n ASCIIDOC_COMMON = $(ASCIIDOC) $(ASCIIDOC_EXTRA) $(ASCIIDOC_CONF) \\\n-\t\t-amanversion=$(GIT_VERSION) \\\n \t\t-amanmanual='Git Manual' -amansource='Git'\n+ASCIIDOC_MANVERSION = -amanversion=$(GIT_VERSION)\n+ASCIIDOC_ALL = $(ASCIIDOC_COMMON) $(ASCIIDOC_MANVERSION)\n ASCIIDOC_DEPS = asciidoc.conf GIT-ASCIIDOCFLAGS\n-TXT_TO_HTML = $(ASCIIDOC_COMMON) -b $(ASCIIDOC_HTML)\n-TXT_TO_XML = $(ASCIIDOC_COMMON) -b $(ASCIIDOC_DOCBOOK)\n+TXT_TO_HTML = $(ASCIIDOC_ALL) -b $(ASCIIDOC_HTML)\n+TXT_TO_XML = $(ASCIIDOC_ALL) -b $(ASCIIDOC_DOCBOOK)\n MANPAGE_XSL = manpage-normal.xsl\n XMLTO = xmlto\n XMLTO_EXTRA =\n@@ -333,6 +334,16 @@ mergetools-list.made: ../git-mergetool--lib.sh $(wildcard ../mergetools/*)\n \t\tshow_tool_names can_merge \"* \" || :' >mergetools-merge.txt && \\\n \tdate >$@\n \n+# We use $(ASCIIDOC_COMMON) here, and not $(ASCIIDOC_ALL). We don't\n+# want to include $(ASCIIDOC_MANVERSION) and have the documentation\n+# re-built every time HEAD changes.\n+#\n+# This is a trade-off requiring a \"clean\" build of the documentation\n+# for release purposes, in the future we might include the version if\n+# there's a cheaper way to re-insert the \"Source\" version during\n+# re-builds. If we detect that that's the only thing we changed we\n+# could insert it with a cheap search/replacement against the existing\n+# files.\n TRACK_ASCIIDOCFLAGS = $(subst ','\\'',$(ASCIIDOC_COMMON):$(ASCIIDOC_HTML):$(ASCIIDOC_DOCBOOK))\n \n GIT-ASCIIDOCFLAGS: FORCE\n-- \n2.32.0.634.g284ac724283\n\n"},{"id":"429060","messageId":"xmqqmtr4pv9r.fsf@gitster.g","threadId":"56000","inReplyTo":"patch-1.1-911881ce19f-20210702T115617Z-avarab@gmail.com","subject":"Re: [PATCH] Documentation/Makefile: don't re-build on 'git version' changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-02T15:53:20Z","receivedAt":"2021-07-02T15:53:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Since 9a71722b4df (Doc: auto-detect changed build flags, 2019-03-17)\n> we've been eagerly re-building the documentation whenever the output\n> of \"git version\" (via the GIT-VERSION file) changed. This was never\n> the intention, and was a regression on what we intended in\n> 7b8a74f39cb (Documentation: Replace @@GIT_VERSION@@ in documentation,\n> 2007-03-25).\n\nI am not sure. Even if there were no changes in say\n'Documentation/git-cat-file.txt' and the sources it depends on\nbetween 'master' and 'next', after doing this:\n\n $ git checkout next\n $ make prefix=$HOME/git-next/ install install-doc\n $ git checkout master\n $ make prefix=$HOME/git-master/ install install-doc\n $ $HOME/git-master/bin/git help cat-file | tail -n 1\n\nI should see that the documentation should say it is from the\n'master' branch in its footer, no?\n\nIn other words, I think 7b8a74f39cb's reasoning (not the\nimplementation), especially the last sentence of its log message, is\nflawed, where it said:\n\n Documentation: Replace @@GIT_VERSION@@ in documentation\n \n Include GIT-VERSION-FILE and replace @@GIT_VERSION@@ in\n the HTML and XML asciidoc output. The documentation\n doesn't depend on GIT-VERSION-FILE so it will not be\n automatically rebuild if nothing else changed.\n\nThanks.\n"},{"id":"429095","messageId":"60dfb7d11cac3_3dd220811@natae.notmuch","threadId":"56000","inReplyTo":"patch-1.1-911881ce19f-20210702T115617Z-avarab@gmail.com","subject":"RE: [PATCH] Documentation/Makefile: don't re-build on 'git version' changes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-07-03T01:05:21Z","receivedAt":"2021-07-03T01:07:15Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n> Since 9a71722b4df (Doc: auto-detect changed build flags, 2019-03-17)\n> we've been eagerly re-building the documentation whenever the output\n> of \"git version\" (via the GIT-VERSION file) changed. This was never\n> the intention, and was a regression on what we intended in\n> 7b8a74f39cb (Documentation: Replace @@GIT_VERSION@@ in documentation,\n> 2007-03-25).\n> \n> So let's add an ASCIIDOC_MANVERSION variable that we exclude from\n> ASCIIDOC_COMMON. The change in 9a71722b4df was only intending to catch\n> cases where we e.g. switched between asciidoc and asciidoctor, not to\n> undo the logic in 7b8a74f39cb and force a re-build every time our HEAD\n> changed in the repository.\n\nOnce again, why do we care that the version is 2.32.0.98.gcfb60a24d6 and\nnot 2.32.0.97.g949e814b27?\n\nNot just in the documentation, but everywhere.\n\nMaybe we can add a GIT_RELEASE variable that unlike GIT_VERSION it\ndoesn't contain the precise commit. For example GIT_RELEASE = 2.33-dev.\n\n-- \nFelipe Contreras"},{"id":"429117","messageId":"87pmvz8v1u.fsf@evledraar.gmail.com","threadId":"56000","inReplyTo":"xmqqmtr4pv9r.fsf@gitster.g","subject":"Re: [PATCH] Documentation/Makefile: don't re-build on 'git version' changes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-07-03T11:58:55Z","receivedAt":"2021-07-03T12:02:26Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Jul 02 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> Since 9a71722b4df (Doc: auto-detect changed build flags, 2019-03-17)\n>> we've been eagerly re-building the documentation whenever the output\n>> of \"git version\" (via the GIT-VERSION file) changed. This was never\n>> the intention, and was a regression on what we intended in\n>> 7b8a74f39cb (Documentation: Replace @@GIT_VERSION@@ in documentation,\n>> 2007-03-25).\n>\n> I am not sure. Even if there were no changes in say\n> 'Documentation/git-cat-file.txt' and the sources it depends on\n> between 'master' and 'next', after doing this:\n>\n> $ git checkout next\n> $ make prefix=$HOME/git-next/ install install-doc\n> $ git checkout master\n> $ make prefix=$HOME/git-master/ install install-doc\n> $ $HOME/git-master/bin/git help cat-file | tail -n 1\n>\n> I should see that the documentation should say it is from the\n> 'master' branch in its footer, no?\n\nYes in theory, in practice it's very annoying to have the very slow\ndocumentation build be re-built so aggressively. Since it wasn't a\npractical issue anyone worried about before 2019 I think it's worth\nreverting it.\n\n> In other words, I think 7b8a74f39cb's reasoning (not the\n> implementation), especially the last sentence of its log message, is\n> flawed, where it said:\n>\n> Documentation: Replace @@GIT_VERSION@@ in documentation\n> \n> Include GIT-VERSION-FILE and replace @@GIT_VERSION@@ in\n> the HTML and XML asciidoc output. The documentation\n> doesn't depend on GIT-VERSION-FILE so it will not be\n> automatically rebuild if nothing else changed.\n\nArguably it's a feature. The point of the version in the documentation\nis to make it clear what version we're discussing. If I build something\non the master SHA-1 and advance to next, and none of the documentation\ndependencies change, it's most useful to refer to the oldest last\nversion we can cover.\n\nI think nobody's doing such a \"chained\" build when building the docs for\na \"real\" release, and having mixed versions might be confusing, but for\nthe \"local build\" case from a development checkout it's arguably more\nuseful.\n"},{"id":"429118","messageId":"87mtr38tvd.fsf@evledraar.gmail.com","threadId":"56000","inReplyTo":"60dfb7d11cac3_3dd220811@natae.notmuch","subject":"Re: [PATCH] Documentation/Makefile: don't re-build on 'git version' changes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-07-03T12:03:40Z","receivedAt":"2021-07-03T12:27:55Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Jul 02 2021, Felipe Contreras wrote:\n\n> Ævar Arnfjörð Bjarmason wrote:\n>> Since 9a71722b4df (Doc: auto-detect changed build flags, 2019-03-17)\n>> we've been eagerly re-building the documentation whenever the output\n>> of \"git version\" (via the GIT-VERSION file) changed. This was never\n>> the intention, and was a regression on what we intended in\n>> 7b8a74f39cb (Documentation: Replace @@GIT_VERSION@@ in documentation,\n>> 2007-03-25).\n>> \n>> So let's add an ASCIIDOC_MANVERSION variable that we exclude from\n>> ASCIIDOC_COMMON. The change in 9a71722b4df was only intending to catch\n>> cases where we e.g. switched between asciidoc and asciidoctor, not to\n>> undo the logic in 7b8a74f39cb and force a re-build every time our HEAD\n>> changed in the repository.\n>\n> Once again, why do we care that the version is 2.32.0.98.gcfb60a24d6 and\n> not 2.32.0.97.g949e814b27?\n>\n> Not just in the documentation, but everywhere.\n\nIt's useful e.g. on my Debian system to see that the \"next\" Debian\npackaged is 2.31.0.291.g576ba9dcdaf in docs & \"git version\", arguably\nless so for documentation.\n\nBut yeah, I think we should at least have some opt-out for sticking the\nexact version everywhere, given the mostly unless re-building it does\nduring development.\n\n> Maybe we can add a GIT_RELEASE variable that unlike GIT_VERSION it\n> doesn't contain the precise commit. For example GIT_RELEASE = 2.33-dev.\n\nI just added this to my pre-make script:\n\n\tgrep -q ^/version .git/info/exclude || echo /version >>.git/info/exclude\n\techo $(grep -o -P '(?<=^DEF_VER=v).*' GIT-VERSION-GEN)-dev >version\n\nIt makes use of GIT-VERSION-GEN picking up a tarball \"version\" file.\n\nSo far it seems like Junio isn't interested in the patch, and in any\ncase it doesn't fix the more general over-rebuilding due to version\nchanges noted upthread when it comes to *.o and libgit. Doing this fixes\nboth for me. Then when I build an installed version I don't use that\ntrick.\n"},{"id":"429133","messageId":"60e0b2f2f198d_2f72081c@natae.notmuch","threadId":"56000","inReplyTo":"87mtr38tvd.fsf@evledraar.gmail.com","subject":"Re: [PATCH] Documentation/Makefile: don't re-build on 'git version' changes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-07-03T18:56:50Z","receivedAt":"2021-07-03T18:56:59Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n> \n> On Fri, Jul 02 2021, Felipe Contreras wrote:\n> \n> > Ævar Arnfjörð Bjarmason wrote:\n> >> Since 9a71722b4df (Doc: auto-detect changed build flags, 2019-03-17)\n> >> we've been eagerly re-building the documentation whenever the output\n> >> of \"git version\" (via the GIT-VERSION file) changed. This was never\n> >> the intention, and was a regression on what we intended in\n> >> 7b8a74f39cb (Documentation: Replace @@GIT_VERSION@@ in documentation,\n> >> 2007-03-25).\n> >> \n> >> So let's add an ASCIIDOC_MANVERSION variable that we exclude from\n> >> ASCIIDOC_COMMON. The change in 9a71722b4df was only intending to catch\n> >> cases where we e.g. switched between asciidoc and asciidoctor, not to\n> >> undo the logic in 7b8a74f39cb and force a re-build every time our HEAD\n> >> changed in the repository.\n> >\n> > Once again, why do we care that the version is 2.32.0.98.gcfb60a24d6 and\n> > not 2.32.0.97.g949e814b27?\n> >\n> > Not just in the documentation, but everywhere.\n> \n> It's useful e.g. on my Debian system to see that the \"next\" Debian\n> packaged is 2.31.0.291.g576ba9dcdaf in docs & \"git version\", arguably\n> less so for documentation.\n\nObviously packagers would use real versions I meant this only for\ndevelopers (e.g. DEVELOPER=1).\n\nAnd BTW, where is that Debian package coming from? I thought\ndistributions didn't package git pre-releases.\n\n> > Maybe we can add a GIT_RELEASE variable that unlike GIT_VERSION it\n> > doesn't contain the precise commit. For example GIT_RELEASE = 2.33-dev.\n> \n> I just added this to my pre-make script:\n> \n> \tgrep -q ^/version .git/info/exclude || echo /version >>.git/info/exclude\n> \techo $(grep -o -P '(?<=^DEF_VER=v).*' GIT-VERSION-GEN)-dev >version\n> \n> It makes use of GIT-VERSION-GEN picking up a tarball \"version\" file.\n\nThis would also do the trick (you need DEVELOPER on the environment\nthough).\n\n--- a/GIT-VERSION-GEN\n+++ b/GIT-VERSION-GEN\n@@ -6,9 +6,10 @@ DEF_VER=v2.32.0\n LF='\n '\n \n-# First see if there is a version file (included in release tarballs),\n-# then try git-describe, then default.\n-if test -f version\n+if test -n \"$DEVELOPER\"\n+then\n+ VN=\"$DEF_VER\"-dev\n+elif test -f version\n then\n VN=$(cat version) || VN=\"$DEF_VER\"\n elif test -d ${GIT_DIR:-.git} -o -f .git &&\n\n\n-- \nFelipe Contreras"},{"id":"429254","messageId":"xmqqh7h8mtzh.fsf@gitster.g","threadId":"56000","inReplyTo":"87mtr38tvd.fsf@evledraar.gmail.com","subject":"Re: [PATCH] Documentation/Makefile: don't re-build on 'git version' changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-05T19:38:26Z","receivedAt":"2021-07-05T19:38:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> It's useful e.g. on my Debian system to see that the \"next\" Debian\n> packaged is 2.31.0.291.g576ba9dcdaf in docs & \"git version\", arguably\n> less so for documentation.\n\nIf it is arguable, perhaps make an argument that is more convincing?\n\nWhat I dislike the most is that in the sample scenario where master\nand next has the same documentation material to build \"git-cat-file.1\",\nthe installed result would be different depending on the order of\nbuilding the documentation, with the change being discussed, i.e.\n\n $ git checkout master && make prefix=$HOME/git-master install-doc\n $ git checkout next && make prefix=$HOME/git-next install-doc\n\nwould make \"~/git-next/bin/git help cat-file\" to claim the\ndocumentation is from the \"master\" version. Which is not all that\nbad, given that there wasn't anything that changed the documentation\nbetween 'master' and 'next'. But if you swap the installation\norder, \"~/git-master/bin/git help cat-file\" would say that the\ndocumentation is from a version much newer than 'master', which is\nnot quite acceptable.\n\nI am OK with the approach you hinted to have an option to _hide_ the\nversion string in the generated documentation (hence they lose their\ndependency on GIT-VERSION-FILE), while keeping the dependency of\nversion.o on GIT-VERSION-FILE, so that something goes wrong in a\nbuilt binary, the developer can still ask \"git version\" to identify\nwhere the binary came from.\n\nThanks.\n\n"},{"id":"429257","messageId":"xmqqczrwmtiq.fsf@gitster.g","threadId":"56000","inReplyTo":"87pmvz8v1u.fsf@evledraar.gmail.com","subject":"Re: [PATCH] Documentation/Makefile: don't re-build on 'git version' changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-05T19:48:29Z","receivedAt":"2021-07-05T19:48:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> I think nobody's doing such a \"chained\" build when building the docs for\n> a \"real\" release, and having mixed versions might be confusing, but for\n> the \"local build\" case from a development checkout it's arguably more\n> useful.\n\nWell, when I prepare a cascade of maintenance releases for CVE, what\ndo you think is done? Yes, I do have \"make distclean\" in the loop\nthat iterates over the maint-$version branches, just to be extra\ncareful, but I shouldn't have to.\n"},{"id":"429372","messageId":"60e4d85d94e80_1c42812084a@natae.notmuch","threadId":"56000","inReplyTo":"xmqqh7h8mtzh.fsf@gitster.g","subject":"Re: [PATCH] Documentation/Makefile: don't re-build on 'git version' changes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-07-06T22:25:33Z","receivedAt":"2021-07-06T22:25:37Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason writes:\n\n> I am OK with the approach you hinted to have an option to _hide_ the\n> version string in the generated documentation (hence they lose their\n> dependency on GIT-VERSION-FILE), while keeping the dependency of\n> version.o on GIT-VERSION-FILE, so that something goes wrong in a\n> built binary, the developer can still ask \"git version\" to identify\n> where the binary came from.\n\nBut it would be even better if the build system knew what was the intent\nof the build.\n\nThere's a difference when building a revision just to run `make test` on\nit, and building it to do `make install`. In the former there's no need\nfor a real version in version.o, only in the latter.\n\nMaybe RELASE=1. Not a release from the point of view of a maintainer,\nbut a release in the sense that the binaries will escape our in-tree\nbuild.\n\n-- \nFelipe Contreras"},{"id":"439123","messageId":"cover-0.8-00000000000-20211020T183533Z-avarab@gmail.com","threadId":"56000","inReplyTo":"YNqBtrXzUlJiuc7y@coredump.intra.peff.net","subject":"[PATCH 0/8] Makefile: make command-list.h 2-5x as fast with -jN","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-20T18:39:51Z","receivedAt":"2021-10-20T18:40:05Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This series is based off an off-hand comment I made about making the\ncmdlist.sh faster, in the meantime much of the same methods are\nalready cooking in \"next\" for the \"lint-docs\" target.\n\nSee 7/8 for the main performance numbers, along the way I stole some\npatches from Johannes Sixt who'd worked on optimizing the script\nbefore, which compliment this new method of generating this file by\npiggy-backing more on GNU make for managing a dependency graph for us.\n\n1. https://lore.kernel.org/git/87r1gqxqxn.fsf@evledraar.gmail.com/\n\nJohannes Sixt (2):\n generate-cmdlist.sh: spawn fewer processes\n generate-cmdlist.sh: replace for loop by printf's auto-repeat feature\n\nÆvar Arnfjörð Bjarmason (6):\n command-list.txt: sort with \"LC_ALL=C sort\"\n generate-cmdlist.sh: trivial whitespace change\n generate-cmdlist.sh: don't call get_categories() from category_list()\n generate-cmdlist.sh: run \"grep | sort\", not \"sort | grep\"\n Makefile: stop having command-list.h depend on a wildcard\n Makefile: assert correct generate-cmdlist.sh output\n\n .gitignore | 1 +\n Makefile | 57 ++++++++++++++++++++++++++++++++++++++++-----\n command-list.txt | 20 ++++++++--------\n generate-cmdlist.sh | 53 ++++++++++++++++++++++++++++-------------\n help.c | 3 ---\n 5 files changed, 99 insertions(+), 35 deletions(-)\n\n-- \n2.33.1.1338.g20da966911a\n\n"},{"id":"439124","messageId":"patch-1.8-96885282988-20211020T183533Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-0.8-00000000000-20211020T183533Z-avarab@gmail.com","subject":"[PATCH 1/8] command-list.txt: sort with \"LC_ALL=C sort\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-20T18:39:52Z","receivedAt":"2021-10-20T18:40:06Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"We should keep these files sorted in the C locale, e.g. in the C\nlocale the order is:\n\n git-check-mailmap\n git-check-ref-format\n git-checkout\n\nBut under en_US.UTF-8 it's:\n\n git-check-mailmap\n git-checkout\n git-check-ref-format\n\nIn a subsequent commit I'll change generate-cmdlist.sh to use C sort\norder, and without this change we'd be led to believe that that change\ncaused a meaningful change in the output, so let's do this as a\nseparate step, right now the generate-cmdlist.sh script just uses the\norder found in this file.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n command-list.txt | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/command-list.txt b/command-list.txt\nindex a289f09ed6f..02fc7ddde68 100644\n--- a/command-list.txt\n+++ b/command-list.txt\n@@ -60,9 +60,9 @@ git-cat-file plumbinginterrogators\n git-check-attr purehelpers\n git-check-ignore purehelpers\n git-check-mailmap purehelpers\n+git-check-ref-format purehelpers\n git-checkout mainporcelain\n git-checkout-index plumbingmanipulators\n-git-check-ref-format purehelpers\n git-cherry plumbinginterrogators complete\n git-cherry-pick mainporcelain\n git-citool mainporcelain\n@@ -111,7 +111,6 @@ git-index-pack plumbingmanipulators\n git-init mainporcelain init\n git-instaweb ancillaryinterrogators complete\n git-interpret-trailers purehelpers\n-gitk mainporcelain\n git-log mainporcelain info\n git-ls-files plumbinginterrogators\n git-ls-remote plumbinginterrogators\n@@ -124,11 +123,11 @@ git-merge-base plumbinginterrogators\n git-merge-file plumbingmanipulators\n git-merge-index plumbingmanipulators\n git-merge-one-file purehelpers\n-git-mergetool ancillarymanipulators complete\n git-merge-tree ancillaryinterrogators\n-git-multi-pack-index plumbingmanipulators\n+git-mergetool ancillarymanipulators complete\n git-mktag plumbingmanipulators\n git-mktree plumbingmanipulators\n+git-multi-pack-index plumbingmanipulators\n git-mv mainporcelain worktree\n git-name-rev plumbinginterrogators\n git-notes mainporcelain\n@@ -154,23 +153,23 @@ git-request-pull foreignscminterface complete\n git-rerere ancillaryinterrogators\n git-reset mainporcelain history\n git-restore mainporcelain worktree\n-git-revert mainporcelain\n git-rev-list plumbinginterrogators\n git-rev-parse plumbinginterrogators\n+git-revert mainporcelain\n git-rm mainporcelain worktree\n git-send-email foreignscminterface complete\n git-send-pack synchingrepositories\n+git-sh-i18n purehelpers\n+git-sh-setup purehelpers\n git-shell synchelpers\n git-shortlog mainporcelain\n git-show mainporcelain info\n git-show-branch ancillaryinterrogators complete\n git-show-index plumbinginterrogators\n git-show-ref plumbinginterrogators\n-git-sh-i18n purehelpers\n-git-sh-setup purehelpers\n git-sparse-checkout mainporcelain worktree\n-git-stash mainporcelain\n git-stage complete\n+git-stash mainporcelain\n git-status mainporcelain info\n git-stripspace purehelpers\n git-submodule mainporcelain\n@@ -189,10 +188,11 @@ git-var plumbinginterrogators\n git-verify-commit ancillaryinterrogators\n git-verify-pack plumbinginterrogators\n git-verify-tag ancillaryinterrogators\n-gitweb ancillaryinterrogators\n git-whatchanged ancillaryinterrogators complete\n git-worktree mainporcelain\n git-write-tree plumbingmanipulators\n+gitk mainporcelain\n+gitweb ancillaryinterrogators\n gitattributes guide\n gitcli guide\n gitcore-tutorial guide\n@@ -211,6 +211,6 @@ gitremote-helpers guide\n gitrepository-layout guide\n gitrevisions guide\n gitsubmodules guide\n-gittutorial-2 guide\n gittutorial guide\n+gittutorial-2 guide\n gitworkflows guide\n-- \n2.33.1.1338.g20da966911a\n\n"},{"id":"439125","messageId":"patch-2.8-5e8fef90e42-20211020T183533Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-0.8-00000000000-20211020T183533Z-avarab@gmail.com","subject":"[PATCH 2/8] generate-cmdlist.sh: trivial whitespace change","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-20T18:39:53Z","receivedAt":"2021-10-20T18:40:07Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This makes a subsequent diff smaller, and won't leave us with this\nsyntax nit at the end.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n generate-cmdlist.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex 9dbbb08e70a..5114f46680a 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -10,7 +10,7 @@ command_list () {\n }\n \n get_categories () {\n-\ttr ' ' '\\012'|\n+\ttr ' ' '\\012' |\n \tgrep -v '^$' |\n \tsort |\n \tuniq\n-- \n2.33.1.1338.g20da966911a\n\n"},{"id":"439126","messageId":"patch-3.8-6b4de6a6088-20211020T183533Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-0.8-00000000000-20211020T183533Z-avarab@gmail.com","subject":"[PATCH 3/8] generate-cmdlist.sh: spawn fewer processes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-20T18:39:54Z","receivedAt":"2021-10-20T18:40:09Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Johannes Sixt \n\nThe function get_categories() is invoked in a loop over all commands.\nAs it runs several processes, this takes an awful lot of time on\nWindows. To reduce the number of processes, move the process that\nfilters empty lines to the other invoker of the function, where it is\nneeded. The invocation of get_categories() in the loop does not need\nthe empty line filtered away because the result is word-split by the\nshell, which eliminates the empty line automatically.\n\nFurthermore, use sort -u instead of sort | uniq to remove yet another\nprocess.\n\n[Ævar: on Linux this seems to speed things up a bit, although with\nhyperfine(1) the results are fuzzy enough to land within the\nconfidence interval]:\n\n$ git show HEAD~:generate-cmdlist.sh >generate-cmdlist.sh.old\n$ hyperfine --warmup 1 -L s ,.old -p 'make clean' 'sh generate-cmdlist.sh{s} command-list.txt'\nBenchmark #1: sh generate-cmdlist.sh command-list.txt\n Time (mean ± σ): 371.3 ms ± 64.2 ms [User: 430.4 ms, System: 72.5 ms]\n Range (min … max): 320.5 ms … 517.7 ms 10 runs\n\nBenchmark #2: sh generate-cmdlist.sh.old command-list.txt\n Time (mean ± σ): 489.9 ms ± 185.4 ms [User: 724.7 ms, System: 141.3 ms]\n Range (min … max): 346.0 ms … 885.3 ms 10 runs\n\nSummary\n 'sh generate-cmdlist.sh command-list.txt' ran\n 1.32 ± 0.55 times faster than 'sh generate-cmdlist.sh.old command-list.txt'\n\nSigned-off-by: Johannes Sixt \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n generate-cmdlist.sh | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex 5114f46680a..27367915611 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -11,15 +11,14 @@ command_list () {\n \n get_categories () {\n \ttr ' ' '\\012' |\n-\tgrep -v '^$' |\n-\tsort |\n-\tuniq\n+\tLC_ALL=C sort -u\n }\n \n category_list () {\n \tcommand_list \"$1\" |\n \tcut -c 40- |\n-\tget_categories\n+\tget_categories |\n+\tgrep -v '^$'\n }\n \n get_synopsis () {\n-- \n2.33.1.1338.g20da966911a\n\n"},{"id":"439127","messageId":"patch-4.8-074685cf714-20211020T183533Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-0.8-00000000000-20211020T183533Z-avarab@gmail.com","subject":"[PATCH 4/8] generate-cmdlist.sh: don't call get_categories() from category_list()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-20T18:39:55Z","receivedAt":"2021-10-20T18:40:10Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This isn't for optimization as the get_categories() is a purely shell\nfunction, but rather for ease of readability, let's just inline these\ntwo lines. We'll be changing this code some more in subsequent commits\nto make this worth it.\n\nRename the get_categories() function to get_category_line(), since\nthat's what it's doing now.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n generate-cmdlist.sh | 7 ++++---\n 1 file changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex 27367915611..16043e38476 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -9,7 +9,7 @@ command_list () {\n \teval \"grep -ve '^#' $exclude_programs\" <\"$1\"\n }\n \n-get_categories () {\n+get_category_line () {\n \ttr ' ' '\\012' |\n \tLC_ALL=C sort -u\n }\n@@ -17,7 +17,8 @@ get_categories () {\n category_list () {\n \tcommand_list \"$1\" |\n \tcut -c 40- |\n-\tget_categories |\n+\ttr ' ' '\\012' |\n+\tLC_ALL=C sort -u |\n \tgrep -v '^$'\n }\n \n@@ -66,7 +67,7 @@ print_command_list () {\n \twhile read cmd rest\n \tdo\n \t\tprintf \"\t{ \\\"$cmd\\\", $(get_synopsis $cmd), 0\"\n-\t\tfor cat in $(echo \"$rest\" | get_categories)\n+\t\tfor cat in $(echo \"$rest\" | get_category_line)\n \t\tdo\n \t\t\tprintf \" | CAT_$cat\"\n \t\tdone\n-- \n2.33.1.1338.g20da966911a\n\n"},{"id":"439128","messageId":"patch-5.8-f01c1fd8088-20211020T183533Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-0.8-00000000000-20211020T183533Z-avarab@gmail.com","subject":"[PATCH 5/8] generate-cmdlist.sh: run \"grep | sort\", not \"sort | grep\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-20T18:39:56Z","receivedAt":"2021-10-20T18:40:11Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This doesn't matter for performance, but let's not include the empty\nlines in our sorting. This makes the intent of the code clearer.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n generate-cmdlist.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex 16043e38476..e517c33710a 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -18,8 +18,8 @@ category_list () {\n \tcommand_list \"$1\" |\n \tcut -c 40- |\n \ttr ' ' '\\012' |\n-\tLC_ALL=C sort -u |\n-\tgrep -v '^$'\n+\tgrep -v '^$' |\n+\tLC_ALL=C sort -u\n }\n \n get_synopsis () {\n-- \n2.33.1.1338.g20da966911a\n\n"},{"id":"439129","messageId":"patch-6.8-e0b11514b8d-20211020T183533Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-0.8-00000000000-20211020T183533Z-avarab@gmail.com","subject":"[PATCH 6/8] generate-cmdlist.sh: replace for loop by printf's auto-repeat feature","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-20T18:39:57Z","receivedAt":"2021-10-20T18:40:13Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Johannes Sixt \n\nThis is just a small code reduction. There is a small probability that\nthe new code breaks when the category list is empty. But that would be\nnoticed during the compile step.\n\nSigned-off-by: Johannes Sixt \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n generate-cmdlist.sh | 5 +----\n 1 file changed, 1 insertion(+), 4 deletions(-)\n\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex e517c33710a..a1ab2b1f077 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -67,10 +67,7 @@ print_command_list () {\n \twhile read cmd rest\n \tdo\n \t\tprintf \"\t{ \\\"$cmd\\\", $(get_synopsis $cmd), 0\"\n-\t\tfor cat in $(echo \"$rest\" | get_category_line)\n-\t\tdo\n-\t\t\tprintf \" | CAT_$cat\"\n-\t\tdone\n+\t\tprintf \" | CAT_%s\" $(echo \"$rest\" | get_category_line)\n \t\techo \" },\"\n \tdone\n \techo \"};\"\n-- \n2.33.1.1338.g20da966911a\n\n"},{"id":"439130","messageId":"patch-7.8-0c6f9b80d3b-20211020T183533Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-0.8-00000000000-20211020T183533Z-avarab@gmail.com","subject":"[PATCH 7/8] Makefile: stop having command-list.h depend on a wildcard","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-20T18:39:58Z","receivedAt":"2021-10-20T18:40:17Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the dependency graph for \"command-list.h\" so that it's not\nre-built in its entirety every time one file in Documentation/git*.txt\nchanges. This makes the generation of the command-list.h use a pattern\nsimilar to that established in 8650c6298c1 (doc lint: make \"lint-docs\"\nnon-.PHONY, 2021-10-15) for the \"lint-docs\" target.\n\nThis change replaces the monolithic \"generate-cmdlist.sh\" invocation\nwith a dependency chain like:\n\n Documentation/git-add.txt ->\n .build/command-list.h.d/git-add.gen (and \"git-add.txt\" intermediary) ->\n command-list.h\n\nI.e. we'll now generate intermediary files when extracting the \"NAME\"\nsection the documentation. Here the 6th line of\n\"Documentation/git.txt\" will be extracted to\n\".build/command-list.h.d/git-add.gen.txt\":\n\n git-add - Add file contents to the index\n\nWe'll then create an intermediate\n\".build/command-list.h.d/git-add.gen\" (leading \"\\t\" stripped):\n\n ./generate-cmdlist.sh --entry-only .build/command-list.h.d/git-add.gen.txt\n { \"git-add\", N_(\"Add file contents to the index\"), 0 | CAT_mainporcelain | CAT_worktree },\n\nThe \"command-list.h\" itself can the be made by having\n\"generate-cmdlist.sh\" emit only the header section, followed by:\n\n LC_ALL=sort .build/command-list.h.d/*.gen\n\nUnfortunately we can't drop the old code completely due to the CMake\nintegration, see 061c2240b1b (Introduce CMake support for configuring\nGit, 2020-06-12). It will keep using the older and slower script.\n\nWith this the initial creation of the command-list.h is a bit slower\nwith -j1, but around 2x as fast -j8[1]. The real benefit comes from\nthe more common case of an incremental build, say when only\n\"Documentation/git-add.txt\" was updated after a \"pull\". There we're\n4-5x as fast with this new method[2].\n\nThe benefit of optimizing this is because this file is very frequently\nre-generated, e.g. for \"git rebase -i --exec 'make git'\" with\n\"git-add.txt\" modified we're around 1.7 times as fast[3]. That target\nwill need to re-make \"git\" (via \"help.o\") due to help.c's use of\ncommand-list.h.\n\nIn terms of implementation: I'm adding $(QUIET) while I'm at it, which\nis here so we don't quiet the equivalent of trace output under V=1,\nthis could be used in other places that use \"@cmd\" to quiet \"cmd\"\noutput.\n\nSome of the dependencies between \"command-list.h\" and\n\"$(COMMAND_LIST_GEN)\" are redundant, considering that the former\ndepends on the latter. I'm sticking to listing dependencies mentioned\nin the rule itself, e.g. \"command-list.h\" itself calls\n\"generate-cmdlist.sh\", so I list the dependency even though it would\nget it via a recursive dependency.\n\nThese rules can be lazy about leaving behind files on error thanks to\nthe .DELETE_ON_ERROR flag, see 7b76d6bf221 (Makefile: add and use the\n\".DELETE_ON_ERROR\" flag, 2021-06-29), or in the case of the\n\"*.txt.gen\" files because we'll unconditionally clobber them anyway if\nthe relevant source file is touched, so we can leave them for \"make\nclean\".\n\n1. Piped through: 2>/dev/null | grep -P -v \"^\\s*$\"; ditto below:\n hyperfine -s basic -L j \", -j8\" -L s ,.old -p 'rm -rf .build command-list.h' 'make{j} -f Makefile{s} command-list.h'\n Benchmark #1: make -f Makefile command-list.h\n Time (mean ± σ): 769.3 ms ± 90.4 ms [User: 892.4 ms, System: 98.8 ms]\n Range (min … max): 665.6 ms … 941.9 ms 10 runs\n Benchmark #2: make -j8 -f Makefile command-list.h\n Time (mean ± σ): 212.5 ms ± 45.0 ms [User: 954.4 ms, System: 136.9 ms]\n Range (min … max): 187.6 ms … 326.2 ms 11 runs\n Benchmark #3: make -f Makefile.old command-list.h\n Time (mean ± σ): 515.0 ms ± 70.5 ms [User: 613.8 ms, System: 110.9 ms]\n Range (min … max): 407.6 ms … 603.9 ms 10 runs\n Benchmark #4: make -j8 -f Makefile.old command-list.h\n Time (mean ± σ): 474.9 ms ± 62.7 ms [User: 569.7 ms, System: 106.0 ms]\n Range (min … max): 407.0 ms … 556.8 ms 10 runs\n Summary\n 'make -j8 -f Makefile command-list.h' ran\n 2.24 ± 0.56 times faster than 'make -j8 -f Makefile.old command-list.h'\n 2.42 ± 0.61 times faster than 'make -f Makefile.old command-list.h'\n 3.62 ± 0.88 times faster than 'make -f Makefile command-list.h'\n2. hyperfine -s basic -L j \", -j8\" -L s ,.old -p 'touch Documentation/git-add.txt' 'make{j} -f Makefile{s} command-list.h'\n Benchmark #1: make -f Makefile command-list.h\n Time (mean ± σ): 94.6 ms ± 26.7 ms [User: 83.0 ms, System: 14.1 ms]\n Range (min … max): 79.4 ms … 186.0 ms 15 runs\n Benchmark #2: make -j8 -f Makefile command-list.h\n Time (mean ± σ): 80.3 ms ± 10.5 ms [User: 81.3 ms, System: 12.9 ms]\n Range (min … max): 76.9 ms … 127.6 ms 37 runs\n Benchmark #3: make -f Makefile.old command-list.h\n Time (mean ± σ): 348.9 ms ± 174.5 ms [User: 415.3 ms, System: 70.6 ms]\n Range (min … max): 66.0 ms … 550.3 ms 43 runs\n Benchmark #4: make -j8 -f Makefile.old command-list.h\n Time (mean ± σ): 406.7 ms ± 157.1 ms [User: 473.4 ms, System: 78.3 ms]\n Range (min … max): 67.4 ms … 560.4 ms 34 runs\n Summary\n 'make -j8 -f Makefile command-list.h' ran\n 1.18 ± 0.37 times faster than 'make -f Makefile command-list.h'\n 4.34 ± 2.25 times faster than 'make -f Makefile.old command-list.h'\n 5.07 ± 2.07 times faster than 'make -j8 -f Makefile.old command-list.h'\n3. hyperfine -s basic -L j \" -j8\" -L s ,.old -p 'touch Documentation/git-add.txt' 'make{j} -f Makefile{s} git'\n Benchmark #1: make -j8 -f Makefile git\n Time (mean ± σ): 484.2 ms ± 53.5 ms [User: 253.2 ms, System: 83.5 ms]\n Range (min … max): 400.7 ms … 580.5 ms 10 runs\n Benchmark #2: make -j8 -f Makefile.old git\n Time (mean ± σ): 836.7 ms ± 37.3 ms [User: 729.2 ms, System: 157.1 ms]\n Range (min … max): 774.2 ms … 885.6 ms 10 runs\n Summary\n 'make -j8 -f Makefile git' ran\n 1.73 ± 0.21 times faster than 'make -j8 -f Makefile.old git'\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n .gitignore | 1 +\n Makefile | 52 +++++++++++++++++++++++++++++++++++++++------\n generate-cmdlist.sh | 36 +++++++++++++++++++++++++------\n 3 files changed, 77 insertions(+), 12 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex 054249b20a8..dc03a988b16 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -198,6 +198,7 @@\n *.[aos]\n *.o.json\n *.py[co]\n+.build/\n .depend/\n *.gcda\n *.gcno\ndiff --git a/Makefile b/Makefile\nindex 381bed2c1d2..ce4cce57eb8 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1954,6 +1954,7 @@ endif\n \n ifneq ($(findstring s,$(MAKEFLAGS)),s)\n ifndef V\n+\tQUIET = @\n \tQUIET_CC = @echo ' ' CC $@;\n \tQUIET_AR = @echo ' ' AR $@;\n \tQUIET_LINK = @echo ' ' LINK $@;\n@@ -2242,12 +2243,50 @@ config-list.h: generate-configlist.sh\n config-list.h: Documentation/*config.txt Documentation/config/*.txt\n \t$(QUIET_GEN)$(SHELL_PATH) ./generate-configlist.sh >$@\n \n-command-list.h: generate-cmdlist.sh command-list.txt\n-\n-command-list.h: $(wildcard Documentation/git*.txt)\n-\t$(QUIET_GEN)$(SHELL_PATH) ./generate-cmdlist.sh \\\n-\t\t$(patsubst %,--exclude-program %,$(EXCLUDED_PROGRAMS)) \\\n-\t\tcommand-list.txt >$@\n+EXCLUDED_COMMAND_LIST =\n+EXCLUDED_COMMAND_LIST += git\n+EXCLUDED_COMMAND_LIST += git-bisect-lk2009\n+EXCLUDED_COMMAND_LIST += git-credential-cache--daemon\n+EXCLUDED_COMMAND_LIST += git-fsck-objects\n+EXCLUDED_COMMAND_LIST += git-init-db\n+EXCLUDED_COMMAND_LIST += git-mergetool--lib\n+EXCLUDED_COMMAND_LIST += git-remote-ext\n+EXCLUDED_COMMAND_LIST += git-remote-fd\n+EXCLUDED_COMMAND_LIST += git-sh-i18n--envsubst\n+EXCLUDED_COMMAND_LIST += git-submodules\n+EXCLUDED_COMMAND_LIST += git-tools\n+EXCLUDED_COMMAND_LIST += git-version\n+EXCLUDED_COMMAND_LIST += git-web--browse\n+EXCLUDED_COMMAND_LIST += gitweb.conf\n+\n+EXCLUDED_TXT += $(patsubst %,Documentation/%.txt,$(EXCLUDED_PROGRAMS) $(EXCLUDED_COMMAND_LIST))\n+COMMAND_LIST_TXT_DEP = $(filter-out $(EXCLUDED_TXT), $(wildcard Documentation/git*.txt))\n+\n+COMMAND_LIST_GEN = $(patsubst Documentation/%.txt,.build/command-list.h.d/%.gen,$(COMMAND_LIST_TXT_DEP))\n+\n+.build:\n+\t$(QUIET)mkdir .build\n+.build/command-list.h.d: | .build\n+\t$(QUIET)mkdir -p .build/command-list.h.d\n+\n+# We must depend on .build/command-list.h as an \"order-only\"\n+# prerequisite, its mtime will change when these targets run.\n+$(COMMAND_LIST_GEN): | .build/command-list.h.d\n+$(COMMAND_LIST_GEN): command-list.txt\n+$(COMMAND_LIST_GEN): generate-cmdlist.sh\n+$(COMMAND_LIST_GEN): .build/command-list.h.d/%.gen: Documentation/%.txt\n+\t$(QUIET)grep \"^$(patsubst .build/command-list.h.d/%.gen,%,$@) \" command-list.txt >$@.txt && \\\n+\t./generate-cmdlist.sh --entry-only $@.txt >$@\n+\n+command-list.h: $(COMMAND_LIST_GEN)\n+command-list.h: generate-cmdlist.sh\n+command-list.h: command-list.txt\n+\t$(QUIET_GEN){ \\\n+\t\t$(SHELL_PATH) ./generate-cmdlist.sh --header-only command-list.txt && \\\n+\t\techo \"static struct cmdname_help command_list[] = {\" && \\\n+\t\tLC_ALL=C sort $(COMMAND_LIST_GEN) && \\\n+\t\techo \"};\"; \\\n+\t} >$@\n \n hook-list.h: generate-hooklist.sh Documentation/githooks.txt\n \t$(QUIET_GEN)$(SHELL_PATH) ./generate-hooklist.sh >$@\n@@ -3238,6 +3277,7 @@ clean: profile-clean coverage-clean cocciclean\n \t$(RM) $(SP_OBJ)\n \t$(RM) $(HCC)\n \t$(RM) -r bin-wrappers $(dep_dirs) $(compdb_dir) compile_commands.json\n+\t$(RM) -r .build/\n \t$(RM) -r po/build/\n \t$(RM) *.pyc *.pyo */*.pyc */*.pyo $(GENERATED_H) $(ETAGS_TARGET) tags cscope*\n \t$(RM) -r .dist-tmp-dir .doc-tmp-dir\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex a1ab2b1f077..2bc528e8cae 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -15,7 +15,7 @@ get_category_line () {\n }\n \n category_list () {\n-\tcommand_list \"$1\" |\n+\tgrep -v '^#' \"$1\" |\n \tcut -c 40- |\n \ttr ' ' '\\012' |\n \tgrep -v '^$' |\n@@ -23,13 +23,14 @@ category_list () {\n }\n \n get_synopsis () {\n+\thead -n 10 \"Documentation/$1.txt\" |\n \tsed -n '\n \t\t/^NAME/,/'\"$1\"'/H\n \t\t${\n \t\t\tx\n \t\t\ts/.*'\"$1\"' - \\(.*\\)/N_(\"\\1\")/\n \t\t\tp\n-\t\t}' \"Documentation/$1.txt\"\n+\t\t}'\n }\n \n define_categories () {\n@@ -61,16 +62,12 @@ define_category_names () {\n }\n \n print_command_list () {\n-\techo \"static struct cmdname_help command_list[] = {\"\n-\n-\tcommand_list \"$1\" |\n \twhile read cmd rest\n \tdo\n \t\tprintf \"\t{ \\\"$cmd\\\", $(get_synopsis $cmd), 0\"\n \t\tprintf \" | CAT_%s\" $(echo \"$rest\" | get_category_line)\n \t\techo \" },\"\n \tdone\n-\techo \"};\"\n }\n \n exclude_programs=\n@@ -81,6 +78,19 @@ do\n \tshift\n done\n \n+header_only=\n+case $1 in\n+--entry-only)\n+\tshift\n+\tprint_command_list $1 <\"$1\"\n+\texit 0\n+\t;;\n+--header-only)\n+\tshift\n+\theader_only=t\n+\t;;\n+esac\n+\n echo \"/* Automatically generated by generate-cmdlist.sh */\n struct cmdname_help {\n \tconst char *name;\n@@ -92,4 +102,18 @@ define_categories \"$1\"\n echo\n define_category_names \"$1\"\n echo\n+\n+if test -n \"$header_only\"\n+then\n+\texit 0\n+fi\n+\n+# The old compatibility mode for CMmake. See 061c2240b1b (Introduce\n+# CMake support for configuring Git, 2020-06-12)\n+echo \"static struct cmdname_help command_list[] = {\"\n+grep -v \\\n+\t-e '^#' \\\n+\t-e '^git-fsck-objects ' \\\n+\t\"$1\" |\n print_command_list \"$1\"\n+echo \"};\"\n-- \n2.33.1.1338.g20da966911a\n\n"},{"id":"439131","messageId":"patch-8.8-23d4cc77b6c-20211020T183533Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-0.8-00000000000-20211020T183533Z-avarab@gmail.com","subject":"[PATCH 8/8] Makefile: assert correct generate-cmdlist.sh output","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-20T18:39:59Z","receivedAt":"2021-10-20T18:40:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Because generate-cmdlist.sh invokes \"sed\" to extract the \"NAME\" blurb\nfrom the Documentation/git-*.txt files, we can end up with bad content\nif those files aren't what we expected. E.g. we'll emit multiple lines\nfrom that \"sed\" one-liner if the \"NAME\" section were to move further\ndown than what our \"head -n 10\" covers.\n\nLet's assert that this can't happen by checking that the number of\nlines we make is what we'd expect to get. This means we can remove an\nassertion added in cfb22a02ab5 (help: use command-list.h for common\ncommand list, 2018-05-10). We'll catch this during compilation\ninstead.\n\nThere are still other cases where it's possible to generate a bad\ncommand-list.h, but the rest should be caught by a C compilation\nerror.\n\nIt would be possible to change our generated \"command-list.h\" to\nsimply include this new \".build/command-list.h.gen\" instead of cat-ing\nit in the Makefile, but let's leave the generated file as it is.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 7 ++++++-\n generate-cmdlist.sh | 2 +-\n help.c | 3 ---\n 3 files changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex ce4cce57eb8..711f0c064e6 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2278,13 +2278,18 @@ $(COMMAND_LIST_GEN): .build/command-list.h.d/%.gen: Documentation/%.txt\n \t$(QUIET)grep \"^$(patsubst .build/command-list.h.d/%.gen,%,$@) \" command-list.txt >$@.txt && \\\n \t./generate-cmdlist.sh --entry-only $@.txt >$@\n \n+.build/command-list.h.gen: $(COMMAND_LIST_GEN)\n+\t$(QUIET)LC_ALL=C sort $(COMMAND_LIST_GEN) >$@ && \\\n+\ttest $$(wc -l <$@) -eq $(words $(COMMAND_LIST_GEN))\n+\n command-list.h: $(COMMAND_LIST_GEN)\n command-list.h: generate-cmdlist.sh\n command-list.h: command-list.txt\n+command-list.h: .build/command-list.h.gen\n \t$(QUIET_GEN){ \\\n \t\t$(SHELL_PATH) ./generate-cmdlist.sh --header-only command-list.txt && \\\n \t\techo \"static struct cmdname_help command_list[] = {\" && \\\n-\t\tLC_ALL=C sort $(COMMAND_LIST_GEN) && \\\n+\t\tcat $< && \\\n \t\techo \"};\"; \\\n \t} >$@\n \ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex 2bc528e8cae..bdefa151ae1 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -23,7 +23,7 @@ category_list () {\n }\n \n get_synopsis () {\n-\thead -n 10 \"Documentation/$1.txt\" |\n+\thead -n 6 \"Documentation/$1.txt\" |\n \tsed -n '\n \t\t/^NAME/,/'\"$1\"'/H\n \t\t${\ndiff --git a/help.c b/help.c\nindex 973e47cdc30..cd6dd4c2440 100644\n--- a/help.c\n+++ b/help.c\n@@ -57,9 +57,6 @@ static void extract_cmds(struct cmdname_help **p_cmds, uint32_t mask)\n \tint i, nr = 0;\n \tstruct cmdname_help *cmds;\n \n-\tif (ARRAY_SIZE(command_list) == 0)\n-\t\tBUG(\"empty command_list[] is a sign of broken generate-cmdlist.sh\");\n-\n \tALLOC_ARRAY(cmds, ARRAY_SIZE(command_list) + 1);\n \n \tfor (i = 0; i < ARRAY_SIZE(command_list); i++) {\n-- \n2.33.1.1338.g20da966911a\n\n"},{"id":"439137","messageId":"YXB9moNHRj+nrnX9@coredump.intra.peff.net","threadId":"56000","inReplyTo":"cover-0.8-00000000000-20211020T183533Z-avarab@gmail.com","subject":"Re: [PATCH 0/8] Makefile: make command-list.h 2-5x as fast with -jN","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-20T20:35:38Z","receivedAt":"2021-10-20T20:35:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 20, 2021 at 08:39:51PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> This series is based off an off-hand comment I made about making the\n> cmdlist.sh faster, in the meantime much of the same methods are\n> already cooking in \"next\" for the \"lint-docs\" target.\n> \n> See 7/8 for the main performance numbers, along the way I stole some\n> patches from Johannes Sixt who'd worked on optimizing the script\n> before, which compliment this new method of generating this file by\n> piggy-backing more on GNU make for managing a dependency graph for us.\n\nI still think this is a much more complicated and error-prone approach\nthan just making the script faster. I know we can't rely on perl, but\ncould we use it optimistically?\n\nThe proof-of-concept below on top of your patch 6 does two things:\n\n - observes that there is no need for get_category_line in the loop; it\n is just sorting and de-duping the bitfields, but since we just OR\n them together, neither of those things matters\n\n - uses perl to open each individual doc file to get the synopsis. It\n _feels_ like this should be something that sed or awk could do, but\n it is beyond me. However, speculatively trying perl is an easy win,\n and we can fall back to the shell loop.\n\nHere are my timings:\n\nBenchmark #1: sh generate-cmdlist.sh command-list.txt\n Time (mean ± σ): 40.4 ms ± 18.1 ms [User: 44.9 ms, System: 7.1 ms]\n Range (min … max): 20.3 ms … 65.5 ms 10 runs\n\nBenchmark #2: sh generate-cmdlist.sh.old command-list.txt\n Time (mean ± σ): 1.414 s ± 0.038 s [User: 1.641 s, System: 0.863 s]\n Range (min … max): 1.344 s … 1.451 s 10 runs\n\nSummary\n 'sh generate-cmdlist.sh command-list.txt' ran\n 34.96 ± 15.66 times faster than 'sh generate-cmdlist.sh.old command-list.txt'\n\nI hate having fallbacks, because the seldom-used version may bitrot. I'm\ntempted to just write that loop in C, but there's a circular dependency\nwith using any of libgit.a (even though it's really only the git\nporcelain that cares about command-list.h, it goes into help.o which\ngoes into libgit.a. We could break that dependency if we wanted,\nthough). If we can do it in awk, that may be worthwhile.\n\nBut either way, I think this is superior to trying to parallelize the\nMakefile:\n\n - it actually uses less CPU, rather than just trying to improve\n wall-clock time by using more cores\n\n - there's little chance of having some subtle dependency problem\n\nParallelizing makes a lot of sense to me when the operation is truly\nexpensive. But in this case it's literally just opening a file, and the\nonly reason it's slow is because we spawn a ton of processes to do it.\n\n---\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex a1ab2b1f07..f922eebe23 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -63,11 +63,23 @@ define_category_names () {\n print_command_list () {\n \techo \"static struct cmdname_help command_list[] = {\"\n \n+\t# try perl first, as we can do it all in one process\n+\tcommand_list \"$1\" |\n+\tperl -ne '\n+\t\tmy ($cmd, @rest) = split;\n+\t\topen(my $fh, \"<\", \"Documentation/$cmd.txt\");\n+\t\twhile (<$fh>) {\n+\t\t\tnext unless /^$cmd - (.*)/;\n+\t\t\tprint \"{ \\\"$cmd\\\", N_(\\\"$1\\\"), 0\";\n+\t\t\tprint \" | CAT_$_\" for (@rest);\n+\t\t\tprint \" },\\n\";\n+\t\t}\n+\t' ||\n \tcommand_list \"$1\" |\n \twhile read cmd rest\n \tdo\n \t\tprintf \"\t{ \\\"$cmd\\\", $(get_synopsis $cmd), 0\"\n-\t\tprintf \" | CAT_%s\" $(echo \"$rest\" | get_category_line)\n+\t\tprintf \" | CAT_%s\" $rest\n \t\techo \" },\"\n \tdone\n \techo \"};\"\n"},{"id":"439142","messageId":"YXCKqAEwtwFozWk6@nand.local","threadId":"56000","inReplyTo":"YXB9moNHRj+nrnX9@coredump.intra.peff.net","subject":"Re: [PATCH 0/8] Makefile: make command-list.h 2-5x as fast with -jN","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-10-20T21:31:20Z","receivedAt":"2021-10-20T21:31:27Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Oct 20, 2021 at 04:35:38PM -0400, Jeff King wrote:\n> On Wed, Oct 20, 2021 at 08:39:51PM +0200, Ævar Arnfjörð Bjarmason wrote:\n>\n> > This series is based off an off-hand comment I made about making the\n> > cmdlist.sh faster, in the meantime much of the same methods are\n> > already cooking in \"next\" for the \"lint-docs\" target.\n> >\n> > See 7/8 for the main performance numbers, along the way I stole some\n> > patches from Johannes Sixt who'd worked on optimizing the script\n> > before, which compliment this new method of generating this file by\n> > piggy-backing more on GNU make for managing a dependency graph for us.\n>\n> I still think this is a much more complicated and error-prone approach\n> than just making the script faster. I know we can't rely on perl, but\n> could we use it optimistically?\n\nI'll take credit for this terrible idea of using Perl when available.\n\nBut I don't think we even need to, since we could just rely on Awk. That\nhas all the benefits you described while still avoiding the circular\ndependency on libgit.a. But the killer feature is that we don't have to\nrely on two implementations, the lesser-used of which is likely to\nbitrot over time.\n\nThe resulting awk is a little ugly, because of the nested-ness. I'm also\nno awk-spert, but I think that something like the below gets the job\ndone.\n\nIt also has the benefit of being slightly faster than the equivalent\nPerl implementation, for whatever those extra ~9 ms are worth ;).\n\nBenchmark #1: sh generate-cmdlist.sh command-list.txt\n Time (mean ± σ): 25.3 ms ± 5.3 ms [User: 31.1 ms, System: 8.3 ms]\n Range (min … max): 15.5 ms … 31.7 ms 95 runs\n\nBenchmark #2: sh generate-cmdlist.sh.old command-list.txt\n Time (mean ± σ): 34.9 ms ± 9.8 ms [User: 41.0 ms, System: 6.9 ms]\n Range (min … max): 22.4 ms … 54.8 ms 64 runs\n\nSummary\n 'sh generate-cmdlist.sh command-list.txt' ran\n 1.38 ± 0.49 times faster than 'sh generate-cmdlist.sh.old command-list.txt'\n\n---\n\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex a1ab2b1f07..39338ef1cc 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -64,12 +64,19 @@ print_command_list () {\n \techo \"static struct cmdname_help command_list[] = {\"\n\n \tcommand_list \"$1\" |\n-\twhile read cmd rest\n-\tdo\n-\t\tprintf \"\t{ \\\"$cmd\\\", $(get_synopsis $cmd), 0\"\n-\t\tprintf \" | CAT_%s\" $(echo \"$rest\" | get_category_line)\n-\t\techo \" },\"\n-\tdone\n+\tawk '{\n+\t\tf=\"Documentation/\" $1 \".txt\"\n+\t\twhile((getline line 0) {\n+\t\t\tif (match(line, \"^\" $1 \" - \")) {\n+\t\t\t\tsyn=substr(line, RLENGTH+1)\n+\t\t\t\tprintf \"\\t{ \\\"%s\\\", N_(\\\"%s\\\"), 0\", $1, syn\n+\t\t\t\tfor (i=2; i<=NF; i++) {\n+\t\t\t\t\tprintf \" | CAT_%s\", $i\n+\t\t\t\t}\n+\t\t\t\tprint \" },\"\n+\t\t\t}\n+\t\t}\n+\t}'\n \techo \"};\"\n }\n"},{"id":"439151","messageId":"211021.86v91rmftn.gmgdl@evledraar.gmail.com","threadId":"56000","inReplyTo":"YXCKqAEwtwFozWk6@nand.local","subject":"Re: [PATCH 0/8] Makefile: make command-list.h 2-5x as fast with -jN","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-20T23:14:37Z","receivedAt":"2021-10-20T23:34:02Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Oct 20 2021, Taylor Blau wrote:\n\n> On Wed, Oct 20, 2021 at 04:35:38PM -0400, Jeff King wrote:\n>> On Wed, Oct 20, 2021 at 08:39:51PM +0200, Ævar Arnfjörð Bjarmason wrote:\n>>\n>> > This series is based off an off-hand comment I made about making the\n>> > cmdlist.sh faster, in the meantime much of the same methods are\n>> > already cooking in \"next\" for the \"lint-docs\" target.\n>> >\n>> > See 7/8 for the main performance numbers, along the way I stole some\n>> > patches from Johannes Sixt who'd worked on optimizing the script\n>> > before, which compliment this new method of generating this file by\n>> > piggy-backing more on GNU make for managing a dependency graph for us.\n>>\n>> I still think this is a much more complicated and error-prone approach\n>> than just making the script faster. I know we can't rely on perl, but\n>> could we use it optimistically?\n\nJeff: Just in terms of error prone both of these implementations will\naccept bad input that's being caught in 8/8 of this series.\n\nWe accept a lot of bad input now, ending up with some combinations of\nbad output or compile errors if you screw with the input *.txt files. I\nthink I've addressed all of those in this series.\n\nIf you mean the general concept of making a \"foo.gen\" from a \"foo.txt\"\nas an intermediate with make as a way to get to \"many-foo.h\" I don't\nreally see how it's error prone conceptually. You get error checking\neach step of the way, and it encourages logic that's simpler each step\nof the way.\n\n> I'll take credit for this terrible idea of using Perl when available.\n>\n> But I don't think we even need to, since we could just rely on Awk. That\n> has all the benefits you described while still avoiding the circular\n> dependency on libgit.a. But the killer feature is that we don't have to\n> rely on two implementations, the lesser-used of which is likely to\n> bitrot over time.\n>\n> The resulting awk is a little ugly, because of the nested-ness. I'm also\n> no awk-spert, but I think that something like the below gets the job\n> done.\n>\n> It also has the benefit of being slightly faster than the equivalent\n> Perl implementation, for whatever those extra ~9 ms are worth ;).\n>\n> Benchmark #1: sh generate-cmdlist.sh command-list.txt\n> Time (mean ± σ): 25.3 ms ± 5.3 ms [User: 31.1 ms, System: 8.3 ms]\n> Range (min … max): 15.5 ms … 31.7 ms 95 runs\n>\n> Benchmark #2: sh generate-cmdlist.sh.old command-list.txt\n> Time (mean ± σ): 34.9 ms ± 9.8 ms [User: 41.0 ms, System: 6.9 ms]\n> Range (min … max): 22.4 ms … 54.8 ms 64 runs\n>\n> Summary\n> 'sh generate-cmdlist.sh command-list.txt' ran\n> 1.38 ± 0.49 times faster than 'sh generate-cmdlist.sh.old command-list.txt'\n>\n> ---\n>\n> diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\n> index a1ab2b1f07..39338ef1cc 100755\n> --- a/generate-cmdlist.sh\n> +++ b/generate-cmdlist.sh\n> @@ -64,12 +64,19 @@ print_command_list () {\n> \techo \"static struct cmdname_help command_list[] = {\"\n>\n> \tcommand_list \"$1\" |\n> -\twhile read cmd rest\n> -\tdo\n> -\t\tprintf \"\t{ \\\"$cmd\\\", $(get_synopsis $cmd), 0\"\n> -\t\tprintf \" | CAT_%s\" $(echo \"$rest\" | get_category_line)\n> -\t\techo \" },\"\n> -\tdone\n> +\tawk '{\n> +\t\tf=\"Documentation/\" $1 \".txt\"\n> +\t\twhile((getline line 0) {\n> +\t\t\tif (match(line, \"^\" $1 \" - \")) {\n> +\t\t\t\tsyn=substr(line, RLENGTH+1)\n> +\t\t\t\tprintf \"\\t{ \\\"%s\\\", N_(\\\"%s\\\"), 0\", $1, syn\n> +\t\t\t\tfor (i=2; i<=NF; i++) {\n> +\t\t\t\t\tprintf \" | CAT_%s\", $i\n> +\t\t\t\t}\n> +\t\t\t\tprint \" },\"\n> +\t\t\t}\n> +\t\t}\n> +\t}'\n> \techo \"};\"\n> }\n\nPer Eric's Sunshine's upthread comments an awk and Perl implementation\nwere both considered before[1].\n\nI also care a bit about the timings of the from-scratch build, but I\nthink they're way less interesting than a partial build.\n\nI.e. I think if you e.g. touch Documentation/git-a*.txt with this series\nwith/without this awk version the difference in runtime is within the\nerror bars. I.e. making the loop faster isn't necessary. It's better to\nget to a point where make can save you from doing all/most of the work\nby checking modification times, rather than making an O(n) loop faster.\n\nThe only reason there's even a loop there is because it's used by the\ncmake logic in contrib/* (how we've ended up with a hard dependency in\ncontrib is another matter...).\n\nI'm also interested in (and have WIP patches for) simplifying things\nmore generally in the Makefile. Once we have a file exploded out has\njust the synopsis line that can be used to replace what's now in\nDocumentation/cmd-list.perl, i.e. those summary blurbs also end up in\n\"man git\".\n\nThere's subtle dependency issues there as well, and just having a\none-off solution for the the command-list.h doesn't get us closer to\naddressing that sibling implementation.\n\nIn terms of future Makefile work I was hoping to get this in, untangle\nsome of the complexity between the inter-dependency of Makefile &\nDocumentation/Makefile (eventually just merging the two, and leaving a\nstub in Documentation/Makefile). I've also got a working implementation\nfor getting rid of all of the \"FORCE\" dependencies (except the version\none).\n\n1. https://lore.kernel.org/git/CAPig+cSzKoOzU-zPOZqfNpPYBFpcWqvDP3mwLvAn5WkiNW0UMw@mail.gmail.com/\n"},{"id":"439153","messageId":"YXCqO7WFET6J7gGf@coredump.intra.peff.net","threadId":"56000","inReplyTo":"211021.86v91rmftn.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 0/8] Makefile: make command-list.h 2-5x as fast with -jN","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-20T23:46:45Z","receivedAt":"2021-10-20T23:46:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 21, 2021 at 01:14:37AM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> Jeff: Just in terms of error prone both of these implementations will\n> accept bad input that's being caught in 8/8 of this series.\n> \n> We accept a lot of bad input now, ending up with some combinations of\n> bad output or compile errors if you screw with the input *.txt files. I\n> think I've addressed all of those in this series.\n\nI don't mind more error-checking, though TBH I don't find a huge value\nin it. But what I did mean was:\n\n> If you mean the general concept of making a \"foo.gen\" from a \"foo.txt\"\n> as an intermediate with make as a way to get to \"many-foo.h\" I don't\n> really see how it's error prone conceptually. You get error checking\n> each step of the way, and it encourages logic that's simpler each step\n> of the way.\n\nYes. It just seems like the Makefile gets more complicated, and\nsometimes that can lead to subtle dependency issues (e.g., the \".build\"\ndependency in the earlier iteration of the series).\n\nAnd in general I'd much rather debug an awk script than a Makefile.\n\n> Per Eric's Sunshine's upthread comments an awk and Perl implementation\n> were both considered before[1].\n\nAh sorry, I thought it was just a perl one that had been the\nshow-stopper. I hadn't noticed the awk one. However, the point of my\npatch was to use perl if available, and fall back otherwise. Maybe\nthat's too ugly, but it does address the concern with Eric's\nimplementation.\n\n> I.e. I think if you e.g. touch Documentation/git-a*.txt with this series\n> with/without this awk version the difference in runtime is within the\n> error bars. I.e. making the loop faster isn't necessary. It's better to\n> get to a point where make can save you from doing all/most of the work\n> by checking modification times, rather than making an O(n) loop faster.\n\nFWIW, I don't agree with this paragraph at all. Parallelizing or reusing\npartial results is IMHO inferior to just making things faster.\n\n> I'm also interested in (and have WIP patches for) simplifying things\n> more generally in the Makefile. Once we have a file exploded out has\n> just the synopsis line that can be used to replace what's now in\n> Documentation/cmd-list.perl, i.e. those summary blurbs also end up in\n> \"man git\".\n> \n> There's subtle dependency issues there as well, and just having a\n> one-off solution for the the command-list.h doesn't get us closer to\n> addressing that sibling implementation.\n\nSo I don't know what \"subtle dependency issues\" you found here, but this\nis exactly the kind of complexity it was my goal to avoid.\n\n-Peff\n"},{"id":"439158","messageId":"211021.86ilxrmbb9.gmgdl@evledraar.gmail.com","threadId":"56000","inReplyTo":"YXCqO7WFET6J7gGf@coredump.intra.peff.net","subject":"Re: [PATCH 0/8] Makefile: make command-list.h 2-5x as fast with -jN","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-21T00:48:24Z","receivedAt":"2021-10-21T01:11:28Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Oct 20 2021, Jeff King wrote:\n\n> On Thu, Oct 21, 2021 at 01:14:37AM +0200, Ævar Arnfjörð Bjarmason wrote:\n>\n>> Jeff: Just in terms of error prone both of these implementations will\n>> accept bad input that's being caught in 8/8 of this series.\n>> \n>> We accept a lot of bad input now, ending up with some combinations of\n>> bad output or compile errors if you screw with the input *.txt files. I\n>> think I've addressed all of those in this series.\n>\n> I don't mind more error-checking, though TBH I don't find a huge value\n> in it. But what I did mean was:\n>\n>> If you mean the general concept of making a \"foo.gen\" from a \"foo.txt\"\n>> as an intermediate with make as a way to get to \"many-foo.h\" I don't\n>> really see how it's error prone conceptually. You get error checking\n>> each step of the way, and it encourages logic that's simpler each step\n>> of the way.\n>\n> Yes. It just seems like the Makefile gets more complicated, and\n> sometimes that can lead to subtle dependency issues (e.g., the \".build\"\n> dependency in the earlier iteration of the series).\n\nFWIW there wasn't an earlier version of the series, just a POC patch I\nhad as a comment in\nhttps://lore.kernel.org/git/87r1gqxqxn.fsf@evledraar.gmail.com/\n\n> And in general I'd much rather debug an awk script than a Makefile.\n>\n>> Per Eric's Sunshine's upthread comments an awk and Perl implementation\n>> were both considered before[1].\n>\n> Ah sorry, I thought it was just a perl one that had been the\n> show-stopper. I hadn't noticed the awk one. However, the point of my\n> patch was to use perl if available, and fall back otherwise. Maybe\n> that's too ugly, but it does address the concern with Eric's\n> implementation.\n\nI think carrying two implementations is worse than just having the one\nslightly slower one.\n\n>> I.e. I think if you e.g. touch Documentation/git-a*.txt with this series\n>> with/without this awk version the difference in runtime is within the\n>> error bars. I.e. making the loop faster isn't necessary. It's better to\n>> get to a point where make can save you from doing all/most of the work\n>> by checking modification times, rather than making an O(n) loop faster.\n>\n> FWIW, I don't agree with this paragraph at all. Parallelizing or reusing\n> partial results is IMHO inferior to just making things faster.\n\nI agree with you in the general case, but for something that's consumed\nby a make dependency graph I find it easier to debug things if\ne.g. changing git-add.txt results in a change to git-add.gen, which is\nthen cat'd together.\n\nIOW if we had a sufficiently fast C compiler I think I'd still prefer\nmake's existing rules over some equivalent of:\n\n cat *.c | super-fast-cc\n\nSince similar to how the *.sp files depend on the the *.o files now,\ndeclaring the dependency graph allows you to easily add more built\nthings.\n\n>> I'm also interested in (and have WIP patches for) simplifying things\n>> more generally in the Makefile. Once we have a file exploded out has\n>> just the synopsis line that can be used to replace what's now in\n>> Documentation/cmd-list.perl, i.e. those summary blurbs also end up in\n>> \"man git\".\n>> \n>> There's subtle dependency issues there as well, and just having a\n>> one-off solution for the the command-list.h doesn't get us closer to\n>> addressing that sibling implementation.\n>\n> So I don't know what \"subtle dependency issues\" you found here, but this\n> is exactly the kind of complexity it was my goal to avoid.\n\nBut how? I don't see how narrowly making the loop in generate-cmdlist.sh\ngets us closer to generating the \"cmds_txt\" in the\nDocumentation/Makefile.\n\nWhereas after this series we're pretty much there in terms of generating\nthose files. i.e. try:\n\n cat Documentation/cmds-mainporcelain.txt\n\nAll of those synopsis blurbs are extracted, and reverse-attributable to\nthe corresponding files.\n\nThe dependencies there are (arguably) subtly broken because those files\naren't re-made if a \"cmd-list.made\" is more recent, so if you remove one\nof the generated text files the Makefile logic will get stuck because\nthe graph is incomplete (which can happen e.g. if \"make clean\" is\ninterrupted, or you run a \"git clean -dxf '*.txt'\".\n\nI did the latter and ran into that recently, because I was trying to\nad-hoc fix another more general dependency issue we tend to have, which\nis using wildcards on potentially generated files, so if you checkout a\nnew verison, build, and then checkout an old version (or are adding one\nof the files involved) a script like build-docdep.perl will \"helpfully\"\npick up bad dependencies.\n\nI guess you could argue that those are all problems with the Makefile,\nbut I think they're ultimately best solved by driving the dependencies\nfrom the Makefile.\n\nI.e. all we need is the one list of built-ins in command-list.txt, pair\nthat up with the \"category\" and we can always generated everything down\nto the manpages correctly without relying on FS wildcards.\n"},{"id":"439159","messageId":"YXDOaNsu8CaytBbr@nand.local","threadId":"56000","inReplyTo":"211021.86ilxrmbb9.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 0/8] Makefile: make command-list.h 2-5x as fast with -jN","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-10-21T02:20:24Z","receivedAt":"2021-10-21T02:20:32Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Oct 21, 2021 at 02:48:24AM +0200, Ævar Arnfjörð Bjarmason wrote:\n> >> Per Eric's Sunshine's upthread comments an awk and Perl implementation\n> >> were both considered before[1].\n> >\n> > Ah sorry, I thought it was just a perl one that had been the\n> > show-stopper. I hadn't noticed the awk one. However, the point of my\n> > patch was to use perl if available, and fall back otherwise. Maybe\n> > that's too ugly, but it does address the concern with Eric's\n> > implementation.\n>\n> I think carrying two implementations is worse than just having the one\n> slightly slower one.\n\nI have no opinion on whether or not assuming that awk or Perl exists and\ncan be relied upon during the build is reasonable or not. It seems like\nthe former might be a slightly safer assumption than the latter, but in\nall honesty it seems like both are always likely to be around.\n\nIn any case, I think the point was that we could improve upon Peff's\npatch by just having a single implementation done in awk. And when I\nwrote that I definitely was in the mindset of being able to rely on awk\nduring compilation.\n\n> >> I.e. I think if you e.g. touch Documentation/git-a*.txt with this series\n> >> with/without this awk version the difference in runtime is within the\n> >> error bars. I.e. making the loop faster isn't necessary. It's better to\n> >> get to a point where make can save you from doing all/most of the work\n> >> by checking modification times, rather than making an O(n) loop faster.\n> >\n> > FWIW, I don't agree with this paragraph at all. Parallelizing or reusing\n> > partial results is IMHO inferior to just making things faster.\n>\n> I agree with you in the general case, but for something that's consumed\n> by a make dependency graph I find it easier to debug things if\n> e.g. changing git-add.txt results in a change to git-add.gen, which is\n> then cat'd together.\n>\n> IOW if we had a sufficiently fast C compiler I think I'd still prefer\n> make's existing rules over some equivalent of:\n>\n> cat *.c | super-fast-cc\n>\n> Since similar to how the *.sp files depend on the the *.o files now,\n> declaring the dependency graph allows you to easily add more built\n> things.\n\nThis seems like an unfair comparison to me. I might be more sympathetic\nif we were generating a more complicated artifact by running\ngenerate-cmdlist.sh, but its inputs and outputs seem very well defined\n(and non-complicated) to me.\n\nIn any case, I agree with Peff that this isn't the approach that I would\nhave taken. But I also think that *just* parallelizing isn't necessarily\na win here. There are two reasons I think that:\n\n - The cognitive load required to parallelize this process is\n complicated; the .build directory seems like another thing to keep\n track of, and it's not clear to me what updates it, or what the\n result of touching some file in that directory is.\n\n - But even if the parallelization was achievable by more\n straightforward means, you still have to do the slow thing when\n you're rebuilding from scratch. So this is strictly worse the first\n time you are compiling, at least on machines with fewer cores.\n\nIn any case, this is all overkill in my mind for what we are talking\nabout. I agree that 'cat *.c | super-fast-cc' is worse than a competent\nMakefile that knows what to build and when. But the problem here is a\nslow loop in shell that is easily made much faster by implementing it\nin a language that can execute the whole loop in a single process.\n\nThanks,\nTaylor\n"},{"id":"439176","messageId":"CAPig+cTX+zbO6QJeWR1yu6Fe+wmhHFqd-=mp_GpwFMLYBbu5DA@mail.gmail.com","threadId":"56000","inReplyTo":"211021.86v91rmftn.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 0/8] Makefile: make command-list.h 2-5x as fast with -jN","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-10-21T05:39:09Z","receivedAt":"2021-10-21T05:39:24Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Oct 20, 2021 at 7:34 PM Ævar Arnfjörð Bjarmason\n wrote:\n> On Wed, Oct 20 2021, Taylor Blau wrote:\n> > But I don't think we even need to, since we could just rely on Awk. That\n> > has all the benefits you described while still avoiding the circular\n> > dependency on libgit.a. But the killer feature is that we don't have to\n> > rely on two implementations, the lesser-used of which is likely to\n> > bitrot over time.\n>\n> Per Eric's Sunshine's upthread comments an awk and Perl implementation\n> were both considered before[1].\n> 1. https://lore.kernel.org/git/CAPig+cSzKoOzU-zPOZqfNpPYBFpcWqvDP3mwLvAn5WkiNW0UMw@mail.gmail.com/\n\nThanks for saving me the trouble of digging up that email reference.\n"},{"id":"439235","messageId":"YXF6Wwcvc0vq5ADZ@coredump.intra.peff.net","threadId":"56000","inReplyTo":"211021.86ilxrmbb9.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 0/8] Makefile: make command-list.h 2-5x as fast with -jN","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-21T14:34:03Z","receivedAt":"2021-10-21T14:34:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 21, 2021 at 02:48:24AM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> >> There's subtle dependency issues there as well, and just having a\n> >> one-off solution for the the command-list.h doesn't get us closer to\n> >> addressing that sibling implementation.\n> >\n> > So I don't know what \"subtle dependency issues\" you found here, but this\n> > is exactly the kind of complexity it was my goal to avoid.\n> \n> But how? I don't see how narrowly making the loop in generate-cmdlist.sh\n> gets us closer to generating the \"cmds_txt\" in the\n> Documentation/Makefile.\n\nWhat I meant is that the work to get everything right in the Makefile to\ncorrectly handle dependencies and a partial rebuild can be tricky. For\ninstance, you're still stuck with a big wildcard dependency on\nDocumentation/git*.txt (and a manual list of exclusions in the Makefile)\nbecause it's hard in make to do make new dynamic rules based on an\nexisting one (i.e., the list _should_ come from what's in\ncommand-list.txt). Or the fact that we apparently need to keep the old\nscript around or cmake anyway.\n\nIt's also much slower. Here are from-scratch builds before and after\nyour patch 7:\n\n $ hyperfine --warmup 1 -p 'make clean' 'make command-list.h'\n Benchmark #1: make command-list.h\n Time (mean ± σ): 1.527 s ± 0.060 s [User: 1.320 s, System: 0.649 s]\n Range (min … max): 1.433 s … 1.625 s 10 runs\n \n \n $ hyperfine --warmup 1 -p 'make clean' 'make command-list.h'\n Benchmark #1: make command-list.h\n Time (mean ± σ): 2.661 s ± 0.080 s [User: 2.359 s, System: 1.082 s]\n Range (min … max): 2.481 s … 2.756 s 10 runs\n\nI know that partial builds will offset that in some cases, but it still\nfeels like a step in the wrong direction. Even with a partial build,\nswapping out \"make clean\" for \"touch Documentation/git-add.txt\" takes\nabout 200ms for me. Whereas with the faster version of\ngenerate-cmdlist.sh I showed, it takes 150ms.\n\nNow performance isn't everything, and it's possible these partial\nsnippets will be useful in other places. But I'm not sure I see any real\nadvantage in this series, and it seems like we're taking a hit in both\nperformance and complexity in the meantime.\n\n-Peff\n"},{"id":"439236","messageId":"YXF8a7OoStzdEwZF@coredump.intra.peff.net","threadId":"56000","inReplyTo":"patch-6.8-e0b11514b8d-20211020T183533Z-avarab@gmail.com","subject":"Re: [PATCH 6/8] generate-cmdlist.sh: replace for loop by printf's auto-repeat feature","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-21T14:42:51Z","receivedAt":"2021-10-21T14:42:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 20, 2021 at 08:39:57PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> From: Johannes Sixt \n> \n> This is just a small code reduction. There is a small probability that\n> the new code breaks when the category list is empty. But that would be\n> noticed during the compile step.\n> \n> Signed-off-by: Johannes Sixt \n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> generate-cmdlist.sh | 5 +----\n> 1 file changed, 1 insertion(+), 4 deletions(-)\n> \n> diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\n> index e517c33710a..a1ab2b1f077 100755\n> --- a/generate-cmdlist.sh\n> +++ b/generate-cmdlist.sh\n> @@ -67,10 +67,7 @@ print_command_list () {\n> \twhile read cmd rest\n> \tdo\n> \t\tprintf \"\t{ \\\"$cmd\\\", $(get_synopsis $cmd), 0\"\n> -\t\tfor cat in $(echo \"$rest\" | get_category_line)\n> -\t\tdo\n> -\t\t\tprintf \" | CAT_$cat\"\n> -\t\tdone\n> +\t\tprintf \" | CAT_%s\" $(echo \"$rest\" | get_category_line)\n> \t\techo \" },\"\n\nI think this is fine, but regardless of what happens in patch 7, it's\nprobably worth dropping this get_category_line call. All it does is sort\nand de-dup the tokens in $rest, but we don't care because we're just\nOR-ing them together. And of the 3 processes spawned by each loop, it is\nresponsible for 2 of them.\n\nEven if this loop is broken out into individual bits of Makefile\nsnippet, avoiding the extra processes is worth doing.\n\nThe patch below gives me:\n\n $ git show HEAD:generate-cmdlist.sh >generate-cmdlist.sh.old\n $ hyperfine --warmup 1 -L s ,.old -p 'make clean' 'sh generate-cmdlist.sh{s} command-list.txt'\n Benchmark #1: sh generate-cmdlist.sh command-list.txt\n Time (mean ± σ): 591.3 ms ± 31.5 ms [User: 392.9 ms, System: 243.7 ms]\n Range (min … max): 543.7 ms … 630.6 ms 10 runs\n \n Benchmark #2: sh generate-cmdlist.sh.old command-list.txt\n Time (mean ± σ): 1.236 s ± 0.060 s [User: 1.100 s, System: 0.556 s]\n Range (min … max): 1.089 s … 1.275 s 10 runs\n \n Summary\n 'sh generate-cmdlist.sh command-list.txt' ran\n 2.09 ± 0.15 times faster than 'sh generate-cmdlist.sh.old command-list.txt'\n\n---\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex a1ab2b1f07..fab9e6a671 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -67,7 +67,7 @@ print_command_list () {\n \twhile read cmd rest\n \tdo\n \t\tprintf \"\t{ \\\"$cmd\\\", $(get_synopsis $cmd), 0\"\n-\t\tprintf \" | CAT_%s\" $(echo \"$rest\" | get_category_line)\n+\t\tprintf \" | CAT_%s\" $rest\n \t\techo \" },\"\n \tdone\n \techo \"};\"\n\nI think you could also delete get_category_line, as it was inlined in\nthe other caller.\n\n-Peff\n"},{"id":"439237","messageId":"YXF9DQuj0541ULku@coredump.intra.peff.net","threadId":"56000","inReplyTo":"patch-7.8-0c6f9b80d3b-20211020T183533Z-avarab@gmail.com","subject":"Re: [PATCH 7/8] Makefile: stop having command-list.h depend on a wildcard","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-21T14:45:33Z","receivedAt":"2021-10-21T14:45:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 20, 2021 at 08:39:58PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> get_synopsis () {\n> +\thead -n 10 \"Documentation/$1.txt\" |\n> \tsed -n '\n> \t\t/^NAME/,/'\"$1\"'/H\n> \t\t${\n> \t\t\tx\n> \t\t\ts/.*'\"$1\"' - \\(.*\\)/N_(\"\\1\")/\n> \t\t\tp\n> -\t\t}' \"Documentation/$1.txt\"\n> +\t\t}'\n> }\n\nBy the way, I'm not sure about the utility of this change. It reduces\nthe number of lines that sed looks at, but at the cost of an extra\nprocess. That's probably a net loss. And if we did want to limit the\ndata sed covers, doing \"pq\" after we matched would be simpler.\n\nIt also feels like it's orthogonal to what this patch is doing, but\nmaybe there's some subtle non-performance reason to want this.\n\n-Peff\n"},{"id":"439250","messageId":"YXGUXEjoB3qofs0U@coredump.intra.peff.net","threadId":"56000","inReplyTo":"YXF8a7OoStzdEwZF@coredump.intra.peff.net","subject":"Re: [PATCH 6/8] generate-cmdlist.sh: replace for loop by printf's auto-repeat feature","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-21T16:25:00Z","receivedAt":"2021-10-21T16:25:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 21, 2021 at 10:42:52AM -0400, Jeff King wrote:\n\n> diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\n> index a1ab2b1f07..fab9e6a671 100755\n> --- a/generate-cmdlist.sh\n> +++ b/generate-cmdlist.sh\n> @@ -67,7 +67,7 @@ print_command_list () {\n> \twhile read cmd rest\n> \tdo\n> \t\tprintf \"\t{ \\\"$cmd\\\", $(get_synopsis $cmd), 0\"\n> -\t\tprintf \" | CAT_%s\" $(echo \"$rest\" | get_category_line)\n> +\t\tprintf \" | CAT_%s\" $rest\n> \t\techo \" },\"\n> \tdone\n> \techo \"};\"\n> \n> I think you could also delete get_category_line, as it was inlined in\n> the other caller.\n\nJust for fun, I did a pure-shell loop to drop get_synopsis, which means\nwe don't exec any processes inside the loop. That patch is below, which\nyields the timings (orig is up to your patch 6, no-sort is the patch\nabove, and pure-shell is the patch below on top):\n\n $ hyperfine --warmup 1 -L v orig,no-sort,pure-shell -p 'make clean' 'sh generate-cmdlist.sh.{v} command-list.txt'\n Benchmark #1: sh generate-cmdlist.sh.orig command-list.txt\n Time (mean ± σ): 1.286 s ± 0.148 s [User: 1.503 s, System: 0.781 s]\n Range (min … max): 0.938 s … 1.451 s 10 runs\n \n Benchmark #2: sh generate-cmdlist.sh.no-sort command-list.txt\n Time (mean ± σ): 553.6 ms ± 143.3 ms [User: 396.7 ms, System: 198.3 ms]\n Range (min … max): 192.6 ms … 683.5 ms 10 runs\n \n Benchmark #3: sh generate-cmdlist.sh.pure-shell command-list.txt\n Time (mean ± σ): 29.7 ms ± 15.6 ms [User: 22.6 ms, System: 19.4 ms]\n Range (min … max): 12.0 ms … 49.1 ms 10 runs\n \n Summary\n 'sh generate-cmdlist.sh.pure-shell command-list.txt' ran\n 18.65 ± 10.93 times faster than 'sh generate-cmdlist.sh.no-sort command-list.txt'\n 43.33 ± 23.32 times faster than 'sh generate-cmdlist.sh.orig command-list.txt'\n\nSo that's building all of the commands faster than I could get even\n\"touch Documentation/git-add.txt && make command-list.h\" to run with\nyour patch (not entirely fair; I'm not invoking make here, which\nprobably does add 100ms of overhead, but I think it's still a net win).\n\nThe patch below doesn't enforce the /NAME/ section as the sed does. IMHO\nthat's not of much value because it uses the line with the command-name\nas the lower bound. But it could be done pretty easily with an extra\n$seen_name variable.\n\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex fab9e6a671..eae4bbb4c7 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -22,16 +22,6 @@ category_list () {\n \tLC_ALL=C sort -u\n }\n \n-get_synopsis () {\n-\tsed -n '\n-\t\t/^NAME/,/'\"$1\"'/H\n-\t\t${\n-\t\t\tx\n-\t\t\ts/.*'\"$1\"' - \\(.*\\)/N_(\"\\1\")/\n-\t\t\tp\n-\t\t}' \"Documentation/$1.txt\"\n-}\n-\n define_categories () {\n \techo\n \techo \"/* Command categories */\"\n@@ -66,7 +56,18 @@ print_command_list () {\n \tcommand_list \"$1\" |\n \twhile read cmd rest\n \tdo\n-\t\tprintf \"\t{ \\\"$cmd\\\", $(get_synopsis $cmd), 0\"\n+\t\tsynopsis=\n+\t\twhile read line\n+\t\tdo\n+\t\t\tcase \"$line\" in\n+\t\t\t\"$cmd - \"*)\n+\t\t\t\tsynopsis=${line#$cmd - }\n+\t\t\t\tbreak\n+\t\t\t\t;;\n+\t\t\tesac\n+\t\tdone <\"Documentation/$cmd.txt\"\n+\n+\t\tprintf '\\t{ \"%s\", N_(\"%s\"), 0' \"$cmd\" \"$synopsis\"\n \t\tprintf \" | CAT_%s\" $rest\n \t\techo \" },\"\n \tdone\n"},{"id":"439258","messageId":"xmqq4k9ame1q.fsf@gitster.g","threadId":"56000","inReplyTo":"YXF9DQuj0541ULku@coredump.intra.peff.net","subject":"Re: [PATCH 7/8] Makefile: stop having command-list.h depend on a wildcard","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-21T18:24:33Z","receivedAt":"2021-10-21T18:24:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, Oct 20, 2021 at 08:39:58PM +0200, Ævar Arnfjörð Bjarmason wrote:\n>\n>> get_synopsis () {\n>> +\thead -n 10 \"Documentation/$1.txt\" |\n>> \tsed -n '\n>> \t\t/^NAME/,/'\"$1\"'/H\n>> \t\t${\n>> \t\t\tx\n>> \t\t\ts/.*'\"$1\"' - \\(.*\\)/N_(\"\\1\")/\n>> \t\t\tp\n>> -\t\t}' \"Documentation/$1.txt\"\n>> +\t\t}'\n>> }\n>\n> By the way, I'm not sure about the utility of this change. It reduces\n> the number of lines that sed looks at, but at the cost of an extra\n> process. That's probably a net loss. And if we did want to limit the\n> data sed covers, doing \"pq\" after we matched would be simpler.\n\nDoesn't the above\n\n - slurp the lines into hold space while we are in the synopsis\n part;\n - otherwise keep reading and discarding;\n - and do the processing at end\n\nSo presumably, instead of waiting till the end, can't we immediately\nprocess the thing and exit?\n\nI guess your \"'pq' after we matched\" is saying the same thing. I\nagree that extra process with an ad-hoc limitation of 10 does leave\na bad taste in my mouth.\n"},{"id":"439298","messageId":"xmqqh7dahur9.fsf@gitster.g","threadId":"56000","inReplyTo":"YXF6Wwcvc0vq5ADZ@coredump.intra.peff.net","subject":"Re: [PATCH 0/8] Makefile: make command-list.h 2-5x as fast with -jN","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-21T22:34:50Z","receivedAt":"2021-10-21T22:34:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Now performance isn't everything, and it's possible these partial\n> snippets will be useful in other places. But I'm not sure I see any real\n> advantage in this series, and it seems like we're taking a hit in both\n> performance and complexity in the meantime.\n\nLet's not forget about a hit we are taking in reviewer bandwidth.\nAnd added complexity will cost more over time. I am not sure if\nthese 8-patches deserved this much attention, compared to other\nneglected topics.\n"},{"id":"439301","messageId":"CAFaJEqtHdg7htFLyztVpJsWViC7rxRxknWpPHHcK_v8PoPxOig@mail.gmail.com","threadId":"56000","inReplyTo":"patch-7.8-0c6f9b80d3b-20211020T183533Z-avarab@gmail.com","subject":"Re: [PATCH 7/8] Makefile: stop having command-list.h depend on a wildcard","fromName":"Øystein Walle","fromEmail":"oystwa@gmail.com","sentAt":"2021-10-21T22:46:52Z","receivedAt":"2021-10-21T22:47:30Z","isPatch":true,"sender":{"key":"oystwa@gmail.com","avatar":"https://avatars.githubusercontent.com/u/794585?v=4"},"body":"On Wed, 20 Oct 2021 at 20:40, Ævar Arnfjörð Bjarmason wrote:\n\n> Unfortunately we can't drop the old code completely due to the CMake\n> integration, see 061c2240b1b (Introduce CMake support for configuring\n> Git, 2020-06-12). It will keep using the older and slower script.\n\nI took a quick stab at this earlier today and it's not too difficult to\nimplement. I can take a deeper dive over the weekend if this patch\nseries gets traction.\n\n> +header_only=\n> +case $1 in\n> +--entry-only)\n> + shift\n> + print_command_list $1 <\"$1\"\n> + exit 0\n> + ;;\n\nWhy is \"$1\" given twice here? As far as I can tell print_command_list\ndoesn't reference its arguments anymore.\n\n> +# The old compatibility mode for CMmake. See 061c2240b1b (Introduce\n> +# CMake support for configuring Git, 2020-06-12)\n\ns/CMmake/CMake/\n\n> +echo \"static struct cmdname_help command_list[] = {\"\n> +grep -v \\\n> + -e '^#' \\\n> + -e '^git-fsck-objects ' \\\n> + \"$1\" |\n> print_command_list \"$1\"\n> +echo \"};\"\n\nI suppose it stems from this unchanged line. That can be changed to just\n`print_command_list`.\n\nØsse\n"},{"id":"439367","messageId":"211022.86h7d9kzgt.gmgdl@evledraar.gmail.com","threadId":"56000","inReplyTo":"YXF6Wwcvc0vq5ADZ@coredump.intra.peff.net","subject":"Re: [PATCH 0/8] Makefile: make command-list.h 2-5x as fast with -jN","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-22T10:51:11Z","receivedAt":"2021-10-22T12:37:12Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Oct 21 2021, Jeff King wrote:\n\n> On Thu, Oct 21, 2021 at 02:48:24AM +0200, Ævar Arnfjörð Bjarmason wrote:\n>\n>> >> There's subtle dependency issues there as well, and just having a\n>> >> one-off solution for the the command-list.h doesn't get us closer to\n>> >> addressing that sibling implementation.\n>> >\n>> > So I don't know what \"subtle dependency issues\" you found here, but this\n>> > is exactly the kind of complexity it was my goal to avoid.\n>> \n>> But how? I don't see how narrowly making the loop in generate-cmdlist.sh\n>> gets us closer to generating the \"cmds_txt\" in the\n>> Documentation/Makefile.\n>\n> What I meant is that the work to get everything right in the Makefile to\n> correctly handle dependencies and a partial rebuild can be tricky. For\n> instance, you're still stuck with a big wildcard dependency on\n> Documentation/git*.txt (and a manual list of exclusions in the Makefile)\n> because it's hard in make to do make new dynamic rules based on an\n> existing one (i.e., the list _should_ come from what's in\n> command-list.txt). Or the fact that we apparently need to keep the old\n> script around or cmake anyway.\n>\n> It's also much slower. Here are from-scratch builds before and after\n> your patch 7:\n>\n> $ hyperfine --warmup 1 -p 'make clean' 'make command-list.h'\n> Benchmark #1: make command-list.h\n> Time (mean ± σ): 1.527 s ± 0.060 s [User: 1.320 s, System: 0.649 s]\n> Range (min … max): 1.433 s … 1.625 s 10 runs\n> \n> \n> $ hyperfine --warmup 1 -p 'make clean' 'make command-list.h'\n> Benchmark #1: make command-list.h\n> Time (mean ± σ): 2.661 s ± 0.080 s [User: 2.359 s, System: 1.082 s]\n> Range (min … max): 2.481 s … 2.756 s 10 runs\n>\n> I know that partial builds will offset that in some cases, but it still\n> feels like a step in the wrong direction. Even with a partial build,\n> swapping out \"make clean\" for \"touch Documentation/git-add.txt\" takes\n> about 200ms for me. Whereas with the faster version of\n> generate-cmdlist.sh I showed, it takes 150ms.\n>\n> Now performance isn't everything, and it's possible these partial\n> snippets will be useful in other places. But I'm not sure I see any real\n> advantage in this series, and it seems like we're taking a hit in both\n> performance and complexity in the meantime.\n\nYes, the same numbers are noted in the 7/8 commit message. I.e. it's\nslower on -j1, but faster with higher -j numbers.\n\nAside from any changes I'm proposing here it seems rather pointless to\nme to optimize the runtime of -j1 runs.\n\nI think we use those in e.g. CI, so of course if they become *really*\nslow it will matter, but the purpose of this change is to make hacking\non git easier, both in terms of runtime and discovering what the\nMakefile is doing wih V=1.\n\nI think anyone hacking on git is going to be on a system with -j2 at\nleast. So again, separate from these specific changes, if we've got a\nchange that speeds up -jN runs at the cost of a -j1 run that seems like\ngood thing.\n\nIn terms of the utility of benchmarks this benchmark uses \"make\" and is\nmeaningful, but e.g. (and I think some\nother ones?) in this thread invoke the shellscript directly.\n\nThose sorts of benchmarks may or may not matter, and in this case the\nscript is always called in the context of a Makefile, so that's really\nthe only meaningful way to test it. If e.g. its performance changes in a\nway that won't be noticed in other Makefile noise it probably won't\nmatter to anyone.\n\n"},{"id":"439368","messageId":"211022.86cznxkyu3.gmgdl@evledraar.gmail.com","threadId":"56000","inReplyTo":"YXDOaNsu8CaytBbr@nand.local","subject":"Re: [PATCH 0/8] Makefile: make command-list.h 2-5x as fast with -jN","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-22T12:37:18Z","receivedAt":"2021-10-22T12:50:49Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Oct 20 2021, Taylor Blau wrote:\n\n> On Thu, Oct 21, 2021 at 02:48:24AM +0200, Ævar Arnfjörð Bjarmason wrote:\n>> >> Per Eric's Sunshine's upthread comments an awk and Perl implementation\n>> >> were both considered before[1].\n>> >\n>> > Ah sorry, I thought it was just a perl one that had been the\n>> > show-stopper. I hadn't noticed the awk one. However, the point of my\n>> > patch was to use perl if available, and fall back otherwise. Maybe\n>> > that's too ugly, but it does address the concern with Eric's\n>> > implementation.\n>>\n>> I think carrying two implementations is worse than just having the one\n>> slightly slower one.\n>\n> I have no opinion on whether or not assuming that awk or Perl exists and\n> can be relied upon during the build is reasonable or not. It seems like\n> the former might be a slightly safer assumption than the latter, but in\n> all honesty it seems like both are always likely to be around.\n>\n> In any case, I think the point was that we could improve upon Peff's\n> patch by just having a single implementation done in awk. And when I\n> wrote that I definitely was in the mindset of being able to rely on awk\n> during compilation.\n>\n>> >> I.e. I think if you e.g. touch Documentation/git-a*.txt with this series\n>> >> with/without this awk version the difference in runtime is within the\n>> >> error bars. I.e. making the loop faster isn't necessary. It's better to\n>> >> get to a point where make can save you from doing all/most of the work\n>> >> by checking modification times, rather than making an O(n) loop faster.\n>> >\n>> > FWIW, I don't agree with this paragraph at all. Parallelizing or reusing\n>> > partial results is IMHO inferior to just making things faster.\n>>\n>> I agree with you in the general case, but for something that's consumed\n>> by a make dependency graph I find it easier to debug things if\n>> e.g. changing git-add.txt results in a change to git-add.gen, which is\n>> then cat'd together.\n>>\n>> IOW if we had a sufficiently fast C compiler I think I'd still prefer\n>> make's existing rules over some equivalent of:\n>>\n>> cat *.c | super-fast-cc\n>>\n>> Since similar to how the *.sp files depend on the the *.o files now,\n>> declaring the dependency graph allows you to easily add more built\n>> things.\n>\n> This seems like an unfair comparison to me. I might be more sympathetic\n> if we were generating a more complicated artifact by running\n> generate-cmdlist.sh, but its inputs and outputs seem very well defined\n> (and non-complicated) to me.\n\nThey are? a foo.o to foo.o input is relatively uncomplicated, and you\ncan discover the exact dependencies with 3rd party tools, like the GCC\nand Clang switches we use generate the .depends dirs[1].\n\nWhereas with the custom shellscripts that have for-loops of their own\nlike generate-cmdlist.sh what it depends on exactly is relatively opaque\nto you until you read the shellscript.\n\nI guess it's a matter of taste, but if you run this with/without this\nseries:\n\n touch Documentation/git-a*.txt; time make -j1 command-list.h --debug=b V=1\n\nYou'll see that before we'd spot that e.g. git-add.txt changed, but\nwe'll run one target in response to that at the end.\n\nSo it's just like what you'd get when you make %.o from %.c to produce a\n\"program\" that links all those %.o together at the end.\n\nSo I do think it's a fair comparison, if anything it's unfair to this\nseries, because as noted you can discover these dependency independently\nwith GCC etc. for C code. But for a custom *.txt format with an ad-hoc\n*.sh to parse it there's no such aid available.\n\n1. $ cat .depend/help.o.d \nhelp.o: help.c cache.h git-compat-util.h compat/bswap.h wildmatch.h \\\n banned.h strbuf.h hashmap.h hash.h repository.h path.h sha1dc_git.h \\\n sha1collisiondetection/lib/sha1.h sha256/block/sha256.h list.h advice.h \\\n gettext.h convert.h string-list.h trace.h trace2.h pack-revindex.h \\\n oid-array.h mem-pool.h config.h builtin.h commit.h object.h tree.h \\\n decorate.h gpg-interface.h pretty.h commit-slab.h commit-slab-decl.h \\\n commit-slab-impl.h exec-cmd.h run-command.h thread-utils.h strvec.h \\\n levenshtein.h help.h command-list.h column.h version.h refs.h \\\n parse-options.h prompt.h\n[...]\n"},{"id":"439405","messageId":"YXMDm3ZzgE1UGSwk@coredump.intra.peff.net","threadId":"56000","inReplyTo":"211022.86h7d9kzgt.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 0/8] Makefile: make command-list.h 2-5x as fast with -jN","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-22T18:31:55Z","receivedAt":"2021-10-22T18:32:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 22, 2021 at 12:51:11PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> Yes, the same numbers are noted in the 7/8 commit message. I.e. it's\n> slower on -j1, but faster with higher -j numbers.\n> \n> Aside from any changes I'm proposing here it seems rather pointless to\n> me to optimize the runtime of -j1 runs.\n> \n> I think we use those in e.g. CI, so of course if they become *really*\n> slow it will matter, but the purpose of this change is to make hacking\n> on git easier, both in terms of runtime and discovering what the\n> Makefile is doing wih V=1.\n> \n> I think anyone hacking on git is going to be on a system with -j2 at\n> least. So again, separate from these specific changes, if we've got a\n> change that speeds up -jN runs at the cost of a -j1 run that seems like\n> good thing.\n\nIt seems weird to me to assume that all of our processors are available\nto build command-list.h. In most cases you are not running \"make -j16\ncommand-list.h\", but rather \"make -j16\", and those other processors are\ndoing useful things, like say, building actual C code.\n\nSo counting CPU time is the interesting thing, because every cycle you\nsave there gets used for other work. And \"make -j1\" just brings\nwall-clock and CPU time together.\n\n-Peff\n"},{"id":"439406","messageId":"cover-v2-00.10-00000000000-20211022T193027Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-0.8-00000000000-20211020T183533Z-avarab@gmail.com","subject":"[PATCH v2 00/10] Makefile: make generate-cmdlist.sh much faster","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-22T19:36:04Z","receivedAt":"2021-10-22T19:36:20Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This version of this series drops the Makefile-powered version of the\ncmdlist in favor of making the shellscript much faster, mostly with\nsuggestions from Jeff King.\n\nI still think that splitting out the generated data into files may be\nuseful for unifying the Documentation/ and C code build processes,\nthere's another custom parser for command-list.txt in\nDocumentation/cmd-list.perl.\n\nBut if and when I've got something for that I can dig that out of the\nv1, in the meantime the v1 of this should be mostly uncontroversial.\n\nThe last tow patches make things a bit slower for me, but since they\nreplace command invocations with pure-shell logic they presumably make\nthings a bit less painful on e.g. Windows, and the 8th patch here\nalready made things quite very fast already.\n\nJeff King (1):\n generate-cmdlist.sh: do not shell out to \"sed\"\n\nJohannes Sixt (2):\n generate-cmdlist.sh: spawn fewer processes\n generate-cmdlist.sh: replace for loop by printf's auto-repeat feature\n\nÆvar Arnfjörð Bjarmason (7):\n command-list.txt: sort with \"LC_ALL=C sort\"\n generate-cmdlist.sh: trivial whitespace change\n generate-cmdlist.sh: don't call get_categories() from category_list()\n generate-cmdlist.sh: run \"grep | sort\", not \"sort | grep\"\n generate-cmdlist.sh: stop sorting category lines\n generate-cmdlist.sh: replace \"grep' invocation with a shell version\n generate-cmdlist.sh: replace \"cut\", \"tr\" and \"grep\" with pure-shell\n\n command-list.txt | 20 +++++++-------\n generate-cmdlist.sh | 66 ++++++++++++++++++++++++++-------------------\n 2 files changed, 48 insertions(+), 38 deletions(-)\n\nRange-diff against v1:\n 1: 96885282988 = 1: 96885282988 command-list.txt: sort with \"LC_ALL=C sort\"\n 2: 5e8fef90e42 = 2: 5e8fef90e42 generate-cmdlist.sh: trivial whitespace change\n 3: 6b4de6a6088 = 3: 6b4de6a6088 generate-cmdlist.sh: spawn fewer processes\n 4: 074685cf714 = 4: 074685cf714 generate-cmdlist.sh: don't call get_categories() from category_list()\n 5: f01c1fd8088 = 5: f01c1fd8088 generate-cmdlist.sh: run \"grep | sort\", not \"sort | grep\"\n 6: e0b11514b8d = 6: e0b11514b8d generate-cmdlist.sh: replace for loop by printf's auto-repeat feature\n 7: 0c6f9b80d3b < -: ----------- Makefile: stop having command-list.h depend on a wildcard\n 8: 23d4cc77b6c < -: ----------- Makefile: assert correct generate-cmdlist.sh output\n -: ----------- > 7: f2f37c2963b generate-cmdlist.sh: stop sorting category lines\n -: ----------- > 8: 83318d6c0da generate-cmdlist.sh: do not shell out to \"sed\"\n -: ----------- > 9: 7903dd1f8c2 generate-cmdlist.sh: replace \"grep' invocation with a shell version\n -: ----------- > 10: e10a43756d1 generate-cmdlist.sh: replace \"cut\", \"tr\" and \"grep\" with pure-shell\n-- \n2.33.1.1505.g075a284c562\n\n"},{"id":"439407","messageId":"patch-v2-01.10-96885282988-20211022T193027Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-v2-00.10-00000000000-20211022T193027Z-avarab@gmail.com","subject":"[PATCH v2 01/10] command-list.txt: sort with \"LC_ALL=C sort\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-22T19:36:05Z","receivedAt":"2021-10-22T19:36:22Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"We should keep these files sorted in the C locale, e.g. in the C\nlocale the order is:\n\n git-check-mailmap\n git-check-ref-format\n git-checkout\n\nBut under en_US.UTF-8 it's:\n\n git-check-mailmap\n git-checkout\n git-check-ref-format\n\nIn a subsequent commit I'll change generate-cmdlist.sh to use C sort\norder, and without this change we'd be led to believe that that change\ncaused a meaningful change in the output, so let's do this as a\nseparate step, right now the generate-cmdlist.sh script just uses the\norder found in this file.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n command-list.txt | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/command-list.txt b/command-list.txt\nindex a289f09ed6f..02fc7ddde68 100644\n--- a/command-list.txt\n+++ b/command-list.txt\n@@ -60,9 +60,9 @@ git-cat-file plumbinginterrogators\n git-check-attr purehelpers\n git-check-ignore purehelpers\n git-check-mailmap purehelpers\n+git-check-ref-format purehelpers\n git-checkout mainporcelain\n git-checkout-index plumbingmanipulators\n-git-check-ref-format purehelpers\n git-cherry plumbinginterrogators complete\n git-cherry-pick mainporcelain\n git-citool mainporcelain\n@@ -111,7 +111,6 @@ git-index-pack plumbingmanipulators\n git-init mainporcelain init\n git-instaweb ancillaryinterrogators complete\n git-interpret-trailers purehelpers\n-gitk mainporcelain\n git-log mainporcelain info\n git-ls-files plumbinginterrogators\n git-ls-remote plumbinginterrogators\n@@ -124,11 +123,11 @@ git-merge-base plumbinginterrogators\n git-merge-file plumbingmanipulators\n git-merge-index plumbingmanipulators\n git-merge-one-file purehelpers\n-git-mergetool ancillarymanipulators complete\n git-merge-tree ancillaryinterrogators\n-git-multi-pack-index plumbingmanipulators\n+git-mergetool ancillarymanipulators complete\n git-mktag plumbingmanipulators\n git-mktree plumbingmanipulators\n+git-multi-pack-index plumbingmanipulators\n git-mv mainporcelain worktree\n git-name-rev plumbinginterrogators\n git-notes mainporcelain\n@@ -154,23 +153,23 @@ git-request-pull foreignscminterface complete\n git-rerere ancillaryinterrogators\n git-reset mainporcelain history\n git-restore mainporcelain worktree\n-git-revert mainporcelain\n git-rev-list plumbinginterrogators\n git-rev-parse plumbinginterrogators\n+git-revert mainporcelain\n git-rm mainporcelain worktree\n git-send-email foreignscminterface complete\n git-send-pack synchingrepositories\n+git-sh-i18n purehelpers\n+git-sh-setup purehelpers\n git-shell synchelpers\n git-shortlog mainporcelain\n git-show mainporcelain info\n git-show-branch ancillaryinterrogators complete\n git-show-index plumbinginterrogators\n git-show-ref plumbinginterrogators\n-git-sh-i18n purehelpers\n-git-sh-setup purehelpers\n git-sparse-checkout mainporcelain worktree\n-git-stash mainporcelain\n git-stage complete\n+git-stash mainporcelain\n git-status mainporcelain info\n git-stripspace purehelpers\n git-submodule mainporcelain\n@@ -189,10 +188,11 @@ git-var plumbinginterrogators\n git-verify-commit ancillaryinterrogators\n git-verify-pack plumbinginterrogators\n git-verify-tag ancillaryinterrogators\n-gitweb ancillaryinterrogators\n git-whatchanged ancillaryinterrogators complete\n git-worktree mainporcelain\n git-write-tree plumbingmanipulators\n+gitk mainporcelain\n+gitweb ancillaryinterrogators\n gitattributes guide\n gitcli guide\n gitcore-tutorial guide\n@@ -211,6 +211,6 @@ gitremote-helpers guide\n gitrepository-layout guide\n gitrevisions guide\n gitsubmodules guide\n-gittutorial-2 guide\n gittutorial guide\n+gittutorial-2 guide\n gitworkflows guide\n-- \n2.33.1.1505.g075a284c562\n\n"},{"id":"439408","messageId":"patch-v2-02.10-5e8fef90e42-20211022T193027Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-v2-00.10-00000000000-20211022T193027Z-avarab@gmail.com","subject":"[PATCH v2 02/10] generate-cmdlist.sh: trivial whitespace change","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-22T19:36:06Z","receivedAt":"2021-10-22T19:36:27Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This makes a subsequent diff smaller, and won't leave us with this\nsyntax nit at the end.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n generate-cmdlist.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex 9dbbb08e70a..5114f46680a 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -10,7 +10,7 @@ command_list () {\n }\n \n get_categories () {\n-\ttr ' ' '\\012'|\n+\ttr ' ' '\\012' |\n \tgrep -v '^$' |\n \tsort |\n \tuniq\n-- \n2.33.1.1505.g075a284c562\n\n"},{"id":"439409","messageId":"patch-v2-03.10-6b4de6a6088-20211022T193027Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-v2-00.10-00000000000-20211022T193027Z-avarab@gmail.com","subject":"[PATCH v2 03/10] generate-cmdlist.sh: spawn fewer processes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-22T19:36:07Z","receivedAt":"2021-10-22T19:36:28Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Johannes Sixt \n\nThe function get_categories() is invoked in a loop over all commands.\nAs it runs several processes, this takes an awful lot of time on\nWindows. To reduce the number of processes, move the process that\nfilters empty lines to the other invoker of the function, where it is\nneeded. The invocation of get_categories() in the loop does not need\nthe empty line filtered away because the result is word-split by the\nshell, which eliminates the empty line automatically.\n\nFurthermore, use sort -u instead of sort | uniq to remove yet another\nprocess.\n\n[Ævar: on Linux this seems to speed things up a bit, although with\nhyperfine(1) the results are fuzzy enough to land within the\nconfidence interval]:\n\n$ git show HEAD~:generate-cmdlist.sh >generate-cmdlist.sh.old\n$ hyperfine --warmup 1 -L s ,.old -p 'make clean' 'sh generate-cmdlist.sh{s} command-list.txt'\nBenchmark #1: sh generate-cmdlist.sh command-list.txt\n Time (mean ± σ): 371.3 ms ± 64.2 ms [User: 430.4 ms, System: 72.5 ms]\n Range (min … max): 320.5 ms … 517.7 ms 10 runs\n\nBenchmark #2: sh generate-cmdlist.sh.old command-list.txt\n Time (mean ± σ): 489.9 ms ± 185.4 ms [User: 724.7 ms, System: 141.3 ms]\n Range (min … max): 346.0 ms … 885.3 ms 10 runs\n\nSummary\n 'sh generate-cmdlist.sh command-list.txt' ran\n 1.32 ± 0.55 times faster than 'sh generate-cmdlist.sh.old command-list.txt'\n\nSigned-off-by: Johannes Sixt \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n generate-cmdlist.sh | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex 5114f46680a..27367915611 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -11,15 +11,14 @@ command_list () {\n \n get_categories () {\n \ttr ' ' '\\012' |\n-\tgrep -v '^$' |\n-\tsort |\n-\tuniq\n+\tLC_ALL=C sort -u\n }\n \n category_list () {\n \tcommand_list \"$1\" |\n \tcut -c 40- |\n-\tget_categories\n+\tget_categories |\n+\tgrep -v '^$'\n }\n \n get_synopsis () {\n-- \n2.33.1.1505.g075a284c562\n\n"},{"id":"439410","messageId":"patch-v2-04.10-074685cf714-20211022T193027Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-v2-00.10-00000000000-20211022T193027Z-avarab@gmail.com","subject":"[PATCH v2 04/10] generate-cmdlist.sh: don't call get_categories() from category_list()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-22T19:36:08Z","receivedAt":"2021-10-22T19:36:28Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This isn't for optimization as the get_categories() is a purely shell\nfunction, but rather for ease of readability, let's just inline these\ntwo lines. We'll be changing this code some more in subsequent commits\nto make this worth it.\n\nRename the get_categories() function to get_category_line(), since\nthat's what it's doing now.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n generate-cmdlist.sh | 7 ++++---\n 1 file changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex 27367915611..16043e38476 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -9,7 +9,7 @@ command_list () {\n \teval \"grep -ve '^#' $exclude_programs\" <\"$1\"\n }\n \n-get_categories () {\n+get_category_line () {\n \ttr ' ' '\\012' |\n \tLC_ALL=C sort -u\n }\n@@ -17,7 +17,8 @@ get_categories () {\n category_list () {\n \tcommand_list \"$1\" |\n \tcut -c 40- |\n-\tget_categories |\n+\ttr ' ' '\\012' |\n+\tLC_ALL=C sort -u |\n \tgrep -v '^$'\n }\n \n@@ -66,7 +67,7 @@ print_command_list () {\n \twhile read cmd rest\n \tdo\n \t\tprintf \"\t{ \\\"$cmd\\\", $(get_synopsis $cmd), 0\"\n-\t\tfor cat in $(echo \"$rest\" | get_categories)\n+\t\tfor cat in $(echo \"$rest\" | get_category_line)\n \t\tdo\n \t\t\tprintf \" | CAT_$cat\"\n \t\tdone\n-- \n2.33.1.1505.g075a284c562\n\n"},{"id":"439411","messageId":"patch-v2-05.10-f01c1fd8088-20211022T193027Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-v2-00.10-00000000000-20211022T193027Z-avarab@gmail.com","subject":"[PATCH v2 05/10] generate-cmdlist.sh: run \"grep | sort\", not \"sort | grep\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-22T19:36:09Z","receivedAt":"2021-10-22T19:36:31Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This doesn't matter for performance, but let's not include the empty\nlines in our sorting. This makes the intent of the code clearer.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n generate-cmdlist.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex 16043e38476..e517c33710a 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -18,8 +18,8 @@ category_list () {\n \tcommand_list \"$1\" |\n \tcut -c 40- |\n \ttr ' ' '\\012' |\n-\tLC_ALL=C sort -u |\n-\tgrep -v '^$'\n+\tgrep -v '^$' |\n+\tLC_ALL=C sort -u\n }\n \n get_synopsis () {\n-- \n2.33.1.1505.g075a284c562\n\n"},{"id":"439412","messageId":"patch-v2-06.10-e0b11514b8d-20211022T193027Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-v2-00.10-00000000000-20211022T193027Z-avarab@gmail.com","subject":"[PATCH v2 06/10] generate-cmdlist.sh: replace for loop by printf's auto-repeat feature","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-22T19:36:10Z","receivedAt":"2021-10-22T19:36:34Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Johannes Sixt \n\nThis is just a small code reduction. There is a small probability that\nthe new code breaks when the category list is empty. But that would be\nnoticed during the compile step.\n\nSigned-off-by: Johannes Sixt \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n generate-cmdlist.sh | 5 +----\n 1 file changed, 1 insertion(+), 4 deletions(-)\n\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex e517c33710a..a1ab2b1f077 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -67,10 +67,7 @@ print_command_list () {\n \twhile read cmd rest\n \tdo\n \t\tprintf \"\t{ \\\"$cmd\\\", $(get_synopsis $cmd), 0\"\n-\t\tfor cat in $(echo \"$rest\" | get_category_line)\n-\t\tdo\n-\t\t\tprintf \" | CAT_$cat\"\n-\t\tdone\n+\t\tprintf \" | CAT_%s\" $(echo \"$rest\" | get_category_line)\n \t\techo \" },\"\n \tdone\n \techo \"};\"\n-- \n2.33.1.1505.g075a284c562\n\n"},{"id":"439413","messageId":"patch-v2-07.10-f2f37c2963b-20211022T193027Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-v2-00.10-00000000000-20211022T193027Z-avarab@gmail.com","subject":"[PATCH v2 07/10] generate-cmdlist.sh: stop sorting category lines","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-22T19:36:11Z","receivedAt":"2021-10-22T19:36:36Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"In a preceding commit we changed the print_command_list() loop to use\nprintf's auto-repeat feature. Let's now get rid of get_category_line()\nentirely by not sorting the categories.\n\nThis will change the output of the generated code from e.g.:\n\n - { \"git-apply\", N_(\"Apply a patch to files and/or to the index\"), 0 | CAT_complete | CAT_plumbingmanipulators },\n\nTo:\n\n + { \"git-apply\", N_(\"Apply a patch to files and/or to the index\"), 0 | CAT_plumbingmanipulators | CAT_complete },\n\nI.e. the categories are no longer sorted, but as they're OR'd together\nit won't matter for the end result.\n\nThis speeds up the generate-cmdlist.sh a bit. Comparing HEAD~ (old)\nand \"master\" to this code:\n\n 'sh generate-cmdlist.sh command-list.txt' ran\n 1.07 ± 0.33 times faster than 'sh generate-cmdlist.sh.old command-list.txt'\n 1.15 ± 0.36 times faster than 'sh generate-cmdlist.sh.master command-list.txt'\n\nSuggested-by: Jeff King \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n generate-cmdlist.sh | 7 +------\n 1 file changed, 1 insertion(+), 6 deletions(-)\n\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex a1ab2b1f077..f50112c50f8 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -9,11 +9,6 @@ command_list () {\n \teval \"grep -ve '^#' $exclude_programs\" <\"$1\"\n }\n \n-get_category_line () {\n-\ttr ' ' '\\012' |\n-\tLC_ALL=C sort -u\n-}\n-\n category_list () {\n \tcommand_list \"$1\" |\n \tcut -c 40- |\n@@ -67,7 +62,7 @@ print_command_list () {\n \twhile read cmd rest\n \tdo\n \t\tprintf \"\t{ \\\"$cmd\\\", $(get_synopsis $cmd), 0\"\n-\t\tprintf \" | CAT_%s\" $(echo \"$rest\" | get_category_line)\n+\t\tprintf \" | CAT_%s\" $rest\n \t\techo \" },\"\n \tdone\n \techo \"};\"\n-- \n2.33.1.1505.g075a284c562\n\n"},{"id":"439414","messageId":"patch-v2-08.10-83318d6c0da-20211022T193027Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-v2-00.10-00000000000-20211022T193027Z-avarab@gmail.com","subject":"[PATCH v2 08/10] generate-cmdlist.sh: do not shell out to \"sed\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-22T19:36:12Z","receivedAt":"2021-10-22T19:36:39Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Jeff King \n\nReplace the \"sed\" invocation in get_synopsis() with a pure-shell\nversion. This speeds up generate-cmdlist.sh significantly. Compared to\nHEAD~ (old) and \"master\" we are, according to hyperfine(1):\n\n 'sh generate-cmdlist.sh command-list.txt' ran\n 12.69 ± 5.01 times faster than 'sh generate-cmdlist.sh.old command-list.txt'\n 18.34 ± 3.03 times faster than 'sh generate-cmdlist.sh.master command-list.txt'\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Jeff King \n---\n generate-cmdlist.sh | 23 ++++++++++++-----------\n 1 file changed, 12 insertions(+), 11 deletions(-)\n\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex f50112c50f8..9b7d6aea629 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -17,16 +17,6 @@ category_list () {\n \tLC_ALL=C sort -u\n }\n \n-get_synopsis () {\n-\tsed -n '\n-\t\t/^NAME/,/'\"$1\"'/H\n-\t\t${\n-\t\t\tx\n-\t\t\ts/.*'\"$1\"' - \\(.*\\)/N_(\"\\1\")/\n-\t\t\tp\n-\t\t}' \"Documentation/$1.txt\"\n-}\n-\n define_categories () {\n \techo\n \techo \"/* Command categories */\"\n@@ -61,7 +51,18 @@ print_command_list () {\n \tcommand_list \"$1\" |\n \twhile read cmd rest\n \tdo\n-\t\tprintf \"\t{ \\\"$cmd\\\", $(get_synopsis $cmd), 0\"\n+\t\tsynopsis=\n+\t\twhile read line\n+\t\tdo\n+\t\t\tcase \"$line\" in\n+\t\t\t\"$cmd - \"*)\n+\t\t\t\tsynopsis=${line#$cmd - }\n+\t\t\t\tbreak\n+\t\t\t\t;;\n+\t\t\tesac\n+\t\tdone <\"Documentation/$cmd.txt\"\n+\n+\t\tprintf '\\t{ \"%s\", N_(\"%s\"), 0' \"$cmd\" \"$synopsis\"\n \t\tprintf \" | CAT_%s\" $rest\n \t\techo \" },\"\n \tdone\n-- \n2.33.1.1505.g075a284c562\n\n"},{"id":"439415","messageId":"patch-v2-10.10-e10a43756d1-20211022T193027Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-v2-00.10-00000000000-20211022T193027Z-avarab@gmail.com","subject":"[PATCH v2 10/10] generate-cmdlist.sh: replace \"cut\", \"tr\" and \"grep\" with pure-shell","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-22T19:36:14Z","receivedAt":"2021-10-22T19:36:42Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Extend the pure-shell parsing of command-list.txt by using having\ncommand_list() take an argument indicating whether we're interested in\nthe \"$cmd\" part of the line, or just the \"$rest\".\n\nThat takes care of the \"cut -d\", and printf's auto-repeat feature can\nreplace the \"tr\". We don't need the \"grep -v\" either, as we're not\nemitting any empty lines here (the command-list.txt doesn't have any).\n\nThis doesn't make things any faster or slower in my tests, but as with\nthe preceding commit let's do it just to get rid of command\ninvocations, it'll probably help on e.g. Windows.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n generate-cmdlist.sh | 12 +++++++-----\n 1 file changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex 2b184bbc65f..394443c66df 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -17,7 +17,12 @@ command_list () {\n \t\t\t\t*\":$cmd:\"*)\n \t\t\t\t;;\n \t\t\t*)\n-\t\t\t\techo \"$cmd $rest\"\n+\t\t\t\tif test -n \"$1\"\n+\t\t\t\tthen\n+\t\t\t\t\tprintf \"%s\\n\" $rest\n+\t\t\t\telse\n+\t\t\t\t\techo \"$cmd $rest\"\n+\t\t\t\tfi\n \t\t\t\t;;\n \t\t\tesac\n \t\tesac\n@@ -25,10 +30,7 @@ command_list () {\n }\n \n category_list () {\n-\tcommand_list <\"$1\" |\n-\tcut -d' ' -f2- |\n-\ttr ' ' '\\012' |\n-\tgrep -v '^$' |\n+\tcommand_list --no-cat <\"$1\" |\n \tLC_ALL=C sort -u\n }\n \n-- \n2.33.1.1505.g075a284c562\n\n"},{"id":"439416","messageId":"patch-v2-09.10-7903dd1f8c2-20211022T193027Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-v2-00.10-00000000000-20211022T193027Z-avarab@gmail.com","subject":"[PATCH v2 09/10] generate-cmdlist.sh: replace \"grep' invocation with a shell version","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-22T19:36:13Z","receivedAt":"2021-10-22T19:36:44Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Replace the \"grep\" we run to exclude certain programs from the\ngenerated output with a pure-shell loop that strips out the comments,\nand sees if the \"cmd\" we're reading is on a list of excluded\nprograms. This uses a trick similar to test_have_prereq() in\ntest-lib-functions.sh.\n\nOn my *nix system this makes things quite a bit slower compared to\nHEAD~, but since the generate-cmdlist.sh is already quite fast, and\nthis likely helps systems where command invocations are more\nexpensive (i.e. Windows) let's use this anyway.\n\n 'sh generate-cmdlist.sh.old command-list.txt' ran\n 1.56 ± 0.11 times faster than 'sh generate-cmdlist.sh command-list.txt'\n 18.00 ± 0.19 times faster than 'sh generate-cmdlist.sh.master command-list.txt'\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n generate-cmdlist.sh | 27 +++++++++++++++++++++------\n 1 file changed, 21 insertions(+), 6 deletions(-)\n\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex 9b7d6aea629..2b184bbc65f 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -6,12 +6,27 @@ die () {\n }\n \n command_list () {\n-\teval \"grep -ve '^#' $exclude_programs\" <\"$1\"\n+\twhile read cmd rest\n+\tdo\n+\t\tcase \"$cmd\" in\n+\t\t\"#\"*)\n+\t\t\tcontinue;\n+\t\t\t;;\n+\t\t*)\n+\t\t\tcase \"$exclude_programs\" in\n+\t\t\t\t*\":$cmd:\"*)\n+\t\t\t\t;;\n+\t\t\t*)\n+\t\t\t\techo \"$cmd $rest\"\n+\t\t\t\t;;\n+\t\t\tesac\n+\t\tesac\n+\tdone\n }\n \n category_list () {\n-\tcommand_list \"$1\" |\n-\tcut -c 40- |\n+\tcommand_list <\"$1\" |\n+\tcut -d' ' -f2- |\n \ttr ' ' '\\012' |\n \tgrep -v '^$' |\n \tLC_ALL=C sort -u\n@@ -48,7 +63,7 @@ define_category_names () {\n print_command_list () {\n \techo \"static struct cmdname_help command_list[] = {\"\n \n-\tcommand_list \"$1\" |\n+\tcommand_list <\"$1\" |\n \twhile read cmd rest\n \tdo\n \t\tsynopsis=\n@@ -69,11 +84,11 @@ print_command_list () {\n \techo \"};\"\n }\n \n-exclude_programs=\n+exclude_programs=:\n while test \"--exclude-program\" = \"$1\"\n do\n \tshift\n-\texclude_programs=\"$exclude_programs -e \\\"^$1 \\\"\"\n+\texclude_programs=\"$exclude_programs$1:\"\n \tshift\n done\n \n-- \n2.33.1.1505.g075a284c562\n\n"},{"id":"439421","messageId":"YXMrBMvkrtfLSeOz@nand.local","threadId":"56000","inReplyTo":"cover-v2-00.10-00000000000-20211022T193027Z-avarab@gmail.com","subject":"Re: [PATCH v2 00/10] Makefile: make generate-cmdlist.sh much faster","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-10-22T21:20:04Z","receivedAt":"2021-10-22T21:20:13Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Fri, Oct 22, 2021 at 09:36:04PM +0200, Ævar Arnfjörð Bjarmason wrote:\n> This version of this series drops the Makefile-powered version of the\n> cmdlist in favor of making the shellscript much faster, mostly with\n> suggestions from Jeff King.\n\nI'd be happy with the version that you suggest here, since at least the\nnet-effect is that generate-commandlist.sh is faster than before without\nmuch additional complexity.\n\nThat said, I did find the structure of these patches somewhat confusing.\nThere is a lot of refactoring of get_categories() and related functions,\nand those patches were a little tricky to read for me. I wonder how much\ncould be cleaned up by placing \"generate-cmdlist.sh: stop sorting\ncategory lines\" earlier in the series, getting rid of the caller.\n\nIt's too bad that the penultimate patch slowed things down a bit, but\nI think that things are so fast now that much more discussion in this\narea is really just splitting hairs. It would be interesting to hear\nfrom somebody on Windows whether or not the speed-up there was worth it\n(otherwise dropping that patch might make sense).\n\nAnyway, I think we've all spent a lot of time discussing a rather\nstraightforward set of patches ;-). So this version looks good to me.\n\nThanks,\nTaylor\n"},{"id":"439426","messageId":"211022.865ytok9ey.gmgdl@evledraar.gmail.com","threadId":"56000","inReplyTo":"YXMDm3ZzgE1UGSwk@coredump.intra.peff.net","subject":"Re: [PATCH 0/8] Makefile: make command-list.h 2-5x as fast with -jN","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-22T20:50:16Z","receivedAt":"2021-10-22T21:59:54Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Oct 22 2021, Jeff King wrote:\n\n> On Fri, Oct 22, 2021 at 12:51:11PM +0200, Ævar Arnfjörð Bjarmason wrote:\n>\n>> Yes, the same numbers are noted in the 7/8 commit message. I.e. it's\n>> slower on -j1, but faster with higher -j numbers.\n>> \n>> Aside from any changes I'm proposing here it seems rather pointless to\n>> me to optimize the runtime of -j1 runs.\n>> \n>> I think we use those in e.g. CI, so of course if they become *really*\n>> slow it will matter, but the purpose of this change is to make hacking\n>> on git easier, both in terms of runtime and discovering what the\n>> Makefile is doing wih V=1.\n>> \n>> I think anyone hacking on git is going to be on a system with -j2 at\n>> least. So again, separate from these specific changes, if we've got a\n>> change that speeds up -jN runs at the cost of a -j1 run that seems like\n>> good thing.\n>\n> It seems weird to me to assume that all of our processors are available\n> to build command-list.h. In most cases you are not running \"make -j16\n> command-list.h\", but rather \"make -j16\", and those other processors are\n> doing useful things, like say, building actual C code.\n>\n> So counting CPU time is the interesting thing, because every cycle you\n> save there gets used for other work. And \"make -j1\" just brings\n> wall-clock and CPU time together.\n\nThere's been a lot of goalpost moving in the discussion around this\nseries, but if you look at the numbers for the v1 I proposed at\n it also had a\nsignificant drop in \"user\" time.\n\nThe only thing that was slower in my tests in either wallclock or user\ntime was the wallclock time on the building from scratch case (the user\ntime was lower). Maybe those measurements were bad or whatever, but\nthat's the context for the above.\n\nNow, since then I've submitted a v2 with just the \"make the shellscript\nfaster\" parts of this, including with some of your suggestions, making\nthe shellscript version faster.\n\nFWIW there's a rather trivial addition to my version that makes the\n\"make all the things\" faster again in wallclock/user time. We don't need\nto re-parse the command-list.txt at all again to emit the headers if\ncommand-list.txt doesn't change, which is the common case. So we can\njust cache the whole header portion in another *.gen file and \"cat\" it.\n\nAnyway, I've been running these changes on top of other Makefile changes\nI've got locally where we re-build almost nothing as HEAD changes. No\nFORCE targets shellscripting (but the same via native GNU make\nfeatures), and no *.sh/*.perl script re-generation on HEAD changes (the\nformer being on-list).\n\nSo with that the common case really is that everything hangs on the\ncommand-list.h generation. Since we have maybe 1-5 *.c files modified,\nthen we need to link everything, and the linking is hanging on help.o,\nwhich needs command-list.h.\n"},{"id":"439464","messageId":"xmqqcznv9yfh.fsf@gitster.g","threadId":"56000","inReplyTo":"patch-v2-09.10-7903dd1f8c2-20211022T193027Z-avarab@gmail.com","subject":"Re: [PATCH v2 09/10] generate-cmdlist.sh: replace \"grep' invocation with a shell version","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-23T22:19:30Z","receivedAt":"2021-10-23T22:21:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Replace the \"grep\" we run to exclude certain programs from the\n> generated output with a pure-shell loop that strips out the comments,\n> and sees if the \"cmd\" we're reading is on a list of excluded\n> programs. This uses a trick similar to test_have_prereq() in\n> test-lib-functions.sh.\n>\n> On my *nix system this makes things quite a bit slower compared to\n> HEAD~, but since the generate-cmdlist.sh is already quite fast, and\n> this likely helps systems where command invocations are more\n> expensive (i.e. Windows) let's use this anyway.\n>\n> 'sh generate-cmdlist.sh.old command-list.txt' ran\n> 1.56 ± 0.11 times faster than 'sh generate-cmdlist.sh command-list.txt'\n> 18.00 ± 0.19 times faster than 'sh generate-cmdlist.sh.master command-list.txt'\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> generate-cmdlist.sh | 27 +++++++++++++++++++++------\n> 1 file changed, 21 insertions(+), 6 deletions(-)\n>\n> diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\n> index 9b7d6aea629..2b184bbc65f 100755\n> --- a/generate-cmdlist.sh\n> +++ b/generate-cmdlist.sh\n> @@ -6,12 +6,27 @@ die () {\n> }\n> \n> command_list () {\n> -\teval \"grep -ve '^#' $exclude_programs\" <\"$1\"\n> +\twhile read cmd rest\n> +\tdo\n> +\t\tcase \"$cmd\" in\n> +\t\t\"#\"*)\n> +\t\t\tcontinue;\n> +\t\t\t;;\n> +\t\t*)\n> +\t\t\tcase \"$exclude_programs\" in\n> +\t\t\t\t*\":$cmd:\"*)\n> +\t\t\t\t;;\n\nFunny indentation.\n\n> +\t\t\t*)\n> +\t\t\t\techo \"$cmd $rest\"\n> +\t\t\t\t;;\n> +\t\t\tesac\n> +\t\tesac\n> +\tdone\n> }\n\n> category_list () {\n> -\tcommand_list \"$1\" |\n> +\tcommand_list <\"$1\" |\n\nThis change is unnecessary if you did\n\n\twhile read cmd rest\n\tdo\n\t\t...\n\tdone <\"$1\"\n\nto keep the external interface to the command_list helper unchanged.\n\n> -\tcut -c 40- |\n> +\tcut -d' ' -f2- |\n\nIs this just a subjective preference or a logical consequence of how\nthe output from command_list looks like got somehow changed?\n\n> @@ -48,7 +63,7 @@ define_category_names () {\n> print_command_list () {\n> \techo \"static struct cmdname_help command_list[] = {\"\n> \n> -\tcommand_list \"$1\" |\n> +\tcommand_list <\"$1\" |\n\nDitto.\n\n> -exclude_programs=\n> +exclude_programs=:\n> while test \"--exclude-program\" = \"$1\"\n> do\n> \tshift\n> -\texclude_programs=\"$exclude_programs -e \\\"^$1 \\\"\"\n> +\texclude_programs=\"$exclude_programs$1:\"\n> \tshift\n> done\n"},{"id":"439465","messageId":"xmqq7de39y4l.fsf@gitster.g","threadId":"56000","inReplyTo":"patch-v2-10.10-e10a43756d1-20211022T193027Z-avarab@gmail.com","subject":"Re: [PATCH v2 10/10] generate-cmdlist.sh: replace \"cut\", \"tr\" and \"grep\" with pure-shell","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-23T22:26:02Z","receivedAt":"2021-10-23T22:26:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Extend the pure-shell parsing of command-list.txt by using having\n\nusing having???\n\n> command_list() take an argument indicating whether we're interested in\n> the \"$cmd\" part of the line, or just the \"$rest\".\n\nOK, --no-cat stands for --no-category? Even if (or especially if,\nperhaps) you do not bother to parse the option in the command_list\nhelper, it would help the readers if it is spelled out. I somehow\nthought if this option has anything to do with \"/bin/cat\".\n\n> That takes care of the \"cut -d\", and printf's auto-repeat feature can\n> replace the \"tr\". We don't need the \"grep -v\" either, as we're not\n> emitting any empty lines here (the command-list.txt doesn't have any).\n\nIt may make sense to ensure that the case arm won't feed an empty \nline that made cmd an empty by tightening the condition.\n\n\tcase \"$cmd\" in\n\t\"#\"*)\n\t\tcontinue\n\t\t;; \n-\t*)\n+\t?*)\n\t\tcase \"$exclude_programs\" in\n\t\t*:\"$cmd\":*)\n\t\t\t;;\n\nIf anything, that would serve as a clear documentation that we are\nsafe even when the input has an empty line.\n"},{"id":"439466","messageId":"xmqq35or9y3q.fsf@gitster.g","threadId":"56000","inReplyTo":"patch-v2-09.10-7903dd1f8c2-20211022T193027Z-avarab@gmail.com","subject":"Re: [PATCH v2 09/10] generate-cmdlist.sh: replace \"grep' invocation with a shell version","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-23T22:26:33Z","receivedAt":"2021-10-23T22:26:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> +\t\t\"#\"*)\n> +\t\t\tcontinue;\n> +\t\t\t;;\n\nStray ';' after continue.\n"},{"id":"439467","messageId":"xmqqsfwr8j66.fsf@gitster.g","threadId":"56000","inReplyTo":"cover-v2-00.10-00000000000-20211022T193027Z-avarab@gmail.com","subject":"Re: [PATCH v2 00/10] Makefile: make generate-cmdlist.sh much faster","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-23T22:34:25Z","receivedAt":"2021-10-23T22:34:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Subject: Re: [PATCH v2 00/10] Makefile: make generate-cmdlist.sh much faster\n\nStale topic as there is no change to the Makefile.\n\n> This version of this series drops the Makefile-powered version of the\n> cmdlist in favor of making the shellscript much faster, mostly with\n> suggestions from Jeff King.\n\nOK.\n\nI looked at the whole thing and it looked almost done, modulo just a\nlittle breakages here and there whose corrections should be fairly\nobvious.\n\nThanks.\n"},{"id":"439557","messageId":"YXbdxDcaZOVrPUBz@coredump.intra.peff.net","threadId":"56000","inReplyTo":"patch-v2-07.10-f2f37c2963b-20211022T193027Z-avarab@gmail.com","subject":"Re: [PATCH v2 07/10] generate-cmdlist.sh: stop sorting category lines","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-25T16:39:32Z","receivedAt":"2021-10-25T16:39:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 22, 2021 at 09:36:11PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> In a preceding commit we changed the print_command_list() loop to use\n> printf's auto-repeat feature. Let's now get rid of get_category_line()\n> entirely by not sorting the categories.\n> \n> This will change the output of the generated code from e.g.:\n> \n> - { \"git-apply\", N_(\"Apply a patch to files and/or to the index\"), 0 | CAT_complete | CAT_plumbingmanipulators },\n> \n> To:\n> \n> + { \"git-apply\", N_(\"Apply a patch to files and/or to the index\"), 0 | CAT_plumbingmanipulators | CAT_complete },\n> \n> I.e. the categories are no longer sorted, but as they're OR'd together\n> it won't matter for the end result.\n\nThanks for picking this up. The commit message here is well explained.\n\n> This speeds up the generate-cmdlist.sh a bit. Comparing HEAD~ (old)\n> and \"master\" to this code:\n> \n> 'sh generate-cmdlist.sh command-list.txt' ran\n> 1.07 ± 0.33 times faster than 'sh generate-cmdlist.sh.old command-list.txt'\n> 1.15 ± 0.36 times faster than 'sh generate-cmdlist.sh.master command-list.txt'\n\nCurious. I get much more dramatic results (as I'd expect, as we are\ncutting out 2 of 3 process spawns in the loop):\n\n 'sh generate-cmdlist.sh command-list.txt' ran\n 2.16 ± 0.17 times faster than 'sh generate-cmdlist.sh.old command-list.txt'\n 2.37 ± 0.28 times faster than 'sh generate-cmdlist.sh.master command-list.txt'\n\nEither way, I think it's a good idea (and it paves the way for the next\npatch, where we get the biggest speedup because we stop spawning any\nprocesses at all).\n\n-Peff\n"},{"id":"439561","messageId":"YXbfdEqKHc5b19u9@coredump.intra.peff.net","threadId":"56000","inReplyTo":"patch-v2-08.10-83318d6c0da-20211022T193027Z-avarab@gmail.com","subject":"Re: [PATCH v2 08/10] generate-cmdlist.sh: do not shell out to \"sed\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-25T16:46:44Z","receivedAt":"2021-10-25T16:46:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 22, 2021 at 09:36:12PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> From: Jeff King \n> \n> Replace the \"sed\" invocation in get_synopsis() with a pure-shell\n> version. This speeds up generate-cmdlist.sh significantly. Compared to\n> HEAD~ (old) and \"master\" we are, according to hyperfine(1):\n\nUnsurprisingly I'm in favor of this. ;)\n\nCuriously again, I get more dramatic results than you:\n\n> 'sh generate-cmdlist.sh command-list.txt' ran\n> 12.69 ± 5.01 times faster than 'sh generate-cmdlist.sh.old command-list.txt'\n> 18.34 ± 3.03 times faster than 'sh generate-cmdlist.sh.master command-list.txt'\n\n 'sh generate-cmdlist.sh command-list.txt' ran\n 22.44 ± 13.59 times faster than 'sh generate-cmdlist.sh.old command-list.txt'\n 57.35 ± 34.10 times faster than 'sh generate-cmdlist.sh.master command-list.txt'\n\nIt's like spawning processes is somehow faster on your machine than\nmine. I wonder if it's a CPU governor thing. This is a laptop, and those\nnumbers come from using \"powersave\". Doing \"cpufreq-set -g performance\",\nI get:\n\n 'sh generate-cmdlist.sh command-list.txt' ran\n 14.35 ± 0.23 times faster than 'sh generate-cmdlist.sh.old command-list.txt'\n 33.15 ± 0.50 times faster than 'sh generate-cmdlist.sh.master command-list.txt'\n\nwhich is closer. But most notably all versions are 3-5x faster than\ntheir \"powersave\" counterparts. I wonder if that has been driving some\nof the confusion in our timings in this thread.\n\nEither way, I think this is still a good direction to go.\n\n-Peff\n"},{"id":"439564","messageId":"YXbh+YR19bygRFaX@coredump.intra.peff.net","threadId":"56000","inReplyTo":"cover-v2-00.10-00000000000-20211022T193027Z-avarab@gmail.com","subject":"Re: [PATCH v2 00/10] Makefile: make generate-cmdlist.sh much faster","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-25T16:57:29Z","receivedAt":"2021-10-25T16:57:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 22, 2021 at 09:36:04PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> This version of this series drops the Makefile-powered version of the\n> cmdlist in favor of making the shellscript much faster, mostly with\n> suggestions from Jeff King.\n> \n> I still think that splitting out the generated data into files may be\n> useful for unifying the Documentation/ and C code build processes,\n> there's another custom parser for command-list.txt in\n> Documentation/cmd-list.perl.\n> \n> But if and when I've got something for that I can dig that out of the\n> v1, in the meantime the v1 of this should be mostly uncontroversial.\n\nThanks, up through patch 8 this all looks good to me.\n\n> The last tow patches make things a bit slower for me, but since they\n> replace command invocations with pure-shell logic they presumably make\n> things a bit less painful on e.g. Windows, and the 8th patch here\n> already made things quite very fast already.\n\nThese ones I could take or leave. They probably do help a little on\nWindows, but I'm much more concerned about O(nr_of_commands) process\ninvocations than I am in reducing the base number of invocations\n(because one gives a 169x speedup over the other).\n\nAnd in patch 9 in particular, we're trading a grep one-liner for a\nmuch-longer shell loop. And I don't think this is hypocritical with\nrespect to patch 8; there we are replacing ugly sed with ugly shell, and\nthe speed benefit is clear and large.\n\n-Peff\n"},{"id":"439571","messageId":"YXbuy3Lk0ZS+8sFQ@coredump.intra.peff.net","threadId":"56000","inReplyTo":"YXbfdEqKHc5b19u9@coredump.intra.peff.net","subject":"Re: [PATCH v2 08/10] generate-cmdlist.sh: do not shell out to \"sed\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-25T17:52:11Z","receivedAt":"2021-10-25T17:52:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 25, 2021 at 12:46:45PM -0400, Jeff King wrote:\n\n> It's like spawning processes is somehow faster on your machine than\n> mine. I wonder if it's a CPU governor thing. This is a laptop, and those\n> numbers come from using \"powersave\". Doing \"cpufreq-set -g performance\",\n> I get:\n> \n> 'sh generate-cmdlist.sh command-list.txt' ran\n> 14.35 ± 0.23 times faster than 'sh generate-cmdlist.sh.old command-list.txt'\n> 33.15 ± 0.50 times faster than 'sh generate-cmdlist.sh.master command-list.txt'\n> \n> which is closer. But most notably all versions are 3-5x faster than\n> their \"powersave\" counterparts. I wonder if that has been driving some\n> of the confusion in our timings in this thread.\n\nBTW, I should have mentioned these governors are used with the\nintel_pstate driver. So \"powersave\" here is not the default \"use the\nlowest frequency\" governor in linux, but the pstate-specific one which\nis more like \"ondemand\".\n\nNot that I expect anybody to look into or pontificate on governor\nissues, but I wanted to make sure I didn't confuse anyone.\n\n-Peff\n"},{"id":"439576","messageId":"xmqqfssp0xge.fsf@gitster.g","threadId":"56000","inReplyTo":"patch-v2-01.10-96885282988-20211022T193027Z-avarab@gmail.com","subject":"Re: [PATCH v2 01/10] command-list.txt: sort with \"LC_ALL=C sort\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-25T18:29:53Z","receivedAt":"2021-10-25T18:29:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> We should keep these files sorted in the C locale, e.g. in the C\n> locale the order is:\n>\n> git-check-mailmap\n> git-check-ref-format\n> git-checkout\n>\n> But under en_US.UTF-8 it's:\n>\n> git-check-mailmap\n> git-checkout\n> git-check-ref-format\n>\n> In a subsequent commit I'll change generate-cmdlist.sh to use C sort\n> order, and without this change we'd be led to believe that that change\n> caused a meaningful change in the output, so let's do this as a\n> separate step, right now the generate-cmdlist.sh script just uses the\n> order found in this file.\n\nHmph, I do not mind sorting this file bytewise like this at all, but\ndoes the justification above still apply to this round? I had an\nimpression that we lose the sorting altogether in the end...\n\nAlso, I am not sure where that \"led to believe\" comes from---do we\nhave a test that checks the output from generate-cmdlist somehow?\n\n\n"},{"id":"439592","messageId":"211025.861r483ij9.gmgdl@evledraar.gmail.com","threadId":"56000","inReplyTo":"xmqqfssp0xge.fsf@gitster.g","subject":"Re: [PATCH v2 01/10] command-list.txt: sort with \"LC_ALL=C sort\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-25T21:22:09Z","receivedAt":"2021-10-25T21:24:00Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Oct 25 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> We should keep these files sorted in the C locale, e.g. in the C\n>> locale the order is:\n>>\n>> git-check-mailmap\n>> git-check-ref-format\n>> git-checkout\n>>\n>> But under en_US.UTF-8 it's:\n>>\n>> git-check-mailmap\n>> git-checkout\n>> git-check-ref-format\n>>\n>> In a subsequent commit I'll change generate-cmdlist.sh to use C sort\n>> order, and without this change we'd be led to believe that that change\n>> caused a meaningful change in the output, so let's do this as a\n>> separate step, right now the generate-cmdlist.sh script just uses the\n>> order found in this file.\n>\n> Hmph, I do not mind sorting this file bytewise like this at all, but\n> does the justification above still apply to this round? I had an\n> impression that we lose the sorting altogether in the end...\n>\n> Also, I am not sure where that \"led to believe\" comes from---do we\n> have a test that checks the output from generate-cmdlist somehow?\n\nWe end up unsorting the categories we bitwise-OR together. I.e. the\nCAT_* on a line like this:\n\n { \"git-whatchanged\", N_(\"Show logs with difference each commit introduces\"), 0 | CAT_ancillaryinterrogators | CAT_complete },\n\nBut we'll still sort the actual command list, and spew it out as-is in\nsome places \"git\" and \"help\" output. So having it be unsorted or\nshuffled wouldn't be nice.\n"},{"id":"439597","messageId":"xmqqv91kzthm.fsf@gitster.g","threadId":"56000","inReplyTo":"211025.861r483ij9.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v2 01/10] command-list.txt: sort with \"LC_ALL=C sort\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-25T21:26:13Z","receivedAt":"2021-10-25T21:26:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n>> Also, I am not sure where that \"led to believe\" comes from---do we\n>> have a test that checks the output from generate-cmdlist somehow?\n>\n> We end up unsorting the categories we bitwise-OR together. I.e. the\n> CAT_* on a line like this:\n>\n> { \"git-whatchanged\", N_(\"Show logs with difference each commit introduces\"), 0 | CAT_ancillaryinterrogators | CAT_complete },\n>\n> But we'll still sort the actual command list, and spew it out as-is in\n> some places \"git\" and \"help\" output. So having it be unsorted or\n> shuffled wouldn't be nice.\n\nThe last paragraph would be a good replacement sentence for the part\nof the proposed log message that made me ask the question, I would\nthink.\n\nThanks.\n"},{"id":"440526","messageId":"patch-v3-01.10-c385e84c04c-20211105T135058Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-v3-00.10-00000000000-20211105T135058Z-avarab@gmail.com","subject":"[PATCH v3 01/10] command-list.txt: sort with \"LC_ALL=C sort\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-05T14:07:59Z","receivedAt":"2021-11-05T14:08:15Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"We should keep these files sorted in the C locale, e.g. in the C\nlocale the order is:\n\n git-check-mailmap\n git-check-ref-format\n git-checkout\n\nBut under en_US.UTF-8 it's:\n\n git-check-mailmap\n git-checkout\n git-check-ref-format\n\nIn a subsequent commit I'll change generate-cmdlist.sh to use C sort\norder, and without this change we'd be led to believe that that change\ncaused a meaningful change in the output, so let's do this as a\nseparate step, right now the generate-cmdlist.sh script just uses the\norder found in this file.\n\nNote that this refers to the sort order of the lines in\ncommand-list.txt, a subsequent commit will also change how we treat\nthe sort order of the \"category\" fields, but that's unrelated to this\nchange.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n command-list.txt | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/command-list.txt b/command-list.txt\nindex eb9cee8dee9..04cde20c3da 100644\n--- a/command-list.txt\n+++ b/command-list.txt\n@@ -60,9 +60,9 @@ git-cat-file plumbinginterrogators\n git-check-attr purehelpers\n git-check-ignore purehelpers\n git-check-mailmap purehelpers\n+git-check-ref-format purehelpers\n git-checkout mainporcelain\n git-checkout-index plumbingmanipulators\n-git-check-ref-format purehelpers\n git-cherry plumbinginterrogators complete\n git-cherry-pick mainporcelain\n git-citool mainporcelain\n@@ -111,7 +111,6 @@ git-index-pack plumbingmanipulators\n git-init mainporcelain init\n git-instaweb ancillaryinterrogators complete\n git-interpret-trailers purehelpers\n-gitk mainporcelain\n git-log mainporcelain info\n git-ls-files plumbinginterrogators\n git-ls-remote plumbinginterrogators\n@@ -124,11 +123,11 @@ git-merge-base plumbinginterrogators\n git-merge-file plumbingmanipulators\n git-merge-index plumbingmanipulators\n git-merge-one-file purehelpers\n-git-mergetool ancillarymanipulators complete\n git-merge-tree ancillaryinterrogators\n-git-multi-pack-index plumbingmanipulators\n+git-mergetool ancillarymanipulators complete\n git-mktag plumbingmanipulators\n git-mktree plumbingmanipulators\n+git-multi-pack-index plumbingmanipulators\n git-mv mainporcelain worktree\n git-name-rev plumbinginterrogators\n git-notes mainporcelain\n@@ -154,23 +153,23 @@ git-request-pull foreignscminterface complete\n git-rerere ancillaryinterrogators\n git-reset mainporcelain history\n git-restore mainporcelain worktree\n-git-revert mainporcelain\n git-rev-list plumbinginterrogators\n git-rev-parse plumbinginterrogators\n+git-revert mainporcelain\n git-rm mainporcelain worktree\n git-send-email foreignscminterface complete\n git-send-pack synchingrepositories\n+git-sh-i18n purehelpers\n+git-sh-setup purehelpers\n git-shell synchelpers\n git-shortlog mainporcelain\n git-show mainporcelain info\n git-show-branch ancillaryinterrogators complete\n git-show-index plumbinginterrogators\n git-show-ref plumbinginterrogators\n-git-sh-i18n purehelpers\n-git-sh-setup purehelpers\n git-sparse-checkout mainporcelain\n-git-stash mainporcelain\n git-stage complete\n+git-stash mainporcelain\n git-status mainporcelain info\n git-stripspace purehelpers\n git-submodule mainporcelain\n@@ -189,7 +188,6 @@ git-var plumbinginterrogators\n git-verify-commit ancillaryinterrogators\n git-verify-pack plumbinginterrogators\n git-verify-tag ancillaryinterrogators\n-gitweb ancillaryinterrogators\n git-whatchanged ancillaryinterrogators complete\n git-worktree mainporcelain\n git-write-tree plumbingmanipulators\n@@ -204,6 +202,7 @@ gitfaq guide\n gitglossary guide\n githooks guide\n gitignore guide\n+gitk mainporcelain\n gitmailmap guide\n gitmodules guide\n gitnamespaces guide\n@@ -211,6 +210,7 @@ gitremote-helpers guide\n gitrepository-layout guide\n gitrevisions guide\n gitsubmodules guide\n-gittutorial-2 guide\n gittutorial guide\n+gittutorial-2 guide\n+gitweb ancillaryinterrogators\n gitworkflows guide\n-- \n2.34.0.rc1.721.ga0c1db665bc\n\n"},{"id":"440527","messageId":"cover-v3-00.10-00000000000-20211105T135058Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-v2-00.10-00000000000-20211022T193027Z-avarab@gmail.com","subject":"[PATCH v3 00/10] generate-cmdlist.sh: make it (and \"make\") run faster","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-05T14:07:58Z","receivedAt":"2021-11-05T14:08:16Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This series makes the rather slow generate-cmdlist.sh shellscript run\nin ~20ms on my box instead of ~400ms. This helps a lot with \"make\"\nruntime, e.g. during interactive rebase.\n\nThis hopefully addresses all the feedback on v2. I kept the\npenultimate patch as explained in an updated commit message it helped\nquite a bit in the CI environment, and presumably on various other\nsetup.\n\nThere's also a new ~1.3x speedup of \"generate-cmdlist.sh: don't parse\ncommand-list.txt thrice\" new at the end here, but on some faster boxes\nthat's against a relative slowdown in 9/10 compared to Jeff's\n8/10[1]. I opted to keep 9/10 to give slower systems where process\nspawning isn't as fast the benefit of the doubt. We spend less user\ntime in 10/10 than in 8/10, but more system time.\n\n1. This is HEAD~n at the tip of the series. So .2 = 8/10, .1 = 9/10 '' = 10/10\n $ hyperfine --warmup 20 -L v .2,.1, 'sh generate-cmdlist.sh{v} command-list.txt'\n Benchmark #1: sh generate-cmdlist.sh.2 command-list.txt\n Time (mean ± σ): 19.5 ms ± 0.2 ms [User: 16.7 ms, System: 8.1 ms]\n Range (min … max): 18.9 ms … 20.5 ms 151 runs\n \n Benchmark #2: sh generate-cmdlist.sh.1 command-list.txt\n Time (mean ± σ): 30.1 ms ± 0.3 ms [User: 24.8 ms, System: 17.1 ms]\n Range (min … max): 29.3 ms … 31.3 ms 97 runs\n \n Benchmark #3: sh generate-cmdlist.sh command-list.txt\n Time (mean ± σ): 22.8 ms ± 0.3 ms [User: 15.2 ms, System: 10.1 ms]\n Range (min … max): 22.5 ms … 23.7 ms 125 runs\n \n Summary\n 'sh generate-cmdlist.sh.2 command-list.txt' ran\n 1.17 ± 0.02 times faster than 'sh generate-cmdlist.sh command-list.txt'\n 1.54 ± 0.02 times faster than 'sh generate-cmdlist.sh.1 command-list.txt'\n \n\nJeff King (1):\n generate-cmdlist.sh: do not shell out to \"sed\"\n\nJohannes Sixt (2):\n generate-cmdlist.sh: spawn fewer processes\n generate-cmdlist.sh: replace for loop by printf's auto-repeat feature\n\nÆvar Arnfjörð Bjarmason (7):\n command-list.txt: sort with \"LC_ALL=C sort\"\n generate-cmdlist.sh: trivial whitespace change\n generate-cmdlist.sh: don't call get_categories() from category_list()\n generate-cmdlist.sh: run \"grep | sort\", not \"sort | grep\"\n generate-cmdlist.sh: stop sorting category lines\n generate-cmdlist.sh: replace \"grep' invocation with a shell version\n generate-cmdlist.sh: don't parse command-list.txt thrice\n\n command-list.txt | 22 ++++++-------\n generate-cmdlist.sh | 78 ++++++++++++++++++++++++++-------------------\n 2 files changed, 56 insertions(+), 44 deletions(-)\n\nRange-diff against v2:\n 1: 96885282988 ! 1: c385e84c04c command-list.txt: sort with \"LC_ALL=C sort\"\n @@ Commit message\n separate step, right now the generate-cmdlist.sh script just uses the\n order found in this file.\n \n + Note that this refers to the sort order of the lines in\n + command-list.txt, a subsequent commit will also change how we treat\n + the sort order of the \"category\" fields, but that's unrelated to this\n + change.\n +\n Signed-off-by: Ævar Arnfjörð Bjarmason \n \n ## command-list.txt ##\n @@ command-list.txt: git-request-pull foreignscminterface\n git-show-ref plumbinginterrogators\n -git-sh-i18n purehelpers\n -git-sh-setup purehelpers\n - git-sparse-checkout mainporcelain worktree\n + git-sparse-checkout mainporcelain\n -git-stash mainporcelain\n git-stage complete\n +git-stash mainporcelain\n @@ command-list.txt: git-var plumbinginterrogators\n git-whatchanged ancillaryinterrogators complete\n git-worktree mainporcelain\n git-write-tree plumbingmanipulators\n +@@ command-list.txt: gitfaq guide\n + gitglossary guide\n + githooks guide\n + gitignore guide\n +gitk mainporcelain\n -+gitweb ancillaryinterrogators\n - gitattributes guide\n - gitcli guide\n - gitcore-tutorial guide\n + gitmailmap guide\n + gitmodules guide\n + gitnamespaces guide\n @@ command-list.txt: gitremote-helpers guide\n gitrepository-layout guide\n gitrevisions guide\n @@ command-list.txt: gitremote-helpers guide\n -gittutorial-2 guide\n gittutorial guide\n +gittutorial-2 guide\n ++gitweb ancillaryinterrogators\n gitworkflows guide\n 2: 5e8fef90e42 ! 2: b4b4c3aa135 generate-cmdlist.sh: trivial whitespace change\n @@ Metadata\n ## Commit message ##\n generate-cmdlist.sh: trivial whitespace change\n \n - This makes a subsequent diff smaller, and won't leave us with this\n - syntax nit at the end.\n + Add \" \" before a \"|\" at the end of a line in generate-cmdlist.sh for\n + consistency with other code in the file. Some of the surrounding code\n + will be modified in subsequent commits.\n \n Signed-off-by: Ævar Arnfjörð Bjarmason \n \n 3: 6b4de6a6088 = 3: 737cca59d99 generate-cmdlist.sh: spawn fewer processes\n 4: 074685cf714 = 4: 6ad17ab56c2 generate-cmdlist.sh: don't call get_categories() from category_list()\n 5: f01c1fd8088 = 5: d7be565b567 generate-cmdlist.sh: run \"grep | sort\", not \"sort | grep\"\n 6: e0b11514b8d = 6: 646363db11f generate-cmdlist.sh: replace for loop by printf's auto-repeat feature\n 7: f2f37c2963b = 7: d8cc7c246b8 generate-cmdlist.sh: stop sorting category lines\n 8: 83318d6c0da = 8: aeeecc575fb generate-cmdlist.sh: do not shell out to \"sed\"\n 9: 7903dd1f8c2 ! 9: e2702bcc1d0 generate-cmdlist.sh: replace \"grep' invocation with a shell version\n @@ Commit message\n test-lib-functions.sh.\n \n On my *nix system this makes things quite a bit slower compared to\n - HEAD~, but since the generate-cmdlist.sh is already quite fast, and\n - this likely helps systems where command invocations are more\n - expensive (i.e. Windows) let's use this anyway.\n -\n + HEAD~:\n + o\n 'sh generate-cmdlist.sh.old command-list.txt' ran\n 1.56 ± 0.11 times faster than 'sh generate-cmdlist.sh command-list.txt'\n 18.00 ± 0.19 times faster than 'sh generate-cmdlist.sh.master command-list.txt'\n \n - Signed-off-by: Ævar Arnfjörð Bjarmason \n + But when I tried running generate-cmdlist.sh 100 times in CI I found\n + that it helped across the board even on OSX & Linux. I tried testing\n + it in CI with this ad-hoc few-liner:\n +\n + for i in $(seq -w 0 11 | sort -nr)\n + do\n + git show HEAD~$i:generate-cmdlist.sh >generate-cmdlist-HEAD$i.sh &&\n + git add generate-cmdlist* &&\n + cp t/t0000-generate-cmdlist.sh t/t00$i-generate-cmdlist.sh || : &&\n + perl -pi -e \"s/HEAD0/HEAD$i/g\" t/t00$i-generate-cmdlist.sh &&\n + git add t/t00*.sh\n + done && git commit -m\"generated it\"\n +\n + Here HEAD~02 and the t0002* file refers to this change, and HEAD~03\n + and t0003* file to the preceding commit, the relevant results were:\n +\n + linux-gcc:\n +\n + [12:05:33] t0002-generate-cmdlist.sh .. ok 14 ms ( 0.00 usr 0.00 sys + 3.64 cusr 3.09 csys = 6.73 CPU)\n + [12:05:30] t0003-generate-cmdlist.sh .. ok 32 ms ( 0.00 usr 0.00 sys + 2.66 cusr 1.81 csys = 4.47 CPU)\n +\n + osx-gcc:\n +\n + [11:58:04] t0002-generate-cmdlist.sh .. ok 80081 ms ( 0.02 usr 0.02 sys + 17.80 cusr 10.07 csys = 27.91 CPU)\n + [11:58:16] t0003-generate-cmdlist.sh .. ok 92127 ms ( 0.02 usr 0.01 sys + 22.54 cusr 14.27 csys = 36.84 CPU)\n +\n + vs-test:\n +\n + [12:03:14] t0002-generate-cmdlist.sh .. ok 30 s ( 0.02 usr 0.00 sys + 13.14 cusr 26.19 csys = 39.35 CPU)\n + [12:03:20] t0003-generate-cmdlist.sh .. ok 32 s ( 0.00 usr 0.02 sys + 13.25 cusr 26.10 csys = 39.37 CPU)\n +\n + I.e. even on *nix running 100 of these in a loop was up to ~2x faster\n + in absolute runtime, I suspect it's due factors that are exacerbated\n + in the CI, e.g. much slower process startup due to some platform\n + limits, or a slower FS.\n +\n + The \"cut -d\" change here is because we're not emitting the\n + 40-character aligned output anymore, i.e. we'll get the output from\n + command_list() now, not an as-is line from command-list.txt.\n +\n + This also makes the parsing more reliable, as we could tweak the\n + whitespace alignment without breaking this parser. Let's reword a\n + now-inaccurate comment in \"command-list.txt\" describing that previous\n + alignment limitation. We'll still need the \"### command-list [...]\"\n + line due to the \"Documentation/cmd-list.perl\" logic added in\n + 11c6659d85d (command-list: prepare machinery for upcoming \"common\n + groups\" section, 2015-05-21).\n +\n + There was a proposed change subsequent to this one[3] which continued\n + moving more logic into the \"command_list() function, i.e. replaced the\n + \"cut | tr | grep\" chain in \"category_list()\" with an argument to\n + \"command_list()\".\n +\n + That change might have had a bit of an effect, but not as much as the\n + preceding commit, so I decided to drop it. The relevant performance\n + numbers from it were:\n +\n + linux-gcc:\n +\n + [12:05:33] t0001-generate-cmdlist.sh .. ok 13 ms ( 0.00 usr 0.00 sys + 3.33 cusr 2.78 csys = 6.11 CPU)\n + [12:05:33] t0002-generate-cmdlist.sh .. ok 14 ms ( 0.00 usr 0.00 sys + 3.64 cusr 3.09 csys = 6.73 CPU)\n +\n + osx-gcc:\n +\n + [11:58:03] t0001-generate-cmdlist.sh .. ok 78416 ms ( 0.02 usr 0.01 sys + 11.78 cusr 6.22 csys = 18.03 CPU)\n + [11:58:04] t0002-generate-cmdlist.sh .. ok 80081 ms ( 0.02 usr 0.02 sys + 17.80 cusr 10.07 csys = 27.91 CPU)\n +\n + vs-test:\n +\n + [12:03:20] t0001-generate-cmdlist.sh .. ok 34 s ( 0.00 usr 0.03 sys + 12.42 cusr 19.55 csys = 32.00 CPU)\n + [12:03:14] t0002-generate-cmdlist.sh .. ok 30 s ( 0.02 usr 0.00 sys + 13.14 cusr 26.19 csys = 39.35 CPU)\n +\n + As above HEAD~2 and t0002* are testing the code in this commit (and\n + the line is the same), but HEAD~1 and t0001* are testing that dropped\n + change in [3].\n +\n + 1. https://lore.kernel.org/git/cover-v2-00.10-00000000000-20211022T193027Z-avarab@gmail.com/\n + 2. https://lore.kernel.org/git/patch-v2-08.10-83318d6c0da-20211022T193027Z-avarab@gmail.com/\n + 3. https://lore.kernel.org/git/patch-v2-10.10-e10a43756d1-20211022T193027Z-avarab@gmail.com/\n +\n + ## command-list.txt ##\n +@@\n + # specified here, which can only have \"guide\" attribute and nothing\n + # else.\n + #\n +-### command list (do not change this line, also do not change alignment)\n ++### command list (do not change this line)\n + # command name category [category] [category]\n + git-add mainporcelain worktree\n + git-am mainporcelain\n \n ## generate-cmdlist.sh ##\n @@ generate-cmdlist.sh: die () {\n @@ generate-cmdlist.sh: die () {\n +\twhile read cmd rest\n +\tdo\n +\t\tcase \"$cmd\" in\n -+\t\t\"#\"*)\n -+\t\t\tcontinue;\n ++\t\t\"#\"* | '')\n ++\t\t\t# Ignore comments and allow empty lines\n ++\t\t\tcontinue\n +\t\t\t;;\n +\t\t*)\n +\t\t\tcase \"$exclude_programs\" in\n -+\t\t\t\t*\":$cmd:\"*)\n ++\t\t\t*\":$cmd:\"*)\n +\t\t\t\t;;\n +\t\t\t*)\n +\t\t\t\techo \"$cmd $rest\"\n +\t\t\t\t;;\n +\t\t\tesac\n +\t\tesac\n -+\tdone\n ++\tdone <\"$1\"\n }\n \n category_list () {\n --\tcommand_list \"$1\" |\n + \tcommand_list \"$1\" |\n -\tcut -c 40- |\n -+\tcommand_list <\"$1\" |\n +\tcut -d' ' -f2- |\n \ttr ' ' '\\012' |\n \tgrep -v '^$' |\n \tLC_ALL=C sort -u\n -@@ generate-cmdlist.sh: define_category_names () {\n - print_command_list () {\n - \techo \"static struct cmdname_help command_list[] = {\"\n - \n --\tcommand_list \"$1\" |\n -+\tcommand_list <\"$1\" |\n - \twhile read cmd rest\n - \tdo\n - \t\tsynopsis=\n @@ generate-cmdlist.sh: print_command_list () {\n \techo \"};\"\n }\n10: e10a43756d1 < -: ----------- generate-cmdlist.sh: replace \"cut\", \"tr\" and \"grep\" with pure-shell\n -: ----------- > 10: 100084070fd generate-cmdlist.sh: don't parse command-list.txt thrice\n-- \n2.34.0.rc1.721.ga0c1db665bc\n\n"},{"id":"440528","messageId":"patch-v3-02.10-b4b4c3aa135-20211105T135058Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-v3-00.10-00000000000-20211105T135058Z-avarab@gmail.com","subject":"[PATCH v3 02/10] generate-cmdlist.sh: trivial whitespace change","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-05T14:08:00Z","receivedAt":"2021-11-05T14:08:18Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add \" \" before a \"|\" at the end of a line in generate-cmdlist.sh for\nconsistency with other code in the file. Some of the surrounding code\nwill be modified in subsequent commits.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n generate-cmdlist.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex 9dbbb08e70a..5114f46680a 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -10,7 +10,7 @@ command_list () {\n }\n \n get_categories () {\n-\ttr ' ' '\\012'|\n+\ttr ' ' '\\012' |\n \tgrep -v '^$' |\n \tsort |\n \tuniq\n-- \n2.34.0.rc1.721.ga0c1db665bc\n\n"},{"id":"440529","messageId":"patch-v3-03.10-737cca59d99-20211105T135058Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-v3-00.10-00000000000-20211105T135058Z-avarab@gmail.com","subject":"[PATCH v3 03/10] generate-cmdlist.sh: spawn fewer processes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-05T14:08:01Z","receivedAt":"2021-11-05T14:08:22Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Johannes Sixt \n\nThe function get_categories() is invoked in a loop over all commands.\nAs it runs several processes, this takes an awful lot of time on\nWindows. To reduce the number of processes, move the process that\nfilters empty lines to the other invoker of the function, where it is\nneeded. The invocation of get_categories() in the loop does not need\nthe empty line filtered away because the result is word-split by the\nshell, which eliminates the empty line automatically.\n\nFurthermore, use sort -u instead of sort | uniq to remove yet another\nprocess.\n\n[Ævar: on Linux this seems to speed things up a bit, although with\nhyperfine(1) the results are fuzzy enough to land within the\nconfidence interval]:\n\n$ git show HEAD~:generate-cmdlist.sh >generate-cmdlist.sh.old\n$ hyperfine --warmup 1 -L s ,.old -p 'make clean' 'sh generate-cmdlist.sh{s} command-list.txt'\nBenchmark #1: sh generate-cmdlist.sh command-list.txt\n Time (mean ± σ): 371.3 ms ± 64.2 ms [User: 430.4 ms, System: 72.5 ms]\n Range (min … max): 320.5 ms … 517.7 ms 10 runs\n\nBenchmark #2: sh generate-cmdlist.sh.old command-list.txt\n Time (mean ± σ): 489.9 ms ± 185.4 ms [User: 724.7 ms, System: 141.3 ms]\n Range (min … max): 346.0 ms … 885.3 ms 10 runs\n\nSummary\n 'sh generate-cmdlist.sh command-list.txt' ran\n 1.32 ± 0.55 times faster than 'sh generate-cmdlist.sh.old command-list.txt'\n\nSigned-off-by: Johannes Sixt \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n generate-cmdlist.sh | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex 5114f46680a..27367915611 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -11,15 +11,14 @@ command_list () {\n \n get_categories () {\n \ttr ' ' '\\012' |\n-\tgrep -v '^$' |\n-\tsort |\n-\tuniq\n+\tLC_ALL=C sort -u\n }\n \n category_list () {\n \tcommand_list \"$1\" |\n \tcut -c 40- |\n-\tget_categories\n+\tget_categories |\n+\tgrep -v '^$'\n }\n \n get_synopsis () {\n-- \n2.34.0.rc1.721.ga0c1db665bc\n\n"},{"id":"440530","messageId":"patch-v3-04.10-6ad17ab56c2-20211105T135058Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-v3-00.10-00000000000-20211105T135058Z-avarab@gmail.com","subject":"[PATCH v3 04/10] generate-cmdlist.sh: don't call get_categories() from category_list()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-05T14:08:02Z","receivedAt":"2021-11-05T14:08:23Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This isn't for optimization as the get_categories() is a purely shell\nfunction, but rather for ease of readability, let's just inline these\ntwo lines. We'll be changing this code some more in subsequent commits\nto make this worth it.\n\nRename the get_categories() function to get_category_line(), since\nthat's what it's doing now.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n generate-cmdlist.sh | 7 ++++---\n 1 file changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex 27367915611..16043e38476 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -9,7 +9,7 @@ command_list () {\n \teval \"grep -ve '^#' $exclude_programs\" <\"$1\"\n }\n \n-get_categories () {\n+get_category_line () {\n \ttr ' ' '\\012' |\n \tLC_ALL=C sort -u\n }\n@@ -17,7 +17,8 @@ get_categories () {\n category_list () {\n \tcommand_list \"$1\" |\n \tcut -c 40- |\n-\tget_categories |\n+\ttr ' ' '\\012' |\n+\tLC_ALL=C sort -u |\n \tgrep -v '^$'\n }\n \n@@ -66,7 +67,7 @@ print_command_list () {\n \twhile read cmd rest\n \tdo\n \t\tprintf \"\t{ \\\"$cmd\\\", $(get_synopsis $cmd), 0\"\n-\t\tfor cat in $(echo \"$rest\" | get_categories)\n+\t\tfor cat in $(echo \"$rest\" | get_category_line)\n \t\tdo\n \t\t\tprintf \" | CAT_$cat\"\n \t\tdone\n-- \n2.34.0.rc1.721.ga0c1db665bc\n\n"},{"id":"440531","messageId":"patch-v3-05.10-d7be565b567-20211105T135058Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-v3-00.10-00000000000-20211105T135058Z-avarab@gmail.com","subject":"[PATCH v3 05/10] generate-cmdlist.sh: run \"grep | sort\", not \"sort | grep\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-05T14:08:03Z","receivedAt":"2021-11-05T14:08:25Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This doesn't matter for performance, but let's not include the empty\nlines in our sorting. This makes the intent of the code clearer.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n generate-cmdlist.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex 16043e38476..e517c33710a 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -18,8 +18,8 @@ category_list () {\n \tcommand_list \"$1\" |\n \tcut -c 40- |\n \ttr ' ' '\\012' |\n-\tLC_ALL=C sort -u |\n-\tgrep -v '^$'\n+\tgrep -v '^$' |\n+\tLC_ALL=C sort -u\n }\n \n get_synopsis () {\n-- \n2.34.0.rc1.721.ga0c1db665bc\n\n"},{"id":"440532","messageId":"patch-v3-06.10-646363db11f-20211105T135058Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-v3-00.10-00000000000-20211105T135058Z-avarab@gmail.com","subject":"[PATCH v3 06/10] generate-cmdlist.sh: replace for loop by printf's auto-repeat feature","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-05T14:08:04Z","receivedAt":"2021-11-05T14:08:26Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Johannes Sixt \n\nThis is just a small code reduction. There is a small probability that\nthe new code breaks when the category list is empty. But that would be\nnoticed during the compile step.\n\nSigned-off-by: Johannes Sixt \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n generate-cmdlist.sh | 5 +----\n 1 file changed, 1 insertion(+), 4 deletions(-)\n\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex e517c33710a..a1ab2b1f077 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -67,10 +67,7 @@ print_command_list () {\n \twhile read cmd rest\n \tdo\n \t\tprintf \"\t{ \\\"$cmd\\\", $(get_synopsis $cmd), 0\"\n-\t\tfor cat in $(echo \"$rest\" | get_category_line)\n-\t\tdo\n-\t\t\tprintf \" | CAT_$cat\"\n-\t\tdone\n+\t\tprintf \" | CAT_%s\" $(echo \"$rest\" | get_category_line)\n \t\techo \" },\"\n \tdone\n \techo \"};\"\n-- \n2.34.0.rc1.721.ga0c1db665bc\n\n"},{"id":"440533","messageId":"patch-v3-07.10-d8cc7c246b8-20211105T135058Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-v3-00.10-00000000000-20211105T135058Z-avarab@gmail.com","subject":"[PATCH v3 07/10] generate-cmdlist.sh: stop sorting category lines","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-05T14:08:05Z","receivedAt":"2021-11-05T14:08:27Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"In a preceding commit we changed the print_command_list() loop to use\nprintf's auto-repeat feature. Let's now get rid of get_category_line()\nentirely by not sorting the categories.\n\nThis will change the output of the generated code from e.g.:\n\n - { \"git-apply\", N_(\"Apply a patch to files and/or to the index\"), 0 | CAT_complete | CAT_plumbingmanipulators },\n\nTo:\n\n + { \"git-apply\", N_(\"Apply a patch to files and/or to the index\"), 0 | CAT_plumbingmanipulators | CAT_complete },\n\nI.e. the categories are no longer sorted, but as they're OR'd together\nit won't matter for the end result.\n\nThis speeds up the generate-cmdlist.sh a bit. Comparing HEAD~ (old)\nand \"master\" to this code:\n\n 'sh generate-cmdlist.sh command-list.txt' ran\n 1.07 ± 0.33 times faster than 'sh generate-cmdlist.sh.old command-list.txt'\n 1.15 ± 0.36 times faster than 'sh generate-cmdlist.sh.master command-list.txt'\n\nSuggested-by: Jeff King \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n generate-cmdlist.sh | 7 +------\n 1 file changed, 1 insertion(+), 6 deletions(-)\n\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex a1ab2b1f077..f50112c50f8 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -9,11 +9,6 @@ command_list () {\n \teval \"grep -ve '^#' $exclude_programs\" <\"$1\"\n }\n \n-get_category_line () {\n-\ttr ' ' '\\012' |\n-\tLC_ALL=C sort -u\n-}\n-\n category_list () {\n \tcommand_list \"$1\" |\n \tcut -c 40- |\n@@ -67,7 +62,7 @@ print_command_list () {\n \twhile read cmd rest\n \tdo\n \t\tprintf \"\t{ \\\"$cmd\\\", $(get_synopsis $cmd), 0\"\n-\t\tprintf \" | CAT_%s\" $(echo \"$rest\" | get_category_line)\n+\t\tprintf \" | CAT_%s\" $rest\n \t\techo \" },\"\n \tdone\n \techo \"};\"\n-- \n2.34.0.rc1.721.ga0c1db665bc\n\n"},{"id":"440534","messageId":"patch-v3-08.10-aeeecc575fb-20211105T135058Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-v3-00.10-00000000000-20211105T135058Z-avarab@gmail.com","subject":"[PATCH v3 08/10] generate-cmdlist.sh: do not shell out to \"sed\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-05T14:08:06Z","receivedAt":"2021-11-05T14:08:28Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Jeff King \n\nReplace the \"sed\" invocation in get_synopsis() with a pure-shell\nversion. This speeds up generate-cmdlist.sh significantly. Compared to\nHEAD~ (old) and \"master\" we are, according to hyperfine(1):\n\n 'sh generate-cmdlist.sh command-list.txt' ran\n 12.69 ± 5.01 times faster than 'sh generate-cmdlist.sh.old command-list.txt'\n 18.34 ± 3.03 times faster than 'sh generate-cmdlist.sh.master command-list.txt'\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Jeff King \n---\n generate-cmdlist.sh | 23 ++++++++++++-----------\n 1 file changed, 12 insertions(+), 11 deletions(-)\n\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex f50112c50f8..9b7d6aea629 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -17,16 +17,6 @@ category_list () {\n \tLC_ALL=C sort -u\n }\n \n-get_synopsis () {\n-\tsed -n '\n-\t\t/^NAME/,/'\"$1\"'/H\n-\t\t${\n-\t\t\tx\n-\t\t\ts/.*'\"$1\"' - \\(.*\\)/N_(\"\\1\")/\n-\t\t\tp\n-\t\t}' \"Documentation/$1.txt\"\n-}\n-\n define_categories () {\n \techo\n \techo \"/* Command categories */\"\n@@ -61,7 +51,18 @@ print_command_list () {\n \tcommand_list \"$1\" |\n \twhile read cmd rest\n \tdo\n-\t\tprintf \"\t{ \\\"$cmd\\\", $(get_synopsis $cmd), 0\"\n+\t\tsynopsis=\n+\t\twhile read line\n+\t\tdo\n+\t\t\tcase \"$line\" in\n+\t\t\t\"$cmd - \"*)\n+\t\t\t\tsynopsis=${line#$cmd - }\n+\t\t\t\tbreak\n+\t\t\t\t;;\n+\t\t\tesac\n+\t\tdone <\"Documentation/$cmd.txt\"\n+\n+\t\tprintf '\\t{ \"%s\", N_(\"%s\"), 0' \"$cmd\" \"$synopsis\"\n \t\tprintf \" | CAT_%s\" $rest\n \t\techo \" },\"\n \tdone\n-- \n2.34.0.rc1.721.ga0c1db665bc\n\n"},{"id":"440535","messageId":"patch-v3-09.10-e2702bcc1d0-20211105T135058Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-v3-00.10-00000000000-20211105T135058Z-avarab@gmail.com","subject":"[PATCH v3 09/10] generate-cmdlist.sh: replace \"grep' invocation with a shell version","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-05T14:08:07Z","receivedAt":"2021-11-05T14:08:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Replace the \"grep\" we run to exclude certain programs from the\ngenerated output with a pure-shell loop that strips out the comments,\nand sees if the \"cmd\" we're reading is on a list of excluded\nprograms. This uses a trick similar to test_have_prereq() in\ntest-lib-functions.sh.\n\nOn my *nix system this makes things quite a bit slower compared to\nHEAD~:\no\n 'sh generate-cmdlist.sh.old command-list.txt' ran\n 1.56 ± 0.11 times faster than 'sh generate-cmdlist.sh command-list.txt'\n 18.00 ± 0.19 times faster than 'sh generate-cmdlist.sh.master command-list.txt'\n\nBut when I tried running generate-cmdlist.sh 100 times in CI I found\nthat it helped across the board even on OSX & Linux. I tried testing\nit in CI with this ad-hoc few-liner:\n\n for i in $(seq -w 0 11 | sort -nr)\n do\n \tgit show HEAD~$i:generate-cmdlist.sh >generate-cmdlist-HEAD$i.sh &&\n \tgit add generate-cmdlist* &&\n \tcp t/t0000-generate-cmdlist.sh t/t00$i-generate-cmdlist.sh || : &&\n \tperl -pi -e \"s/HEAD0/HEAD$i/g\" t/t00$i-generate-cmdlist.sh &&\n \tgit add t/t00*.sh\n done && git commit -m\"generated it\"\n\nHere HEAD~02 and the t0002* file refers to this change, and HEAD~03\nand t0003* file to the preceding commit, the relevant results were:\n\n linux-gcc:\n\n [12:05:33] t0002-generate-cmdlist.sh .. ok 14 ms ( 0.00 usr 0.00 sys + 3.64 cusr 3.09 csys = 6.73 CPU)\n [12:05:30] t0003-generate-cmdlist.sh .. ok 32 ms ( 0.00 usr 0.00 sys + 2.66 cusr 1.81 csys = 4.47 CPU)\n\n osx-gcc:\n\n [11:58:04] t0002-generate-cmdlist.sh .. ok 80081 ms ( 0.02 usr 0.02 sys + 17.80 cusr 10.07 csys = 27.91 CPU)\n [11:58:16] t0003-generate-cmdlist.sh .. ok 92127 ms ( 0.02 usr 0.01 sys + 22.54 cusr 14.27 csys = 36.84 CPU)\n\n vs-test:\n\n [12:03:14] t0002-generate-cmdlist.sh .. ok 30 s ( 0.02 usr 0.00 sys + 13.14 cusr 26.19 csys = 39.35 CPU)\n [12:03:20] t0003-generate-cmdlist.sh .. ok 32 s ( 0.00 usr 0.02 sys + 13.25 cusr 26.10 csys = 39.37 CPU)\n\nI.e. even on *nix running 100 of these in a loop was up to ~2x faster\nin absolute runtime, I suspect it's due factors that are exacerbated\nin the CI, e.g. much slower process startup due to some platform\nlimits, or a slower FS.\n\nThe \"cut -d\" change here is because we're not emitting the\n40-character aligned output anymore, i.e. we'll get the output from\ncommand_list() now, not an as-is line from command-list.txt.\n\nThis also makes the parsing more reliable, as we could tweak the\nwhitespace alignment without breaking this parser. Let's reword a\nnow-inaccurate comment in \"command-list.txt\" describing that previous\nalignment limitation. We'll still need the \"### command-list [...]\"\nline due to the \"Documentation/cmd-list.perl\" logic added in\n11c6659d85d (command-list: prepare machinery for upcoming \"common\ngroups\" section, 2015-05-21).\n\nThere was a proposed change subsequent to this one[3] which continued\nmoving more logic into the \"command_list() function, i.e. replaced the\n\"cut | tr | grep\" chain in \"category_list()\" with an argument to\n\"command_list()\".\n\nThat change might have had a bit of an effect, but not as much as the\npreceding commit, so I decided to drop it. The relevant performance\nnumbers from it were:\n\n linux-gcc:\n\n [12:05:33] t0001-generate-cmdlist.sh .. ok 13 ms ( 0.00 usr 0.00 sys + 3.33 cusr 2.78 csys = 6.11 CPU)\n [12:05:33] t0002-generate-cmdlist.sh .. ok 14 ms ( 0.00 usr 0.00 sys + 3.64 cusr 3.09 csys = 6.73 CPU)\n\n osx-gcc:\n\n [11:58:03] t0001-generate-cmdlist.sh .. ok 78416 ms ( 0.02 usr 0.01 sys + 11.78 cusr 6.22 csys = 18.03 CPU)\n [11:58:04] t0002-generate-cmdlist.sh .. ok 80081 ms ( 0.02 usr 0.02 sys + 17.80 cusr 10.07 csys = 27.91 CPU)\n\n vs-test:\n\n [12:03:20] t0001-generate-cmdlist.sh .. ok 34 s ( 0.00 usr 0.03 sys + 12.42 cusr 19.55 csys = 32.00 CPU)\n [12:03:14] t0002-generate-cmdlist.sh .. ok 30 s ( 0.02 usr 0.00 sys + 13.14 cusr 26.19 csys = 39.35 CPU)\n\nAs above HEAD~2 and t0002* are testing the code in this commit (and\nthe line is the same), but HEAD~1 and t0001* are testing that dropped\nchange in [3].\n\n1. https://lore.kernel.org/git/cover-v2-00.10-00000000000-20211022T193027Z-avarab@gmail.com/\n2. https://lore.kernel.org/git/patch-v2-08.10-83318d6c0da-20211022T193027Z-avarab@gmail.com/\n3. https://lore.kernel.org/git/patch-v2-10.10-e10a43756d1-20211022T193027Z-avarab@gmail.com/\n---\n command-list.txt | 2 +-\n generate-cmdlist.sh | 24 ++++++++++++++++++++----\n 2 files changed, 21 insertions(+), 5 deletions(-)\n\ndiff --git a/command-list.txt b/command-list.txt\nindex 04cde20c3da..675c28f0bd0 100644\n--- a/command-list.txt\n+++ b/command-list.txt\n@@ -43,7 +43,7 @@\n # specified here, which can only have \"guide\" attribute and nothing\n # else.\n #\n-### command list (do not change this line, also do not change alignment)\n+### command list (do not change this line)\n # command name category [category] [category]\n git-add mainporcelain worktree\n git-am mainporcelain\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex 9b7d6aea629..cfe0454d1de 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -6,12 +6,28 @@ die () {\n }\n \n command_list () {\n-\teval \"grep -ve '^#' $exclude_programs\" <\"$1\"\n+\twhile read cmd rest\n+\tdo\n+\t\tcase \"$cmd\" in\n+\t\t\"#\"* | '')\n+\t\t\t# Ignore comments and allow empty lines\n+\t\t\tcontinue\n+\t\t\t;;\n+\t\t*)\n+\t\t\tcase \"$exclude_programs\" in\n+\t\t\t*\":$cmd:\"*)\n+\t\t\t\t;;\n+\t\t\t*)\n+\t\t\t\techo \"$cmd $rest\"\n+\t\t\t\t;;\n+\t\t\tesac\n+\t\tesac\n+\tdone <\"$1\"\n }\n \n category_list () {\n \tcommand_list \"$1\" |\n-\tcut -c 40- |\n+\tcut -d' ' -f2- |\n \ttr ' ' '\\012' |\n \tgrep -v '^$' |\n \tLC_ALL=C sort -u\n@@ -69,11 +85,11 @@ print_command_list () {\n \techo \"};\"\n }\n \n-exclude_programs=\n+exclude_programs=:\n while test \"--exclude-program\" = \"$1\"\n do\n \tshift\n-\texclude_programs=\"$exclude_programs -e \\\"^$1 \\\"\"\n+\texclude_programs=\"$exclude_programs$1:\"\n \tshift\n done\n \n-- \n2.34.0.rc1.721.ga0c1db665bc\n\n"},{"id":"440536","messageId":"patch-v3-10.10-100084070fd-20211105T135058Z-avarab@gmail.com","threadId":"56000","inReplyTo":"cover-v3-00.10-00000000000-20211105T135058Z-avarab@gmail.com","subject":"[PATCH v3 10/10] generate-cmdlist.sh: don't parse command-list.txt thrice","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-05T14:08:08Z","receivedAt":"2021-11-05T14:08:30Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the \"define_categories()\" and \"define_category_names()\" functions\nto take the already-parsed output of \"category_list()\" as an argument,\nwhich brings our number of passes over \"command-list.txt\" from three\nto two.\n\nThen have \"category_list()\" itself take the output of \"command_list()\"\nas an argument, bringing the number of times we parse the file to one.\n\nCompared to the pre-image this speeds us up quite a bit:\n\n $ git show HEAD~:generate-cmdlist.sh >generate-cmdlist.sh.old\n $ hyperfine --warmup 10 -L v ,.old 'sh generate-cmdlist.sh{v} command-list.txt'\n Benchmark #1: sh generate-cmdlist.sh command-list.txt\n Time (mean ± σ): 22.9 ms ± 0.3 ms [User: 15.8 ms, System: 9.6 ms]\n Range (min … max): 22.5 ms … 24.0 ms 125 runs\n\n Benchmark #2: sh generate-cmdlist.sh.old command-list.txt\n Time (mean ± σ): 30.1 ms ± 0.4 ms [User: 24.4 ms, System: 17.5 ms]\n Range (min … max): 29.5 ms … 32.3 ms 96 runs\n\n Summary\n 'sh generate-cmdlist.sh command-list.txt' ran\n 1.32 ± 0.02 times faster than 'sh generate-cmdlist.sh.old command-list.txt'\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n generate-cmdlist.sh | 17 ++++++++++-------\n 1 file changed, 10 insertions(+), 7 deletions(-)\n\ndiff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\nindex cfe0454d1de..205541e0f7f 100755\n--- a/generate-cmdlist.sh\n+++ b/generate-cmdlist.sh\n@@ -26,7 +26,7 @@ command_list () {\n }\n \n category_list () {\n-\tcommand_list \"$1\" |\n+\techo \"$1\" |\n \tcut -d' ' -f2- |\n \ttr ' ' '\\012' |\n \tgrep -v '^$' |\n@@ -37,7 +37,7 @@ define_categories () {\n \techo\n \techo \"/* Command categories */\"\n \tbit=0\n-\tcategory_list \"$1\" |\n+\techo \"$1\" |\n \twhile read cat\n \tdo\n \t\techo \"#define CAT_$cat (1UL << $bit)\"\n@@ -51,7 +51,7 @@ define_category_names () {\n \techo \"/* Category names */\"\n \techo \"static const char *category_names[] = {\"\n \tbit=0\n-\tcategory_list \"$1\" |\n+\techo \"$1\" |\n \twhile read cat\n \tdo\n \t\techo \"\t\\\"$cat\\\", /* (1UL << $bit) */\"\n@@ -64,7 +64,7 @@ define_category_names () {\n print_command_list () {\n \techo \"static struct cmdname_help command_list[] = {\"\n \n-\tcommand_list \"$1\" |\n+\techo \"$1\" |\n \twhile read cmd rest\n \tdo\n \t\tsynopsis=\n@@ -93,6 +93,9 @@ do\n \tshift\n done\n \n+commands=\"$(command_list \"$1\")\"\n+categories=\"$(category_list \"$commands\")\"\n+\n echo \"/* Automatically generated by generate-cmdlist.sh */\n struct cmdname_help {\n \tconst char *name;\n@@ -100,8 +103,8 @@ struct cmdname_help {\n \tuint32_t category;\n };\n \"\n-define_categories \"$1\"\n+define_categories \"$categories\"\n echo\n-define_category_names \"$1\"\n+define_category_names \"$categories\"\n echo\n-print_command_list \"$1\"\n+print_command_list \"$commands\"\n-- \n2.34.0.rc1.721.ga0c1db665bc\n\n"},{"id":"440557","messageId":"xmqqbl2yw7b6.fsf@gitster.g","threadId":"56000","inReplyTo":"patch-v3-01.10-c385e84c04c-20211105T135058Z-avarab@gmail.com","subject":"Re: [PATCH v3 01/10] command-list.txt: sort with \"LC_ALL=C sort\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-05T22:45:17Z","receivedAt":"2021-11-05T22:45:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> In a subsequent commit I'll change generate-cmdlist.sh to use C sort\n> order, and without this change we'd be led to believe that that change\n> caused a meaningful change in the output,\n\nWhat I found misleading in this sentence (which hasn't changed after\nI pointed it out in the previous iterations) is that\ncommand-list.txt is an input file, and if the sort order used in the\nscript that reads this to produce some other file as its output\nchanges, nobody will be \"led to believe\" anything. Not unless/until\nwhich output file to look at and compare between revisions.\n\nIs this talking about the order of entries in command-list.h file?\n\nAlso, if the script sorts the input, whether it is in C locale or\nother locale, it would not matter how the input originally is\nsorted, as the input does not have duplicated entries to make sort\nstability matter, no?\n"},{"id":"440558","messageId":"xmqq7ddmw77y.fsf@gitster.g","threadId":"56000","inReplyTo":"patch-v3-03.10-737cca59d99-20211105T135058Z-avarab@gmail.com","subject":"Re: [PATCH v3 03/10] generate-cmdlist.sh: spawn fewer processes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-05T22:47:13Z","receivedAt":"2021-11-05T22:47:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\n> index 5114f46680a..27367915611 100755\n> --- a/generate-cmdlist.sh\n> +++ b/generate-cmdlist.sh\n> @@ -11,15 +11,14 @@ command_list () {\n> \n> get_categories () {\n> \ttr ' ' '\\012' |\n> -\tgrep -v '^$' |\n> -\tsort |\n> -\tuniq\n> +\tLC_ALL=C sort -u\n> }\n> \n> category_list () {\n> \tcommand_list \"$1\" |\n> \tcut -c 40- |\n> -\tget_categories\n> +\tget_categories |\n> +\tgrep -v '^$'\n> }\n\nIt is funny that this changes \"grep then sort\" into \"sort then\ngrep\", which will be \"corrected\" in two steps down. The series\nseems a bit over-engineered and broken down too much, at least to\nme, but let's not waste any more time on it by an extra reroll.\n\n"},{"id":"440566","messageId":"211106.86bl2xhpxf.gmgdl@evledraar.gmail.com","threadId":"56000","inReplyTo":"xmqq7ddmw77y.fsf@gitster.g","subject":"Re: [PATCH v3 03/10] generate-cmdlist.sh: spawn fewer processes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-06T04:23:05Z","receivedAt":"2021-11-06T04:24:32Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Nov 05 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh\n>> index 5114f46680a..27367915611 100755\n>> --- a/generate-cmdlist.sh\n>> +++ b/generate-cmdlist.sh\n>> @@ -11,15 +11,14 @@ command_list () {\n>> \n>> get_categories () {\n>> \ttr ' ' '\\012' |\n>> -\tgrep -v '^$' |\n>> -\tsort |\n>> -\tuniq\n>> +\tLC_ALL=C sort -u\n>> }\n>> \n>> category_list () {\n>> \tcommand_list \"$1\" |\n>> \tcut -c 40- |\n>> -\tget_categories\n>> +\tget_categories |\n>> +\tgrep -v '^$'\n>> }\n>\n> It is funny that this changes \"grep then sort\" into \"sort then\n> grep\", which will be \"corrected\" in two steps down. The series\n> seems a bit over-engineered and broken down too much, at least to\n> me, but let's not waste any more time on it by an extra reroll.\n\nYes, it's a bit of back and forth, but I didn't want to outright drop\nJohannes's patches which I'd integrated here, and thought it would be\nhelpful to others to distill the history of various optimization steps\n(starting with Johannes's work here) into the permanent commit history.\n"},{"id":"440567","messageId":"211106.867ddlhp9j.gmgdl@evledraar.gmail.com","threadId":"56000","inReplyTo":"xmqqbl2yw7b6.fsf@gitster.g","subject":"Re: [PATCH v3 01/10] command-list.txt: sort with \"LC_ALL=C sort\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-06T04:26:04Z","receivedAt":"2021-11-06T04:38:54Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Nov 05 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> In a subsequent commit I'll change generate-cmdlist.sh to use C sort\n>> order, and without this change we'd be led to believe that that change\n>> caused a meaningful change in the output,\n>\n> What I found misleading in this sentence (which hasn't changed after\n> I pointed it out in the previous iterations) is that[...]\n\nI tried to clarify what you raised in the previous iteration with the\nnew paragraph after the one you're quoting. I.e.:\n\n Note that this refers to the sort order of the lines in\n command-list.txt, a subsequent commit will also change how we treat\n the sort order of the \"category\" fields, but that's unrelated to this\n change.\n\n> command-list.txt is an input file, and if the sort order used in the\n> script that reads this to produce some other file as its output\n> changes, nobody will be \"led to believe\" anything. Not unless/until\n> which output file to look at and compare between revisions.\n\nI'd read your comment on the previous iteration as you being unclear on\nwhether we were changing the sort order of lines in the file, or the\ncategory fields found within those lines.\n\nWe don't sort the lines, and never have, just the fields, and within\nthis series we stop doing that sorting (as it ends up in an OR'd\nbitfield, so it doesn't matter).\n\n> Is this talking about the order of entries in command-list.h file?\n>\n> Also, if the script sorts the input, whether it is in C locale or\n> other locale, it would not matter how the input originally is\n> sorted, as the input does not have duplicated entries to make sort\n> stability matter, no?\n\nThis change is just to make this consistent for human editors. I think\nwe re-sort this wherever we display this in git, whether that's via\nhelp.c or the completion powered by git.c.\n"},{"id":"440676","messageId":"xmqq35o6v4ky.fsf@gitster.g","threadId":"56000","inReplyTo":"211106.867ddlhp9j.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v3 01/10] command-list.txt: sort with \"LC_ALL=C sort\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-08T19:18:37Z","receivedAt":"2021-11-08T19:18:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> On Fri, Nov 05 2021, Junio C Hamano wrote:\n>\n>> Ævar Arnfjörð Bjarmason writes:\n>>\n>>> In a subsequent commit I'll change generate-cmdlist.sh to use C sort\n>>> order, and without this change we'd be led to believe that that change\n>>> caused a meaningful change in the output,\n>>\n>> What I found misleading in this sentence (which hasn't changed after\n>> I pointed it out in the previous iterations) is that[...]\n>\n> I tried to clarify what you raised in the previous iteration with the\n> new paragraph after the one you're quoting. I.e.:\n>\n> Note that this refers to the sort order of the lines in\n> command-list.txt, a subsequent commit will also change how we treat\n> the sort order of the \"category\" fields, but that's unrelated to this\n> change.\n\nYeah, but my question was not about the order of category tokens on\neach line. My question was the order of these lines in the file.\n\nThe new pargraph is answering a question that wasn't asked.\n\n>> Is this talking about the order of entries in command-list.h file?\n>>\n>> Also, if the script sorts the input, whether it is in C locale or\n>> other locale, it would not matter how the input originally is\n>> sorted, as the input does not have duplicated entries to make sort\n>> stability matter, no?\n>\n> This change is just to make this consistent for human editors. I think\n> we re-sort this wherever we display this in git, whether that's via\n> help.c or the completion powered by git.c.\n\nSo\n\n>> order, and without this change we'd be led to believe that that change\n>> caused a meaningful change in the output,\n\nis something irrelevant to explain what this change is, no?\n"}]}