{"thread":{"id":"64286","subject":"[PATCH] commit-graph: add new config for changed-paths & recommend it in scalar","startedAt":"2025-10-09T21:01:53Z","lastAt":"2025-10-29T21:04:29Z","messageCount":9,"participants":["Emily Yang via GitGitGadget","Junio C Hamano","Derrick Stolee","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"528404","messageId":"pull.1983.git.1760043710502.gitgitgadget@gmail.com","threadId":"64286","inReplyTo":null,"subject":"[PATCH] commit-graph: add new config for changed-paths & recommend it in scalar","fromName":"Emily Yang via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-10-09T21:01:50Z","receivedAt":"2025-10-09T21:01:53Z","isPatch":true,"sender":{"key":"name:Emily Yang","avatar":null},"body":"From: Emily Yang <emilyyang.git@gmail.com>\n\nThe changed-path Bloom filters feature has proven stable and reliable\nover several years of use, delivering significant performance\nimprovement for file history computation in large monorepos. Currently\na user can opt-in to writing the changed-path Bloom filters using the\n\"--changed-paths\" option to \"git commit-graph write\". The filters will\nbe persisted until the user drops the filters using the\n\"--no-changed-paths\" option.\n\nLarge monorepos using Git's background maintenance to build and update\ncommit-graph files could use an easy switch to enable this feature\nwithout a foreground computation. In this commit, we're proposing a new\nconfig option \"commitGraph.changedPaths\" - \"true\" value acts like\n\"--changed-paths\"; \"false\" disables a previous \"true\" config value but\ndoesn't imply \"--no-changed-paths\". This config will always respect the\nprecedence of command line option \"--changed-paths\" and\n\"--no-changed-paths\".\n\nWe also set this new config as optional recommended config in scalar to\nturn on this feature for large repos.\n\nHelped-by: Derrick Stolee <stolee@gmail.com>\nSigned-off-by: Emily Yang <emilyyang.git@gmail.com>\n---\n    commit-graph: add new config for changed-paths & recommend it in scalar\n    \n    Hello,\n    \n    I'm Emily and I'm interested in contributing to Git. This is my first\n    contribution to Git, super excited!\n    \n    I'm from Microsoft and spend most of my time working in the Office\n    MonoRepo (OMR, one of the largest repos in the world). Recently I've\n    been working with Derrick Stolee on Git performance related topics. We'd\n    love to propose a small enhancement on the existing changed-paths Bloom\n    filters feature to benefit large repos like OMR. Please kindly review\n    the code and provide your feedback!\n    \n    Thanks, Emily\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1983%2Femilyyang-ms%2Fchanged-paths-config-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1983/emilyyang-ms/changed-paths-config-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1983\n\n Documentation/config/commitgraph.adoc |  8 +++++\n builtin/commit-graph.c                |  2 ++\n scalar.c                              |  1 +\n t/t5318-commit-graph.sh               | 44 +++++++++++++++++++++++++++\n 4 files changed, 55 insertions(+)\n\ndiff --git a/Documentation/config/commitgraph.adoc b/Documentation/config/commitgraph.adoc\nindex 7f8c9d6638..c540e8a43d 100644\n--- a/Documentation/config/commitgraph.adoc\n+++ b/Documentation/config/commitgraph.adoc\n@@ -8,6 +8,14 @@ commitGraph.maxNewFilters::\n \tSpecifies the default value for the `--max-new-filters` option of `git\n \tcommit-graph write` (c.f., linkgit:git-commit-graph[1]).\n \n+commitGraph.changedPaths::\n+\tIf true, then `git commit-graph write` will compute and write\n+\tchanged-path Bloom filters by default, equivalent to passing\n+\t`--changed-paths`. If false or unset, changed-path Bloom filters\n+\twill only be written when explicitly requested via `--changed-paths`.\n+\tCommand-line options always take precedence over this configuration.\n+\tDefaults to unset.\n+\n commitGraph.readChangedPaths::\n \tDeprecated. Equivalent to commitGraph.changedPathsVersion=-1 if true, and\n \tcommitGraph.changedPathsVersion=0 if false. (If commitGraph.changedPathVersion\ndiff --git a/builtin/commit-graph.c b/builtin/commit-graph.c\nindex fe3ebaadad..d62005edc0 100644\n--- a/builtin/commit-graph.c\n+++ b/builtin/commit-graph.c\n@@ -210,6 +210,8 @@ static int git_commit_graph_write_config(const char *var, const char *value,\n {\n \tif (!strcmp(var, \"commitgraph.maxnewfilters\"))\n \t\twrite_opts.max_new_filters = git_config_int(var, value, ctx->kvi);\n+\telse if (!strcmp(var, \"commitgraph.changedpaths\"))\n+\t\topts.enable_changed_paths = git_config_bool(var, value) ? 1 : -1;\n \t/*\n \t * No need to fall-back to 'git_default_config', since this was already\n \t * called in 'cmd_commit_graph()'.\ndiff --git a/scalar.c b/scalar.c\nindex 4a373c133d..f754311627 100644\n--- a/scalar.c\n+++ b/scalar.c\n@@ -166,6 +166,7 @@ static int set_recommended_config(int reconfigure)\n #endif\n \t\t/* Optional */\n \t\t{ \"status.aheadBehind\", \"false\" },\n+\t\t{ \"commitGraph.changedPaths\", \"true\" },\n \t\t{ \"commitGraph.generationVersion\", \"1\" },\n \t\t{ \"core.autoCRLF\", \"false\" },\n \t\t{ \"core.safeCRLF\", \"false\" },\ndiff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\nindex 0b3404f58f..98c6910963 100755\n--- a/t/t5318-commit-graph.sh\n+++ b/t/t5318-commit-graph.sh\n@@ -946,4 +946,48 @@ test_expect_success 'stale commit cannot be parsed when traversing graph' '\n \t)\n '\n \n+test_expect_success 'config commitGraph.changedPaths acts like --changed-paths' '\n+\tgit init config-changed-paths &&\n+\t(\n+\t\tcd config-changed-paths &&\n+\n+\t\t# commitGraph.changedPaths is not set and it should not write Bloom filters\n+\t\ttest_commit first &&\n+\t\tGIT_PROGRESS_DELAY=0 git commit-graph write --reachable --progress 2>error &&\n+\t\ttest_grep ! \"Bloom filters\" error &&\n+\n+\t\t# Set commitGraph.changedPaths to true and it should write Bloom filters\n+\t\ttest_commit second &&\n+\t\tgit config commitGraph.changedPaths true &&\n+\t\tGIT_PROGRESS_DELAY=0 git commit-graph write --reachable --progress 2>error &&\n+\t\ttest_grep \"Bloom filters\" error &&\n+\n+\t\t# Add one more config commitGraph.changedPaths as false to disable the previous true config value\n+\t\t# It should still write Bloom filters due to existing filters\n+\t\ttest_commit third &&\n+\t\tgit config --add commitGraph.changedPaths false &&\n+\t\tGIT_PROGRESS_DELAY=0 git commit-graph write --reachable --progress 2>error &&\n+\t\ttest_grep \"Bloom filters\" error &&\n+\n+\t\t# commitGraph.changedPaths is still false and command line options should take precedence\n+\t\ttest_commit fourth &&\n+\t\tGIT_PROGRESS_DELAY=0 git commit-graph write --no-changed-paths --reachable --progress 2>error &&\n+\t\ttest_grep ! \"Bloom filters\" error &&\n+\t\tGIT_PROGRESS_DELAY=0 git commit-graph write --reachable --progress 2>error &&\n+\t\ttest_grep ! \"Bloom filters\" error &&\n+\n+\t\t# commitGraph.changedPaths is all cleared and then set to false again, command line options should take precedence\n+\t\ttest_commit fifth &&\n+\t\tgit config --unset-all commitGraph.changedPaths &&\n+\t\tgit config commitGraph.changedPaths false &&\n+\t\tGIT_PROGRESS_DELAY=0 git commit-graph write --changed-paths --reachable --progress 2>error &&\n+\t\ttest_grep \"Bloom filters\" error &&\n+\n+\t\t# commitGraph.changedPaths is still false and it should write Bloom filters due to existing filters\n+\t\ttest_commit sixth &&\n+\t\tGIT_PROGRESS_DELAY=0 git commit-graph write --reachable --progress 2>error &&\n+\t\ttest_grep \"Bloom filters\" error\n+\t)\n+'\n+\n test_done\n\nbase-commit: 79cf913ea9321f774da29b2330b5781d5ff420ef\n-- \ngitgitgadget\n"},{"id":"528428","messageId":"xmqqecrbd7yh.fsf@gitster.g","threadId":"64286","inReplyTo":"pull.1983.git.1760043710502.gitgitgadget@gmail.com","subject":"Re: [PATCH] commit-graph: add new config for changed-paths & recommend it in scalar","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-09T22:30:14Z","receivedAt":"2025-10-09T22:30:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Emily Yang via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Emily Yang <emilyyang.git@gmail.com>\n>\n> The changed-path Bloom filters feature has proven stable and reliable\n> over several years of use, delivering significant performance\n> improvement for file history computation in large monorepos. Currently\n> a user can opt-in to writing the changed-path Bloom filters using the\n> \"--changed-paths\" option to \"git commit-graph write\". The filters will\n> be persisted until the user drops the filters using the\n> \"--no-changed-paths\" option.\n\nMakes sense.\n\n> Large monorepos using Git's background maintenance to build and update\n> commit-graph files could use an easy switch to enable this feature\n> without a foreground computation.\n\nAgain makes sense.\n\n> In this commit, we're proposing a new\n> config option \"commitGraph.changedPaths\" - \"true\" value acts like\n> \"--changed-paths\"; \"false\" disables a previous \"true\" config value but\n> doesn't imply \"--no-changed-paths\".\n\nThe way the above is phrased is so unusual that I am afraid it would\nconfuse readers.\n\nWhen a configuration variable gives an opportunity for the users to\noverride the hardcoded default (in this case, --no-changed-paths has\nbeen the traditional default, and graph.changedPaths=true would make\nus pretend as if --changed-paths were given from the command line).\nSo if we were to have this configuration variable, setting it false\nMUST make it pretend as if --no-changed-paths were given from the\ncommand line, and MUST continue to do so even in some future we\nchanged the hardcoded default to be \"true\" (i.e., unless the user\nsays graph.changedPath=false in the configuration and/or declines\nwith \"--no-changed-paths\" from the command line, we will record the\nchanged paths filter by default).\n\nSetting commitGraph.changedPaths to true should mean that the\n\"git commit-graph write\" command behaves as if --changed-paths\nwere given immediately after that \"write\", so that an end-user\ncommmand\n\n    $ git commit-graph write\n\nshould behave as if it was written like this\n\n    $ git commit-graph write --changed-paths\n\nand\n\n    $ git commit-graph write --no-changed-paths\n\nshould behave as if it was written like this\n\n    $ git commit-graph write --changed-paths --no-changed-paths\n\ni.e. allowing the command line --no-changed-paths to override it.\n\nSetting commitGraph.changedPaths to false should similarly mean that\n\"--no-changed-paths\" implicitly is added immediately after \"write\",\nmeaning that \n\n    $ git commit-graph write\n\nshould behave as if it was written like this\n\n    $ git commit-graph write --no-changed-paths\n\nAs it is the default not to write changed-paths filter, this has no\neffect, but I would say it still \"implies\" --no-changed-paths, and I\nhope you'd agree once you imagine a hypothetical future in which the\ndefault for \"git commit-graph write\" is to write changed-paths\nfilter by default.\n\n> This config will always respect the\n> precedence of command line option \"--changed-paths\" and\n> \"--no-changed-paths\".\n\nThis is a bit unusual way to phrase this, but I think it makes sense\nfor the configuraiton variable to be overridden by the command line\noption, as that is the bog-standard way configuration variables and\ncommand line options interact with each other; it is so standard\nthat it is probably not even worth saying it.\n\n> We also set this new config as optional recommended config in scalar to\n> turn on this feature for large repos.\n\nGreat.  Yes, from the start of the description above, anybody who is\naware of the \"scalar\" effort would be anticipating this conclusion.\n\n> Helped-by: Derrick Stolee <stolee@gmail.com>\n> Signed-off-by: Emily Yang <emilyyang.git@gmail.com>\n> ---\n>     commit-graph: add new config for changed-paths & recommend it in scalar\n>     \n>     Hello,\n>     \n>     I'm Emily and I'm interested in contributing to Git. This is my first\n>     contribution to Git, super excited!\n>     \n>     I'm from Microsoft and spend most of my time working in the Office\n>     MonoRepo (OMR, one of the largest repos in the world). Recently I've\n>     been working with Derrick Stolee on Git performance related topics. We'd\n>     love to propose a small enhancement on the existing changed-paths Bloom\n>     filters feature to benefit large repos like OMR. Please kindly review\n>     the code and provide your feedback!\n>     \n>     Thanks, Emily\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1983%2Femilyyang-ms%2Fchanged-paths-config-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1983/emilyyang-ms/changed-paths-config-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1983\n>\n>  Documentation/config/commitgraph.adoc |  8 +++++\n>  builtin/commit-graph.c                |  2 ++\n>  scalar.c                              |  1 +\n>  t/t5318-commit-graph.sh               | 44 +++++++++++++++++++++++++++\n>  4 files changed, 55 insertions(+)\n>\n> diff --git a/Documentation/config/commitgraph.adoc b/Documentation/config/commitgraph.adoc\n> index 7f8c9d6638..c540e8a43d 100644\n> --- a/Documentation/config/commitgraph.adoc\n> +++ b/Documentation/config/commitgraph.adoc\n> @@ -8,6 +8,14 @@ commitGraph.maxNewFilters::\n>  \tSpecifies the default value for the `--max-new-filters` option of `git\n>  \tcommit-graph write` (c.f., linkgit:git-commit-graph[1]).\n>  \n> +commitGraph.changedPaths::\n> +\tIf true, then `git commit-graph write` will compute and write\n> +\tchanged-path Bloom filters by default, equivalent to passing\n> +\t`--changed-paths`. If false or unset, changed-path Bloom filters\n> +\twill only be written when explicitly requested via `--changed-paths`.\n> +\tCommand-line options always take precedence over this configuration.\n> +\tDefaults to unset.\n> +\n>  commitGraph.readChangedPaths::\n>  \tDeprecated. Equivalent to commitGraph.changedPathsVersion=-1 if true, and\n>  \tcommitGraph.changedPathsVersion=0 if false. (If commitGraph.changedPathVersion\n> diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c\n> index fe3ebaadad..d62005edc0 100644\n> --- a/builtin/commit-graph.c\n> +++ b/builtin/commit-graph.c\n> @@ -210,6 +210,8 @@ static int git_commit_graph_write_config(const char *var, const char *value,\n>  {\n>  \tif (!strcmp(var, \"commitgraph.maxnewfilters\"))\n>  \t\twrite_opts.max_new_filters = git_config_int(var, value, ctx->kvi);\n> +\telse if (!strcmp(var, \"commitgraph.changedpaths\"))\n> +\t\topts.enable_changed_paths = git_config_bool(var, value) ? 1 : -1;\n\nThis is iffy.\n\nUnless the way existing command line parser figures out if the user\nwants or does not want to use the feature is so screwed up, you\nshouldn't have to do any such thing.\n\nWhy do you need to special case 'false' this way?  The usual\npractice is\n\n * First, you initialize the variable \"enable_changed_paths\" with the\n   hardcoded default.  In this case, as changed-paths is not written\n   by default, you'd initialize it to 0 (not -1).\n\n * Then you read from the configuration variables to update it.  If\n   you see commitgraph.changedPaths configuration, you take its\n   value (either 0 or 1 as it is a Boolean variable) and overwrite\n   the hardcoded default in the \"enable_changed_paths\" variable.  Otherwise\n   you leave \"enable_changed_paths\" as-is.\n\n * If you also have environment variable override, then you see if\n   there is the environment variable you care about, and if so,\n   override \"enable_changed_paths\" with its value.  Otherwise you leave\n   \"enable_changed_paths\" as-is.\n\n * Finally you read from the command line options using\n   parse_options().  If there are command line options given,\n   \"enable_changed_paths\" would be overriden again.\n\nIf the way the existing parser sets up enable_changed_paths is\nscrewed up and does not follow the above pattern (I didn't check),\nperhaps you'd need a preliminary clean-up patch before adding this\nnew feature.\n\nThanks.\n"},{"id":"528504","messageId":"80ab806c-1a53-408b-9120-cae4faae0491@gmail.com","threadId":"64286","inReplyTo":"pull.1983.git.1760043710502.gitgitgadget@gmail.com","subject":"Re: [PATCH] commit-graph: add new config for changed-paths & recommend it in scalar","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2025-10-10T12:32:23Z","receivedAt":"2025-10-10T12:32:25Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/9/2025 5:01 PM, Emily Yang via GitGitGadget wrote:\n> From: Emily Yang <emilyyang.git@gmail.com>\n>     Hello,\n>     \n>     I'm Emily and I'm interested in contributing to Git. This is my first\n>     contribution to Git, super excited!\n>     \n>     I'm from Microsoft and spend most of my time working in the Office\n>     MonoRepo (OMR, one of the largest repos in the world). Recently I've\n>     been working with Derrick Stolee on Git performance related topics. We'd\n>     love to propose a small enhancement on the existing changed-paths Bloom\n>     filters feature to benefit large repos like OMR. Please kindly review\n>     the code and provide your feedback!\n\nCongratulations on your first Git submission, Emily!\n\nFor the rest on the list, Emily and I work together in support of engineering\nsystems at Microsoft, and her team is particularly interested in Git\nperformance for the Office monorepo. This first patch is hopefully one of\nmany to follow as we build up more people with the right expertise to make\nchanges to Git, especially at our boundaries of scale.\n\nI've already done a \"pre-review\" of this patch as part of mentoring Emily in\nher journey to Git contribution.\n\nThanks,\n-Stolee\n"},{"id":"528505","messageId":"1a88e577-a808-4815-b390-e5d2253e670c@gmail.com","threadId":"64286","inReplyTo":"xmqqecrbd7yh.fsf@gitster.g","subject":"Re: [PATCH] commit-graph: add new config for changed-paths & recommend it in scalar","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2025-10-10T12:48:23Z","receivedAt":"2025-10-10T12:48:25Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/9/2025 6:30 PM, Junio C Hamano wrote:\n> \"Emily Yang via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n> \n>> From: Emily Yang <emilyyang.git@gmail.com>\n\n>> In this commit, we're proposing a new\n>> config option \"commitGraph.changedPaths\" - \"true\" value acts like\n>> \"--changed-paths\"; \"false\" disables a previous \"true\" config value but\n>> doesn't imply \"--no-changed-paths\".\n> \n> The way the above is phrased is so unusual that I am afraid it would\n> confuse readers.\n> \n> When a configuration variable gives an opportunity for the users to\n> override the hardcoded default (in this case, --no-changed-paths has\n> been the traditional default,\n\n(I'm pointing out this statement and how it's not quite right. I'll\nexplain more fully lower in this reply.)\n\n> and graph.changedPaths=true would make\n> us pretend as if --changed-paths were given from the command line).\n> So if we were to have this configuration variable, setting it false\n> MUST make it pretend as if --no-changed-paths were given from the\n> command line, and MUST continue to do so even in some future we\n> changed the hardcoded default to be \"true\" (i.e., unless the user\n> says graph.changedPath=false in the configuration and/or declines\n> with \"--no-changed-paths\" from the command line, we will record the\n> changed paths filter by default).\n> \n> Setting commitGraph.changedPaths to true should mean that the\n> \"git commit-graph write\" command behaves as if --changed-paths\n> were given immediately after that \"write\", so that an end-user\n> commmand\n> \n>     $ git commit-graph write\n> \n> should behave as if it was written like this\n> \n>     $ git commit-graph write --changed-paths\n> \n> and\n> \n>     $ git commit-graph write --no-changed-paths\n> \n> should behave as if it was written like this\n> \n>     $ git commit-graph write --changed-paths --no-changed-paths\n> \n> i.e. allowing the command line --no-changed-paths to override it.\n> \n> Setting commitGraph.changedPaths to false should similarly mean that\n> \"--no-changed-paths\" implicitly is added immediately after \"write\",\n> meaning that \n> \n>     $ git commit-graph write\n> \n> should behave as if it was written like this\n> \n>     $ git commit-graph write --no-changed-paths\n\nOne thing that is tricky about --[no-]changed-paths is that it is a\n\"tri-state\" argument due to 0087a87ba8 (commit-graph: persist\nexistence of changed-paths, 2020-07-01):\n\n * --changed-paths : Definitely write the data, even if it didn't\n   exist already.\n\n * --no-changed-paths : Definitely _don't_ write the data, even if\n   it exists already.\n\n * (not present) : Update filters that do exist, but don't write them\n   if they don't exist.\n\nThis is reflected in how opts.enable_changed_paths is initialized to\n-1 in the existing version. Then, the config is loaded before the\narguments are parsed (this is already enforcing the precedence of\n'--max-new-filters=<N>' over the 'commitGraph.maxNewFilters' config).\n\nLater, opts.enable_changed_paths is converted into\nCOMMIT_GRAPH_WRITE_BLOOM_FILTERS or COMMIT_GRAPH_NO_WRITE_BLOOM_FILTERS\nflags for the underlying commit-graph API, with the default of -1\npassing neither flag (which will use any existing commit-graph to\npersist and extend filters that already exist).\n\nThe big reason for this is so users can use a foreground process to\ninitialize filters, then background maintenance will respect and persist\nthat behavior. The big change here is that the config allows a user to\nenable the filters and have them be computed entirely in the background.\n\nSo I think this is the root of your concerns here.\n>> @@ -210,6 +210,8 @@ static int git_commit_graph_write_config(const char *var, const char *value,\n>>  {\n>>  \tif (!strcmp(var, \"commitgraph.maxnewfilters\"))\n>>  \t\twrite_opts.max_new_filters = git_config_int(var, value, ctx->kvi);\n>> +\telse if (!strcmp(var, \"commitgraph.changedpaths\"))\n>> +\t\topts.enable_changed_paths = git_config_bool(var, value) ? 1 : -1;\n> \n> This is iffy.\n> \n> Unless the way existing command line parser figures out if the user\n> wants or does not want to use the feature is so screwed up, you\n> shouldn't have to do any such thing.\n> \n> Why do you need to special case 'false' this way? \n\nThe config now has this implication:\n\n * true : turn '(not present)' into '--changed-paths'.\n * false/unset : Continue to assume '(not present)'.\n\nAnd the typical case is that we would have 'false' imply\n'--no-changed-paths' which _removes_ filters that may exist. I\ncould see a case for this.\n\nThe situation that I wanted to think about was this:\n\n * A user sets the config to 'true' in global config.\n * They then set the config to 'false' in a specific repo.\n\nIn this case, the 'false' _disables the config_ but doesn't cause\nany existing filters to be deleted.\n\nI hope this helps. I could see a case for 'false' implying\n'--no-changed-filters' but as Emily was investigating this and\nnoting this discrepancy, we leaned in the direction of being non-\ndestructive with the config.\n\nThanks,\n-Stolee\n\n\n"},{"id":"528522","messageId":"xmqq4is6afag.fsf@gitster.g","threadId":"64286","inReplyTo":"1a88e577-a808-4815-b390-e5d2253e670c@gmail.com","subject":"Re: [PATCH] commit-graph: add new config for changed-paths & recommend it in scalar","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-10T16:32:23Z","receivedAt":"2025-10-10T16:32:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee <stolee@gmail.com> writes:\n\n> One thing that is tricky about --[no-]changed-paths is that it is a\n> \"tri-state\" argument due to 0087a87ba8 (commit-graph: persist\n> existence of changed-paths, 2020-07-01):\n>\n>  * --changed-paths : Definitely write the data, even if it didn't\n>    exist already.\n>\n>  * --no-changed-paths : Definitely _don't_ write the data, even if\n>    it exists already.\n>\n>  * (not present) : Update filters that do exist, but don't write them\n>    if they don't exist.\n\nOK, so \"--no-\" is not the usual \"no\"; it is more like \"strip\nexisting\" that implies \"even existing ones are getting nuked, there\nis no way we write new ones\".  OK, that may explain the construct I\nfound funny.  Thanks for clarifying.\n\n> The situation that I wanted to think about was this:\n>\n>  * A user sets the config to 'true' in global config.\n>  * They then set the config to 'false' in a specific repo.\n>\n> In this case, the 'false' _disables the config_ but doesn't cause\n> any existing filters to be deleted.\n\nOuch, that hurts, as they expected this specific one would drop\nexisting filters but that does not happen.\n\nPerhaps we need to strengthen the description of --no-* (if not\nrenaming it to --drop-* or something to clarify what it really\ndoes).\n\nAt least the configuration needs to be explained not like: \"setting\nit to false is different from --no-changed-paths option\".  The\ndocumentation should not stop at saying what it is not, but should\nalso say what it does.  Perhaps \"setting it to true is like always\ngiving --changed-paths, setting it to false stops writing new\nchanged paths filters, but without dropping existing changed paths\nfilters\" or something along that line.\n\nThanks.\n"},{"id":"529102","messageId":"pull.1983.v2.git.1760734739642.gitgitgadget@gmail.com","threadId":"64286","inReplyTo":"pull.1983.git.1760043710502.gitgitgadget@gmail.com","subject":"[PATCH v2] commit-graph: add new config for changed-paths & recommend it in scalar","fromName":"Emily Yang via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-10-17T20:58:59Z","receivedAt":"2025-10-17T20:59:02Z","isPatch":true,"sender":{"key":"name:Emily Yang","avatar":null},"body":"From: Emily Yang <emilyyang.git@gmail.com>\n\nThe changed-path Bloom filters feature has proven stable and reliable\nover several years of use, delivering significant performance\nimprovement for file history computation in large monorepos. Currently\na user can opt-in to writing the changed-path Bloom filters using the\n\"--changed-paths\" option to \"git commit-graph write\". The filters will\nbe persisted until the user drops the filters using the\n\"--no-changed-paths\" option. For this functionality, refer to 0087a87ba8\n(commit-graph: persist existence of changed-paths, 2020-07-01).\n\nLarge monorepos using Git's background maintenance to build and update\ncommit-graph files could use an easy switch to enable this feature\nwithout a foreground computation. In this commit, we're proposing a new\nconfig option \"commitGraph.changedPaths\":\n\n* If \"true\", \"git commit-graph write\" will write Bloom filters,\n  equivalent to passing \"--changed-paths\";\n* If \"false\" or \"unset\", Bloom filters will be written during \"git\n  commit-graph write\" only if the filters already exist in the current\n  commit-graph file. This matches the default behaviour of \"git\n  commit-graph write\" without any \"--[no-]changed-paths\" option. Note\n  \"false\" can disable a previous \"true\" config value but doesn't imply\n  \"--no-changed-paths\".\n\nThis config will always respect the precedence of command line option\n\"--[no-]changed-paths\".\n\nWe also set this new config as optional recommended config in scalar to\nturn on this feature for large repos.\n\nHelped-by: Derrick Stolee <stolee@gmail.com>\nSigned-off-by: Emily Yang <emilyyang.git@gmail.com>\n---\n    commit-graph: add new config for changed-paths & recommend it in scalar\n    \n    Hello,\n    \n    I'm Emily and I'm interested in contributing to Git. This is my first\n    contribution to Git, super excited!\n    \n    I'm from Microsoft and spend most of my time working in the Office\n    MonoRepo (OMR, one of the largest repos in the world). Recently I've\n    been working with Derrick Stolee on Git performance related topics. We'd\n    love to propose a small enhancement on the existing changed-paths Bloom\n    filters feature to benefit large repos like OMR. Please kindly review\n    the code and provide your feedback!\n    \n    What's included in v2:\n    \n    I received feedback about the confusion around the config explanation,\n    so in v2 I added more clarification in the doc and commit message,\n    hopefully it helps!\n    \n    Thanks, Emily\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1983%2Femilyyang-ms%2Fchanged-paths-config-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1983/emilyyang-ms/changed-paths-config-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1983\n\nRange-diff vs v1:\n\n 1:  90b271e905 ! 1:  365db79f4d commit-graph: add new config for changed-paths & recommend it in scalar\n     @@ Commit message\n          a user can opt-in to writing the changed-path Bloom filters using the\n          \"--changed-paths\" option to \"git commit-graph write\". The filters will\n          be persisted until the user drops the filters using the\n     -    \"--no-changed-paths\" option.\n     +    \"--no-changed-paths\" option. For this functionality, refer to 0087a87ba8\n     +    (commit-graph: persist existence of changed-paths, 2020-07-01).\n      \n          Large monorepos using Git's background maintenance to build and update\n          commit-graph files could use an easy switch to enable this feature\n          without a foreground computation. In this commit, we're proposing a new\n     -    config option \"commitGraph.changedPaths\" - \"true\" value acts like\n     -    \"--changed-paths\"; \"false\" disables a previous \"true\" config value but\n     -    doesn't imply \"--no-changed-paths\". This config will always respect the\n     -    precedence of command line option \"--changed-paths\" and\n     -    \"--no-changed-paths\".\n     +    config option \"commitGraph.changedPaths\":\n     +\n     +    * If \"true\", \"git commit-graph write\" will write Bloom filters,\n     +      equivalent to passing \"--changed-paths\";\n     +    * If \"false\" or \"unset\", Bloom filters will be written during \"git\n     +      commit-graph write\" only if the filters already exist in the current\n     +      commit-graph file. This matches the default behaviour of \"git\n     +      commit-graph write\" without any \"--[no-]changed-paths\" option. Note\n     +      \"false\" can disable a previous \"true\" config value but doesn't imply\n     +      \"--no-changed-paths\".\n     +\n     +    This config will always respect the precedence of command line option\n     +    \"--[no-]changed-paths\".\n      \n          We also set this new config as optional recommended config in scalar to\n          turn on this feature for large repos.\n     @@ Documentation/config/commitgraph.adoc: commitGraph.maxNewFilters::\n      +commitGraph.changedPaths::\n      +\tIf true, then `git commit-graph write` will compute and write\n      +\tchanged-path Bloom filters by default, equivalent to passing\n     -+\t`--changed-paths`. If false or unset, changed-path Bloom filters\n     -+\twill only be written when explicitly requested via `--changed-paths`.\n     -+\tCommand-line options always take precedence over this configuration.\n     -+\tDefaults to unset.\n     ++\t`--changed-paths`. If false or unset, changed-paths Bloom filters will\n     ++\tbe written during `git commit-graph write` only if the filters already\n     ++\texist in the current commit-graph file. This matches the default\n     ++\tbehavior of `git commit-graph write` without any `--[no-]changed-paths`\n     ++\toption. To rewrite a commit-graph file without any filters, use the\n     ++\t`--no-changed-paths` option. Command-line option `--[no-]changed-paths`\n     ++\talways takes precedence over this configuration. Defaults to unset.\n      +\n       commitGraph.readChangedPaths::\n       \tDeprecated. Equivalent to commitGraph.changedPathsVersion=-1 if true, and\n       \tcommitGraph.changedPathsVersion=0 if false. (If commitGraph.changedPathVersion\n      \n     + ## Documentation/git-commit-graph.adoc ##\n     +@@ Documentation/git-commit-graph.adoc: take a while on large repositories. It provides significant performance gains\n     + for getting history of a directory or a file with `git log -- <path>`. If\n     + this option is given, future commit-graph writes will automatically assume\n     + that this option was intended. Use `--no-changed-paths` to stop storing this\n     +-data.\n     ++data. `--changed-paths` is implied by config `commitGraph.changedPaths=true`.\n     + +\n     + With the `--max-new-filters=<n>` option, generate at most `n` new Bloom\n     + filters (if `--changed-paths` is specified). If `n` is `-1`, no limit is\n     +\n       ## builtin/commit-graph.c ##\n      @@ builtin/commit-graph.c: static int git_commit_graph_write_config(const char *var, const char *value,\n       {\n\n\n Documentation/config/commitgraph.adoc | 11 +++++++\n Documentation/git-commit-graph.adoc   |  2 +-\n builtin/commit-graph.c                |  2 ++\n scalar.c                              |  1 +\n t/t5318-commit-graph.sh               | 44 +++++++++++++++++++++++++++\n 5 files changed, 59 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config/commitgraph.adoc b/Documentation/config/commitgraph.adoc\nindex 7f8c9d6638..70a56c53d2 100644\n--- a/Documentation/config/commitgraph.adoc\n+++ b/Documentation/config/commitgraph.adoc\n@@ -8,6 +8,17 @@ commitGraph.maxNewFilters::\n \tSpecifies the default value for the `--max-new-filters` option of `git\n \tcommit-graph write` (c.f., linkgit:git-commit-graph[1]).\n \n+commitGraph.changedPaths::\n+\tIf true, then `git commit-graph write` will compute and write\n+\tchanged-path Bloom filters by default, equivalent to passing\n+\t`--changed-paths`. If false or unset, changed-paths Bloom filters will\n+\tbe written during `git commit-graph write` only if the filters already\n+\texist in the current commit-graph file. This matches the default\n+\tbehavior of `git commit-graph write` without any `--[no-]changed-paths`\n+\toption. To rewrite a commit-graph file without any filters, use the\n+\t`--no-changed-paths` option. Command-line option `--[no-]changed-paths`\n+\talways takes precedence over this configuration. Defaults to unset.\n+\n commitGraph.readChangedPaths::\n \tDeprecated. Equivalent to commitGraph.changedPathsVersion=-1 if true, and\n \tcommitGraph.changedPathsVersion=0 if false. (If commitGraph.changedPathVersion\ndiff --git a/Documentation/git-commit-graph.adoc b/Documentation/git-commit-graph.adoc\nindex e9558173c0..6d19026035 100644\n--- a/Documentation/git-commit-graph.adoc\n+++ b/Documentation/git-commit-graph.adoc\n@@ -71,7 +71,7 @@ take a while on large repositories. It provides significant performance gains\n for getting history of a directory or a file with `git log -- <path>`. If\n this option is given, future commit-graph writes will automatically assume\n that this option was intended. Use `--no-changed-paths` to stop storing this\n-data.\n+data. `--changed-paths` is implied by config `commitGraph.changedPaths=true`.\n +\n With the `--max-new-filters=<n>` option, generate at most `n` new Bloom\n filters (if `--changed-paths` is specified). If `n` is `-1`, no limit is\ndiff --git a/builtin/commit-graph.c b/builtin/commit-graph.c\nindex fe3ebaadad..d62005edc0 100644\n--- a/builtin/commit-graph.c\n+++ b/builtin/commit-graph.c\n@@ -210,6 +210,8 @@ static int git_commit_graph_write_config(const char *var, const char *value,\n {\n \tif (!strcmp(var, \"commitgraph.maxnewfilters\"))\n \t\twrite_opts.max_new_filters = git_config_int(var, value, ctx->kvi);\n+\telse if (!strcmp(var, \"commitgraph.changedpaths\"))\n+\t\topts.enable_changed_paths = git_config_bool(var, value) ? 1 : -1;\n \t/*\n \t * No need to fall-back to 'git_default_config', since this was already\n \t * called in 'cmd_commit_graph()'.\ndiff --git a/scalar.c b/scalar.c\nindex 4a373c133d..f754311627 100644\n--- a/scalar.c\n+++ b/scalar.c\n@@ -166,6 +166,7 @@ static int set_recommended_config(int reconfigure)\n #endif\n \t\t/* Optional */\n \t\t{ \"status.aheadBehind\", \"false\" },\n+\t\t{ \"commitGraph.changedPaths\", \"true\" },\n \t\t{ \"commitGraph.generationVersion\", \"1\" },\n \t\t{ \"core.autoCRLF\", \"false\" },\n \t\t{ \"core.safeCRLF\", \"false\" },\ndiff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\nindex 0b3404f58f..98c6910963 100755\n--- a/t/t5318-commit-graph.sh\n+++ b/t/t5318-commit-graph.sh\n@@ -946,4 +946,48 @@ test_expect_success 'stale commit cannot be parsed when traversing graph' '\n \t)\n '\n \n+test_expect_success 'config commitGraph.changedPaths acts like --changed-paths' '\n+\tgit init config-changed-paths &&\n+\t(\n+\t\tcd config-changed-paths &&\n+\n+\t\t# commitGraph.changedPaths is not set and it should not write Bloom filters\n+\t\ttest_commit first &&\n+\t\tGIT_PROGRESS_DELAY=0 git commit-graph write --reachable --progress 2>error &&\n+\t\ttest_grep ! \"Bloom filters\" error &&\n+\n+\t\t# Set commitGraph.changedPaths to true and it should write Bloom filters\n+\t\ttest_commit second &&\n+\t\tgit config commitGraph.changedPaths true &&\n+\t\tGIT_PROGRESS_DELAY=0 git commit-graph write --reachable --progress 2>error &&\n+\t\ttest_grep \"Bloom filters\" error &&\n+\n+\t\t# Add one more config commitGraph.changedPaths as false to disable the previous true config value\n+\t\t# It should still write Bloom filters due to existing filters\n+\t\ttest_commit third &&\n+\t\tgit config --add commitGraph.changedPaths false &&\n+\t\tGIT_PROGRESS_DELAY=0 git commit-graph write --reachable --progress 2>error &&\n+\t\ttest_grep \"Bloom filters\" error &&\n+\n+\t\t# commitGraph.changedPaths is still false and command line options should take precedence\n+\t\ttest_commit fourth &&\n+\t\tGIT_PROGRESS_DELAY=0 git commit-graph write --no-changed-paths --reachable --progress 2>error &&\n+\t\ttest_grep ! \"Bloom filters\" error &&\n+\t\tGIT_PROGRESS_DELAY=0 git commit-graph write --reachable --progress 2>error &&\n+\t\ttest_grep ! \"Bloom filters\" error &&\n+\n+\t\t# commitGraph.changedPaths is all cleared and then set to false again, command line options should take precedence\n+\t\ttest_commit fifth &&\n+\t\tgit config --unset-all commitGraph.changedPaths &&\n+\t\tgit config commitGraph.changedPaths false &&\n+\t\tGIT_PROGRESS_DELAY=0 git commit-graph write --changed-paths --reachable --progress 2>error &&\n+\t\ttest_grep \"Bloom filters\" error &&\n+\n+\t\t# commitGraph.changedPaths is still false and it should write Bloom filters due to existing filters\n+\t\ttest_commit sixth &&\n+\t\tGIT_PROGRESS_DELAY=0 git commit-graph write --reachable --progress 2>error &&\n+\t\ttest_grep \"Bloom filters\" error\n+\t)\n+'\n+\n test_done\n\nbase-commit: 79cf913ea9321f774da29b2330b5781d5ff420ef\n-- \ngitgitgadget\n"},{"id":"529423","messageId":"dfb978ab-993f-49c3-ba55-d12d47dc659f@gmail.com","threadId":"64286","inReplyTo":"pull.1983.v2.git.1760734739642.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] commit-graph: add new config for changed-paths & recommend it in scalar","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2025-10-22T14:53:47Z","receivedAt":"2025-10-22T14:53:50Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/17/2025 4:58 PM, Emily Yang via GitGitGadget wrote:\n> From: Emily Yang <emilyyang.git@gmail.com>\n\n>     What's included in v2:\n>     \n>     I received feedback about the confusion around the config explanation,\n>     so in v2 I added more clarification in the doc and commit message,\n>     hopefully it helps!\n>     \n>     Thanks, Emily\n\nThanks for these updates. I'm happy with the new version.\n\nThanks,\n-Stolee\n"},{"id":"529431","messageId":"xmqq8qh2zvd6.fsf@gitster.g","threadId":"64286","inReplyTo":"dfb978ab-993f-49c3-ba55-d12d47dc659f@gmail.com","subject":"Re: [PATCH v2] commit-graph: add new config for changed-paths & recommend it in scalar","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-22T17:42:13Z","receivedAt":"2025-10-22T17:42:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee <stolee@gmail.com> writes:\n\n> On 10/17/2025 4:58 PM, Emily Yang via GitGitGadget wrote:\n>> From: Emily Yang <emilyyang.git@gmail.com>\n>\n>>     What's included in v2:\n>>     \n>>     I received feedback about the confusion around the config explanation,\n>>     so in v2 I added more clarification in the doc and commit message,\n>>     hopefully it helps!\n>>     \n>>     Thanks, Emily\n>\n> Thanks for these updates. I'm happy with the new version.\n\nThanks, both.  Will queue and mark it for 'next'.\n"},{"id":"529890","messageId":"aQKBTQRQkGTgXkd6@szeder.dev","threadId":"64286","inReplyTo":"pull.1983.v2.git.1760734739642.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] commit-graph: add new config for changed-paths & recommend it in scalar","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2025-10-29T21:04:13Z","receivedAt":"2025-10-29T21:04:29Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Oct 17, 2025 at 08:58:59PM +0000, Emily Yang via GitGitGadget wrote:\n> From: Emily Yang <emilyyang.git@gmail.com>\n> \n> The changed-path Bloom filters feature has proven stable and reliable\n> over several years of use, delivering significant performance\n> improvement for file history computation in large monorepos. Currently\n> a user can opt-in to writing the changed-path Bloom filters using the\n> \"--changed-paths\" option to \"git commit-graph write\". The filters will\n> be persisted until the user drops the filters using the\n> \"--no-changed-paths\" option. For this functionality, refer to 0087a87ba8\n> (commit-graph: persist existence of changed-paths, 2020-07-01).\n> \n> Large monorepos using Git's background maintenance to build and update\n> commit-graph files could use an easy switch to enable this feature\n> without a foreground computation. In this commit, we're proposing a new\n> config option \"commitGraph.changedPaths\":\n> \n> * If \"true\", \"git commit-graph write\" will write Bloom filters,\n>   equivalent to passing \"--changed-paths\";\n> * If \"false\" or \"unset\", Bloom filters will be written during \"git\n>   commit-graph write\" only if the filters already exist in the current\n>   commit-graph file. This matches the default behaviour of \"git\n>   commit-graph write\" without any \"--[no-]changed-paths\" option. Note\n>   \"false\" can disable a previous \"true\" config value but doesn't imply\n>   \"--no-changed-paths\".\n\nSo if the commit-graph contains changed path Bloom filters, and the\nuser takes the effort, and explicitly sets this config variable to\nfalse, then Git will just ignore that, and will continue to waste\nresources to compute the Bloom filters?!  This doesn't seems like a\nsensible behavior to me.\n\n\n> diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\n> index 0b3404f58f..98c6910963 100755\n> --- a/t/t5318-commit-graph.sh\n> +++ b/t/t5318-commit-graph.sh\n> @@ -946,4 +946,48 @@ test_expect_success 'stale commit cannot be parsed when traversing graph' '\n>  \t)\n>  '\n>  \n> +test_expect_success 'config commitGraph.changedPaths acts like --changed-paths' '\n> +\tgit init config-changed-paths &&\n> +\t(\n> +\t\tcd config-changed-paths &&\n> +\n> +\t\t# commitGraph.changedPaths is not set and it should not write Bloom filters\n> +\t\ttest_commit first &&\n> +\t\tGIT_PROGRESS_DELAY=0 git commit-graph write --reachable --progress 2>error &&\n> +\t\ttest_grep ! \"Bloom filters\" error &&\n> +\n> +\t\t# Set commitGraph.changedPaths to true and it should write Bloom filters\n> +\t\ttest_commit second &&\n> +\t\tgit config commitGraph.changedPaths true &&\n> +\t\tGIT_PROGRESS_DELAY=0 git commit-graph write --reachable --progress 2>error &&\n> +\t\ttest_grep \"Bloom filters\" error &&\n> +\n> +\t\t# Add one more config commitGraph.changedPaths as false to disable the previous true config value\n> +\t\t# It should still write Bloom filters due to existing filters\n> +\t\ttest_commit third &&\n> +\t\tgit config --add commitGraph.changedPaths false &&\n> +\t\tGIT_PROGRESS_DELAY=0 git commit-graph write --reachable --progress 2>error &&\n> +\t\ttest_grep \"Bloom filters\" error &&\n> +\n> +\t\t# commitGraph.changedPaths is still false and command line options should take precedence\n> +\t\ttest_commit fourth &&\n> +\t\tGIT_PROGRESS_DELAY=0 git commit-graph write --no-changed-paths --reachable --progress 2>error &&\n> +\t\ttest_grep ! \"Bloom filters\" error &&\n> +\t\tGIT_PROGRESS_DELAY=0 git commit-graph write --reachable --progress 2>error &&\n> +\t\ttest_grep ! \"Bloom filters\" error &&\n> +\n> +\t\t# commitGraph.changedPaths is all cleared and then set to false again, command line options should take precedence\n> +\t\ttest_commit fifth &&\n> +\t\tgit config --unset-all commitGraph.changedPaths &&\n> +\t\tgit config commitGraph.changedPaths false &&\n> +\t\tGIT_PROGRESS_DELAY=0 git commit-graph write --changed-paths --reachable --progress 2>error &&\n> +\t\ttest_grep \"Bloom filters\" error &&\n> +\n> +\t\t# commitGraph.changedPaths is still false and it should write Bloom filters due to existing filters\n> +\t\ttest_commit sixth &&\n> +\t\tGIT_PROGRESS_DELAY=0 git commit-graph write --reachable --progress 2>error &&\n> +\t\ttest_grep \"Bloom filters\" error\n> +\t)\n> +'\n\nThe interaction of split commit-graphs and changed path Bloom filters\nused to be buggy, even after attempts to fix it.  Therefore, I think\nthis config variable should be tested with split commit-graphs as\nwell.\n\n"}]}