{"thread":{"id":"61095","subject":"[PATCH 0/5] Fixes for Documentation/MyFirstObjectWalk.txt","startedAt":"2024-03-11T21:43:51Z","lastAt":"2024-03-12T19:29:59Z","messageCount":11,"participants":["Dirk Gouders","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"490407","messageId":"cover.1710192973.git.dirk@gouders.net","threadId":"61095","inReplyTo":null,"subject":"[PATCH 0/5] Fixes for Documentation/MyFirstObjectWalk.txt","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-11T21:36:13Z","receivedAt":"2024-03-11T21:43:51Z","isPatch":true,"sender":{"key":"dirk@gouders.net","avatar":"https://avatars.githubusercontent.com/u/81326422?v=4"},"body":"This series contains fixes for some problems in\nDocumentation/MyFirstObjectWalk.txt that I noticed while working\nthrough that document.\n\nAll patches except [4/5] fix real errors that cause comilation\nfailures or don't produce the wanted output.\nWith the fix in [3/5] I'm rather uncertain if it is correct and I\nwould like to ask for special attention, here.\n\nFor this first round I send separate patches in the hope that\nsimplifies reviews and also, because the reasons for those problems\ndiffer. Finally, those patches could be merged into one should this be\nmore appropriate.\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\n-- \n2.43.0\n"},{"id":"490408","messageId":"15b74566e07a39902556cd620e5cd6df1da3c7df.1710192973.git.dirk@gouders.net","threadId":"61095","inReplyTo":"cover.1710192973.git.dirk@gouders.net","subject":"[PATCH 1/5] MyFirstObjectWalk: use additional arg in config_fn_t","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-11T10:11:11Z","receivedAt":"2024-03-11T21:44:43Z","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\nFixes: a4e7e317 (config: add ctx arg to config_fn_t)\nCc: Glen Choo \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":"490409","messageId":"c1ac70584006f5a7906acf7ffbb88bfb11ca2c95.1710192973.git.dirk@gouders.net","threadId":"61095","inReplyTo":"cover.1710192973.git.dirk@gouders.net","subject":"[PATCH 2/5] MyFirstObjectWalk: fix misspelled \"builtins/\"","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-11T10:26:35Z","receivedAt":"2024-03-11T21:45:31Z","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":"490410","messageId":"0f67a161efc2635bb453eff2f6ab34b94c8794ae.1710192973.git.dirk@gouders.net","threadId":"61095","inReplyTo":"cover.1710192973.git.dirk@gouders.net","subject":"[PATCH 3/5] MyFirstObjectWalk: fix filtered object walk","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-11T12:47:24Z","receivedAt":"2024-03-11T21:46:13Z","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\nFixes: f0d2f849 (MyFirstObjectWalk: update recommended usage)\nCc: Derrick Stolee \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":"490411","messageId":"637070dd48ceefc5c2b7a902dd5eb3ecde503894.1710192973.git.dirk@gouders.net","threadId":"61095","inReplyTo":"cover.1710192973.git.dirk@gouders.net","subject":"[PATCH 4/5] MyFirstObjectWalk: fix description for counting omitted objects","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-11T13:29:24Z","receivedAt":"2024-03-11T21:46:57Z","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":"490412","messageId":"a2d30eff217e560873c49fe45116c784dc12fd32.1710192973.git.dirk@gouders.net","threadId":"61095","inReplyTo":"cover.1710192973.git.dirk@gouders.net","subject":"[PATCH 5/5] MyFirstObjectWalk: add stderr to pipe processing","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-11T21:00:12Z","receivedAt":"2024-03-11T21:47:39Z","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 using the operator '|&' to additionally connect stderr to\nstdin 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..b96724c4d7 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 |& 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 |& 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":"490434","messageId":"xmqqh6hcmhmu.fsf@gitster.g","threadId":"61095","inReplyTo":"a2d30eff217e560873c49fe45116c784dc12fd32.1710192973.git.dirk@gouders.net","subject":"Re: [PATCH 5/5] MyFirstObjectWalk: add stderr to pipe processing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-12T00:13:45Z","receivedAt":"2024-03-12T00:13:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dirk Gouders writes:\n\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 using the operator '|&' to additionally connect stderr to\n> stdin of the latter command.\n\nIsn't |& a bash-ism?\n\n upstream command >&2 | downstream command\n\nis how you would do the same more portably.\n"},{"id":"490435","messageId":"xmqqcys0mhjp.fsf@gitster.g","threadId":"61095","inReplyTo":"cover.1710192973.git.dirk@gouders.net","subject":"Re: [PATCH 0/5] Fixes for Documentation/MyFirstObjectWalk.txt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-12T00:15:38Z","receivedAt":"2024-03-12T00:15:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dirk Gouders writes:\n\n> This series contains fixes for some problems in\n> Documentation/MyFirstObjectWalk.txt that I noticed while working\n> through that document.\n>\n> All patches except [4/5] fix real errors that cause comilation\n> failures or don't produce the wanted output.\n> With the fix in [3/5] I'm rather uncertain if it is correct and I\n> would like to ask for special attention, here.\n>\n> For this first round I send separate patches in the hope that\n> simplifies reviews and also, because the reasons for those problems\n> differ. Finally, those patches could be merged into one should this be\n> more appropriate.\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\nEmily, if I am not misremembering, this document came from your\ndirection, so perhaps you have enough familiarity to lend these\npatches an eye or two to look them over?\n\nThanks.\n"},{"id":"490436","messageId":"xmqq4jdcmhfm.fsf@gitster.g","threadId":"61095","inReplyTo":"15b74566e07a39902556cd620e5cd6df1da3c7df.1710192973.git.dirk@gouders.net","subject":"Re: [PATCH 1/5] MyFirstObjectWalk: use additional arg in config_fn_t","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-12T00:18:05Z","receivedAt":"2024-03-12T00:18:08Z","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> 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\n> Fixes: a4e7e317 (config: add ctx arg to config_fn_t)\n> Cc: Glen Choo \n\nI know what you wanted to convey with these, but in this project we\ndo not use them. Both becomes unnecessary once you write the first\npart of the log message above ;-)\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":"490484","messageId":"ghbk7ja5k2.fsf@gouders.net","threadId":"61095","inReplyTo":"xmqqh6hcmhmu.fsf@gitster.g","subject":"Re: [PATCH 5/5] MyFirstObjectWalk: add stderr to pipe processing","fromName":"Dirk Gouders","fromEmail":"dirk@gouders.net","sentAt":"2024-03-12T14:27:41Z","receivedAt":"2024-03-12T14:27:51Z","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>> 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 using the operator '|&' to additionally connect stderr to\n>> stdin of the latter command.\n>\n> Isn't |& a bash-ism?\n>\n> upstream command >&2 | downstream command\n>\n> is how you would do the same more portably.\n\nAh yes, it is a bash-ism, thank you.\n\nI guess you meant\n\n upstream command 2>&1 | downstream command\n\nI will wait a bit (some days) for other comments\nand then at least remove this bash-ism plus the wrong tags you also\nmentioned -- also thanks for that.\n\nDirk\n"},{"id":"490499","messageId":"xmqqwmq7jljk.fsf@gitster.g","threadId":"61095","inReplyTo":"ghbk7ja5k2.fsf@gouders.net","subject":"Re: [PATCH 5/5] MyFirstObjectWalk: add stderr to pipe processing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-12T19:29:51Z","receivedAt":"2024-03-12T19:29:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dirk Gouders writes:\n\n> Ah yes, it is a bash-ism, thank you.\n>\n> I guess you meant\n\nYup, thanks, that was a typo.\n\n> upstream command 2>&1 | downstream command\n"}]}