{"thread":{"id":"54201","subject":"`git describe --dirty` doesn't consider untracked files to be dirty","startedAt":"2020-09-07T09:17:49Z","lastAt":"2020-09-20T02:12:27Z","messageCount":9,"participants":["Ash Holland","Raymond E. Pasco","Junio C Hamano","Aaron Schrab"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"405121","messageId":"CAHJUbDg2KA9Xo_CAO=cgrZewOH0zfEhOVydhMN8fLvVDmji4sQ@mail.gmail.com","threadId":"54201","inReplyTo":null,"subject":"`git describe --dirty` doesn't consider untracked files to be dirty","fromName":"Ash Holland","fromEmail":"ash@sorrel.sh","sentAt":"2020-09-07T09:04:17Z","receivedAt":"2020-09-07T09:17:49Z","isPatch":false,"sender":{"key":"ash@sorrel.sh","avatar":"https://avatars.githubusercontent.com/u/9433472?v=4"},"body":"Hi,\n\nThere seems to be a discrepancy between how `git describe --dirty` is\ndocumented and how it actually behaves. The documentation describes\nthe --dirty flag like this:\n\n> If the working tree has local modification \"-dirty\" is appended to it.\n\nbut certain kinds of \"local modification\", namely untracked files,\ndon't cause \"-dirty\" to be included.\n\nPlease could this be fixed, either in the documentation or in git describe?\n\nthanks,\nAsh\nshe/they\n"},{"id":"405153","messageId":"C5HTGCE96RJ4.DT7CCU2SIG3Q@ziyou.local","threadId":"54201","inReplyTo":"CAHJUbDg2KA9Xo_CAO=cgrZewOH0zfEhOVydhMN8fLvVDmji4sQ@mail.gmail.com","subject":"Re: `git describe --dirty` doesn't consider untracked files to be dirty","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2020-09-08T07:40:50Z","receivedAt":"2020-09-08T07:49:40Z","isPatch":false,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"On Mon Sep 7, 2020 at 5:04 AM EDT, Ash Holland wrote:\n> There seems to be a discrepancy between how `git describe --dirty` is\n> documented and how it actually behaves. The documentation describes\n> the --dirty flag like this:\n>\n> > If the working tree has local modification \"-dirty\" is appended to it.\n>\n> but certain kinds of \"local modification\", namely untracked files,\n> don't cause \"-dirty\" to be included.\n\nI think the documentation here could be made clearer, but I'm not sure\nof the precise wording that would be best.\n\nI wonder if describe should have an option that considers the presence\nof untracked (but not ignored, i.e. anything that would be flagged by\nstatus) files to count as a dirty worktree. Implementing this option\nmight be the lazy way to make the documentation easier to rewrite.\n"},{"id":"405157","messageId":"xmqqh7s8z0qw.fsf@gitster.c.googlers.com","threadId":"54201","inReplyTo":"CAHJUbDg2KA9Xo_CAO=cgrZewOH0zfEhOVydhMN8fLvVDmji4sQ@mail.gmail.com","subject":"Re: `git describe --dirty` doesn't consider untracked files to be dirty","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-08T16:33:59Z","receivedAt":"2020-09-08T16:34:25Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ash Holland writes:\n\n> There seems to be a discrepancy between how `git describe --dirty` is\n> documented and how it actually behaves. The documentation describes\n> the --dirty flag like this:\n>\n>> If the working tree has local modification \"-dirty\" is appended to it.\n\nNot limited to what \"describe\" does, whenever we mention \"local\nmodification\", we only mean modification to tracked contents,\nbecause by definition we do not detect or track \"modifications\" to\nanything that is not tracked. Untracked paths may have been\nmodified multiple times, but since they are not even added, we do\nnot notice nor care.\n\nThat is to say that the documentation and the code are consistent\nwith each other.\n\nHaving said all that, a source that was forgotten to be added, yet\naffects the built product by a build rule with wildcard e.g.\n\"compile all *.c files and link them into a single binary\", would\nhappen in real life, so from that point of view, appending \"-dirty\"\nonly when there is a local modification may not be all that useful,\nand tweaking the \"--dirty\" option to also pay attention to untracked\n(but not ignored) might have merit. \n\nI do not think this is something we want to hide behind a\nconfiguration knob, but I am undecided between (1) declare that this\nis a bug and change the behaviour of \"--dirty\" and (2) declare that\nwe discovered another useful behaviour and add a new option next to\n\"--dirty\".\n\nThanks.\n"},{"id":"405210","messageId":"20200908231652.GC1014@pug.qqx.org","threadId":"54201","inReplyTo":"xmqqh7s8z0qw.fsf@gitster.c.googlers.com","subject":"Re: `git describe --dirty` doesn't consider untracked files to be dirty","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2020-09-08T23:16:52Z","receivedAt":"2020-09-08T23:25:50Z","isPatch":false,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"At 09:33 -0700 08 Sep 2020, Junio C Hamano wrote:\n>Ash Holland writes:\n>\n>> There seems to be a discrepancy between how `git describe --dirty` is\n>> documented and how it actually behaves. The documentation describes\n>> the --dirty flag like this:\n>>\n>>> If the working tree has local modification \"-dirty\" is appended to it.\n>\n>Not limited to what \"describe\" does, whenever we mention \"local\n>modification\", we only mean modification to tracked contents,\n>because by definition we do not detect or track \"modifications\" to\n>anything that is not tracked. Untracked paths may have been\n>modified multiple times, but since they are not even added, we do\n>not notice nor care.\n\nIt's perhaps worth noting that submodules are already considered dirty \nwhen untracked files are added:\n\n$ git diff vim/bundle/fugitive\n\n$ echo foo >vim/bundle/fugitive/foo\n\n$ git diff vim/bundle/fugitive\ndiff --git i/vim/bundle/fugitive w/vim/bundle/fugitive\n--- i/vim/bundle/fugitive\n+++ w/vim/bundle/fugitive\n@@ -1 +1 @@\n-Subproject commit caf3b1d5696e8d39a905e48f1e89d8c0c565168c\n+Subproject commit caf3b1d5696e8d39a905e48f1e89d8c0c565168c-dirty\n"},{"id":"405212","messageId":"xmqqft7rx1k7.fsf@gitster.c.googlers.com","threadId":"54201","inReplyTo":"20200908231652.GC1014@pug.qqx.org","subject":"Re: `git describe --dirty` doesn't consider untracked files to be dirty","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-08T23:59:20Z","receivedAt":"2020-09-08T23:59:26Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Aaron Schrab writes:\n\n> It's perhaps worth noting that submodules are already considered dirty\n> when untracked files are added:\n>\n> $ git diff vim/bundle/fugitive\n>\n> $ echo foo >vim/bundle/fugitive/foo\n>\n> $ git diff vim/bundle/fugitive\n> diff --git i/vim/bundle/fugitive w/vim/bundle/fugitive\n> --- i/vim/bundle/fugitive\n> +++ w/vim/bundle/fugitive\n> @@ -1 +1 @@\n> -Subproject commit caf3b1d5696e8d39a905e48f1e89d8c0c565168c\n> +Subproject commit caf3b1d5696e8d39a905e48f1e89d8c0c565168c-dirty\n\nIt gives one vote for (1) to the part you did not quote from the\nmessage you are responding to, which was:\n\n>> I do not think this is something we want to hide behind a\n>> configuration knob, but I am undecided between (1) declare that this\n>> is a bug and change the behaviour of \"--dirty\" and (2) declare that\n>> we discovered another useful behaviour and add a new option next to\n>> \"--dirty\".\n\nI tend to agree the consistency with that behaviour would be more\nuseful. The discrepanthy shows the relative age of features and how\nour thinking has changed over time ;-)\n"},{"id":"405945","messageId":"xmqqo8m1k542.fsf@gitster.c.googlers.com","threadId":"54201","inReplyTo":"20200908231652.GC1014@pug.qqx.org","subject":"Re: `git describe --dirty` doesn't consider untracked files to be dirty","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-19T18:12:45Z","receivedAt":"2020-09-19T18:12:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Aaron Schrab writes:\n\n> It's perhaps worth noting that submodules are already considered dirty\n> when untracked files are added:\n>\n> $ git diff vim/bundle/fugitive\n>\n> $ echo foo >vim/bundle/fugitive/foo\n>\n> $ git diff vim/bundle/fugitive\n> diff --git i/vim/bundle/fugitive w/vim/bundle/fugitive\n> --- i/vim/bundle/fugitive\n> +++ w/vim/bundle/fugitive\n> @@ -1 +1 @@\n> -Subproject commit caf3b1d5696e8d39a905e48f1e89d8c0c565168c\n> +Subproject commit caf3b1d5696e8d39a905e48f1e89d8c0c565168c-dirty\n\nIn other words, if we do this in the state:\n\n $ git -C vim/bundle/fugitive describe --dirty\n\nthe submodule directory is not reported as dirty.\n\nThis is worth fixing. I am leaning towards saying that `diff` is\nwrong in this case, but I am OK to consider unifying the behaviour\nthe other way and making `describe --dirty` more strict.\n\nThanks.\n"},{"id":"405960","messageId":"CAHJUbDjSS-fWjeJkD49yEPmRKZQLYSW0R9-PhzFem1QsEuJUOQ@mail.gmail.com","threadId":"54201","inReplyTo":"xmqqo8m1k542.fsf@gitster.c.googlers.com","subject":"Re: `git describe --dirty` doesn't consider untracked files to be dirty","fromName":"Ash Holland","fromEmail":"ash@sorrel.sh","sentAt":"2020-09-20T00:17:32Z","receivedAt":"2020-09-20T00:30:20Z","isPatch":false,"sender":{"key":"ash@sorrel.sh","avatar":"https://avatars.githubusercontent.com/u/9433472?v=4"},"body":"On Sat, 19 Sep 2020 at 19:12, Junio C Hamano wrote:\n> This is worth fixing. I am leaning towards saying that `diff` is\n> wrong in this case, but I am OK to consider unifying the behaviour\n> the other way and making `describe --dirty` more strict.\n\nfwiw, my preference would be for the second behaviour; I have a release\nscript which complains at me if I've forgotten to commit something, and\nto avoid making a release with new uncommitted files I currently have to\nuse both `git describe --dirty` (to check for modifications to tracked\nfiles) and also `git ls-files --others --exclude-standard` (to check for\nuntracked files).\n\nmaybe there's a better plumbing command I should be using in a script,\nbut your example of the wildcard build rule also would suggest that\n`describe` should be changed, not `diff`:\n\n> Having said all that, a source that was forgotten to be added, yet\n> affects the built product by a build rule with wildcard e.g.\n> \"compile all *.c files and link them into a single binary\", would\n> happen in real life, so from that point of view, appending \"-dirty\"\n> only when there is a local modification may not be all that useful,\n> and tweaking the \"--dirty\" option to also pay attention to untracked\n> (but not ignored) might have merit.\n\nlastly, by appeal to `git clean`'s documentation: \"Remove untracked\nfiles from the working tree\"\n\nif you clean a repository by removing untracked files, then untracked\nfiles surely make the working tree dirty :)\n"},{"id":"405962","messageId":"xmqq5z89i5j3.fsf@gitster.c.googlers.com","threadId":"54201","inReplyTo":"xmqqo8m1k542.fsf@gitster.c.googlers.com","subject":"Re: `git describe --dirty` doesn't consider untracked files to be dirty","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-20T01:46:40Z","receivedAt":"2020-09-20T01:47:25Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Aaron Schrab writes:\n>\n>> It's perhaps worth noting that submodules are already considered dirty\n>> when untracked files are added:\n>>\n>> $ git diff vim/bundle/fugitive\n>>\n>> $ echo foo >vim/bundle/fugitive/foo\n>>\n>> $ git diff vim/bundle/fugitive\n>> diff --git i/vim/bundle/fugitive w/vim/bundle/fugitive\n>> --- i/vim/bundle/fugitive\n>> +++ w/vim/bundle/fugitive\n>> @@ -1 +1 @@\n>> -Subproject commit caf3b1d5696e8d39a905e48f1e89d8c0c565168c\n>> +Subproject commit caf3b1d5696e8d39a905e48f1e89d8c0c565168c-dirty\n>\n> In other words, if we do this in the state:\n>\n> $ git -C vim/bundle/fugitive describe --dirty\n>\n> the submodule directory is not reported as dirty.\n>\n> This is worth fixing. I am leaning towards saying that `diff` is\n> wrong in this case, but I am OK to consider unifying the behaviour\n> the other way and making `describe --dirty` more strict.\n\n\"git diff\" family of commands know the \"--ignore-submodules=\"\noption, and it seems that by default they do not ignore \"untracked\".\n\nThis seems to be what causes its output fail to pretend as if output\nfrom \"git describe --dirty\" in the submodule directory were used on\nthe working-tree side of the comparison and leads to this\ninconsistency. Obviously we can tweak the default of \"diff\" family\nof commands to ignore untracked paths in submodules and that would\nmake them consistent with \"git describe --dirty\", but that would not\ngive us a new way to tweak behaviour of \"git describe\" like we can\ndo with \"git diff --ignore-submodules=\".\n\nThe current \"untracked files do not count as part of dirtiness\"\ndefault behaviour of \"git describe --dirty\" is relied upon by\npeople's existing scripts, and changing it from under them would\ncause unnecessary breakage. But that does not have to stop us from\nteaching \"git describe --dirty\" an optional \"--ignore=\"\noption, similar to what \"diff --ignore-submodules=\" option\ndoes to the submodules.\n\nThe first step would be to allow those who want their \"git describe\n--dirty --ignore=none\" (untracked files are counted as dirtiness, to\nbe consistent with how \"git diff\" sees submodule directories by\ndefault) to use presence of untracked files as dirty. This is a\nsafe first step and can be done without breaking any existing users.\n\nAfter that materializes and users gain experience, we may want to\ndiscuss if we want to change the default behaviour of \"git describe\n--dirty\" or what value the future default should be, how bad the\ncompatibility breakage would be if we change the default, and what\nthe transition plan and schedule looks like. But we do not have to\ndo such a longer-term planning before the first step happens.\n\n\n\n"},{"id":"405963","messageId":"xmqq1rixi4cb.fsf@gitster.c.googlers.com","threadId":"54201","inReplyTo":"xmqq5z89i5j3.fsf@gitster.c.googlers.com","subject":"Re: `git describe --dirty` doesn't consider untracked files to be dirty","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-20T02:12:20Z","receivedAt":"2020-09-20T02:12:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> The first step would be to allow those who want their \"git describe\n> --dirty --ignore=none\" (untracked files are counted as dirtiness, to\n> be consistent with how \"git diff\" sees submodule directories by\n> default) to use presence of untracked files as dirty. This is a\n> safe first step and can be done without breaking any existing users.\n\nIt is worse than I thought. There is zero-th step we need to have,\nto fix \"git describe --dirty\" itself.\n\nBecause the command internally uses \"diff-index\", and by default it\nconsiders that a submodule with untracked path *is* dirty. Because\nof that, you get an inexplicable inconsistent behaviour.\n\n * If you start from a pristine checkout, and then add an untracked\n path to the current project, \"git describe --dirty\" won't give\n the -dirty suffix.\n\n * But if you add an untracked path in its submodule, the command\n does give you the -dirty suffix.\n\nA fix, without the first-step to give the command configurable\ndefinition of what makes a repository 'dirty', would probably look\nlike the attached untested patch. A fix to \"diff\" machinery to make\n\"--ignore-submodules=untracked\" the default would also make \"describe\"\ninternally consistent, too.\n\n builtin/describe.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 7668591d57..af08d7d8cf 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -45,7 +45,7 @@ static struct commit_names commit_names;\n \n /* diff-index command arguments to check if working tree is dirty. */\n static const char *diff_index_args[] = {\n-\t\"diff-index\", \"--quiet\", \"HEAD\", \"--\", NULL\n+\t\"diff-index\", \"--quiet\", \"--ignore-submodules=untracked\", \"HEAD\", \"--\", NULL\n };\n \n struct commit_name {\n\n \n"}]}