{"thread":{"id":"60470","subject":"[RFC PATCH] status: avoid reporting worktrees as \"Untracked files\"","startedAt":"2023-11-04T00:02:47Z","lastAt":"2023-11-12T23:52:43Z","messageCount":6,"participants":["Edmundo Carmona Antoranz","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"484420","messageId":"20231104000209.916189-1-eantoranz@gmail.com","threadId":"60470","inReplyTo":null,"subject":"[RFC PATCH] status: avoid reporting worktrees as \"Untracked files\"","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2023-11-04T00:02:08Z","receivedAt":"2023-11-04T00:02:47Z","isPatch":true,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"Given that worktrees are tracked in their own special fashion separately,\nit makes sense to _not_ report them as \"untracked\". Also, when seeing the\ndirectory of a worktree listed as Untracked, it might be tempting to try\nto do operations (like 'git add') on them from the parent worktree which,\nat the moment, will silently do nothing.\n\nWith this patch, we check items against the list of worktrees to add\nthem into the untracked items list effectively hiding them.\n\nEND OF PATCH\n\nHere are a few questions more inline with the \"RFC\" part of the patch.\n\nAbout UI\n- Would it make more sense to separate them from Untracked files instead\n  of hiding them (perhaps add a --worktrees option to display them)?\n- Follow-up if the previous answer is 'yes': List a worktree only if it\n  is not clean?\n\nAbout code:\n- If keeping the idea/patch, Would it make more sense (performance-wise) to\n  fist check an item in the list of worktrees before checking it in the\n  index? In other words, reverse the conditions to add an item to the\n  untracked list?\n---\n wt-status.c | 9 ++++++++-\n 1 file changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex 9f45bf6949..5fd1e6007a 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -775,6 +775,7 @@ static void wt_status_collect_untracked(struct wt_status *s)\n \tstruct dir_struct dir = DIR_INIT;\n \tuint64_t t_begin = getnanotime();\n \tstruct index_state *istate = s->repo->index;\n+\tstruct worktree **worktrees;\n \n \tif (!s->show_untracked_files)\n \t\treturn;\n@@ -795,9 +796,12 @@ static void wt_status_collect_untracked(struct wt_status *s)\n \n \tfill_directory(&dir, istate, &s->pathspec);\n \n+\tworktrees = get_worktrees();\n+\n \tfor (i = 0; i < dir.nr; i++) {\n \t\tstruct dir_entry *ent = dir.entries[i];\n-\t\tif (index_name_is_other(istate, ent->name, ent->len))\n+\t\tif (index_name_is_other(istate, ent->name, ent->len) &&\n+\t\t    !find_worktree_by_path(worktrees, ent->name))\n \t\t\tstring_list_insert(&s->untracked, ent->name);\n \t}\n \n@@ -809,6 +813,9 @@ static void wt_status_collect_untracked(struct wt_status *s)\n \n \tdir_clear(&dir);\n \n+\tif (worktrees)\n+\t\tfree_worktrees(worktrees);\n+\n \tif (advice_enabled(ADVICE_STATUS_U_OPTION))\n \t\ts->untracked_in_ms = (getnanotime() - t_begin) / 1000000;\n }\n-- \n2.42.0\n\n"},{"id":"484426","messageId":"CAPig+cTL6b5ANb-KJt7ZMkfmJ3X3-FMSXu-ThaQaFAdGV73www@mail.gmail.com","threadId":"60470","inReplyTo":"20231104000209.916189-1-eantoranz@gmail.com","subject":"Re: [RFC PATCH] status: avoid reporting worktrees as \"Untracked files\"","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-11-04T06:15:59Z","receivedAt":"2023-11-04T06:16:13Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Nov 3, 2023 at 8:03 PM Edmundo Carmona Antoranz\n<eantoranz@gmail.com> wrote:\n> Given that worktrees are tracked in their own special fashion separately,\n> it makes sense to _not_ report them as \"untracked\". Also, when seeing the\n> directory of a worktree listed as Untracked, it might be tempting to try\n> to do operations (like 'git add') on them from the parent worktree which,\n> at the moment, will silently do nothing.\n>\n> With this patch, we check items against the list of worktrees to add\n> them into the untracked items list effectively hiding them.\n>\n> END OF PATCH\n>\n> Here are a few questions more inline with the \"RFC\" part of the patch.\n>\n> About UI\n> - Would it make more sense to separate them from Untracked files instead\n>   of hiding them (perhaps add a --worktrees option to display them)?\n> - Follow-up if the previous answer is 'yes': List a worktree only if it\n>   is not clean?\n>\n> About code:\n> - If keeping the idea/patch, Would it make more sense (performance-wise) to\n>   fist check an item in the list of worktrees before checking it in the\n>   index? In other words, reverse the conditions to add an item to the\n>   untracked list?\n\nI have slightly mixed feelings about this idea since I'm sympathetic\nto the motivation, however, my knee-jerk reaction is that these really\n_are_ untracked considering that Git is a \"content tracker\" and\nworktrees are not project content. Git already has general mechanisms\nsuch as .git/info/exclude and .gitignore for suppressing certain\nuntracked items, so introducing special-purpose code to suppress\nworktrees from being considered untracked may be a case of adding\ncomplexity for little gain.\n\nMoreover, although your personal workflow may be to create worktrees\nwithin your main directory:\n\n    git worktree add new-feature\n\nother people use a workflow in which worktrees are created at other\nlocations, such as making them siblings:\n\n    git worktree add ../new-feature\n\nFor the former case, it's easy enough to mention worktrees in\n.git/info/exclude, especially if you use a standard naming convention\nfor your worktrees, in which case a single wildcard pattern may allow\nyou to set it once and forget about it. For the latter workflow, the\nextra \"is untracked\" checking is simply wasteful.\n\nHaving said all that, I think that someone may have recently floated\nthe idea on the mailing list about suppressing dirty submodules from\nshowing up as \"dirty\" in git-status. Although the underlying concepts\nand mechanisms are quite distinct (especially since a submodule _is_\ncontent), perhaps there is some sort of analogy between worktrees and\ndirty submodules which invalidates my knee-jerk reaction. Also, I'm\njust one person responding without having put all that much thought\ninto it. Others may feel differently.\n\nRegarding the patch itself...\n\n> diff --git a/wt-status.c b/wt-status.c\n> @@ -795,9 +796,12 @@ static void wt_status_collect_untracked(struct wt_status *s)\n> +       worktrees = get_worktrees();\n> +\n>         for (i = 0; i < dir.nr; i++) {\n>                 struct dir_entry *ent = dir.entries[i];\n> -               if (index_name_is_other(istate, ent->name, ent->len))\n> +               if (index_name_is_other(istate, ent->name, ent->len) &&\n> +                   !find_worktree_by_path(worktrees, ent->name))\n>                         string_list_insert(&s->untracked, ent->name);\n>         }\n\nThis first-stab implementation unfortunately has worse than quadratic\ncomplexity, perhaps even cubic complexity since\nfind_worktree_by_path() performs a linear scan through the worktree\nlist. So, for each path in `dir`, it's performing a\ncharacter-by-character string comparison with each path in\n`worktrees`. Worse, find_worktree_by_path() calls strbuf_realpath()\nwhich hits the filesystem for each path in `worktrees` each time it's\ncalled.\n\nSo, a real (non-RFC) implementation would probably need to perform a\npreparatory step of creating a hash-table/set in which the keys are\nthe realpath'd elements from `worktrees`, and then simply consult the\nhash-table/set for each path in `dir`.\n\n> @@ -809,6 +813,9 @@ static void wt_status_collect_untracked(struct wt_status *s)\n> +       if (worktrees)\n> +               free_worktrees(worktrees);\n\nNit: At this point, we _know_ that `worktrees` is non-NULL, so\nfree_worktrees() can be called unconditionally.\n"},{"id":"484429","messageId":"xmqqjzqygg3i.fsf@gitster.g","threadId":"60470","inReplyTo":"20231104000209.916189-1-eantoranz@gmail.com","subject":"Re: [RFC PATCH] status: avoid reporting worktrees as \"Untracked files\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-04T06:58:09Z","receivedAt":"2023-11-04T06:58:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Edmundo Carmona Antoranz <eantoranz@gmail.com> writes:\n\n> Given that worktrees are tracked in their own special fashion separately,\n> it makes sense to _not_ report them as \"untracked\".\n\nMy gut feeling is that a much better solution to the unstated\nproblem you are solving is to make sure that \"git worktree add\" will\ncomplain and not allow adding a subdirectory of any existing working\ntree of a repository as a new worktree.\n\nWhat problem are you trying to solve?  \"git add foo\" where \"foo\" is\nactually a different worktree of the repository would add it as a\nsubmodule that causes confusion?  If that is the case, I think the\nright solution is not to get into such a state, i.e. not create a\nworktree of the repository inside a different worktree in the first\nplace.\n\n"},{"id":"484751","messageId":"CAOc6etbowajhHsctFJN4ZQ0gND0jzZUrhEkep_pLYtE9y9RBCQ@mail.gmail.com","threadId":"60470","inReplyTo":"xmqqjzqygg3i.fsf@gitster.g","subject":"Re: [RFC PATCH] status: avoid reporting worktrees as \"Untracked files\"","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2023-11-11T09:22:37Z","receivedAt":"2023-11-11T09:22:54Z","isPatch":true,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"Hey, guys! Thanks Junio and Eric for sharing your thoughts.\n\nAnd I candidly thought this was going to be an \"easy sell\".... :-D\n\nOn Sat, Nov 4, 2023 at 7:58 AM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> What problem are you trying to solve?  \"git add foo\" where \"foo\" is\n> actually a different worktree of the repository would add it as a\n> submodule that causes confusion?  If that is the case, I think the\n> right solution is not to get into such a state, i.e. not create a\n> worktree of the repository inside a different worktree in the first\n> place.\n>\n\nI am not against the idea of creating worktrees outside of the\nrepository... however, I like them to be _inside_ the repository. Am I\nthe only one? IDK. I might be! It feels completely natural, if you ask\nme.... but that's just my opinion, I acknowledge that.\n\nWhile I was running a couple of quick tests to add more information\nabout git behaviour with \"my use case\" I think I found something to\nwork on so more RFCs might be on the way in the next few days or\nweeks.\n\nAbout adding an error message when 'git add' will skip doing something\nbecause it is working on a different worktree, I think it makes\nsense.... will probably work on that too.\n\nThanks again! BR!\n"},{"id":"484775","messageId":"CAPig+cQbwcJOQiYyb7bma3pH1hxjE_X_yeAp3JeHWVCeJtySfQ@mail.gmail.com","threadId":"60470","inReplyTo":"CAOc6etbowajhHsctFJN4ZQ0gND0jzZUrhEkep_pLYtE9y9RBCQ@mail.gmail.com","subject":"Re: [RFC PATCH] status: avoid reporting worktrees as \"Untracked files\"","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-11-12T17:13:59Z","receivedAt":"2023-11-12T17:14:12Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Nov 11, 2023 at 4:22 AM Edmundo Carmona Antoranz\n<eantoranz@gmail.com> wrote:\n> On Sat, Nov 4, 2023 at 7:58 AM Junio C Hamano <gitster@pobox.com> wrote:\n> > What problem are you trying to solve?  \"git add foo\" where \"foo\" is\n> > actually a different worktree of the repository would add it as a\n> > submodule that causes confusion?  If that is the case, I think the\n> > right solution is not to get into such a state, i.e. not create a\n> > worktree of the repository inside a different worktree in the first\n> > place.\n> >\n> Hey, guys! Thanks Junio and Eric for sharing your thoughts.\n>\n> I am not against the idea of creating worktrees outside of the\n> repository... however, I like them to be _inside_ the repository. Am I\n> the only one? IDK. I might be! It feels completely natural, if you ask\n> me.... but that's just my opinion, I acknowledge that.\n\nI doubt you're the only one, but, based upon, list emails over the\nyears, it seems that both in-main-tree and outside-main-tree (often\nsibling) worktrees are common. More recently, we've also heard from\npeople who don't even have a main-worktree; instead, they hang their\nmultiple worktrees off of a bare repository (which is an\nexplicitly-supported use-case); i.e.:\n\n    git clone --bare https://.../foobar.git\n    git -C foobar.git worktree add worktree1\n    git -C foobar.git worktree add worktree2\n    ...\n"},{"id":"484779","messageId":"xmqqjzqmmsvx.fsf@gitster.g","threadId":"60470","inReplyTo":"CAPig+cQbwcJOQiYyb7bma3pH1hxjE_X_yeAp3JeHWVCeJtySfQ@mail.gmail.com","subject":"Re: [RFC PATCH] status: avoid reporting worktrees as \"Untracked files\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-12T23:52:34Z","receivedAt":"2023-11-12T23:52:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> I doubt you're the only one, but, based upon, list emails over the\n> years, it seems that both in-main-tree and outside-main-tree (often\n> sibling) worktrees are common. More recently, we've also heard from\n> people who don't even have a main-worktree; instead, they hang their\n> multiple worktrees off of a bare repository (which is an\n> explicitly-supported use-case); i.e.:\n>\n>     git clone --bare https://.../foobar.git\n>     git -C foobar.git worktree add worktree1\n>     git -C foobar.git worktree add worktree2\n>     ...\n\nI am not sure why you brought in that layout in this discussion,\nbecause it places worktree1 and worktree2 next to each other, just\nlike placing worktree1 and worktree2 next to the non-bare repository.\n\n    git clone https://.../foobar.git foobar\n    git -C foobar worktree add worktree1\n    git -C foobar worktree add worktree2\n\nThe layout to create worktrees attached to a bare repository and add\nthem next to each other, and the same starting from a non-bare\nrepository, share an important trait.  They do not have an untracked\nand untrackable \"cruft\" in their working tree, unlike the crazy\nlayout that places worktrees of the repository inside the working\ntree of the primary worktree as untracked subdirectories.\n\nReally, what is the advantage of doing so?  It is not like the build\nrecipe recorded in the primary worktree can work recursively on\ndifferent branches that are checked out---worktree names and paths\nat which they are checked out are totally local matter, and the\nupstream project that supplies the build recipe would not know or\ncare.\n\nEven worse, when the project wants to add a new subdirectory or a\nfile, the name chosen for the subdirectory may happen to collide\nwith the name of an untracked subdirectory you happened to have used\n(again, because the worktree names and locations are totally local\nmatter, the upstream project are unaware of them and cannot avoid\nsuch name clashes even if they cared).  You can imagine the\nconfusion that happens to your next \"git pull\".\n\nCompared to such an insanity, attaching worktrees to a bare\nrepository, so that all worktrees are equals and there is no\n\"primary\" worktree that you cannot remove, behave just as normal as\na set of worktrees attached to a non-bare repository and sit outside\nthe primary worktree, often as immediate siblings.\n\n\n\n"}]}