{"thread":{"id":"61151","subject":"[PATCH v2 1/5] MyFirstObjectWalk: use additional arg in config_fn_t","startedAt":"2024-03-19T11:24:10Z","lastAt":"2024-03-27T11:23:16Z","messageCount":49,"participants":["Dirk Gouders","Kyle Lippincott","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":5},"messages":[{"id":"490925","messageId":"cover.1710840596.git.dirk@gouders.net","threadId":"61151","inReplyTo":"cover.1710192973.git.dirk@gouders.net","subject":"[PATCH v2 0/5] Fixes for Documentation/MyFirstObjectWalk.txt","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-19T11:23:10Z","receivedAt":"2024-03-19T11:24:10Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"The second spin for this series.\n---\nChanges since v1:\n* Added Emily to Cc in the hope for a review\n* Remove superfluous tags from [1/5] and [3/5]\n* Replace bashism `|&` by `2>&1 |` in [5/5]\n---\nDirk Gouders (5):\n MyFirstObjectWalk: use additional arg in config_fn_t\n MyFirstObjectWalk: fix misspelled \"builtins/\"\n MyFirstObjectWalk: fix filtered object walk\n MyFirstObjectWalk: fix description for counting omitted objects\n MyFirstObjectWalk: add stderr to pipe processing\n\n Documentation/MyFirstObjectWalk.txt | 36 ++++++++++++++++-------------\n 1 file changed, 20 insertions(+), 16 deletions(-)\n\nRange-diff against v1:\n1: 15b74566e0 ! 1: babf04295e MyFirstObjectWalk: use additional arg in config_fn_t\n @@ Commit message\n Fix those calls and the example git_walken_config() to use\n that additional argument.\n \n - Fixes: a4e7e317 (config: add ctx arg to config_fn_t)\n - Cc: Glen Choo \n Signed-off-by: Dirk Gouders \n \n ## Documentation/MyFirstObjectWalk.txt ##\n2: c1ac705840 = 2: ab0b820df7 MyFirstObjectWalk: fix misspelled \"builtins/\"\n3: 0f67a161ef ! 3: fac6886af3 MyFirstObjectWalk: fix filtered object walk\n @@ Commit message\n rev->filter to parse_list_objects_filter() in accordance to\n such a call in revisions.c, for example.\n \n - Fixes: f0d2f849 (MyFirstObjectWalk: update recommended usage)\n - Cc: Derrick Stolee \n Signed-off-by: Dirk Gouders \n \n ## Documentation/MyFirstObjectWalk.txt ##\n4: 637070dd48 = 4: 33a1845889 MyFirstObjectWalk: fix description for counting omitted objects\n5: a2d30eff21 ! 5: 64c36dbf16 MyFirstObjectWalk: add stderr to pipe processing\n @@ Commit message\n trace messages are sent to stderr if GIT_TRACE is set to '1', so those\n commands do not produce the described results.\n \n - Fix this by using the operator '|&' to additionally connect stderr to\n - stdin of the latter command.\n + Fix this by redirecting stderr to stdout prior to the pipe operator\n + to additionally connect stderr to stdin of the latter command.\n \n Signed-off-by: Dirk Gouders \n \n @@ Documentation/MyFirstObjectWalk.txt: those lines without having to recompile.\n \n ----\n -$ GIT_TRACE=1 ./bin-wrappers/git walken | head -n 10\n -+$ GIT_TRACE=1 ./bin-wrappers/git walken |& head -n 10\n ++$ GIT_TRACE=1 ./bin-wrappers/git walken 2>&1 | head -n 10\n ----\n \n Take a look at the top commit with `git show` and the object ID you printed; it\n @@ Documentation/MyFirstObjectWalk.txt: of the first handful:\n ----\n $ make\n -$ GIT_TRACE=1 ./bin-wrappers git walken | tail -n 10\n -+$ GIT_TRACE=1 ./bin-wrappers git walken |& tail -n 10\n ++$ GIT_TRACE=1 ./bin-wrappers git walken 2>&1 | tail -n 10\n ----\n \n The last commit object given should have the same OID as the one we saw at the\n-- \n2.43.0\n\n"},{"id":"490924","messageId":"babf04295ee461a5c37574a032c6bc105bb1f31e.1710840596.git.dirk@gouders.net","threadId":"61151","inReplyTo":"cover.1710840596.git.dirk@gouders.net","subject":"[PATCH v2 1/5] MyFirstObjectWalk: use additional arg in config_fn_t","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-19T11:23:11Z","receivedAt":"2024-03-19T11:24:11Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"Commit a4e7e317 (config: add ctx arg to config_fn_t) added a fourth\nargument to config_fn_t but did not change relevant function calls\nin Documentation/MyFirstObjectWalk.txt.\n\nFix those calls and the example git_walken_config() to use\nthat additional argument.\n\nSigned-off-by: Dirk Gouders \n---\n Documentation/MyFirstObjectWalk.txt | 12 +++++++-----\n 1 file changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\nindex c68cdb11b9..cceac2df95 100644\n--- a/Documentation/MyFirstObjectWalk.txt\n+++ b/Documentation/MyFirstObjectWalk.txt\n@@ -210,13 +210,14 @@ We'll also need to include the `config.h` header:\n \n ...\n \n-static int git_walken_config(const char *var, const char *value, void *cb)\n+static int git_walken_config(const char *var, const char *value,\n+\t\t\t const struct config_context *ctx, void *cb)\n {\n \t/*\n \t * For now, we don't have any custom configuration, so fall back to\n \t * the default config.\n \t */\n-\treturn git_default_config(var, value, cb);\n+\treturn git_default_config(var, value, ctx, cb);\n }\n ----\n \n@@ -389,10 +390,11 @@ modifying `rev_info.grep_filter`, which is a `struct grep_opt`.\n First some setup. Add `grep_config()` to `git_walken_config()`:\n \n ----\n-static int git_walken_config(const char *var, const char *value, void *cb)\n+static int git_walken_config(const char *var, const char *value,\n+\t\t\t const struct config_context *ctx, void *cb)\n {\n-\tgrep_config(var, value, cb);\n-\treturn git_default_config(var, value, cb);\n+\tgrep_config(var, value, ctx, cb);\n+\treturn git_default_config(var, value, ctx, cb);\n }\n ----\n \n-- \n2.43.0\n\n"},{"id":"490926","messageId":"ab0b820df7ea2bd15f6c4abdfd0964f931b86791.1710840596.git.dirk@gouders.net","threadId":"61151","inReplyTo":"cover.1710840596.git.dirk@gouders.net","subject":"[PATCH v2 2/5] MyFirstObjectWalk: fix misspelled \"builtins/\"","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-19T11:23:12Z","receivedAt":"2024-03-19T11:24:15Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"pack-objects.c resides in builtin/ (not builtins/).\n\nFix the misspelled directory name.\n\nSigned-off-by: Dirk Gouders \n---\n Documentation/MyFirstObjectWalk.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\nindex cceac2df95..c33d22ae99 100644\n--- a/Documentation/MyFirstObjectWalk.txt\n+++ b/Documentation/MyFirstObjectWalk.txt\n@@ -525,7 +525,7 @@ about each one.\n \n We can base our work on an example. `git pack-objects` prepares all kinds of\n objects for packing into a bitmap or packfile. The work we are interested in\n-resides in `builtins/pack-objects.c:get_object_list()`; examination of that\n+resides in `builtin/pack-objects.c:get_object_list()`; examination of that\n function shows that the all-object walk is being performed by\n `traverse_commit_list()` or `traverse_commit_list_filtered()`. Those two\n functions reside in `list-objects.c`; examining the source shows that, despite\n-- \n2.43.0\n\n"},{"id":"490927","messageId":"fac6886af36c3b99e8e590dd916c09beb7f397cd.1710840596.git.dirk@gouders.net","threadId":"61151","inReplyTo":"cover.1710840596.git.dirk@gouders.net","subject":"[PATCH v2 3/5] MyFirstObjectWalk: fix filtered object walk","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-19T11:23:13Z","receivedAt":"2024-03-19T11:24:20Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"Commit f0d2f849 (MyFirstObjectWalk: update recommended usage)\nchanged a call of parse_list_objects_filter() in a way that\nprobably never worked: parse_list_objects_filter() always needed a\npointer as its first argument.\n\nFix this by removing the CALLOC_ARRAY and passing the address of\nrev->filter to parse_list_objects_filter() in accordance to\nsuch a call in revisions.c, for example.\n\nSigned-off-by: Dirk Gouders \n---\n Documentation/MyFirstObjectWalk.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\nindex c33d22ae99..a06c712e46 100644\n--- a/Documentation/MyFirstObjectWalk.txt\n+++ b/Documentation/MyFirstObjectWalk.txt\n@@ -734,8 +734,8 @@ walk we've just performed:\n \t} else {\n \t\ttrace_printf(\n \t\t\t_(\"Filtered object walk with filterspec 'tree:1'.\\n\"));\n-\t\tCALLOC_ARRAY(rev->filter, 1);\n-\t\tparse_list_objects_filter(rev->filter, \"tree:1\");\n+\n+\t\tparse_list_objects_filter(&rev->filter, \"tree:1\");\n \t}\n \ttraverse_commit_list(rev, walken_show_commit,\n \t\t\t walken_show_object, NULL);\n-- \n2.43.0\n\n"},{"id":"490928","messageId":"33a18458891259565e553ab39301108ce642d02f.1710840596.git.dirk@gouders.net","threadId":"61151","inReplyTo":"cover.1710840596.git.dirk@gouders.net","subject":"[PATCH v2 4/5] MyFirstObjectWalk: fix description for counting omitted objects","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-19T11:23:14Z","receivedAt":"2024-03-19T11:24:25Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"Before the changes to count omitted objects, the function\ntraverse_commit_list() was used and its call cannot be changed to pass\na pointer to an oidset to record omitted objects.\n\nFix the text to clarify that we now use another traversal function to\nbe able to pass the pointer to the introduced oidset.\n\nSigned-off-by: Dirk Gouders \n---\n Documentation/MyFirstObjectWalk.txt | 14 ++++++++------\n 1 file changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\nindex a06c712e46..981dbf917b 100644\n--- a/Documentation/MyFirstObjectWalk.txt\n+++ b/Documentation/MyFirstObjectWalk.txt\n@@ -754,10 +754,11 @@ points to the same tree object as its grandparent.)\n === Counting Omitted Objects\n \n We also have the capability to enumerate all objects which were omitted by a\n-filter, like with `git log --filter= --filter-print-omitted`. Asking\n-`traverse_commit_list_filtered()` to populate the `omitted` list means that our\n-object walk does not perform any better than an unfiltered object walk; all\n-reachable objects are walked in order to populate the list.\n+filter, like with `git log --filter= --filter-print-omitted`. We\n+can ask `traverse_commit_list_filtered()` to populate the `omitted`\n+list which means that our object walk does not perform any better than\n+an unfiltered object walk; all reachable objects are walked in order\n+to populate the list.\n \n First, add the `struct oidset` and related items we will use to iterate it:\n \n@@ -778,8 +779,9 @@ static void walken_object_walk(\n \t...\n ----\n \n-Modify the call to `traverse_commit_list_filtered()` to include your `omitted`\n-object:\n+You need to replace the call to `traverse_commit_list()` to\n+`traverse_commit_list_filtered()` to be able to pass a pointer to the\n+oidset defined and initialized above:\n \n ----\n \t...\n-- \n2.43.0\n\n"},{"id":"490929","messageId":"64c36dbf16108353635a7315a3bd5eb60f2aa92e.1710840596.git.dirk@gouders.net","threadId":"61151","inReplyTo":"cover.1710840596.git.dirk@gouders.net","subject":"[PATCH v2 5/5] MyFirstObjectWalk: add stderr to pipe processing","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-19T11:23:15Z","receivedAt":"2024-03-19T11:24:31Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"In the last chapter of this document, pipes are used in commands to\nfilter out the first/last trace messages. But according to git(1),\ntrace messages are sent to stderr if GIT_TRACE is set to '1', so those\ncommands do not produce the described results.\n\nFix this by redirecting stderr to stdout prior to the pipe operator\nto additionally connect stderr to stdin of the latter command.\n\nSigned-off-by: Dirk Gouders \n---\n Documentation/MyFirstObjectWalk.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\nindex 981dbf917b..2e6ae4d7fc 100644\n--- a/Documentation/MyFirstObjectWalk.txt\n+++ b/Documentation/MyFirstObjectWalk.txt\n@@ -847,7 +847,7 @@ those lines without having to recompile.\n With only that change, run again (but save yourself some scrollback):\n \n ----\n-$ GIT_TRACE=1 ./bin-wrappers/git walken | head -n 10\n+$ GIT_TRACE=1 ./bin-wrappers/git walken 2>&1 | head -n 10\n ----\n \n Take a look at the top commit with `git show` and the object ID you printed; it\n@@ -875,7 +875,7 @@ of the first handful:\n \n ----\n $ make\n-$ GIT_TRACE=1 ./bin-wrappers git walken | tail -n 10\n+$ GIT_TRACE=1 ./bin-wrappers git walken 2>&1 | tail -n 10\n ----\n \n The last commit object given should have the same OID as the one we saw at the\n-- \n2.43.0\n\n"},{"id":"491314","messageId":"2jc6piaupfi33npe6kpjo6ij46ioyofoimdybc7budsgmvyp7b@ttbhwbyv6xgz","threadId":"61151","inReplyTo":"babf04295ee461a5c37574a032c6bc105bb1f31e.1710840596.git.dirk@gouders.net","subject":"Re: [PATCH v2 1/5] MyFirstObjectWalk: use additional arg in config_fn_t","fromName":"Kyle Lippincott","fromEmail":"spectral@google.com","sentAt":"2024-03-23T19:28:09Z","receivedAt":"2024-03-23T19:28:14Z","isPatch":true,"sender":{"key":"spectral@google.com","avatar":"https://avatars.githubusercontent.com/u/6371650?v=4"},"body":"On Tue, Mar 19, 2024 at 12:23:11PM +0100, Dirk Gouders wrote:\n> Commit a4e7e317 (config: add ctx arg to config_fn_t) added a fourth\n> argument to config_fn_t but did not change relevant function calls\n> in Documentation/MyFirstObjectWalk.txt.\n> \n> Fix those calls and the example git_walken_config() to use\n> that additional argument.\n> \n> Signed-off-by: Dirk Gouders \n> ---\n> Documentation/MyFirstObjectWalk.txt | 12 +++++++-----\n> 1 file changed, 7 insertions(+), 5 deletions(-)\n\nLooks good, thanks.\n\n> \n> diff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\n> index c68cdb11b9..cceac2df95 100644\n> --- a/Documentation/MyFirstObjectWalk.txt\n> +++ b/Documentation/MyFirstObjectWalk.txt\n> @@ -210,13 +210,14 @@ We'll also need to include the `config.h` header:\n> \n> ...\n> \n> -static int git_walken_config(const char *var, const char *value, void *cb)\n> +static int git_walken_config(const char *var, const char *value,\n> +\t\t\t const struct config_context *ctx, void *cb)\n> {\n> \t/*\n> \t * For now, we don't have any custom configuration, so fall back to\n> \t * the default config.\n> \t */\n> -\treturn git_default_config(var, value, cb);\n> +\treturn git_default_config(var, value, ctx, cb);\n> }\n> ----\n> \n> @@ -389,10 +390,11 @@ modifying `rev_info.grep_filter`, which is a `struct grep_opt`.\n> First some setup. Add `grep_config()` to `git_walken_config()`:\n> \n> ----\n> -static int git_walken_config(const char *var, const char *value, void *cb)\n> +static int git_walken_config(const char *var, const char *value,\n> +\t\t\t const struct config_context *ctx, void *cb)\n> {\n> -\tgrep_config(var, value, cb);\n> -\treturn git_default_config(var, value, cb);\n> +\tgrep_config(var, value, ctx, cb);\n> +\treturn git_default_config(var, value, ctx, cb);\n> }\n> ----\n> \n> -- \n> 2.43.0\n> \n> \n"},{"id":"491316","messageId":"bwm73ljwyva36idgouda53qlm7hefkpnt7nomlc5d3c2zje47g@cwkt4dtmx7le","threadId":"61151","inReplyTo":"64c36dbf16108353635a7315a3bd5eb60f2aa92e.1710840596.git.dirk@gouders.net","subject":"Re: [PATCH v2 5/5] MyFirstObjectWalk: add stderr to pipe processing","fromName":"Kyle Lippincott","fromEmail":"spectral@google.com","sentAt":"2024-03-23T19:48:35Z","receivedAt":"2024-03-23T19:48:39Z","isPatch":true,"sender":{"key":"spectral@google.com","avatar":"https://avatars.githubusercontent.com/u/6371650?v=4"},"body":"On Tue, Mar 19, 2024 at 12:23:15PM +0100, Dirk Gouders wrote:\n> In the last chapter of this document, pipes are used in commands to\n> filter out the first/last trace messages. But according to git(1),\n> trace messages are sent to stderr if GIT_TRACE is set to '1', so those\n> commands do not produce the described results.\n> \n> Fix this by redirecting stderr to stdout prior to the pipe operator\n> to additionally connect stderr to stdin of the latter command.\n> \n> Signed-off-by: Dirk Gouders \n> ---\n> Documentation/MyFirstObjectWalk.txt | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\n> index 981dbf917b..2e6ae4d7fc 100644\n> --- a/Documentation/MyFirstObjectWalk.txt\n> +++ b/Documentation/MyFirstObjectWalk.txt\n> @@ -847,7 +847,7 @@ those lines without having to recompile.\n> With only that change, run again (but save yourself some scrollback):\n> \n> ----\n> -$ GIT_TRACE=1 ./bin-wrappers/git walken | head -n 10\n> +$ GIT_TRACE=1 ./bin-wrappers/git walken 2>&1 | head -n 10\n> ----\n> \n> Take a look at the top commit with `git show` and the object ID you printed; it\n> @@ -875,7 +875,7 @@ of the first handful:\n> \n> ----\n> $ make\n> -$ GIT_TRACE=1 ./bin-wrappers git walken | tail -n 10\n> +$ GIT_TRACE=1 ./bin-wrappers git walken 2>&1 | tail -n 10\n\nI think there's a second issue here: this should be `./bin-wrappers/git`, right?\n\n> ----\n> \n> The last commit object given should have the same OID as the one we saw at the\n> -- \n> 2.43.0\n> \n> \n"},{"id":"491318","messageId":"ghcyrkzot1.fsf@gouders.net","threadId":"61151","inReplyTo":"bwm73ljwyva36idgouda53qlm7hefkpnt7nomlc5d3c2zje47g@cwkt4dtmx7le","subject":"Re: [PATCH v2 5/5] MyFirstObjectWalk: add stderr to pipe processing","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-23T20:16:42Z","receivedAt":"2024-03-23T20:17:05Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"Kyle Lippincott writes:\n\n> On Tue, Mar 19, 2024 at 12:23:15PM +0100, Dirk Gouders wrote:\n\n>> -$ GIT_TRACE=1 ./bin-wrappers/git walken | head -n 10\n>> +$ GIT_TRACE=1 ./bin-wrappers/git walken 2>&1 | head -n 10\n>> ----\n>> \n>> Take a look at the top commit with `git show` and the object ID you printed; it\n>> @@ -875,7 +875,7 @@ of the first handful:\n>> \n>> ----\n>> $ make\n>> -$ GIT_TRACE=1 ./bin-wrappers git walken | tail -n 10\n>> +$ GIT_TRACE=1 ./bin-wrappers git walken 2>&1 | tail -n 10\n>\n> I think there's a second issue here: this should be `./bin-wrappers/git`, right?\n\nOh yes, that is a second issue -- thank you very much for spending the\ntime to look at this series.\n\nDirk\n\n"},{"id":"491320","messageId":"jikdly5drampltlvxuex67iz77isgt7uqcnf45ocmrozgbhyaq@2g7kucs2ordg","threadId":"61151","inReplyTo":"33a18458891259565e553ab39301108ce642d02f.1710840596.git.dirk@gouders.net","subject":"Re: [PATCH v2 4/5] MyFirstObjectWalk: fix description for counting omitted objects","fromName":"Kyle Lippincott","fromEmail":"spectral@google.com","sentAt":"2024-03-23T21:59:35Z","receivedAt":"2024-03-23T21:59:39Z","isPatch":true,"sender":{"key":"spectral@google.com","avatar":"https://avatars.githubusercontent.com/u/6371650?v=4"},"body":"On Tue, Mar 19, 2024 at 12:23:14PM +0100, Dirk Gouders wrote:\n> Before the changes to count omitted objects, the function\n> traverse_commit_list() was used and its call cannot be changed to pass\n> a pointer to an oidset to record omitted objects.\n> \n> Fix the text to clarify that we now use another traversal function to\n> be able to pass the pointer to the introduced oidset.\n> \n> Signed-off-by: Dirk Gouders \n> ---\n> Documentation/MyFirstObjectWalk.txt | 14 ++++++++------\n> 1 file changed, 8 insertions(+), 6 deletions(-)\n> \n> diff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\n> index a06c712e46..981dbf917b 100644\n> --- a/Documentation/MyFirstObjectWalk.txt\n> +++ b/Documentation/MyFirstObjectWalk.txt\n> @@ -754,10 +754,11 @@ points to the same tree object as its grandparent.)\n> === Counting Omitted Objects\n> \n> We also have the capability to enumerate all objects which were omitted by a\n> -filter, like with `git log --filter= --filter-print-omitted`. Asking\n> -`traverse_commit_list_filtered()` to populate the `omitted` list means that our\n> -object walk does not perform any better than an unfiltered object walk; all\n> -reachable objects are walked in order to populate the list.\n> +filter, like with `git log --filter= --filter-print-omitted`. We\n> +can ask `traverse_commit_list_filtered()` to populate the `omitted`\n> +list which means that our object walk does not perform any better than\n> +an unfiltered object walk; all reachable objects are walked in order\n> +to populate the list.\n\nThe way the original was phrased makes it sound to me like \"Doing via\n is potentially slow.\", and I expect a counter-proposal of using\nmechanismB to resolve that. The rewrite partially avoids that, but I think could\ntake it further to really drive home that this is a consequence of using this\nnew function, and is not a failing we will be proposing a solution for:\n\n We can ask `traverse_commit_list_filtered()` to populate the `omitted` list.\n+Note that this means that our object walk will not perform any better than\n an unfiltered object walk; all reachable objects are walked in order\n to populate the list.\n\nSince that first sentence is now shorter, we could also add a bit more nuance to\nit, calling out that we're going to switch which function we're using earlier\n(and technically redundantly, but I think that's fine); something like the\nfollowing:\n\n We also have the capability to enumerate all objects which were omitted by a\n-filter, like with `git log --filter= --filter-print-omitted`. Asking\n+filter, like with `git log --filter= --filter-print-omitted`. To do this,\n+change `traverse_commit_list()` to `traverse_commit_list_filtered()`, which is\n+able to populate an `omitted` list. Note that this means that our object walk\n+will not perform any better than an unfiltered object walk; all reachable\n+objects are walked in order to populate the list.\n\nFeel free to wordsmith any of my proposed text, and I apologize that these are\njust me typing in something that looks \"patch like\" in my mail client, not\nproperly formatted patches. I think what you have is already an improvement,\nthough, so if you think my proposed text is too verbose, I'm fine with what you\nhave.\n\n> \n> First, add the `struct oidset` and related items we will use to iterate it:\n> \n> @@ -778,8 +779,9 @@ static void walken_object_walk(\n> \t...\n> ----\n> \n> -Modify the call to `traverse_commit_list_filtered()` to include your `omitted`\n> -object:\n> +You need to replace the call to `traverse_commit_list()` to\n\nIf my proposal to introduce the point that we're switching which function we use\nin the earlier diff hunk is accepted, there's a small nit here: saying \"You need\nto\" would feel (very slightly) awkward, since we already mentioned that it was\nnecessary to accomplish the goal. If we accept the previous proposal, we may\nwant to change this to remove the \"You need to\", and just state something like\n\"Replace the call...\"\n\nRegardless, I think saying \"replace the call to A _with_ B\" (instead of \"A _to_\nB\") reads slightly better. I don't know if that's just a personal\npreference/dialect though.\n\n> +`traverse_commit_list_filtered()` to be able to pass a pointer to the\n\nIf we remove the \"You need to\", then we should probably rephrase this to more\nof an instruction, changing \"to be able to\" to \"and\".\n\nSomething like this:\n\n-Modify the call to `traverse_commit_list_filtered()` to include your `omitted`\n-object:\n+Replace the call to `traverse_commit_list()` with\n+`traverse_commit_list_filtered()` and pass a pointer to the `omitted` oidset\n+defined and initialized above:\n\n> +oidset defined and initialized above:\n> \n> ----\n> \t...\n> -- \n> 2.43.0\n> \n> \n"},{"id":"491321","messageId":"p5fii3tmfb6l7dgc2p35euwv4cp6ag64qqhb63wfu75iphqknu@tdo5ru6modr3","threadId":"61151","inReplyTo":"cover.1710840596.git.dirk@gouders.net","subject":"Re: [PATCH v2 0/5] Fixes for Documentation/MyFirstObjectWalk.txt","fromName":"Kyle Lippincott","fromEmail":"spectral@google.com","sentAt":"2024-03-23T22:00:42Z","receivedAt":"2024-03-23T22:00:47Z","isPatch":true,"sender":{"key":"spectral@google.com","avatar":"https://avatars.githubusercontent.com/u/6371650?v=4"},"body":"On Tue, Mar 19, 2024 at 12:23:10PM +0100, Dirk Gouders wrote:\n> The second spin for this series.\n> ---\n> Changes since v1:\n> * Added Emily to Cc in the hope for a review\n> * Remove superfluous tags from [1/5] and [3/5]\n> * Replace bashism `|&` by `2>&1 |` in [5/5]\n> ---\n> Dirk Gouders (5):\n> MyFirstObjectWalk: use additional arg in config_fn_t\n> MyFirstObjectWalk: fix misspelled \"builtins/\"\n> MyFirstObjectWalk: fix filtered object walk\n> MyFirstObjectWalk: fix description for counting omitted objects\n> MyFirstObjectWalk: add stderr to pipe processing\n> \n> Documentation/MyFirstObjectWalk.txt | 36 ++++++++++++++++-------------\n> 1 file changed, 20 insertions(+), 16 deletions(-)\n\nAside from the small comments on 4 and 5, series looks good to me, thanks for\nworking on this.\n"},{"id":"491322","messageId":"gh8r28zhw5.fsf@gouders.net","threadId":"61151","inReplyTo":"jikdly5drampltlvxuex67iz77isgt7uqcnf45ocmrozgbhyaq@2g7kucs2ordg","subject":"Re: [PATCH v2 4/5] MyFirstObjectWalk: fix description for counting omitted objects","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-23T22:46:02Z","receivedAt":"2024-03-23T22:46:19Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"Kyle Lippincott writes:\n\n> On Tue, Mar 19, 2024 at 12:23:14PM +0100, Dirk Gouders wrote:\n>> Before the changes to count omitted objects, the function\n>> traverse_commit_list() was used and its call cannot be changed to pass\n>> a pointer to an oidset to record omitted objects.\n>> \n>> Fix the text to clarify that we now use another traversal function to\n>> be able to pass the pointer to the introduced oidset.\n>> \n>> Signed-off-by: Dirk Gouders \n>> ---\n>> Documentation/MyFirstObjectWalk.txt | 14 ++++++++------\n>> 1 file changed, 8 insertions(+), 6 deletions(-)\n>> \n>> diff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\n>> index a06c712e46..981dbf917b 100644\n>> --- a/Documentation/MyFirstObjectWalk.txt\n>> +++ b/Documentation/MyFirstObjectWalk.txt\n>> @@ -754,10 +754,11 @@ points to the same tree object as its grandparent.)\n>> === Counting Omitted Objects\n>> \n>> We also have the capability to enumerate all objects which were omitted by a\n>> -filter, like with `git log --filter= --filter-print-omitted`. Asking\n>> -`traverse_commit_list_filtered()` to populate the `omitted` list means that our\n>> -object walk does not perform any better than an unfiltered object walk; all\n>> -reachable objects are walked in order to populate the list.\n>> +filter, like with `git log --filter= --filter-print-omitted`. We\n>> +can ask `traverse_commit_list_filtered()` to populate the `omitted`\n>> +list which means that our object walk does not perform any better than\n>> +an unfiltered object walk; all reachable objects are walked in order\n>> +to populate the list.\n>\n> The way the original was phrased makes it sound to me like \"Doing via\n> is potentially slow.\", and I expect a counter-proposal of using\n> mechanismB to resolve that. The rewrite partially avoids that, but I think could\n> take it further to really drive home that this is a consequence of using this\n> new function, and is not a failing we will be proposing a solution for:\n\nYes, I had similar thoughts.\n\n> We can ask `traverse_commit_list_filtered()` to populate the `omitted` list.\n> +Note that this means that our object walk will not perform any better than\n> an unfiltered object walk; all reachable objects are walked in order\n> to populate the list.\n>\n> Since that first sentence is now shorter, we could also add a bit more nuance to\n> it, calling out that we're going to switch which function we're using earlier\n> (and technically redundantly, but I think that's fine); something like the\n> following:\n>\n> We also have the capability to enumerate all objects which were omitted by a\n> -filter, like with `git log --filter= --filter-print-omitted`. Asking\n> +filter, like with `git log --filter= --filter-print-omitted`. To do this,\n> +change `traverse_commit_list()` to `traverse_commit_list_filtered()`, which is\n> +able to populate an `omitted` list. Note that this means that our object walk\n> +will not perform any better than an unfiltered object walk; all reachable\n> +objects are walked in order to populate the list.\n>\n> Feel free to wordsmith any of my proposed text, and I apologize that these are\n> just me typing in something that looks \"patch like\" in my mail client, not\n> properly formatted patches. I think what you have is already an improvement,\n> though, so if you think my proposed text is too verbose, I'm fine with what you\n> have.\n\nThank you for your suggestion. To me, this fits much better and I will\nuse it should no further improvements being asked for.\n\n>> \n>> First, add the `struct oidset` and related items we will use to iterate it:\n>> \n>> @@ -778,8 +779,9 @@ static void walken_object_walk(\n>> \t...\n>> ----\n>> \n>> -Modify the call to `traverse_commit_list_filtered()` to include your `omitted`\n>> -object:\n>> +You need to replace the call to `traverse_commit_list()` to\n>\n> If my proposal to introduce the point that we're switching which function we use\n> in the earlier diff hunk is accepted, there's a small nit here: saying \"You need\n> to\" would feel (very slightly) awkward, since we already mentioned that it was\n> necessary to accomplish the goal. If we accept the previous proposal, we may\n> want to change this to remove the \"You need to\", and just state something like\n> \"Replace the call...\"\n>\n> Regardless, I think saying \"replace the call to A _with_ B\" (instead of \"A _to_\n> B\") reads slightly better. I don't know if that's just a personal\n> preference/dialect though.\n\nWhen I wrote that \"You need to\" it felt semi-optimal even to me\nnon-native speaker, but I didn't exactly know what to do with it. So,\nI'm very glad you are helping me to do all that better.\n\n>> +`traverse_commit_list_filtered()` to be able to pass a pointer to the\n>\n> If we remove the \"You need to\", then we should probably rephrase this to more\n> of an instruction, changing \"to be able to\" to \"and\".\n>\n> Something like this:\n>\n> -Modify the call to `traverse_commit_list_filtered()` to include your `omitted`\n> -object:\n> +Replace the call to `traverse_commit_list()` with\n> +`traverse_commit_list_filtered()` and pass a pointer to the `omitted` oidset\n> +defined and initialized above:\n\nSounds way better and I'd use it.\n\nThanks again,\n\nDirk\n"},{"id":"491323","messageId":"gh4jcwzgxz.fsf@gouders.net","threadId":"61151","inReplyTo":"p5fii3tmfb6l7dgc2p35euwv4cp6ag64qqhb63wfu75iphqknu@tdo5ru6modr3","subject":"Re: [PATCH v2 0/5] Fixes for Documentation/MyFirstObjectWalk.txt","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-23T23:06:32Z","receivedAt":"2024-03-23T23:06:48Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"Kyle Lippincott writes:\n\n> On Tue, Mar 19, 2024 at 12:23:10PM +0100, Dirk Gouders wrote:\n>> The second spin for this series.\n>> ---\n>> Changes since v1:\n>> * Added Emily to Cc in the hope for a review\n>> * Remove superfluous tags from [1/5] and [3/5]\n>> * Replace bashism `|&` by `2>&1 |` in [5/5]\n>> ---\n>> Dirk Gouders (5):\n>> MyFirstObjectWalk: use additional arg in config_fn_t\n>> MyFirstObjectWalk: fix misspelled \"builtins/\"\n>> MyFirstObjectWalk: fix filtered object walk\n>> MyFirstObjectWalk: fix description for counting omitted objects\n>> MyFirstObjectWalk: add stderr to pipe processing\n>> \n>> Documentation/MyFirstObjectWalk.txt | 36 ++++++++++++++++-------------\n>> 1 file changed, 20 insertions(+), 16 deletions(-)\n>\n> Aside from the small comments on 4 and 5, series looks good to me, thanks for\n> working on this.\n\nThanks for the review -- especially for the detailed explanation and\nsuggestions on 4.\n\nDirk\n"},{"id":"491339","messageId":"xmqqil1cv09e.fsf@gitster.g","threadId":"61151","inReplyTo":"gh4jcwzgxz.fsf@gouders.net","subject":"Re: [PATCH v2 0/5] Fixes for Documentation/MyFirstObjectWalk.txt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-24T02:20:29Z","receivedAt":"2024-03-24T02:20:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dirk Gouders writes:\n\n> Kyle Lippincott writes:\n>\n>> On Tue, Mar 19, 2024 at 12:23:10PM +0100, Dirk Gouders wrote:\n>>> The second spin for this series.\n>>> ---\n>>> Changes since v1:\n>>> * Added Emily to Cc in the hope for a review\n>>> * Remove superfluous tags from [1/5] and [3/5]\n>>> * Replace bashism `|&` by `2>&1 |` in [5/5]\n>>> ---\n>>> Dirk Gouders (5):\n>>> MyFirstObjectWalk: use additional arg in config_fn_t\n>>> MyFirstObjectWalk: fix misspelled \"builtins/\"\n>>> MyFirstObjectWalk: fix filtered object walk\n>>> MyFirstObjectWalk: fix description for counting omitted objects\n>>> MyFirstObjectWalk: add stderr to pipe processing\n>>> \n>>> Documentation/MyFirstObjectWalk.txt | 36 ++++++++++++++++-------------\n>>> 1 file changed, 20 insertions(+), 16 deletions(-)\n>>\n>> Aside from the small comments on 4 and 5, series looks good to me, thanks for\n>> working on this.\n>\n> Thanks for the review -- especially for the detailed explanation and\n> suggestions on 4.\n\nYeah, I too liked the comments on [4/5]. Thanks for working well\ntogether.\n"},{"id":"491450","messageId":"3122ae247263fb65d5900c67f7955d68de7f874c.1711368499.git.dirk@gouders.net","threadId":"61151","inReplyTo":"cover.1711368498.git.dirk@gouders.net","subject":"[PATCH v3 2/5] MyFirstObjectWalk: fix misspelled \"builtins/\"","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-25T12:33:33Z","receivedAt":"2024-03-25T13:19:41Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"pack-objects.c resides in builtin/ (not builtins/).\n\nFix the misspelled directory name.\n\nSigned-off-by: Dirk Gouders \n---\n Documentation/MyFirstObjectWalk.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\nindex cceac2df95..c33d22ae99 100644\n--- a/Documentation/MyFirstObjectWalk.txt\n+++ b/Documentation/MyFirstObjectWalk.txt\n@@ -525,7 +525,7 @@ about each one.\n \n We can base our work on an example. `git pack-objects` prepares all kinds of\n objects for packing into a bitmap or packfile. The work we are interested in\n-resides in `builtins/pack-objects.c:get_object_list()`; examination of that\n+resides in `builtin/pack-objects.c:get_object_list()`; examination of that\n function shows that the all-object walk is being performed by\n `traverse_commit_list()` or `traverse_commit_list_filtered()`. Those two\n functions reside in `list-objects.c`; examining the source shows that, despite\n-- \n2.43.0\n\n"},{"id":"491451","messageId":"f21348ab80718b0c6ea1ad9066e0b674b11573ad.1711368499.git.dirk@gouders.net","threadId":"61151","inReplyTo":"cover.1711368498.git.dirk@gouders.net","subject":"[PATCH v3 3/5] MyFirstObjectWalk: fix filtered object walk","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-25T12:33:34Z","receivedAt":"2024-03-25T13:19:47Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"Commit f0d2f849 (MyFirstObjectWalk: update recommended usage)\nchanged a call of parse_list_objects_filter() in a way that\nprobably never worked: parse_list_objects_filter() always needed a\npointer as its first argument.\n\nFix this by removing the CALLOC_ARRAY and passing the address of\nrev->filter to parse_list_objects_filter() in accordance to\nsuch a call in revisions.c, for example.\n\nSigned-off-by: Dirk Gouders \n---\n Documentation/MyFirstObjectWalk.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\nindex c33d22ae99..a06c712e46 100644\n--- a/Documentation/MyFirstObjectWalk.txt\n+++ b/Documentation/MyFirstObjectWalk.txt\n@@ -734,8 +734,8 @@ walk we've just performed:\n \t} else {\n \t\ttrace_printf(\n \t\t\t_(\"Filtered object walk with filterspec 'tree:1'.\\n\"));\n-\t\tCALLOC_ARRAY(rev->filter, 1);\n-\t\tparse_list_objects_filter(rev->filter, \"tree:1\");\n+\n+\t\tparse_list_objects_filter(&rev->filter, \"tree:1\");\n \t}\n \ttraverse_commit_list(rev, walken_show_commit,\n \t\t\t walken_show_object, NULL);\n-- \n2.43.0\n\n"},{"id":"491452","messageId":"cfa4b9ce503e98035d3ce09b0c9e00bcfb6ff70a.1711368499.git.dirk@gouders.net","threadId":"61151","inReplyTo":"cover.1711368498.git.dirk@gouders.net","subject":"[PATCH v3 4/5] MyFirstObjectWalk: fix description for counting omitted objects","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-25T12:33:35Z","receivedAt":"2024-03-25T13:19:53Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"Before the changes to count omitted objects, the function\ntraverse_commit_list() was used and its call cannot be changed to pass\na pointer to an oidset to record omitted objects.\n\nFix the text to clarify that we now use another traversal function to\nbe able to pass the pointer to the introduced oidset.\n\nHelped-by: Kyle Lippincott \nSigned-off-by: Dirk Gouders \n---\n Documentation/MyFirstObjectWalk.txt | 14 ++++++++------\n 1 file changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\nindex a06c712e46..811175837c 100644\n--- a/Documentation/MyFirstObjectWalk.txt\n+++ b/Documentation/MyFirstObjectWalk.txt\n@@ -754,10 +754,11 @@ points to the same tree object as its grandparent.)\n === Counting Omitted Objects\n \n We also have the capability to enumerate all objects which were omitted by a\n-filter, like with `git log --filter= --filter-print-omitted`. Asking\n-`traverse_commit_list_filtered()` to populate the `omitted` list means that our\n-object walk does not perform any better than an unfiltered object walk; all\n-reachable objects are walked in order to populate the list.\n+filter, like with `git log --filter= --filter-print-omitted`. To do this,\n+change `traverse_commit_list()` to `traverse_commit_list_filtered()`, which is\n+able to populate an `omitted` list. Note that this means that our object walk\n+will not perform any better than an unfiltered object walk; all reachable\n+objects are walked in order to populate the list.\n \n First, add the `struct oidset` and related items we will use to iterate it:\n \n@@ -778,8 +779,9 @@ static void walken_object_walk(\n \t...\n ----\n \n-Modify the call to `traverse_commit_list_filtered()` to include your `omitted`\n-object:\n+Replace the call to `traverse_commit_list()` with\n+`traverse_commit_list_filtered()` and pass a pointer to the `omitted` oidset\n+defined and initialized above:\n \n ----\n \t...\n-- \n2.43.0\n\n"},{"id":"491453","messageId":"c571abb49dc3c03d86434deeaddf91c477bb3ce1.1711368499.git.dirk@gouders.net","threadId":"61151","inReplyTo":"cover.1711368498.git.dirk@gouders.net","subject":"[PATCH v3 5/5] MyFirstObjectWalk: add stderr to pipe processing","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-25T12:33:36Z","receivedAt":"2024-03-25T13:19:58Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"In the last chapter of this document, pipes are used in commands to\nfilter out the first/last trace messages. But according to git(1),\ntrace messages are sent to stderr if GIT_TRACE is set to '1', so those\ncommands do not produce the described results.\n\nFix this by redirecting stderr to stdout prior to the pipe operator\nto additionally connect stderr to stdin of the latter command.\n\nFurther, while reviewing the above fix, Kyle Lippincott noticed\na second issue with the second of the examples: a missing slash in the\nexecutable path \"./bin-wrappers git\".\n\nAdd the missing slash.\n\nHelped-by: Kyle Lippincott \nSigned-off-by: Dirk Gouders \n---\n Documentation/MyFirstObjectWalk.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\nindex 811175837c..3d78403c4a 100644\n--- a/Documentation/MyFirstObjectWalk.txt\n+++ b/Documentation/MyFirstObjectWalk.txt\n@@ -847,7 +847,7 @@ those lines without having to recompile.\n With only that change, run again (but save yourself some scrollback):\n \n ----\n-$ GIT_TRACE=1 ./bin-wrappers/git walken | head -n 10\n+$ GIT_TRACE=1 ./bin-wrappers/git walken 2>&1 | head -n 10\n ----\n \n Take a look at the top commit with `git show` and the object ID you printed; it\n@@ -875,7 +875,7 @@ of the first handful:\n \n ----\n $ make\n-$ GIT_TRACE=1 ./bin-wrappers git walken | tail -n 10\n+$ GIT_TRACE=1 ./bin-wrappers/git walken 2>&1 | tail -n 10\n ----\n \n The last commit object given should have the same OID as the one we saw at the\n-- \n2.43.0\n\n"},{"id":"491454","messageId":"0eeb4b78ac91c2bddf775fdea34ce5c0515ff205.1711368499.git.dirk@gouders.net","threadId":"61151","inReplyTo":"cover.1711368498.git.dirk@gouders.net","subject":"[PATCH v3 1/5] MyFirstObjectWalk: use additional arg in config_fn_t","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-25T12:33:32Z","receivedAt":"2024-03-25T13:28:33Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"Commit a4e7e317 (config: add ctx arg to config_fn_t) added a fourth\nargument to config_fn_t but did not change relevant function calls\nin Documentation/MyFirstObjectWalk.txt.\n\nFix those calls and the example git_walken_config() to use\nthat additional argument.\n\nSigned-off-by: Dirk Gouders \n---\n Documentation/MyFirstObjectWalk.txt | 12 +++++++-----\n 1 file changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\nindex c68cdb11b9..cceac2df95 100644\n--- a/Documentation/MyFirstObjectWalk.txt\n+++ b/Documentation/MyFirstObjectWalk.txt\n@@ -210,13 +210,14 @@ We'll also need to include the `config.h` header:\n \n ...\n \n-static int git_walken_config(const char *var, const char *value, void *cb)\n+static int git_walken_config(const char *var, const char *value,\n+\t\t\t const struct config_context *ctx, void *cb)\n {\n \t/*\n \t * For now, we don't have any custom configuration, so fall back to\n \t * the default config.\n \t */\n-\treturn git_default_config(var, value, cb);\n+\treturn git_default_config(var, value, ctx, cb);\n }\n ----\n \n@@ -389,10 +390,11 @@ modifying `rev_info.grep_filter`, which is a `struct grep_opt`.\n First some setup. Add `grep_config()` to `git_walken_config()`:\n \n ----\n-static int git_walken_config(const char *var, const char *value, void *cb)\n+static int git_walken_config(const char *var, const char *value,\n+\t\t\t const struct config_context *ctx, void *cb)\n {\n-\tgrep_config(var, value, cb);\n-\treturn git_default_config(var, value, cb);\n+\tgrep_config(var, value, ctx, cb);\n+\treturn git_default_config(var, value, ctx, cb);\n }\n ----\n \n-- \n2.43.0\n\n"},{"id":"491455","messageId":"cover.1711368498.git.dirk@gouders.net","threadId":"61151","inReplyTo":"cover.1710840596.git.dirk@gouders.net","subject":"[PATCH v3 0/5] Fixes for Documentation/MyFirstObjectWalk.txt","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-25T12:33:31Z","receivedAt":"2024-03-25T13:28:36Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"The 3rd iteration for this series.\n\nI tried to credit Kyle's suggestions for 4 and 5 with Helped-by tags and\nhope it was adequate to do so. Actually, at least #4 was a lot more\nthan a Helped-by, I would say...\n---\nChanges in v3:\n* Reword the description in [4/5]\n* Add a missing slash in [5/5]\n\nChanges in v2:\n* Added Emily to Cc in the hope for a review\n* Remove superfluous tags from [1/5] and [3/5]\n* Replace bashism `|&` by `2>&1 |` in [5/5]\n---\nDirk Gouders (5):\n MyFirstObjectWalk: use additional arg in config_fn_t\n MyFirstObjectWalk: fix misspelled \"builtins/\"\n MyFirstObjectWalk: fix filtered object walk\n MyFirstObjectWalk: fix description for counting omitted objects\n MyFirstObjectWalk: add stderr to pipe processing\n\n Documentation/MyFirstObjectWalk.txt | 36 ++++++++++++++++-------------\n 1 file changed, 20 insertions(+), 16 deletions(-)\n\nRange-diff against v2:\n-: ---------- > 1: 0eeb4b78ac MyFirstObjectWalk: use additional arg in config_fn_t\n-: ---------- > 2: 3122ae2472 MyFirstObjectWalk: fix misspelled \"builtins/\"\n-: ---------- > 3: f21348ab80 MyFirstObjectWalk: fix filtered object walk\n1: 4219237868 ! 4: cfa4b9ce50 MyFirstObjectWalk: fix description for counting omitted objects\n @@ Commit message\n Fix the text to clarify that we now use another traversal function to\n be able to pass the pointer to the introduced oidset.\n \n + Helped-by: Kyle Lippincott \n Signed-off-by: Dirk Gouders \n \n ## Documentation/MyFirstObjectWalk.txt ##\n @@ Documentation/MyFirstObjectWalk.txt: points to the same tree object as its grand\n -`traverse_commit_list_filtered()` to populate the `omitted` list means that our\n -object walk does not perform any better than an unfiltered object walk; all\n -reachable objects are walked in order to populate the list.\n -+filter, like with `git log --filter= --filter-print-omitted`. We\n -+can ask `traverse_commit_list_filtered()` to populate the `omitted`\n -+list which means that our object walk does not perform any better than\n -+an unfiltered object walk; all reachable objects are walked in order\n -+to populate the list.\n ++filter, like with `git log --filter= --filter-print-omitted`. To do this,\n ++change `traverse_commit_list()` to `traverse_commit_list_filtered()`, which is\n ++able to populate an `omitted` list. Note that this means that our object walk\n ++will not perform any better than an unfiltered object walk; all reachable\n ++objects are walked in order to populate the list.\n \n First, add the `struct oidset` and related items we will use to iterate it:\n \n @@ Documentation/MyFirstObjectWalk.txt: static void walken_object_walk(\n \n -Modify the call to `traverse_commit_list_filtered()` to include your `omitted`\n -object:\n -+You need to replace the call to `traverse_commit_list()` to\n -+`traverse_commit_list_filtered()` to be able to pass a pointer to the\n -+oidset defined and initialized above:\n ++Replace the call to `traverse_commit_list()` with\n ++`traverse_commit_list_filtered()` and pass a pointer to the `omitted` oidset\n ++defined and initialized above:\n \n ----\n \t...\n2: 9b0f0832b7 ! 5: c571abb49d MyFirstObjectWalk: add stderr to pipe processing\n @@ Commit message\n Fix this by redirecting stderr to stdout prior to the pipe operator\n to additionally connect stderr to stdin of the latter command.\n \n + Further, while reviewing the above fix, Kyle Lippincott noticed\n + a second issue with the second of the examples: a missing slash in the\n + executable path \"./bin-wrappers git\".\n +\n + Add the missing slash.\n +\n + Helped-by: Kyle Lippincott \n Signed-off-by: Dirk Gouders \n \n ## Documentation/MyFirstObjectWalk.txt ##\n @@ Documentation/MyFirstObjectWalk.txt: of the first handful:\n ----\n $ make\n -$ GIT_TRACE=1 ./bin-wrappers git walken | tail -n 10\n -+$ GIT_TRACE=1 ./bin-wrappers git walken 2>&1 | tail -n 10\n ++$ GIT_TRACE=1 ./bin-wrappers/git walken 2>&1 | tail -n 10\n ----\n \n The last commit object given should have the same OID as the one we saw at the\n-- \n2.43.0\n\n"},{"id":"491466","messageId":"CAO_smVgb4uWZQGv9bKUEL1AGC2DXBb8xL6KeYOkvHgMCXSAhow@mail.gmail.com","threadId":"61151","inReplyTo":"cover.1711368498.git.dirk@gouders.net","subject":"Re: [PATCH v3 0/5] Fixes for Documentation/MyFirstObjectWalk.txt","fromName":"Kyle Lippincott","fromEmail":"spectral@google.com","sentAt":"2024-03-25T17:05:55Z","receivedAt":"2024-03-25T17:06:13Z","isPatch":true,"sender":{"key":"spectral@google.com","avatar":"https://avatars.githubusercontent.com/u/6371650?v=4"},"body":"On Mon, Mar 25, 2024 at 6:19 AM Dirk Gouders wrote:\n>\n> The 3rd iteration for this series.\n>\n> I tried to credit Kyle's suggestions for 4 and 5 with Helped-by tags and\n> hope it was adequate to do so. Actually, at least #4 was a lot more\n> than a Helped-by, I would say...\n> ---\n> Changes in v3:\n> * Reword the description in [4/5]\n> * Add a missing slash in [5/5]\n>\n> Changes in v2:\n> * Added Emily to Cc in the hope for a review\n> * Remove superfluous tags from [1/5] and [3/5]\n> * Replace bashism `|&` by `2>&1 |` in [5/5]\n> ---\n> Dirk Gouders (5):\n> MyFirstObjectWalk: use additional arg in config_fn_t\n> MyFirstObjectWalk: fix misspelled \"builtins/\"\n> MyFirstObjectWalk: fix filtered object walk\n> MyFirstObjectWalk: fix description for counting omitted objects\n> MyFirstObjectWalk: add stderr to pipe processing\n>\n> Documentation/MyFirstObjectWalk.txt | 36 ++++++++++++++++-------------\n> 1 file changed, 20 insertions(+), 16 deletions(-)\n>\n> Range-diff against v2:\n> -: ---------- > 1: 0eeb4b78ac MyFirstObjectWalk: use additional arg in config_fn_t\n> -: ---------- > 2: 3122ae2472 MyFirstObjectWalk: fix misspelled \"builtins/\"\n> -: ---------- > 3: f21348ab80 MyFirstObjectWalk: fix filtered object walk\n\nLooks good, thanks again!\n"},{"id":"491467","messageId":"xmqqcyrimdtx.fsf@gitster.g","threadId":"61151","inReplyTo":"0eeb4b78ac91c2bddf775fdea34ce5c0515ff205.1711368499.git.dirk@gouders.net","subject":"Re: [PATCH v3 1/5] MyFirstObjectWalk: use additional arg in config_fn_t","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-25T17:16:42Z","receivedAt":"2024-03-25T17:16:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dirk Gouders writes:\n\n> Commit a4e7e317 (config: add ctx arg to config_fn_t) added a fourth\n\nIn your next topic, use \"git show -s --pretty=reference a4e7e317\" to\nshow \"a4e7e317f8 (config: add ctx arg to config_fn_t, 2023-06-28)\"\nwith dates. It makes it easier to see how long what is being fixed\nis broken, giving reviewers a sense of urgency for a fix. It is not\nnecessary to reroll this commit only to update the reference, though.\n\n> argument to config_fn_t but did not change relevant function calls\n> in Documentation/MyFirstObjectWalk.txt.\n>\n> Fix those calls and the example git_walken_config() to use\n> that additional argument.\n>\n> Signed-off-by: Dirk Gouders \n> ---\n> Documentation/MyFirstObjectWalk.txt | 12 +++++++-----\n> 1 file changed, 7 insertions(+), 5 deletions(-)\n>\n> diff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\n> index c68cdb11b9..cceac2df95 100644\n> --- a/Documentation/MyFirstObjectWalk.txt\n> +++ b/Documentation/MyFirstObjectWalk.txt\n> @@ -210,13 +210,14 @@ We'll also need to include the `config.h` header:\n> \n> ...\n> \n> -static int git_walken_config(const char *var, const char *value, void *cb)\n> +static int git_walken_config(const char *var, const char *value,\n> +\t\t\t const struct config_context *ctx, void *cb)\n> {\n> \t/*\n> \t * For now, we don't have any custom configuration, so fall back to\n> \t * the default config.\n> \t */\n> -\treturn git_default_config(var, value, cb);\n> +\treturn git_default_config(var, value, ctx, cb);\n> }\n> ----\n> \n> @@ -389,10 +390,11 @@ modifying `rev_info.grep_filter`, which is a `struct grep_opt`.\n> First some setup. Add `grep_config()` to `git_walken_config()`:\n> \n> ----\n> -static int git_walken_config(const char *var, const char *value, void *cb)\n> +static int git_walken_config(const char *var, const char *value,\n> +\t\t\t const struct config_context *ctx, void *cb)\n> {\n> -\tgrep_config(var, value, cb);\n> -\treturn git_default_config(var, value, cb);\n> +\tgrep_config(var, value, ctx, cb);\n> +\treturn git_default_config(var, value, ctx, cb);\n> }\n> ----\n"},{"id":"491481","messageId":"xmqq34semden.fsf@gitster.g","threadId":"61151","inReplyTo":"cfa4b9ce503e98035d3ce09b0c9e00bcfb6ff70a.1711368499.git.dirk@gouders.net","subject":"Re: [PATCH v3 4/5] MyFirstObjectWalk: fix description for counting omitted objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-25T17:25:52Z","receivedAt":"2024-03-25T17:25:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dirk Gouders writes:\n\n> Before the changes to count omitted objects, the function\n> traverse_commit_list() was used and its call cannot be changed to pass\n> a pointer to an oidset to record omitted objects.\n>\n> Fix the text to clarify that we now use another traversal function to\n> be able to pass the pointer to the introduced oidset.\n>\n> Helped-by: Kyle Lippincott \n> Signed-off-by: Dirk Gouders \n> ---\n> Documentation/MyFirstObjectWalk.txt | 14 ++++++++------\n> 1 file changed, 8 insertions(+), 6 deletions(-)\n>\n> diff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\n> index a06c712e46..811175837c 100644\n> --- a/Documentation/MyFirstObjectWalk.txt\n> +++ b/Documentation/MyFirstObjectWalk.txt\n> @@ -754,10 +754,11 @@ points to the same tree object as its grandparent.)\n> === Counting Omitted Objects\n> \n> We also have the capability to enumerate all objects which were omitted by a\n> -filter, like with `git log --filter= --filter-print-omitted`. Asking\n> -`traverse_commit_list_filtered()` to populate the `omitted` list means that our\n> -object walk does not perform any better than an unfiltered object walk; all\n> -reachable objects are walked in order to populate the list.\n> +filter, like with `git log --filter= --filter-print-omitted`. To do this,\n> +change `traverse_commit_list()` to `traverse_commit_list_filtered()`, which is\n> +able to populate an `omitted` list. Note that this means that our object walk\n\n\"this means that\" could be rephrased in a way a bit more helpful and\nto readers with clarity, perhaps:\n\n\tNote that our object walk will not perform any better than\n\tan unfiltered walk with this function, because all reachable\n\tobjects need to be walked in order to ...\n\n> +will not perform any better than an unfiltered object walk; all reachable\n> +objects are walked in order to populate the list.\n\nOther than that, looking very good.\n\nThanks, both.\n\n> First, add the `struct oidset` and related items we will use to iterate it:\n> \n> @@ -778,8 +779,9 @@ static void walken_object_walk(\n> \t...\n> ----\n> \n> -Modify the call to `traverse_commit_list_filtered()` to include your `omitted`\n> -object:\n> +Replace the call to `traverse_commit_list()` with\n> +`traverse_commit_list_filtered()` and pass a pointer to the `omitted` oidset\n> +defined and initialized above:\n> \n> ----\n> \t...\n"},{"id":"491483","messageId":"xmqqsf0ekxpu.fsf@gitster.g","threadId":"61151","inReplyTo":"cover.1711368498.git.dirk@gouders.net","subject":"Re: [PATCH v3 0/5] Fixes for Documentation/MyFirstObjectWalk.txt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-25T17:50:05Z","receivedAt":"2024-03-25T17:50:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dirk Gouders writes:\n\n> The 3rd iteration for this series.\n>\n> I tried to credit Kyle's suggestions for 4 and 5 with Helped-by tags and\n> hope it was adequate to do so. Actually, at least #4 was a lot more\n> than a Helped-by, I would say...\n\nIt seemed adequate, at least to me, but I'll leave the final say up\nto Kyle.\n\nI left a few comments but overall the series is looking much nicer.\nThanks for working on it (and thanks for reviewing and helping,\nKyle).\n\nThis is an unrelated tangent, but I wonder if we can come up with a\nway to find breakages coming from API updates to these \"tutorial\"\ndocuments. The original \"user-manual\" also shares the same issue,\nand the issue may be deeper there as it also needs to catch up with\nend-user facing UI updates. In any case, we somehow ended up with\ntwo more \"tutorial\"-ish documents (MyFirstContribution.txt is the\nother one) that somebody needs to keep an eye on.\n\nIdeally if we can have automated tests, it would be nice. Perhaps\nsprinkling some special instruction in comments that is hidden from\nAsciiDoc mark-up to help our custom program to assemble the bits\ninto the state of the tutorial program that the readers should be\narriving at at different points in the tutorial document, and make\nsure they compile, link, and test well? Or \"follow one of our three\ntutorial documents to the letter to see if they need adjusting, and\ncome up with a set of patches to adjust them\" can be listed as one\nof the microproject ideas? I'll leave a #leftoverbits mark here, but\nwhat I want to see discussed (and eventually implemented) is not the\nclean-up itself (which can go stale over time) but the strategy to\nkeep the \"tutorial\" material up-to-date.\n\nTHanks.\n\n"},{"id":"491484","messageId":"CAO_smVh9TBiJivDrB6wFhAS6adKRqFiu7FE=H5ui5yQ5ssiuqA@mail.gmail.com","threadId":"61151","inReplyTo":"xmqqsf0ekxpu.fsf@gitster.g","subject":"Re: [PATCH v3 0/5] Fixes for Documentation/MyFirstObjectWalk.txt","fromName":"Kyle Lippincott","fromEmail":"spectral@google.com","sentAt":"2024-03-25T18:01:07Z","receivedAt":"2024-03-25T18:01:24Z","isPatch":true,"sender":{"key":"spectral@google.com","avatar":"https://avatars.githubusercontent.com/u/6371650?v=4"},"body":"On Mon, Mar 25, 2024 at 10:50 AM Junio C Hamano wrote:\n>\n> Dirk Gouders writes:\n>\n> > The 3rd iteration for this series.\n> >\n> > I tried to credit Kyle's suggestions for 4 and 5 with Helped-by tags and\n> > hope it was adequate to do so. Actually, at least #4 was a lot more\n> > than a Helped-by, I would say...\n>\n> It seemed adequate, at least to me, but I'll leave the final say up\n> to Kyle.\n>\n> I left a few comments but overall the series is looking much nicer.\n> Thanks for working on it (and thanks for reviewing and helping,\n> Kyle).\n>\n> This is an unrelated tangent, but I wonder if we can come up with a\n> way to find breakages coming from API updates to these \"tutorial\"\n> documents. The original \"user-manual\" also shares the same issue,\n> and the issue may be deeper there as it also needs to catch up with\n> end-user facing UI updates. In any case, we somehow ended up with\n> two more \"tutorial\"-ish documents (MyFirstContribution.txt is the\n> other one) that somebody needs to keep an eye on.\n>\n> Ideally if we can have automated tests, it would be nice. Perhaps\n> sprinkling some special instruction in comments that is hidden from\n> AsciiDoc mark-up to help our custom program to assemble the bits\n> into the state of the tutorial program that the readers should be\n> arriving at at different points in the tutorial document, and make\n> sure they compile, link, and test well?\n\nOn another project, I've had a (separate) test file that just does\nwhat the tutorial says to do, and there's an automatic notice for\n\"you're touching tutorial-test.sh, make sure you make any required\nchanges to tutorial.txt as well\". I don't know if we have that second\npart available to us here, though.\n\n> Or \"follow one of our three\n> tutorial documents to the letter to see if they need adjusting, and\n> come up with a set of patches to adjust them\" can be listed as one\n> of the microproject ideas? I'll leave a #leftoverbits mark here, but\n> what I want to see discussed (and eventually implemented) is not the\n> clean-up itself (which can go stale over time) but the strategy to\n> keep the \"tutorial\" material up-to-date.\n>\n> THanks.\n>\n"},{"id":"491504","messageId":"ghttkuks53.fsf@gouders.net","threadId":"61151","inReplyTo":"xmqqcyrimdtx.fsf@gitster.g","subject":"Re: [PATCH v3 1/5] MyFirstObjectWalk: use additional arg in config_fn_t","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-25T19:50:32Z","receivedAt":"2024-03-25T20:35:44Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"Junio C Hamano writes:\n\n> Dirk Gouders writes:\n>\n>> Commit a4e7e317 (config: add ctx arg to config_fn_t) added a fourth\n>\n> In your next topic, use \"git show -s --pretty=reference a4e7e317\" to\n> show \"a4e7e317f8 (config: add ctx arg to config_fn_t, 2023-06-28)\"\n> with dates. It makes it easier to see how long what is being fixed\n> is broken, giving reviewers a sense of urgency for a fix. It is not\n> necessary to reroll this commit only to update the reference, though.\n\nYes, thanks, will do.\n(This means I read SubmittingPatches too fast and have to re-read it to\nsee if I missed more details.)\n\nDirk\n"},{"id":"491512","messageId":"ghplvikrdk.fsf@gouders.net","threadId":"61151","inReplyTo":"xmqq34semden.fsf@gitster.g","subject":"Re: [PATCH v3 4/5] MyFirstObjectWalk: fix description for counting omitted objects","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-25T20:07:03Z","receivedAt":"2024-03-25T20:52:15Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"Junio C Hamano writes:\n\n> Dirk Gouders writes:\n>\n>> Before the changes to count omitted objects, the function\n>> traverse_commit_list() was used and its call cannot be changed to pass\n>> a pointer to an oidset to record omitted objects.\n>>\n>> Fix the text to clarify that we now use another traversal function to\n>> be able to pass the pointer to the introduced oidset.\n>>\n>> Helped-by: Kyle Lippincott \n>> Signed-off-by: Dirk Gouders \n>> ---\n>> Documentation/MyFirstObjectWalk.txt | 14 ++++++++------\n>> 1 file changed, 8 insertions(+), 6 deletions(-)\n>>\n>> diff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\n>> index a06c712e46..811175837c 100644\n>> --- a/Documentation/MyFirstObjectWalk.txt\n>> +++ b/Documentation/MyFirstObjectWalk.txt\n>> @@ -754,10 +754,11 @@ points to the same tree object as its grandparent.)\n>> === Counting Omitted Objects\n>> \n>> We also have the capability to enumerate all objects which were omitted by a\n>> -filter, like with `git log --filter= --filter-print-omitted`. Asking\n>> -`traverse_commit_list_filtered()` to populate the `omitted` list means that our\n>> -object walk does not perform any better than an unfiltered object walk; all\n>> -reachable objects are walked in order to populate the list.\n>> +filter, like with `git log --filter= --filter-print-omitted`. To do this,\n>> +change `traverse_commit_list()` to `traverse_commit_list_filtered()`, which is\n>> +able to populate an `omitted` list. Note that this means that our object walk\n>\n> \"this means that\" could be rephrased in a way a bit more helpful and\n> to readers with clarity, perhaps:\n>\n> \tNote that our object walk will not perform any better than\n> \tan unfiltered walk with this function, because all reachable\n> \tobjects need to be walked in order to ...\n\nWould it be OK to rearrange it even more? To me, the above raises the\nnew question \"How do I use traverse_commit_list_filtered() to do an\nunfiltered walk?\":\n\n \tNote that our object walk with this function will not perform\n\tany better than the previous unfiltered walk, because all\n\treachable objects need to be walked in order to ...\n\nDirk\n"},{"id":"491513","messageId":"ghle66krc4.fsf@gouders.net","threadId":"61151","inReplyTo":"CAO_smVgb4uWZQGv9bKUEL1AGC2DXBb8xL6KeYOkvHgMCXSAhow@mail.gmail.com","subject":"Re: [PATCH v3 0/5] Fixes for Documentation/MyFirstObjectWalk.txt","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-25T20:07:55Z","receivedAt":"2024-03-25T20:53:06Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"Kyle Lippincott writes:\n\n> On Mon, Mar 25, 2024 at 6:19 AM Dirk Gouders wrote:\n>>\n>> The 3rd iteration for this series.\n>>\n>> I tried to credit Kyle's suggestions for 4 and 5 with Helped-by tags and\n>> hope it was adequate to do so. Actually, at least #4 was a lot more\n>> than a Helped-by, I would say...\n>> ---\n>> Changes in v3:\n>> * Reword the description in [4/5]\n>> * Add a missing slash in [5/5]\n>>\n>> Changes in v2:\n>> * Added Emily to Cc in the hope for a review\n>> * Remove superfluous tags from [1/5] and [3/5]\n>> * Replace bashism `|&` by `2>&1 |` in [5/5]\n>> ---\n>> Dirk Gouders (5):\n>> MyFirstObjectWalk: use additional arg in config_fn_t\n>> MyFirstObjectWalk: fix misspelled \"builtins/\"\n>> MyFirstObjectWalk: fix filtered object walk\n>> MyFirstObjectWalk: fix description for counting omitted objects\n>> MyFirstObjectWalk: add stderr to pipe processing\n>>\n>> Documentation/MyFirstObjectWalk.txt | 36 ++++++++++++++++-------------\n>> 1 file changed, 20 insertions(+), 16 deletions(-)\n>>\n>> Range-diff against v2:\n>> -: ---------- > 1: 0eeb4b78ac MyFirstObjectWalk: use additional arg in config_fn_t\n>> -: ---------- > 2: 3122ae2472 MyFirstObjectWalk: fix misspelled \"builtins/\"\n>> -: ---------- > 3: f21348ab80 MyFirstObjectWalk: fix filtered object walk\n>\n> Looks good, thanks again!\n\nThank you for looking at it.\n\nDirk\n"},{"id":"491517","messageId":"CAO_smVhTQS2+QH0--ZtiCKyrFWx9fak0-0mbN-xY=_Ayx8JW=A@mail.gmail.com","threadId":"61151","inReplyTo":"xmqq34semden.fsf@gitster.g","subject":"Re: [PATCH v3 4/5] MyFirstObjectWalk: fix description for counting omitted objects","fromName":"Kyle Lippincott","fromEmail":"spectral@google.com","sentAt":"2024-03-25T20:59:59Z","receivedAt":"2024-03-25T21:00:15Z","isPatch":true,"sender":{"key":"spectral@google.com","avatar":"https://avatars.githubusercontent.com/u/6371650?v=4"},"body":"On Mon, Mar 25, 2024 at 10:25 AM Junio C Hamano wrote:\n>\n> Dirk Gouders writes:\n>\n> > Before the changes to count omitted objects, the function\n> > traverse_commit_list() was used and its call cannot be changed to pass\n> > a pointer to an oidset to record omitted objects.\n> >\n> > Fix the text to clarify that we now use another traversal function to\n> > be able to pass the pointer to the introduced oidset.\n> >\n> > Helped-by: Kyle Lippincott \n> > Signed-off-by: Dirk Gouders \n> > ---\n> > Documentation/MyFirstObjectWalk.txt | 14 ++++++++------\n> > 1 file changed, 8 insertions(+), 6 deletions(-)\n> >\n> > diff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\n> > index a06c712e46..811175837c 100644\n> > --- a/Documentation/MyFirstObjectWalk.txt\n> > +++ b/Documentation/MyFirstObjectWalk.txt\n> > @@ -754,10 +754,11 @@ points to the same tree object as its grandparent.)\n> > === Counting Omitted Objects\n> >\n> > We also have the capability to enumerate all objects which were omitted by a\n> > -filter, like with `git log --filter= --filter-print-omitted`. Asking\n> > -`traverse_commit_list_filtered()` to populate the `omitted` list means that our\n> > -object walk does not perform any better than an unfiltered object walk; all\n> > -reachable objects are walked in order to populate the list.\n> > +filter, like with `git log --filter= --filter-print-omitted`. To do this,\n> > +change `traverse_commit_list()` to `traverse_commit_list_filtered()`, which is\n> > +able to populate an `omitted` list. Note that this means that our object walk\n>\n> \"this means that\" could be rephrased in a way a bit more helpful and\n> to readers with clarity, perhaps:\n>\n> Note that our object walk will not perform any better than\n> an unfiltered walk with this function, because all reachable\n> objects need to be walked in order to ...\n\nThis proposed text has a small ambiguity, it can be parsed as:\n- Note that (with this function) our object walk will not perform any\nbetter than an unfiltered walk [implying that the function change\nitself is the cause of the performance concern]\nor\n- Note that (our object walk) will not perform any better than an\n(unfiltered walk with this function) [implying that\n`traverse_commit_list_filtered` has a filtered and an unfiltered mode\nof operation [which it does...]]\n\nThe issue is that the name `traverse_commit_list_filtered` is poorly\nnamed: `traverse_commit_list` and `traverse_commit_list_filtered` are\nthe exact same function (both support filtering!), it's just that\n`traverse_commit_list_filtered` is able to announce what was filtered.\n\nPerhaps:\n\n Note that requesting the list of filtered objects may have\nperformance implications; all reachable objects will be visited in\norder to populate the list of filtered objects.\n\nI'm intentionally being ambiguous about it _definitely_ having\nperformance implications, because it's context dependent. It looks\nlike only the `filter_trees_depth` function actually changes what it\nvisits depending on whether the omits list was specified or not.\n\n>\n> > +will not perform any better than an unfiltered object walk; all reachable\n> > +objects are walked in order to populate the list.\n>\n> Other than that, looking very good.\n>\n> Thanks, both.\n>\n> > First, add the `struct oidset` and related items we will use to iterate it:\n> >\n> > @@ -778,8 +779,9 @@ static void walken_object_walk(\n> > ...\n> > ----\n> >\n> > -Modify the call to `traverse_commit_list_filtered()` to include your `omitted`\n> > -object:\n> > +Replace the call to `traverse_commit_list()` with\n> > +`traverse_commit_list_filtered()` and pass a pointer to the `omitted` oidset\n> > +defined and initialized above:\n> >\n> > ----\n> > ...\n"},{"id":"491521","messageId":"ghh6gukqn6.fsf@gouders.net","threadId":"61151","inReplyTo":"xmqqsf0ekxpu.fsf@gitster.g","subject":"Re: [PATCH v3 0/5] Fixes for Documentation/MyFirstObjectWalk.txt","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-25T20:22:53Z","receivedAt":"2024-03-25T21:08:05Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"Junio C Hamano writes:\n\n> Dirk Gouders writes:\n>\n>> The 3rd iteration for this series.\n>>\n>> I tried to credit Kyle's suggestions for 4 and 5 with Helped-by tags and\n>> hope it was adequate to do so. Actually, at least #4 was a lot more\n>> than a Helped-by, I would say...\n>\n> It seemed adequate, at least to me, but I'll leave the final say up\n> to Kyle.\n>\n> I left a few comments but overall the series is looking much nicer.\n> Thanks for working on it (and thanks for reviewing and helping,\n> Kyle).\n>\n> This is an unrelated tangent, but I wonder if we can come up with a\n> way to find breakages coming from API updates to these \"tutorial\"\n> documents. The original \"user-manual\" also shares the same issue,\n> and the issue may be deeper there as it also needs to catch up with\n> end-user facing UI updates. In any case, we somehow ended up with\n> two more \"tutorial\"-ish documents (MyFirstContribution.txt is the\n> other one) that somebody needs to keep an eye on.\n\nMy plan was to also work through MyFirstContribution.txt and test\neverything which at least should disclose currently existing issues.\n\nBut, besides those tutorial documents, many manual pages also contain\nexamples and I wonder how these are maintained.\n\nDirk\n"},{"id":"491524","messageId":"xmqqwmpqgg1g.fsf@gitster.g","threadId":"61151","inReplyTo":"ghplvikrdk.fsf@gouders.net","subject":"Re: [PATCH v3 4/5] MyFirstObjectWalk: fix description for counting omitted objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-25T21:25:31Z","receivedAt":"2024-03-25T21:25:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dirk Gouders writes:\n\n>> \"this means that\" could be rephrased in a way a bit more helpful and\n>> to readers with clarity, perhaps:\n>>\n>> \tNote that our object walk will not perform any better than\n>> \tan unfiltered walk with this function, because all reachable\n>> \tobjects need to be walked in order to ...\n>\n> Would it be OK to rearrange it even more?\n\nSure. We are in the business of clarifying this document, so making\nit easier to read is very much welcomed.\n\n"},{"id":"491572","messageId":"20240326130902.7111-1-dirk@gouders.net","threadId":"61151","inReplyTo":"cover.1711368498.git.dirk@gouders.net","subject":"[PATCH v4 0/5] Fixes for Documentation/MyFirstObjectWalk.txt","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-26T13:08:35Z","receivedAt":"2024-03-26T13:09:18Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"The 4th round of this series.\n\nChances are that I just waste your time with my attemt [4/5].\nMy appologies in advance, should this be the case.\n\nRecently, there was a discussion [1] on the groff mailing list and I\nguess I couldn't resist to try to practice what I read in the linked\nresources ;-)\n\n[1] https://lists.gnu.org/archive/html/groff/2024-03/msg00014.html\n\nCould be that the remaining controversal part of [4/5] should just be\nleft untouched, because it is consuming so much time -- I summarized\nall those versions, so that all incarnations can be compared in one\nview:\n\n* Original:\n\nAsking `traverse_commit_list_filtered()` to populate the `omitted`\nlist means that our object walk does not perform any better than an\nunfiltered object walk; all reachable objects are walked in order to\npopulate the list.\n\n* v3:\n\nNote that this means that our object walk will not perform any better\nthan an unfiltered object walk; all reachable objects are walked in\norder to populate the list.\n\n* Junio's suggestion (with minor rearrangement):\n\nNote that our object walk with this function will not perform any\nbetter than the previous unfiltered walk, because all reachable\nobjects need to be walked in order to populate the list of filtered\nobjects.\n\n* Kyle's suggestion:\n\nNote that requesting the list of filtered objects may have performance\nimplications; all reachable objects will be visited in order to\npopulate the list of filtered objects.\n\n* My new attempt (v4):\n\nThis list of filtered objects may have performance implications,\nhowever, because despite filtering objects, the possibly much larger\nset of all reachable objects must be processed in order to populate\nthat list.\n\n--\nChanges in v4:\n* Used the proper `git show` for references in [1/5] and [3/5]\n* Another attempt to write clear speach in [4/5]\n\nChanges in v3:\n* Reword the description in [4/5]\n* Add a missing slash in [5/5]\n\nChanges in v2:\n* Added Emily to Cc in the hope for a review\n* Remove superfluous tags from [1/5] and [3/5]\n* Replace bashism `|&` by `2>&1 |` in [5/5]\n--\nDirk Gouders (5):\n MyFirstObjectWalk: use additional arg in config_fn_t\n MyFirstObjectWalk: fix misspelled \"builtins/\"\n MyFirstObjectWalk: fix filtered object walk\n MyFirstObjectWalk: fix description for counting omitted objects\n MyFirstObjectWalk: add stderr to pipe processing\n\n Documentation/MyFirstObjectWalk.txt | 37 ++++++++++++++++-------------\n 1 file changed, 21 insertions(+), 16 deletions(-)\n\nRange-diff against v3:\n1: 0eeb4b78ac ! 1: 102cbc54c4 MyFirstObjectWalk: use additional arg in config_fn_t\n @@ Metadata\n ## Commit message ##\n MyFirstObjectWalk: use additional arg in config_fn_t\n \n - Commit a4e7e317 (config: add ctx arg to config_fn_t) added a fourth\n - argument to config_fn_t but did not change relevant function calls\n - in Documentation/MyFirstObjectWalk.txt.\n + Commit a4e7e317f8 (config: add ctx arg to config_fn_t, 2023-06-28)\n + added a fourth argument to config_fn_t but did not change relevant\n + function calls in Documentation/MyFirstObjectWalk.txt.\n \n Fix those calls and the example git_walken_config() to use\n that additional argument.\n2: 3122ae2472 = 2: 5fb7953f31 MyFirstObjectWalk: fix misspelled \"builtins/\"\n3: f21348ab80 ! 3: b88518df0b MyFirstObjectWalk: fix filtered object walk\n @@ Metadata\n ## Commit message ##\n MyFirstObjectWalk: fix filtered object walk\n \n - Commit f0d2f849 (MyFirstObjectWalk: update recommended usage)\n - changed a call of parse_list_objects_filter() in a way that\n - probably never worked: parse_list_objects_filter() always needed a\n - pointer as its first argument.\n + Commit f0d2f84919 (MyFirstObjectWalk: update recommended usage,\n + 2022-03-09) changed a call of parse_list_objects_filter() in a way\n + that probably never worked: parse_list_objects_filter() always needed\n + a pointer as its first argument.\n \n Fix this by removing the CALLOC_ARRAY and passing the address of\n rev->filter to parse_list_objects_filter() in accordance to\n4: cfa4b9ce50 ! 4: 11510630af MyFirstObjectWalk: fix description for counting omitted objects\n @@ Documentation/MyFirstObjectWalk.txt: points to the same tree object as its grand\n -reachable objects are walked in order to populate the list.\n +filter, like with `git log --filter= --filter-print-omitted`. To do this,\n +change `traverse_commit_list()` to `traverse_commit_list_filtered()`, which is\n -+able to populate an `omitted` list. Note that this means that our object walk\n -+will not perform any better than an unfiltered object walk; all reachable\n -+objects are walked in order to populate the list.\n ++able to populate an `omitted` list. This list of filtered objects may have\n ++performance implications, however, because despite filtering objects, the possibly\n ++much larger set of all reachable objects must be processed in order to\n ++populate that list.\n \n First, add the `struct oidset` and related items we will use to iterate it:\n \n5: c571abb49d = 5: 8920313ee2 MyFirstObjectWalk: add stderr to pipe processing\n-- \n2.43.0\n\n"},{"id":"491573","messageId":"20240326130902.7111-2-dirk@gouders.net","threadId":"61151","inReplyTo":"cover.1711368498.git.dirk@gouders.net","subject":"[PATCH v4 1/5] MyFirstObjectWalk: use additional arg in config_fn_t","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-26T13:08:36Z","receivedAt":"2024-03-26T13:09:24Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"Commit a4e7e317f8 (config: add ctx arg to config_fn_t, 2023-06-28)\nadded a fourth argument to config_fn_t but did not change relevant\nfunction calls in Documentation/MyFirstObjectWalk.txt.\n\nFix those calls and the example git_walken_config() to use\nthat additional argument.\n\nSigned-off-by: Dirk Gouders \n---\n Documentation/MyFirstObjectWalk.txt | 12 +++++++-----\n 1 file changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\nindex c68cdb11b9..cceac2df95 100644\n--- a/Documentation/MyFirstObjectWalk.txt\n+++ b/Documentation/MyFirstObjectWalk.txt\n@@ -210,13 +210,14 @@ We'll also need to include the `config.h` header:\n \n ...\n \n-static int git_walken_config(const char *var, const char *value, void *cb)\n+static int git_walken_config(const char *var, const char *value,\n+\t\t\t const struct config_context *ctx, void *cb)\n {\n \t/*\n \t * For now, we don't have any custom configuration, so fall back to\n \t * the default config.\n \t */\n-\treturn git_default_config(var, value, cb);\n+\treturn git_default_config(var, value, ctx, cb);\n }\n ----\n \n@@ -389,10 +390,11 @@ modifying `rev_info.grep_filter`, which is a `struct grep_opt`.\n First some setup. Add `grep_config()` to `git_walken_config()`:\n \n ----\n-static int git_walken_config(const char *var, const char *value, void *cb)\n+static int git_walken_config(const char *var, const char *value,\n+\t\t\t const struct config_context *ctx, void *cb)\n {\n-\tgrep_config(var, value, cb);\n-\treturn git_default_config(var, value, cb);\n+\tgrep_config(var, value, ctx, cb);\n+\treturn git_default_config(var, value, ctx, cb);\n }\n ----\n \n-- \n2.43.0\n\n"},{"id":"491574","messageId":"20240326130902.7111-3-dirk@gouders.net","threadId":"61151","inReplyTo":"cover.1711368498.git.dirk@gouders.net","subject":"[PATCH v4 2/5] MyFirstObjectWalk: fix misspelled \"builtins/\"","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-26T13:08:37Z","receivedAt":"2024-03-26T13:09:30Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"pack-objects.c resides in builtin/ (not builtins/).\n\nFix the misspelled directory name.\n\nSigned-off-by: Dirk Gouders \n---\n Documentation/MyFirstObjectWalk.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\nindex cceac2df95..c33d22ae99 100644\n--- a/Documentation/MyFirstObjectWalk.txt\n+++ b/Documentation/MyFirstObjectWalk.txt\n@@ -525,7 +525,7 @@ about each one.\n \n We can base our work on an example. `git pack-objects` prepares all kinds of\n objects for packing into a bitmap or packfile. The work we are interested in\n-resides in `builtins/pack-objects.c:get_object_list()`; examination of that\n+resides in `builtin/pack-objects.c:get_object_list()`; examination of that\n function shows that the all-object walk is being performed by\n `traverse_commit_list()` or `traverse_commit_list_filtered()`. Those two\n functions reside in `list-objects.c`; examining the source shows that, despite\n-- \n2.43.0\n\n"},{"id":"491575","messageId":"20240326130902.7111-4-dirk@gouders.net","threadId":"61151","inReplyTo":"cover.1711368498.git.dirk@gouders.net","subject":"[PATCH v4 3/5] MyFirstObjectWalk: fix filtered object walk","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-26T13:08:38Z","receivedAt":"2024-03-26T13:09:35Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"Commit f0d2f84919 (MyFirstObjectWalk: update recommended usage,\n2022-03-09) changed a call of parse_list_objects_filter() in a way\nthat probably never worked: parse_list_objects_filter() always needed\na pointer as its first argument.\n\nFix this by removing the CALLOC_ARRAY and passing the address of\nrev->filter to parse_list_objects_filter() in accordance to\nsuch a call in revisions.c, for example.\n\nSigned-off-by: Dirk Gouders \n---\n Documentation/MyFirstObjectWalk.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\nindex c33d22ae99..a06c712e46 100644\n--- a/Documentation/MyFirstObjectWalk.txt\n+++ b/Documentation/MyFirstObjectWalk.txt\n@@ -734,8 +734,8 @@ walk we've just performed:\n \t} else {\n \t\ttrace_printf(\n \t\t\t_(\"Filtered object walk with filterspec 'tree:1'.\\n\"));\n-\t\tCALLOC_ARRAY(rev->filter, 1);\n-\t\tparse_list_objects_filter(rev->filter, \"tree:1\");\n+\n+\t\tparse_list_objects_filter(&rev->filter, \"tree:1\");\n \t}\n \ttraverse_commit_list(rev, walken_show_commit,\n \t\t\t walken_show_object, NULL);\n-- \n2.43.0\n\n"},{"id":"491576","messageId":"20240326130902.7111-5-dirk@gouders.net","threadId":"61151","inReplyTo":"cover.1711368498.git.dirk@gouders.net","subject":"[PATCH v4 4/5] MyFirstObjectWalk: fix description for counting omitted objects","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-26T13:08:39Z","receivedAt":"2024-03-26T13:09:41Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"Before the changes to count omitted objects, the function\ntraverse_commit_list() was used and its call cannot be changed to pass\na pointer to an oidset to record omitted objects.\n\nFix the text to clarify that we now use another traversal function to\nbe able to pass the pointer to the introduced oidset.\n\nHelped-by: Kyle Lippincott \nSigned-off-by: Dirk Gouders \n---\n Documentation/MyFirstObjectWalk.txt | 15 +++++++++------\n 1 file changed, 9 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\nindex a06c712e46..6901561263 100644\n--- a/Documentation/MyFirstObjectWalk.txt\n+++ b/Documentation/MyFirstObjectWalk.txt\n@@ -754,10 +754,12 @@ points to the same tree object as its grandparent.)\n === Counting Omitted Objects\n \n We also have the capability to enumerate all objects which were omitted by a\n-filter, like with `git log --filter= --filter-print-omitted`. Asking\n-`traverse_commit_list_filtered()` to populate the `omitted` list means that our\n-object walk does not perform any better than an unfiltered object walk; all\n-reachable objects are walked in order to populate the list.\n+filter, like with `git log --filter= --filter-print-omitted`. To do this,\n+change `traverse_commit_list()` to `traverse_commit_list_filtered()`, which is\n+able to populate an `omitted` list. This list of filtered objects may have\n+performance implications, however, because despite filtering objects, the possibly\n+much larger set of all reachable objects must be processed in order to\n+populate that list.\n \n First, add the `struct oidset` and related items we will use to iterate it:\n \n@@ -778,8 +780,9 @@ static void walken_object_walk(\n \t...\n ----\n \n-Modify the call to `traverse_commit_list_filtered()` to include your `omitted`\n-object:\n+Replace the call to `traverse_commit_list()` with\n+`traverse_commit_list_filtered()` and pass a pointer to the `omitted` oidset\n+defined and initialized above:\n \n ----\n \t...\n-- \n2.43.0\n\n"},{"id":"491577","messageId":"20240326130902.7111-6-dirk@gouders.net","threadId":"61151","inReplyTo":"cover.1711368498.git.dirk@gouders.net","subject":"[PATCH v4 5/5] MyFirstObjectWalk: add stderr to pipe processing","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-26T13:08:40Z","receivedAt":"2024-03-26T13:09:46Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"In the last chapter of this document, pipes are used in commands to\nfilter out the first/last trace messages. But according to git(1),\ntrace messages are sent to stderr if GIT_TRACE is set to '1', so those\ncommands do not produce the described results.\n\nFix this by redirecting stderr to stdout prior to the pipe operator\nto additionally connect stderr to stdin of the latter command.\n\nFurther, while reviewing the above fix, Kyle Lippincott noticed\na second issue with the second of the examples: a missing slash in the\nexecutable path \"./bin-wrappers git\".\n\nAdd the missing slash.\n\nHelped-by: Kyle Lippincott \nSigned-off-by: Dirk Gouders \n---\n Documentation/MyFirstObjectWalk.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\nindex 6901561263..90446c30a4 100644\n--- a/Documentation/MyFirstObjectWalk.txt\n+++ b/Documentation/MyFirstObjectWalk.txt\n@@ -848,7 +848,7 @@ those lines without having to recompile.\n With only that change, run again (but save yourself some scrollback):\n \n ----\n-$ GIT_TRACE=1 ./bin-wrappers/git walken | head -n 10\n+$ GIT_TRACE=1 ./bin-wrappers/git walken 2>&1 | head -n 10\n ----\n \n Take a look at the top commit with `git show` and the object ID you printed; it\n@@ -876,7 +876,7 @@ of the first handful:\n \n ----\n $ make\n-$ GIT_TRACE=1 ./bin-wrappers git walken | tail -n 10\n+$ GIT_TRACE=1 ./bin-wrappers/git walken 2>&1 | tail -n 10\n ----\n \n The last commit object given should have the same OID as the one we saw at the\n-- \n2.43.0\n\n"},{"id":"491585","messageId":"xmqqo7b1apx3.fsf@gitster.g","threadId":"61151","inReplyTo":"20240326130902.7111-5-dirk@gouders.net","subject":"Re: [PATCH v4 4/5] MyFirstObjectWalk: fix description for counting omitted objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-26T17:00:56Z","receivedAt":"2024-03-26T17:01:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dirk Gouders writes:\n\n> diff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\n> index a06c712e46..6901561263 100644\n> --- a/Documentation/MyFirstObjectWalk.txt\n> +++ b/Documentation/MyFirstObjectWalk.txt\n> @@ -754,10 +754,12 @@ points to the same tree object as its grandparent.)\n> === Counting Omitted Objects\n> \n> We also have the capability to enumerate all objects which were omitted by a\n> -filter, like with `git log --filter= --filter-print-omitted`. Asking\n> -`traverse_commit_list_filtered()` to populate the `omitted` list means that our\n> -object walk does not perform any better than an unfiltered object walk; all\n> -reachable objects are walked in order to populate the list.\n> +filter, like with `git log --filter= --filter-print-omitted`. To do this,\n> +change `traverse_commit_list()` to `traverse_commit_list_filtered()`, which is\n> +able to populate an `omitted` list. This list of filtered objects may have\n> +performance implications, however, because despite filtering objects, the possibly\n> +much larger set of all reachable objects must be processed in order to\n> +populate that list.\n\nIt may be just me not reading what is obvious to everybody else\nclearly, in which case I am happy to take the above text as-is, but\nthe updated text that says a \"list\" may have \"performance\nimplications\" reads a bit odd. It would be understandable if you\nsaid \"asking for list of filtered objects may have\", though.\n\nAre you contrasting a call to traverse_commit_list() and\ntraverse_commit_list_filtered() and discussing their relative\nperformance? \n\nOf are you contrasting a call to traverse_commit_list_filtered()\nwith and without the omitted parameter, and saying that a call with\nomitted parameter asks the machinery to do more work so it has to\ncost more?\n\nOther than that I had no trouble with this latest round.\n\nThanks.\n"},{"id":"491596","messageId":"gh34scvjq3.fsf@gouders.net","threadId":"61151","inReplyTo":"xmqqo7b1apx3.fsf@gitster.g","subject":"Re: [PATCH v4 4/5] MyFirstObjectWalk: fix description for counting omitted objects","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-26T20:09:08Z","receivedAt":"2024-03-26T20:09:32Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"Junio C Hamano writes:\n\n> Dirk Gouders writes:\n>\n>> diff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\n>> index a06c712e46..6901561263 100644\n>> --- a/Documentation/MyFirstObjectWalk.txt\n>> +++ b/Documentation/MyFirstObjectWalk.txt\n>> @@ -754,10 +754,12 @@ points to the same tree object as its grandparent.)\n>> === Counting Omitted Objects\n>> \n>> We also have the capability to enumerate all objects which were omitted by a\n>> -filter, like with `git log --filter= --filter-print-omitted`. Asking\n>> -`traverse_commit_list_filtered()` to populate the `omitted` list means that our\n>> -object walk does not perform any better than an unfiltered object walk; all\n>> -reachable objects are walked in order to populate the list.\n>> +filter, like with `git log --filter= --filter-print-omitted`. To do this,\n>> +change `traverse_commit_list()` to `traverse_commit_list_filtered()`, which is\n>> +able to populate an `omitted` list. This list of filtered objects may have\n>> +performance implications, however, because despite filtering objects, the possibly\n>> +much larger set of all reachable objects must be processed in order to\n>> +populate that list.\n>\n> It may be just me not reading what is obvious to everybody else\n> clearly, in which case I am happy to take the above text as-is, but\n> the updated text that says a \"list\" may have \"performance\n> implications\" reads a bit odd. It would be understandable if you\n> said \"asking for list of filtered objects may have\", though.\n\nOh yes, you are right (as far as I can say): I would change this to\nsomething like:\n\n\"Asking for this list of filtered objects may cause performance\nimplications, however, because in this case, despite filtering objects,\nthe possibly much larger set of all reachable objects must be processed\nin order to populate that list.\"\n\n(Later in the document, it is suggested to do timing with the two\nversions, which kind of follows up on the performance impact that is\nfocused on, here. So, this doesn't remain an unresolved detail.)\n\n> Are you contrasting a call to traverse_commit_list() and\n> traverse_commit_list_filtered() and discussing their relative\n> performance? \n>\n> Of are you contrasting a call to traverse_commit_list_filtered()\n> with and without the omitted parameter, and saying that a call with\n> omitted parameter asks the machinery to do more work so it has to\n> cost more?\n\nThis answer has the potential to cause an enhancement request, anyway:\n\nPreviously, the document didn't state that\ntraverse_commit_list_filtered() can be used without asking for a\n`omitted` list (and I didn't change that), so the contrasting\nin my understanding explicitely is traverse_commit_list()\nvs. traverse_commit_list_filtered().\n\nThe second of your cases is only included implicitely, for those who\nknow or can guess they could use NULL as the pointer to `omitted` list.\n\nThank you for looking at this one more time!\n\nDirk\n\n> Other than that I had no trouble with this latest round.\n>\n> Thanks.\n"},{"id":"491599","messageId":"xmqq1q7waghc.fsf@gitster.g","threadId":"61151","inReplyTo":"gh34scvjq3.fsf@gouders.net","subject":"Re: [PATCH v4 4/5] MyFirstObjectWalk: fix description for counting omitted objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-26T20:24:47Z","receivedAt":"2024-03-26T20:24:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dirk Gouders writes:\n\n> Oh yes, you are right (as far as I can say): I would change this to\n> something like:\n>\n> \"Asking for this list of filtered objects may cause performance\n> implications, however, because in this case, despite filtering objects,\n> the possibly much larger set of all reachable objects must be processed\n> in order to populate that list.\"\n\nBetter, but the verb \"cause\" applied to \"performance implications\"\nfeels funny. It may \"have\" implications. Alternatively, it may\n\"cause\" degradations. As implications can be both positive or\nnegative, it would be better to say \"cause performancedegradations\"\nwhen you know if it is negative.\n\n> (Later in the document, it is suggested to do timing with the two\n> versions, which kind of follows up on the performance impact that is\n> focused on, here. So, this doesn't remain an unresolved detail.)\n\nGreat.\n\nThanks.\n\n\n"},{"id":"491625","messageId":"ezbbu7ibxxrzogb4w46qvqgtwowtaf7e5o5y5dcb53gq7ko33c@g6eror27glzn","threadId":"61151","inReplyTo":"20240326130902.7111-1-dirk@gouders.net","subject":"Re: [PATCH v4 0/5] Fixes for Documentation/MyFirstObjectWalk.txt","fromName":"Kyle Lippincott","fromEmail":"spectral@google.com","sentAt":"2024-03-27T01:04:39Z","receivedAt":"2024-03-27T01:04:51Z","isPatch":true,"sender":{"key":"spectral@google.com","avatar":"https://avatars.githubusercontent.com/u/6371650?v=4"},"body":"On Tue, Mar 26, 2024 at 02:08:35PM +0100, Dirk Gouders wrote:\n> The 4th round of this series.\n> \n> Chances are that I just waste your time with my attemt [4/5].\n> My appologies in advance, should this be the case.\n> \n> Recently, there was a discussion [1] on the groff mailing list and I\n> guess I couldn't resist to try to practice what I read in the linked\n> resources ;-)\n> \n> [1] https://lists.gnu.org/archive/html/groff/2024-03/msg00014.html\n> \n> Could be that the remaining controversal part of [4/5] should just be\n> left untouched, because it is consuming so much time -- I summarized\n> all those versions, so that all incarnations can be compared in one\n> view:\n> \n> * Original:\n> \n> Asking `traverse_commit_list_filtered()` to populate the `omitted`\n> list means that our object walk does not perform any better than an\n> unfiltered object walk; all reachable objects are walked in order to\n> populate the list.\n> \n> * v3:\n> \n> Note that this means that our object walk will not perform any better\n> than an unfiltered object walk; all reachable objects are walked in\n> order to populate the list.\n> \n> * Junio's suggestion (with minor rearrangement):\n> \n> Note that our object walk with this function will not perform any\n> better than the previous unfiltered walk, because all reachable\n> objects need to be walked in order to populate the list of filtered\n> objects.\n> \n> * Kyle's suggestion:\n> \n> Note that requesting the list of filtered objects may have performance\n> implications; all reachable objects will be visited in order to\n> populate the list of filtered objects.\n> \n> * My new attempt (v4):\n> \n> This list of filtered objects may have performance implications,\n> however, because despite filtering objects, the possibly much larger\n> set of all reachable objects must be processed in order to populate\n> that list.\n\nI agree with the issues Junio raised on this phrasing, and trust in Junio's\njudgement to get to a clear phrasing :) I'll be unresponsive to email for at\nleast the next two weeks, so please don't block awaiting my response on any\nfuture rerolls.\n\n> \n> --\n> Changes in v4:\n> * Used the proper `git show` for references in [1/5] and [3/5]\n> * Another attempt to write clear speach in [4/5]\n> \n> Changes in v3:\n> * Reword the description in [4/5]\n> * Add a missing slash in [5/5]\n> \n> Changes in v2:\n> * Added Emily to Cc in the hope for a review\n> * Remove superfluous tags from [1/5] and [3/5]\n> * Replace bashism `|&` by `2>&1 |` in [5/5]\n> --\n> Dirk Gouders (5):\n> MyFirstObjectWalk: use additional arg in config_fn_t\n> MyFirstObjectWalk: fix misspelled \"builtins/\"\n> MyFirstObjectWalk: fix filtered object walk\n> MyFirstObjectWalk: fix description for counting omitted objects\n> MyFirstObjectWalk: add stderr to pipe processing\n> \n> Documentation/MyFirstObjectWalk.txt | 37 ++++++++++++++++-------------\n> 1 file changed, 21 insertions(+), 16 deletions(-)\n> \n> Range-diff against v3:\n> 1: 0eeb4b78ac ! 1: 102cbc54c4 MyFirstObjectWalk: use additional arg in config_fn_t\n> @@ Metadata\n> ## Commit message ##\n> MyFirstObjectWalk: use additional arg in config_fn_t\n> \n> - Commit a4e7e317 (config: add ctx arg to config_fn_t) added a fourth\n> - argument to config_fn_t but did not change relevant function calls\n> - in Documentation/MyFirstObjectWalk.txt.\n> + Commit a4e7e317f8 (config: add ctx arg to config_fn_t, 2023-06-28)\n> + added a fourth argument to config_fn_t but did not change relevant\n> + function calls in Documentation/MyFirstObjectWalk.txt.\n> \n> Fix those calls and the example git_walken_config() to use\n> that additional argument.\n> 2: 3122ae2472 = 2: 5fb7953f31 MyFirstObjectWalk: fix misspelled \"builtins/\"\n> 3: f21348ab80 ! 3: b88518df0b MyFirstObjectWalk: fix filtered object walk\n> @@ Metadata\n> ## Commit message ##\n> MyFirstObjectWalk: fix filtered object walk\n> \n> - Commit f0d2f849 (MyFirstObjectWalk: update recommended usage)\n> - changed a call of parse_list_objects_filter() in a way that\n> - probably never worked: parse_list_objects_filter() always needed a\n> - pointer as its first argument.\n> + Commit f0d2f84919 (MyFirstObjectWalk: update recommended usage,\n> + 2022-03-09) changed a call of parse_list_objects_filter() in a way\n> + that probably never worked: parse_list_objects_filter() always needed\n> + a pointer as its first argument.\n> \n> Fix this by removing the CALLOC_ARRAY and passing the address of\n> rev->filter to parse_list_objects_filter() in accordance to\n> 4: cfa4b9ce50 ! 4: 11510630af MyFirstObjectWalk: fix description for counting omitted objects\n> @@ Documentation/MyFirstObjectWalk.txt: points to the same tree object as its grand\n> -reachable objects are walked in order to populate the list.\n> +filter, like with `git log --filter= --filter-print-omitted`. To do this,\n> +change `traverse_commit_list()` to `traverse_commit_list_filtered()`, which is\n> -+able to populate an `omitted` list. Note that this means that our object walk\n> -+will not perform any better than an unfiltered object walk; all reachable\n> -+objects are walked in order to populate the list.\n> ++able to populate an `omitted` list. This list of filtered objects may have\n> ++performance implications, however, because despite filtering objects, the possibly\n> ++much larger set of all reachable objects must be processed in order to\n> ++populate that list.\n> \n> First, add the `struct oidset` and related items we will use to iterate it:\n> \n> 5: c571abb49d = 5: 8920313ee2 MyFirstObjectWalk: add stderr to pipe processing\n> -- \n> 2.43.0\n> \n"},{"id":"491630","messageId":"ghttkstclo.fsf@gouders.net","threadId":"61151","inReplyTo":"ezbbu7ibxxrzogb4w46qvqgtwowtaf7e5o5y5dcb53gq7ko33c@g6eror27glzn","subject":"Re: [PATCH v4 0/5] Fixes for Documentation/MyFirstObjectWalk.txt","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-27T06:25:55Z","receivedAt":"2024-03-27T06:26:14Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"Kyle Lippincott writes:\n\n> On Tue, Mar 26, 2024 at 02:08:35PM +0100, Dirk Gouders wrote:\n>> The 4th round of this series.\n>> \n>> Chances are that I just waste your time with my attemt [4/5].\n>> My appologies in advance, should this be the case.\n>> \n>> Recently, there was a discussion [1] on the groff mailing list and I\n>> guess I couldn't resist to try to practice what I read in the linked\n>> resources ;-)\n>> \n>> [1] https://lists.gnu.org/archive/html/groff/2024-03/msg00014.html\n>> \n>> Could be that the remaining controversal part of [4/5] should just be\n>> left untouched, because it is consuming so much time -- I summarized\n>> all those versions, so that all incarnations can be compared in one\n>> view:\n>> \n>> * Original:\n>> \n>> Asking `traverse_commit_list_filtered()` to populate the `omitted`\n>> list means that our object walk does not perform any better than an\n>> unfiltered object walk; all reachable objects are walked in order to\n>> populate the list.\n>> \n>> * v3:\n>> \n>> Note that this means that our object walk will not perform any better\n>> than an unfiltered object walk; all reachable objects are walked in\n>> order to populate the list.\n>> \n>> * Junio's suggestion (with minor rearrangement):\n>> \n>> Note that our object walk with this function will not perform any\n>> better than the previous unfiltered walk, because all reachable\n>> objects need to be walked in order to populate the list of filtered\n>> objects.\n>> \n>> * Kyle's suggestion:\n>> \n>> Note that requesting the list of filtered objects may have performance\n>> implications; all reachable objects will be visited in order to\n>> populate the list of filtered objects.\n>> \n>> * My new attempt (v4):\n>> \n>> This list of filtered objects may have performance implications,\n>> however, because despite filtering objects, the possibly much larger\n>> set of all reachable objects must be processed in order to populate\n>> that list.\n>\n> I agree with the issues Junio raised on this phrasing, and trust in Junio's\n> judgement to get to a clear phrasing :) I'll be unresponsive to email for at\n> least the next two weeks, so please don't block awaiting my response on any\n> future rerolls.\n\nThank you.\n"},{"id":"491631","messageId":"gho7b0tceh.fsf@gouders.net","threadId":"61151","inReplyTo":"xmqq1q7waghc.fsf@gitster.g","subject":"Re: [PATCH v4 4/5] MyFirstObjectWalk: fix description for counting omitted objects","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-27T06:30:14Z","receivedAt":"2024-03-27T06:30:33Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"Junio C Hamano writes:\n\n> Dirk Gouders writes:\n>\n>> Oh yes, you are right (as far as I can say): I would change this to\n>> something like:\n>>\n>> \"Asking for this list of filtered objects may cause performance\n>> implications, however, because in this case, despite filtering objects,\n>> the possibly much larger set of all reachable objects must be processed\n>> in order to populate that list.\"\n>\n> Better, but the verb \"cause\" applied to \"performance implications\"\n> feels funny. It may \"have\" implications. Alternatively, it may\n> \"cause\" degradations. As implications can be both positive or\n> negative, it would be better to say \"cause performancedegradations\"\n> when you know if it is negative.\n\nThank you for the clarification with \"implications\"\nI will fix it.\n\nDirk\n\n>> (Later in the document, it is suggested to do timing with the two\n>> versions, which kind of follows up on the performance impact that is\n>> focused on, here. So, this doesn't remain an unresolved detail.)\n>\n> Great.\n>\n> Thanks.\n"},{"id":"491684","messageId":"cover.1711537370.git.dirk@gouders.net","threadId":"61151","inReplyTo":"20240326130902.7111-1-dirk@gouders.net","subject":"[PATCH v5 0/5] Fixes for Documentation/MyFirstObjectWalk.txt","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-27T11:22:11Z","receivedAt":"2024-03-27T11:22:50Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"The fifth round with the correction that a list itself cannot have\nperformance implications -- asking for one may have.\n\n---\nChanges in v5:\n* Rephrase implications that asking for an `omitted` list\n may have in [4/5]\n\nChanges in v4:\n* Used the proper `git show` for references in [1/5] and [3/5]\n* Another attempt to write clear speach in [4/5]\n\nChanges in v3:\n* Reword the description in [4/5]\n* Add a missing slash in [5/5]\n\nChanges in v2:\n* Added Emily to Cc in the hope for a review\n* Remove superfluous tags from [1/5] and [3/5]\n* Replace bashism `|&` by `2>&1 |` in [5/5]\n---\nDirk Gouders (5):\n MyFirstObjectWalk: use additional arg in config_fn_t\n MyFirstObjectWalk: fix misspelled \"builtins/\"\n MyFirstObjectWalk: fix filtered object walk\n MyFirstObjectWalk: fix description for counting omitted objects\n MyFirstObjectWalk: add stderr to pipe processing\n\n Documentation/MyFirstObjectWalk.txt | 37 ++++++++++++++++-------------\n 1 file changed, 21 insertions(+), 16 deletions(-)\n\nRange-diff against v4:\n1: 2501fea789 = 1: 292ae67548 MyFirstObjectWalk: use additional arg in config_fn_t\n2: 6b336e15d6 = 2: 99284db8c1 MyFirstObjectWalk: fix misspelled \"builtins/\"\n3: 83034594e2 = 3: e1b4a4c996 MyFirstObjectWalk: fix filtered object walk\n4: f23ff9fd1b ! 4: e6030f1c0a MyFirstObjectWalk: fix description for counting omitted objects\n @@ Documentation/MyFirstObjectWalk.txt: points to the same tree object as its grand\n -reachable objects are walked in order to populate the list.\n +filter, like with `git log --filter= --filter-print-omitted`. To do this,\n +change `traverse_commit_list()` to `traverse_commit_list_filtered()`, which is\n -+able to populate an `omitted` list. This list of filtered objects may have\n -+performance implications, however, because despite filtering objects, the possibly\n -+much larger set of all reachable objects must be processed in order to\n -+populate that list.\n ++able to populate an `omitted` list. Asking for this list of filtered objects\n ++may cause performance degradations, however, because in this case, despite\n ++filtering objects, the possibly much larger set of all reachable objects must\n ++be processed in order to populate that list.\n \n First, add the `struct oidset` and related items we will use to iterate it:\n \n5: 582cb7d44a = 5: 0a3dbd1452 MyFirstObjectWalk: add stderr to pipe processing\n-- \n2.43.0\n\n"},{"id":"491685","messageId":"292ae6754843c2d400dd54fb162f71642b73c8f1.1711537370.git.dirk@gouders.net","threadId":"61151","inReplyTo":"cover.1711537370.git.dirk@gouders.net","subject":"[PATCH v5 1/5] MyFirstObjectWalk: use additional arg in config_fn_t","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-27T11:22:12Z","receivedAt":"2024-03-27T11:22:56Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"Commit a4e7e317f8 (config: add ctx arg to config_fn_t, 2023-06-28)\nadded a fourth argument to config_fn_t but did not change relevant\nfunction calls in Documentation/MyFirstObjectWalk.txt.\n\nFix those calls and the example git_walken_config() to use\nthat additional argument.\n\nSigned-off-by: Dirk Gouders \n---\n Documentation/MyFirstObjectWalk.txt | 12 +++++++-----\n 1 file changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\nindex c68cdb11b9..cceac2df95 100644\n--- a/Documentation/MyFirstObjectWalk.txt\n+++ b/Documentation/MyFirstObjectWalk.txt\n@@ -210,13 +210,14 @@ We'll also need to include the `config.h` header:\n \n ...\n \n-static int git_walken_config(const char *var, const char *value, void *cb)\n+static int git_walken_config(const char *var, const char *value,\n+\t\t\t const struct config_context *ctx, void *cb)\n {\n \t/*\n \t * For now, we don't have any custom configuration, so fall back to\n \t * the default config.\n \t */\n-\treturn git_default_config(var, value, cb);\n+\treturn git_default_config(var, value, ctx, cb);\n }\n ----\n \n@@ -389,10 +390,11 @@ modifying `rev_info.grep_filter`, which is a `struct grep_opt`.\n First some setup. Add `grep_config()` to `git_walken_config()`:\n \n ----\n-static int git_walken_config(const char *var, const char *value, void *cb)\n+static int git_walken_config(const char *var, const char *value,\n+\t\t\t const struct config_context *ctx, void *cb)\n {\n-\tgrep_config(var, value, cb);\n-\treturn git_default_config(var, value, cb);\n+\tgrep_config(var, value, ctx, cb);\n+\treturn git_default_config(var, value, ctx, cb);\n }\n ----\n \n-- \n2.43.0\n\n"},{"id":"491686","messageId":"99284db8c18b4b53765126e04f6f7248058ac1b4.1711537370.git.dirk@gouders.net","threadId":"61151","inReplyTo":"cover.1711537370.git.dirk@gouders.net","subject":"[PATCH v5 2/5] MyFirstObjectWalk: fix misspelled \"builtins/\"","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-27T11:22:13Z","receivedAt":"2024-03-27T11:23:02Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"pack-objects.c resides in builtin/ (not builtins/).\n\nFix the misspelled directory name.\n\nSigned-off-by: Dirk Gouders \n---\n Documentation/MyFirstObjectWalk.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\nindex cceac2df95..c33d22ae99 100644\n--- a/Documentation/MyFirstObjectWalk.txt\n+++ b/Documentation/MyFirstObjectWalk.txt\n@@ -525,7 +525,7 @@ about each one.\n \n We can base our work on an example. `git pack-objects` prepares all kinds of\n objects for packing into a bitmap or packfile. The work we are interested in\n-resides in `builtins/pack-objects.c:get_object_list()`; examination of that\n+resides in `builtin/pack-objects.c:get_object_list()`; examination of that\n function shows that the all-object walk is being performed by\n `traverse_commit_list()` or `traverse_commit_list_filtered()`. Those two\n functions reside in `list-objects.c`; examining the source shows that, despite\n-- \n2.43.0\n\n"},{"id":"491687","messageId":"e1b4a4c996e5d29d441a630ebccc4c55de1925e3.1711537370.git.dirk@gouders.net","threadId":"61151","inReplyTo":"cover.1711537370.git.dirk@gouders.net","subject":"[PATCH v5 3/5] MyFirstObjectWalk: fix filtered object walk","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-27T11:22:14Z","receivedAt":"2024-03-27T11:23:08Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"Commit f0d2f84919 (MyFirstObjectWalk: update recommended usage,\n2022-03-09) changed a call of parse_list_objects_filter() in a way\nthat probably never worked: parse_list_objects_filter() always needed\na pointer as its first argument.\n\nFix this by removing the CALLOC_ARRAY and passing the address of\nrev->filter to parse_list_objects_filter() in accordance to\nsuch a call in revisions.c, for example.\n\nSigned-off-by: Dirk Gouders \n---\n Documentation/MyFirstObjectWalk.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\nindex c33d22ae99..a06c712e46 100644\n--- a/Documentation/MyFirstObjectWalk.txt\n+++ b/Documentation/MyFirstObjectWalk.txt\n@@ -734,8 +734,8 @@ walk we've just performed:\n \t} else {\n \t\ttrace_printf(\n \t\t\t_(\"Filtered object walk with filterspec 'tree:1'.\\n\"));\n-\t\tCALLOC_ARRAY(rev->filter, 1);\n-\t\tparse_list_objects_filter(rev->filter, \"tree:1\");\n+\n+\t\tparse_list_objects_filter(&rev->filter, \"tree:1\");\n \t}\n \ttraverse_commit_list(rev, walken_show_commit,\n \t\t\t walken_show_object, NULL);\n-- \n2.43.0\n\n"},{"id":"491688","messageId":"e6030f1c0a918deae6f7d9a6353cb8cc3df1ef80.1711537370.git.dirk@gouders.net","threadId":"61151","inReplyTo":"cover.1711537370.git.dirk@gouders.net","subject":"[PATCH v5 4/5] MyFirstObjectWalk: fix description for counting omitted objects","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-27T11:22:15Z","receivedAt":"2024-03-27T11:23:13Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"Before the changes to count omitted objects, the function\ntraverse_commit_list() was used and its call cannot be changed to pass\na pointer to an oidset to record omitted objects.\n\nFix the text to clarify that we now use another traversal function to\nbe able to pass the pointer to the introduced oidset.\n\nHelped-by: Kyle Lippincott \nSigned-off-by: Dirk Gouders \n---\n Documentation/MyFirstObjectWalk.txt | 15 +++++++++------\n 1 file changed, 9 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\nindex a06c712e46..e969a3a68a 100644\n--- a/Documentation/MyFirstObjectWalk.txt\n+++ b/Documentation/MyFirstObjectWalk.txt\n@@ -754,10 +754,12 @@ points to the same tree object as its grandparent.)\n === Counting Omitted Objects\n \n We also have the capability to enumerate all objects which were omitted by a\n-filter, like with `git log --filter= --filter-print-omitted`. Asking\n-`traverse_commit_list_filtered()` to populate the `omitted` list means that our\n-object walk does not perform any better than an unfiltered object walk; all\n-reachable objects are walked in order to populate the list.\n+filter, like with `git log --filter= --filter-print-omitted`. To do this,\n+change `traverse_commit_list()` to `traverse_commit_list_filtered()`, which is\n+able to populate an `omitted` list. Asking for this list of filtered objects\n+may cause performance degradations, however, because in this case, despite\n+filtering objects, the possibly much larger set of all reachable objects must\n+be processed in order to populate that list.\n \n First, add the `struct oidset` and related items we will use to iterate it:\n \n@@ -778,8 +780,9 @@ static void walken_object_walk(\n \t...\n ----\n \n-Modify the call to `traverse_commit_list_filtered()` to include your `omitted`\n-object:\n+Replace the call to `traverse_commit_list()` with\n+`traverse_commit_list_filtered()` and pass a pointer to the `omitted` oidset\n+defined and initialized above:\n \n ----\n \t...\n-- \n2.43.0\n\n"},{"id":"491689","messageId":"0a3dbd14525ba9784ed8d5c329e757d5068e8b36.1711537370.git.dirk@gouders.net","threadId":"61151","inReplyTo":"cover.1711537370.git.dirk@gouders.net","subject":"[PATCH v5 5/5] MyFirstObjectWalk: add stderr to pipe processing","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-27T11:22:16Z","receivedAt":"2024-03-27T11:23:16Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"In the last chapter of this document, pipes are used in commands to\nfilter out the first/last trace messages. But according to git(1),\ntrace messages are sent to stderr if GIT_TRACE is set to '1', so those\ncommands do not produce the described results.\n\nFix this by redirecting stderr to stdout prior to the pipe operator\nto additionally connect stderr to stdin of the latter command.\n\nFurther, while reviewing the above fix, Kyle Lippincott noticed\na second issue with the second of the examples: a missing slash in the\nexecutable path \"./bin-wrappers git\".\n\nAdd the missing slash.\n\nHelped-by: Kyle Lippincott \nSigned-off-by: Dirk Gouders \n---\n Documentation/MyFirstObjectWalk.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt\nindex e969a3a68a..dec8afe5b1 100644\n--- a/Documentation/MyFirstObjectWalk.txt\n+++ b/Documentation/MyFirstObjectWalk.txt\n@@ -848,7 +848,7 @@ those lines without having to recompile.\n With only that change, run again (but save yourself some scrollback):\n \n ----\n-$ GIT_TRACE=1 ./bin-wrappers/git walken | head -n 10\n+$ GIT_TRACE=1 ./bin-wrappers/git walken 2>&1 | head -n 10\n ----\n \n Take a look at the top commit with `git show` and the object ID you printed; it\n@@ -876,7 +876,7 @@ of the first handful:\n \n ----\n $ make\n-$ GIT_TRACE=1 ./bin-wrappers git walken | tail -n 10\n+$ GIT_TRACE=1 ./bin-wrappers/git walken 2>&1 | tail -n 10\n ----\n \n The last commit object given should have the same OID as the one we saw at the\n-- \n2.43.0\n\n"}]}