{"thread":{"id":"54367","subject":"[Outreachy] Introduction","startedAt":"2020-10-07T20:10:58Z","lastAt":"2020-10-16T13:26:17Z","messageCount":17,"participants":["Sangeeta NB","Phillip Wood","Kaartic Sivaraam","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"407097","messageId":"CAHjREB4gsyOrdnhp0_9rs0wv5q5H47-3RcB3fm5NY+L=3SYnMA@mail.gmail.com","threadId":"54367","inReplyTo":null,"subject":"[Outreachy] Introduction","fromName":"Sangeeta NB","fromEmail":"sangunb09@gmail.com","sentAt":"2020-10-07T20:10:44Z","receivedAt":"2020-10-07T20:10:58Z","isPatch":false,"sender":{"key":"sangunb09@gmail.com","avatar":"https://avatars.githubusercontent.com/u/64827599?v=4"},"body":"Hello everyone,\n\nMy name is Sangeeta and I’m one of the Outreachy applicants. I would\nlike to work on the microproject \"Unify the meaning of dirty between\ndiff and describe\".\n\nWhile looking at the files for `describe` and `diff` commands I found\nthat the `describe.c` is present in builtin[1] folder whereas diff.c\nis found in the root[2] folder as well as builtin[3] folder. I could\nnot find any implementation of --dirty in the diff.c present in\nbuiltin[3] folder. So is it that I have to compare the implementation\nof describe.c[1] and diff.c(of root folder)?\n\nAlso, I was curious to know why is there a builtin folder when many\ncommands described in that are described again in the root folder?\n\nLooking forward to working with you all.\n\nSangeeta\n[1] https://github.com/git/git/blob/master/builtin/describe.c\n[2] https://github.com/git/git/blob/master/builtin/diff.c\n[3] https://github.com/git/git/blob/master/diff.c\n"},{"id":"407136","messageId":"22e3d737-8621-9f20-307e-fc4c2a47ec0c@gmail.com","threadId":"54367","inReplyTo":"CAHjREB4gsyOrdnhp0_9rs0wv5q5H47-3RcB3fm5NY+L=3SYnMA@mail.gmail.com","subject":"Re: [Outreachy] Introduction","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2020-10-08T09:07:41Z","receivedAt":"2020-10-08T09:07:47Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Sangeeta\n\nOn 07/10/2020 21:10, Sangeeta NB wrote:\n> Hello everyone,\n\nWelcome to the list\n\n> My name is Sangeeta and I’m one of the Outreachy applicants. I would\n> like to work on the microproject \"Unify the meaning of dirty between\n> diff and describe\".\n> \n> While looking at the files for `describe` and `diff` commands I found\n> that the `describe.c` is present in builtin[1] folder whereas diff.c\n> is found in the root[2] folder as well as builtin[3] folder. I could\n> not find any implementation of --dirty in the diff.c present in\n> builtin[3] folder. So is it that I have to compare the implementation\n> of describe.c[1] and diff.c(of root folder)?\n> \n> Also, I was curious to know why is there a builtin folder when many\n> commands described in that are described again in the root folder?\n\nThe files in the root directory are (mostly) library code that ends up \nin libgit.a. The builtin directory contains the individual git commands \nthat form the git binary that is linked with libgit.a. builtin/diff.c \ncontains cmd_diff() which will be called when the user runs `git diff`. \nThat function parses the command line options and sets up the necessary \ndata to pass to the diff implementation in /diff.c. The diff and log \nfamily of commands are a bit different to most of the other commands in \nthat the option parsing is mostly done by calling setup_revisions() in \n/revision.c rather than using the option parsing library routines in \n/parse-options.c directly. I think the `--dirty` option for diff ends up \nbeing handled by handle_ignore_submodules_arg() in submodule.c, I'll \nleave it to you to see where that is called from (you can use `git grep`).\n\nI'm going to be off line for the rest of today, hopefully someone else \nwill be able to help if you get stuck or I'll try and answer any other \nquestions tomorrow.\n\nBest Wishes\n\nPhillip\n\n> Looking forward to working with you all.\n> \n> Sangeeta\n> [1] https://github.com/git/git/blob/master/builtin/describe.c\n> [2] https://github.com/git/git/blob/master/builtin/diff.c\n> [3] https://github.com/git/git/blob/master/diff.c\n> \n"},{"id":"407199","messageId":"CAHjREB59HjZAs98wMPtMANcHUaXGou7CYg9vAacc3m4uz1yUug@mail.gmail.com","threadId":"54367","inReplyTo":"22e3d737-8621-9f20-307e-fc4c2a47ec0c@gmail.com","subject":"Re: [Outreachy] Introduction","fromName":"Sangeeta NB","fromEmail":"sangunb09@gmail.com","sentAt":"2020-10-09T07:41:59Z","receivedAt":"2020-10-09T07:42:12Z","isPatch":false,"sender":{"key":"sangunb09@gmail.com","avatar":"https://avatars.githubusercontent.com/u/64827599?v=4"},"body":"Thanks for the explanation, Philips. I think there's a long road ahead\nto understand how everything is implemented and put together.\n\nComing to the microproject, it was said that there is an inconsistency\nin --dirty behavior shown by `git diff` and `git describe --dirty` for\nsubmodule state when the files are untracked.\n\nFrom what I understood by looking at the code, the diff files states\nthat we should ignore untracked submodule states. So is it that I have\nto make changes in the way git describe is implemented by ignoring the\nchanges in the untracked submodule?\n\nAlso, I wasn't able to look for this inconsistency in my local\nmachine. Any pointers on how to reproduce this might be helpful.\n\nThanks and regards,\n\nSangeeta\n\nOn Thu, Oct 8, 2020 at 2:37 PM Phillip Wood wrote:\n>\n> Hi Sangeeta\n>\n> On 07/10/2020 21:10, Sangeeta NB wrote:\n> > Hello everyone,\n>\n> Welcome to the list\n>\n> > My name is Sangeeta and I’m one of the Outreachy applicants. I would\n> > like to work on the microproject \"Unify the meaning of dirty between\n> > diff and describe\".\n> >\n> > While looking at the files for `describe` and `diff` commands I found\n> > that the `describe.c` is present in builtin[1] folder whereas diff.c\n> > is found in the root[2] folder as well as builtin[3] folder. I could\n> > not find any implementation of --dirty in the diff.c present in\n> > builtin[3] folder. So is it that I have to compare the implementation\n> > of describe.c[1] and diff.c(of root folder)?\n> >\n> > Also, I was curious to know why is there a builtin folder when many\n> > commands described in that are described again in the root folder?\n>\n> The files in the root directory are (mostly) library code that ends up\n> in libgit.a. The builtin directory contains the individual git commands\n> that form the git binary that is linked with libgit.a. builtin/diff.c\n> contains cmd_diff() which will be called when the user runs `git diff`.\n> That function parses the command line options and sets up the necessary\n> data to pass to the diff implementation in /diff.c. The diff and log\n> family of commands are a bit different to most of the other commands in\n> that the option parsing is mostly done by calling setup_revisions() in\n> /revision.c rather than using the option parsing library routines in\n> /parse-options.c directly. I think the `--dirty` option for diff ends up\n> being handled by handle_ignore_submodules_arg() in submodule.c, I'll\n> leave it to you to see where that is called from (you can use `git grep`).\n>\n> I'm going to be off line for the rest of today, hopefully someone else\n> will be able to help if you get stuck or I'll try and answer any other\n> questions tomorrow.\n>\n> Best Wishes\n>\n> Phillip\n>\n> > Looking forward to working with you all.\n> >\n> > Sangeeta\n> > [1] https://github.com/git/git/blob/master/builtin/describe.c\n> > [2] https://github.com/git/git/blob/master/builtin/diff.c\n> > [3] https://github.com/git/git/blob/master/diff.c\n> >\n"},{"id":"407231","messageId":"7d691f37-b8b7-510d-ba46-51a4a7587226@gmail.com","threadId":"54367","inReplyTo":"CAHjREB59HjZAs98wMPtMANcHUaXGou7CYg9vAacc3m4uz1yUug@mail.gmail.com","subject":"Re: [Outreachy] Introduction","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2020-10-09T18:29:13Z","receivedAt":"2020-10-09T18:29:19Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Sangeeta\n\nOn 09/10/2020 08:41, Sangeeta NB wrote:\n> Thanks for the explanation, Philips. I think there's a long road ahead\n> to understand how everything is implemented and put together.\n> \n> Coming to the microproject, it was said that there is an inconsistency\n> in --dirty behavior shown by `git diff` and `git describe --dirty` for\n> submodule state when the files are untracked.\n\nI struggled to find the mircoprojects page - I must have missed the link \non the outreachy site. In case anyone else is struggling to find it here \nis the project\n\n Unify the meaning of -dirty between diff and describe\n\n git diff reports a submodule directory, whose tracked\n contents match the commit at the HEAD in the submodule, as\n -dirty when there is an untracked file in the submodule\n directory. This is inconsistent with what git describe\n --dirty says when run in the submodule directory in that\n state. [1]\n\n [1] https://lore.kernel.org/git/xmqqo8m1k542.fsf@gitster.c.googlers.com/\n\n> From what I understood by looking at the code, the diff files states\n> that we should ignore untracked submodule states. So is it that I have\n> to make changes in the way git describe is implemented by ignoring the\n> changes in the untracked submodule?\n\nAs I understand it if a submodule contains any untracked files (i.e. a \nfile that has not been added with `git add` and is not ignored by any \n.gitignore or .git/info/exclude entries) then running `git diff` in the \nsuperproject will report that the submodule is dirty - there will be a \nline something like \"+Subproject commit abcdef-dirty\". However if we run \n`git describe --dirty` in the submodule directory then it will not \nappend \"-dirty\" to it's output unless there are changes to tracked files.\n\n> Also, I wasn't able to look for this inconsistency in my local\n> machine. Any pointers on how to reproduce this might be helpful.\n\nI'd start my trying to build git and running t4027-diff-submodule.sh.\nIf you look at the start of the test 'git diff HEAD with dirty submodule \n(untracked)' in t/t4027-diff-submodule.sh it sets up a submodule with an \nuntracked file. If you add \"test_pause &&\" after the diff command in \nthat test it will start a shell in the test directory and you can run \n`git diff HEAD` yourself to see the output and also `git -C sub diff \nHEAD` which will run diff in the submodule directory. The latter command \nshould show that there are no changes in the tracked files of the \nsubmodule. Just exit the shell to get the test to continue. (you can see \nin builtin/describe.c that when it is run with `--dirty` it runs `git \ndiff-index HEAD` to determine if a repository is dirty). To change the \noutput of diff I would look for the string \"Subproject commit\" in diff.c \nto find the code that adds '-dirty' and try working backwards from \nthere. Let me know if you get stuck - it took we a while to work \nbackwards to find where we check if the submodule is dirty.\n\nBest Wishes\n\nPhillip\n\n\n> Thanks and regards,\n> \n> Sangeeta\n> \n> On Thu, Oct 8, 2020 at 2:37 PM Phillip Wood wrote:\n>>\n>> Hi Sangeeta\n>>\n>> On 07/10/2020 21:10, Sangeeta NB wrote:\n>>> Hello everyone,\n>>\n>> Welcome to the list\n>>\n>>> My name is Sangeeta and I’m one of the Outreachy applicants. I would\n>>> like to work on the microproject \"Unify the meaning of dirty between\n>>> diff and describe\".\n>>>\n>>> While looking at the files for `describe` and `diff` commands I found\n>>> that the `describe.c` is present in builtin[1] folder whereas diff.c\n>>> is found in the root[2] folder as well as builtin[3] folder. I could\n>>> not find any implementation of --dirty in the diff.c present in\n>>> builtin[3] folder. So is it that I have to compare the implementation\n>>> of describe.c[1] and diff.c(of root folder)?\n>>>\n>>> Also, I was curious to know why is there a builtin folder when many\n>>> commands described in that are described again in the root folder?\n>>\n>> The files in the root directory are (mostly) library code that ends up\n>> in libgit.a. The builtin directory contains the individual git commands\n>> that form the git binary that is linked with libgit.a. builtin/diff.c\n>> contains cmd_diff() which will be called when the user runs `git diff`.\n>> That function parses the command line options and sets up the necessary\n>> data to pass to the diff implementation in /diff.c. The diff and log\n>> family of commands are a bit different to most of the other commands in\n>> that the option parsing is mostly done by calling setup_revisions() in\n>> /revision.c rather than using the option parsing library routines in\n>> /parse-options.c directly. I think the `--dirty` option for diff ends up\n>> being handled by handle_ignore_submodules_arg() in submodule.c, I'll\n>> leave it to you to see where that is called from (you can use `git grep`).\n>>\n>> I'm going to be off line for the rest of today, hopefully someone else\n>> will be able to help if you get stuck or I'll try and answer any other\n>> questions tomorrow.\n>>\n>> Best Wishes\n>>\n>> Phillip\n>>\n>>> Looking forward to working with you all.\n>>>\n>>> Sangeeta\n>>> [1] https://github.com/git/git/blob/master/builtin/describe.c\n>>> [2] https://github.com/git/git/blob/master/builtin/diff.c\n>>> [3] https://github.com/git/git/blob/master/diff.c\n>>>\n"},{"id":"407299","messageId":"CAHjREB6j6BqZ49wX5uqEOiysTAm8Oo7N=EFpcoovWKkBghBjxQ@mail.gmail.com","threadId":"54367","inReplyTo":"7d691f37-b8b7-510d-ba46-51a4a7587226@gmail.com","subject":"Re: [Outreachy] Introduction","fromName":"Sangeeta NB","fromEmail":"sangunb09@gmail.com","sentAt":"2020-10-11T11:30:56Z","receivedAt":"2020-10-11T11:31:11Z","isPatch":false,"sender":{"key":"sangunb09@gmail.com","avatar":"https://avatars.githubusercontent.com/u/64827599?v=4"},"body":"Thanks for the help, Philip.\n\nOn Fri, Oct 9, 2020 at 11:59 PM Phillip Wood wrote:\n\n> I struggled to find the mircoprojects page - I must have missed the link\n> on the outreachy site.\n\nIn case anyone else is struggling to find the microprojects page,\nhere's the link [1]\n\n[1] https://git.github.io/Outreachy-21-Microprojects/\n\n> As I understand it if a submodule contains any untracked files (i.e. a\n> file that has not been added with `git add` and is not ignored by any\n> .gitignore or .git/info/exclude entries) then running `git diff` in the\n> superproject will report that the submodule is dirty - there will be a\n> line something like \"+Subproject commit abcdef-dirty\". However if we run\n> `git describe --dirty` in the submodule directory then it will not\n> append \"-dirty\" to it's output unless there are changes to tracked files.\n\nOn running `git diff HEAD --ignore-submodules=untracked` the submodule\nwasn't reported as dirty.\n\nI guess this is what we are expecting. So should I make it the default\nbehavior for diff?\n\nA fix for making this as the default behaviour can be:\n\n--- a/diff.c\n+++ b/diff.c\n@@ -422,6 +422,7 @@ int git_diff_ui_config(const char *var, const char\n*value, void *cb)\n if (git_color_config(var, value, cb) < 0)\n return -1;\n\n+ handle_ignore_submodules_arg(&default_diff_options, \"untracked\");\n return git_diff_basic_config(var, value, cb);\n }\n\nBut this would also involve a lot of changes in the way tests are\nwritten as 12 out of 19 tests in t4027-diff-submodule.sh failed after\nadding this patch. I am working on any other workaround for this. Let\nme know whether I am on right path or not. Also any pointers on how to\nproceed would be helpful. Thanks!\n"},{"id":"407321","messageId":"eb1d6d2a-868f-7d1a-e004-30efc1950d9a@gmail.com","threadId":"54367","inReplyTo":"CAHjREB6j6BqZ49wX5uqEOiysTAm8Oo7N=EFpcoovWKkBghBjxQ@mail.gmail.com","subject":"Re: [Outreachy] Introduction","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2020-10-12T10:18:33Z","receivedAt":"2020-10-12T10:18:38Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Sangeeta\n\nOn 11/10/2020 12:30, Sangeeta NB wrote:\n> Thanks for the help, Philip.\n> \n> On Fri, Oct 9, 2020 at 11:59 PM Phillip Wood wrote:\n> \n>> I struggled to find the mircoprojects page - I must have missed the link\n>> on the outreachy site.\n> \n> In case anyone else is struggling to find the microprojects page,\n> here's the link [1]\n> \n> [1] https://git.github.io/Outreachy-21-Microprojects/\n> \n>> As I understand it if a submodule contains any untracked files (i.e. a\n>> file that has not been added with `git add` and is not ignored by any\n>> .gitignore or .git/info/exclude entries) then running `git diff` in the\n>> superproject will report that the submodule is dirty - there will be a\n>> line something like \"+Subproject commit abcdef-dirty\". However if we run\n>> `git describe --dirty` in the submodule directory then it will not\n>> append \"-dirty\" to it's output unless there are changes to tracked files.\n> \n> On running `git diff HEAD --ignore-submodules=untracked` the submodule\n> wasn't reported as dirty.\n\nThat's great\n\n> I guess this is what we are expecting. So should I make it the default\n> behavior for diff?\n\nI think that is a good route forward, we probably want to change the \ndefault for `diff-index` and `diff-files` as well\n\n> \n> A fix for making this as the default behaviour can be:\n> \n> --- a/diff.c\n> +++ b/diff.c\n> @@ -422,6 +422,7 @@ int git_diff_ui_config(const char *var, const char\n> *value, void *cb)\n> if (git_color_config(var, value, cb) < 0)\n> return -1;\n> \n> + handle_ignore_submodules_arg(&default_diff_options, \"untracked\");\n> return git_diff_basic_config(var, value, cb);\n> }\n> \n> But this would also involve a lot of changes in the way tests are\n> written as 12 out of 19 tests in t4027-diff-submodule.sh failed after\n> adding this patch. I am working on any other workaround for this. Let\n> me know whether I am on right path or not. Also any pointers on how to\n> proceed would be helpful. Thanks!\n\nWe'd expect some tests to fail but only the ones that are testing if \nuntracked files cause the submodule to be considered dirty.\n\ngit_diff_ui_config() is a callback that is invoked once per config key \nin the config files so I don't think it is a good place to make the \nchange as it is inefficient and overrides the users' \n`diff.ignoreSubmodules` setting . It also only applies to `diff` and not \n`diff-index` or `diff-files`. I think it would be better to set the \ndefault in diff_setup() though we need to be careful not to override \n`diff.ignoreSubmodules` setting so we might need to add a global flag to \nremember if the user has set `diff.ignoreSubmodules` in their config\n\nBest Wishes\n\nPhillip\n\n> \n\n"},{"id":"407328","messageId":"943d84e6-98fb-29e3-086a-3763c9e73ef8@gmail.com","threadId":"54367","inReplyTo":"CAHjREB6j6BqZ49wX5uqEOiysTAm8Oo7N=EFpcoovWKkBghBjxQ@mail.gmail.com","subject":"Re: [Outreachy] Introduction","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2020-10-12T11:22:14Z","receivedAt":"2020-10-12T11:22:21Z","isPatch":false,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"Hi Sangeeta,\n\nOn 11/10/20 5:00 pm, Sangeeta NB wrote:\n> \n>> As I understand it if a submodule contains any untracked files (i.e. a\n>> file that has not been added with `git add` and is not ignored by any\n>> .gitignore or .git/info/exclude entries) then running `git diff` in the\n>> superproject will report that the submodule is dirty - there will be a\n>> line something like \"+Subproject commit abcdef-dirty\". However if we run\n>> `git describe --dirty` in the submodule directory then it will not\n>> append \"-dirty\" to it's output unless there are changes to tracked files.\n> \n> On running `git diff HEAD --ignore-submodules=untracked` the submodule\n> wasn't reported as dirty.\n>\n\nRight.\n\n> I guess this is what we are expecting. So should I make it the default\n> behavior for diff?\n> \n\nYeah. Changing the default behaviour of 'diff' looks like one of the\noptions that Junio mentions in [1].\n\n> A fix for making this as the default behaviour can be:\n> \n> --- a/diff.c\n> +++ b/diff.c\n> @@ -422,6 +422,7 @@ int git_diff_ui_config(const char *var, const char\n> *value, void *cb)\n> if (git_color_config(var, value, cb) < 0)\n> return -1;\n> \n> + handle_ignore_submodules_arg(&default_diff_options, \"untracked\");\n> return git_diff_basic_config(var, value, cb);\n> }\n> \n\nI'm not sure about whether 'git_diff_ui_config' is the right\nplace to do this, though. I'm also not sure about what the right\napproach would be, off-hand. But I believe the Junio's e-mail I\nreference might be of help in pointing you in the right direction, in\ngeneral.\n\n> But this would also involve a lot of changes in the way tests are\n> written as 12 out of 19 tests in t4027-diff-submodule.sh failed after\n> adding this patch. I am working on any other workaround for this. Let\n> me know whether I am on right path or not. Also any pointers on how to\n> proceed would be helpful. Thanks!\n> \n\nSome test failures are likely to happens as a consequence of the change\ngiven that we would be changing default behaviour. So, adjusting the\ntests appropriately would indeed be necessary. We would've to be careful\nin evaluating the failures so that we don't break other _valid_ use\ncases as a side-effect.\n\n[ References ]\n\n[1]: https://lore.kernel.org/git/xmqq1rixi4cb.fsf@gitster.c.googlers.com/\n\n--\nSivaraam\n"},{"id":"407341","messageId":"xmqq362jsahk.fsf@gitster.c.googlers.com","threadId":"54367","inReplyTo":"CAHjREB6j6BqZ49wX5uqEOiysTAm8Oo7N=EFpcoovWKkBghBjxQ@mail.gmail.com","subject":"Re: [Outreachy] Introduction","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-10-12T15:57:27Z","receivedAt":"2020-10-12T15:57:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sangeeta NB writes:\n\n> A fix for making this as the default behaviour can be:\n>\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -422,6 +422,7 @@ int git_diff_ui_config(const char *var, const char\n> *value, void *cb)\n> if (git_color_config(var, value, cb) < 0)\n> return -1;\n>\n> + handle_ignore_submodules_arg(&default_diff_options, \"untracked\");\n> return git_diff_basic_config(var, value, cb);\n\nThis function is called for each and every element of configuration\nitem in your ~/.gitconfig and .git/config; by definition, the\ndefault behaviour is what is used when the user did not specify\nanything so what is usually done is to do that kind of defaulting\nbefore the code calls git_config() with a callback function like\nthis.\n\nAnd more importantly, the users may have\n\n [diff] ignoresubmodules=\n\nin their configuration file. After calling handle_ignore_submodules_arg()\nwith the value the user desires, the above code will overwrite it with\na hardcoded default---at that point that is no longer \"the default\"\nto be used when the user didn't specify.\n\nI am wondering if the init_diff_ui_defaults() function is the right\nlocation to add the above call.\n\n> }\n>\n> But this would also involve a lot of changes in the way tests are\n> written as 12 out of 19 tests in t4027-diff-submodule.sh failed after\n> adding this patch.\n\nIf the tests expect that the -dirty suffix is added at the end of\n\"Subproject commit 2f256705...\" when the submodule directory has a\nuntracked file, it is expected that such tests need to be updated\nto the new world order you are introducing, which is \"just like 'git\ndescribe --dirty' does not consider having an untracked file does not\nmake otherwise clean checkout a dirty one, 'git diff' should not\nshow that a submodule is dirty in its output if its working tree has\nan untracked file but is otherwise clean\".\n\n\n\nWhat follows is a note for more experienced developers, but I notice\nthat over the years, we seems to have done a shoddy job adjusting\nthe implementation in diff.c file in the hope of adding support to\nwork in multiple repositories; most file-scope static globals like\ndefault_diff_options and diff_detect_rename_default are still only\nread while in the main repository, yet repo_diff_setup() pretends as\nif an invocation of the diff machinery in a different repository can\nuse settings that are repository specific. Again, this is not\nsomething you need to be worried about.\n"},{"id":"407520","messageId":"CAHjREB4Qw78xS-T=LA80yVf_u3=Hbpxg5tqBnP1QK44FRkBFOQ@mail.gmail.com","threadId":"54367","inReplyTo":"xmqq362jsahk.fsf@gitster.c.googlers.com","subject":"Re: [Outreachy] Introduction","fromName":"Sangeeta NB","fromEmail":"sangunb09@gmail.com","sentAt":"2020-10-14T15:52:27Z","receivedAt":"2020-10-14T15:52:41Z","isPatch":false,"sender":{"key":"sangunb09@gmail.com","avatar":"https://avatars.githubusercontent.com/u/64827599?v=4"},"body":"Hey everyone,\n\nI created a PR at gitgitgadget here[1] but it is failing at three\ntests of git rm[2]. I looked at the behavior of git status at some\nother places( by pausing 'git diff HEAD with dirty\nsubmodule(untracked)' in t/t4027-diff-submodule.sh and looking at `git\nstatus` behavior) but it was working perfectly fine(was giving what\noutput was expected). But here[2] I couldn't understand why is it\nfailing. Can someone please have a look at the PR and give me some\npointers? I know I am asking way out of too much but I tried a lot on\nwhat could have been missing but couldn't find anything.\n\n[1] https://github.com/gitgitgadget/git/pull/751\n[2] https://github.com/git/git/blob/master/t/t3600-rm.sh#L691\n\nRegards,\nSangeeta\n\nOn Mon, Oct 12, 2020 at 9:27 PM Junio C Hamano wrote:\n>\n> Sangeeta NB writes:\n>\n> > A fix for making this as the default behaviour can be:\n> >\n> > --- a/diff.c\n> > +++ b/diff.c\n> > @@ -422,6 +422,7 @@ int git_diff_ui_config(const char *var, const char\n> > *value, void *cb)\n> > if (git_color_config(var, value, cb) < 0)\n> > return -1;\n> >\n> > + handle_ignore_submodules_arg(&default_diff_options, \"untracked\");\n> > return git_diff_basic_config(var, value, cb);\n>\n> This function is called for each and every element of configuration\n> item in your ~/.gitconfig and .git/config; by definition, the\n> default behaviour is what is used when the user did not specify\n> anything so what is usually done is to do that kind of defaulting\n> before the code calls git_config() with a callback function like\n> this.\n>\n> And more importantly, the users may have\n>\n> [diff] ignoresubmodules=\n>\n> in their configuration file. After calling handle_ignore_submodules_arg()\n> with the value the user desires, the above code will overwrite it with\n> a hardcoded default---at that point that is no longer \"the default\"\n> to be used when the user didn't specify.\n>\n> I am wondering if the init_diff_ui_defaults() function is the right\n> location to add the above call.\n>\n> > }\n> >\n> > But this would also involve a lot of changes in the way tests are\n> > written as 12 out of 19 tests in t4027-diff-submodule.sh failed after\n> > adding this patch.\n>\n> If the tests expect that the -dirty suffix is added at the end of\n> \"Subproject commit 2f256705...\" when the submodule directory has a\n> untracked file, it is expected that such tests need to be updated\n> to the new world order you are introducing, which is \"just like 'git\n> describe --dirty' does not consider having an untracked file does not\n> make otherwise clean checkout a dirty one, 'git diff' should not\n> show that a submodule is dirty in its output if its working tree has\n> an untracked file but is otherwise clean\".\n>\n>\n>\n> What follows is a note for more experienced developers, but I notice\n> that over the years, we seems to have done a shoddy job adjusting\n> the implementation in diff.c file in the hope of adding support to\n> work in multiple repositories; most file-scope static globals like\n> default_diff_options and diff_detect_rename_default are still only\n> read while in the main repository, yet repo_diff_setup() pretends as\n> if an invocation of the diff machinery in a different repository can\n> use settings that are repository specific. Again, this is not\n> something you need to be worried about.\n"},{"id":"407586","messageId":"3c0bf417-b3d5-b615-c605-49a5d8be4e06@gmail.com","threadId":"54367","inReplyTo":"CAHjREB4Qw78xS-T=LA80yVf_u3=Hbpxg5tqBnP1QK44FRkBFOQ@mail.gmail.com","subject":"Re: [Outreachy] Introduction","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2020-10-15T09:23:59Z","receivedAt":"2020-10-15T09:24:09Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Sangeeta\n\nOn 14/10/2020 16:52, Sangeeta NB wrote:\n> Hey everyone,\n> \n> I created a PR at gitgitgadget here[1] but it is failing at three\n> tests of git rm[2]. I looked at the behavior of git status at some\n> other places( by pausing 'git diff HEAD with dirty\n> submodule(untracked)' in t/t4027-diff-submodule.sh and looking at `git\n> status` behavior) but it was working perfectly fine(was giving what\n> output was expected). But here[2] I couldn't understand why is it\n> failing. Can someone please have a look at the PR and give me some\n> pointers? I know I am asking way out of too much but I tried a lot on\n> what could have been missing but couldn't find anything.\n\nI've spent some time looking at it and I cannot understand what is going \non :-( I think it may possibly be something to do with that test looking \nat a nested submodule and the options not being properly propagated down \nto that submodule - it might be worth looking at the diff code that \nhandles submodules.\n\nI've got a fixup which I'll post after this which gets rid of the global \nflag and instead uses a flag in struct diff_options. I had a quick look \nthrough the test changes and I thinking it would be worth considering if \nsome of them should instead be changed to pass --ignore-submodules=none \nrather than changing the expected result.\n\nBest Wishes\n\nPhillip\n\n> \n> [1] https://github.com/gitgitgadget/git/pull/751\n> [2] https://github.com/git/git/blob/master/t/t3600-rm.sh#L691\n> \n> Regards,\n> Sangeeta\n> \n> On Mon, Oct 12, 2020 at 9:27 PM Junio C Hamano wrote:\n>>\n>> Sangeeta NB writes:\n>>\n>>> A fix for making this as the default behaviour can be:\n>>>\n>>> --- a/diff.c\n>>> +++ b/diff.c\n>>> @@ -422,6 +422,7 @@ int git_diff_ui_config(const char *var, const char\n>>> *value, void *cb)\n>>> if (git_color_config(var, value, cb) < 0)\n>>> return -1;\n>>>\n>>> + handle_ignore_submodules_arg(&default_diff_options, \"untracked\");\n>>> return git_diff_basic_config(var, value, cb);\n>>\n>> This function is called for each and every element of configuration\n>> item in your ~/.gitconfig and .git/config; by definition, the\n>> default behaviour is what is used when the user did not specify\n>> anything so what is usually done is to do that kind of defaulting\n>> before the code calls git_config() with a callback function like\n>> this.\n>>\n>> And more importantly, the users may have\n>>\n>> [diff] ignoresubmodules=\n>>\n>> in their configuration file. After calling handle_ignore_submodules_arg()\n>> with the value the user desires, the above code will overwrite it with\n>> a hardcoded default---at that point that is no longer \"the default\"\n>> to be used when the user didn't specify.\n>>\n>> I am wondering if the init_diff_ui_defaults() function is the right\n>> location to add the above call.\n>>\n>>> }\n>>>\n>>> But this would also involve a lot of changes in the way tests are\n>>> written as 12 out of 19 tests in t4027-diff-submodule.sh failed after\n>>> adding this patch.\n>>\n>> If the tests expect that the -dirty suffix is added at the end of\n>> \"Subproject commit 2f256705...\" when the submodule directory has a\n>> untracked file, it is expected that such tests need to be updated\n>> to the new world order you are introducing, which is \"just like 'git\n>> describe --dirty' does not consider having an untracked file does not\n>> make otherwise clean checkout a dirty one, 'git diff' should not\n>> show that a submodule is dirty in its output if its working tree has\n>> an untracked file but is otherwise clean\".\n>>\n>>\n>>\n>> What follows is a note for more experienced developers, but I notice\n>> that over the years, we seems to have done a shoddy job adjusting\n>> the implementation in diff.c file in the hope of adding support to\n>> work in multiple repositories; most file-scope static globals like\n>> default_diff_options and diff_detect_rename_default are still only\n>> read while in the main repository, yet repo_diff_setup() pretends as\n>> if an invocation of the diff machinery in a different repository can\n>> use settings that are repository specific. Again, this is not\n>> something you need to be worried about.\n"},{"id":"407587","messageId":"2d51c999-61cc-d490-46c9-0fac19aff066@gmail.com","threadId":"54367","inReplyTo":"3c0bf417-b3d5-b615-c605-49a5d8be4e06@gmail.com","subject":"[PATCH] fixup! diff: do not show submodule with untracked files as \"-dirty\"","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2020-10-15T09:26:19Z","receivedAt":"2020-10-15T09:26:24Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Simplify checking if we've already set the flags for ignoring submodules\n---\n builtin/checkout.c | 1 -\n diff.c | 12 +++++-------\n diff.h | 7 +------\n submodule.c | 5 ++---\n wt-status.c | 3 ---\n 5 files changed, 8 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 428201380e..0951f8fee5 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1095,7 +1095,6 @@ static int git_checkout_config(const char *var, const char *value, void *cb)\n {\n \tif (!strcmp(var, \"diff.ignoresubmodules\")) {\n \t\tstruct checkout_opts *opts = cb;\n-\t\tdiff_ignore_submodule_config =1;\n \t\thandle_ignore_submodules_arg(&opts->diff_options, value);\n \t\treturn 0;\n \t}\ndiff --git a/diff.c b/diff.c\nindex 6c9d28d363..93bbb57f88 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -51,7 +51,6 @@ static int diff_no_prefix;\n static int diff_relative;\n static int diff_stat_graph_width;\n static int diff_dirstat_permille_default = 30;\n-int diff_ignore_submodule_config = 0;\n static struct diff_options default_diff_options;\n static long diff_algorithm;\n static unsigned ws_error_highlight_default = WSEH_NEW;\n@@ -403,10 +402,8 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)\n \tif (!strcmp(var, \"diff.orderfile\"))\n \t\treturn git_config_pathname(&diff_order_file_cfg, var, value);\n \n-\tif (!strcmp(var, \"diff.ignoresubmodules\")) {\n-\t\tdiff_ignore_submodule_config =1;\n+\tif (!strcmp(var, \"diff.ignoresubmodules\"))\n \t\thandle_ignore_submodules_arg(&default_diff_options, value);\n-\t}\n \n \tif (!strcmp(var, \"diff.submodule\")) {\n \t\tif (parse_submodule_params(&default_diff_options, value))\n@@ -4598,8 +4595,10 @@ void repo_diff_setup(struct repository *r, struct diff_options *options)\n \n \toptions->color_moved = diff_color_moved_default;\n \toptions->color_moved_ws_handling = diff_color_moved_ws_default;\n-\tif(!diff_ignore_submodule_config)\n-\t\thandle_ignore_submodules_arg(options, \"untracked\");\n+\n+\tif(!options->flags.ignore_submodule_set)\n+\t\toptions->flags.ignore_untracked_in_submodules = 1;\n+\n \tprep_parse_options(options);\n }\n \n@@ -5142,7 +5141,6 @@ static int diff_opt_ignore_submodules(const struct option *opt,\n \tif (!arg)\n \t\targ = \"all\";\n \toptions->flags.override_submodule_config = 1;\n-\tdiff_ignore_submodule_config =1;\n \thandle_ignore_submodules_arg(options, arg);\n \treturn 0;\n }\ndiff --git a/diff.h b/diff.h\nindex 9bfce084d8..2f5612ff7d 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -180,6 +180,7 @@ struct diff_flags {\n \tunsigned ignore_untracked_in_submodules;\n \tunsigned ignore_dirty_submodules;\n \tunsigned override_submodule_config;\n+\tunsigned ignore_submodule_set;\n \tunsigned dirstat_by_line;\n \tunsigned funccontext;\n \tunsigned default_follow_renames;\n@@ -556,12 +557,6 @@ int diff_queue_is_empty(void);\n void diff_flush(struct diff_options*);\n void diff_warn_rename_limit(const char *varname, int needed, int degraded_cc);\n \n-/* \n- * This is for keeping a track whether ignoreSubmodules\n- * is present in user defined config or not.\n- */\n-extern int diff_ignore_submodule_config;\n-\n /* diff-raw status letters */\n #define DIFF_STATUS_ADDED\t\t'A'\n #define DIFF_STATUS_COPIED\t\t'C'\ndiff --git a/submodule.c b/submodule.c\nindex a526c96ce2..8f6227c993 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -198,10 +198,8 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,\n \t\t\tignore = submodule->ignore;\n \t\tfree(key);\n \n-\t\tif (ignore){\n-\t\t\tdiff_ignore_submodule_config =1;\n+\t\tif (ignore)\n \t\t\thandle_ignore_submodules_arg(diffopt, ignore);\n-\t\t}\n \t\telse if (is_gitmodules_unmerged(the_repository->index))\n \t\t\tdiffopt->flags.ignore_submodules = 1;\n \t}\n@@ -422,6 +420,7 @@ const char *submodule_strategy_to_string(const struct submodule_update_strategy\n void handle_ignore_submodules_arg(struct diff_options *diffopt,\n \t\t\t\t const char *arg)\n {\n+\tdiffopt->flags.ignore_submodule_set = 1;\n \tdiffopt->flags.ignore_submodules = 0;\n \tdiffopt->flags.ignore_untracked_in_submodules = 0;\n \tdiffopt->flags.ignore_dirty_submodules = 0;\ndiff --git a/wt-status.c b/wt-status.c\nindex 791af9321a..7074bbdd53 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -605,7 +605,6 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)\n \t\trev.diffopt.flags.ignore_untracked_in_submodules = 1;\n \tif (s->ignore_submodule_arg) {\n \t\trev.diffopt.flags.override_submodule_config = 1;\n-\t\tdiff_ignore_submodule_config =1;\n \t\thandle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg);\n \t}\n \trev.diffopt.format_callback = wt_status_collect_changed_cb;\n@@ -630,7 +629,6 @@ static void wt_status_collect_changes_index(struct wt_status *s)\n \trev.diffopt.flags.override_submodule_config = 1;\n \trev.diffopt.ita_invisible_in_index = 1;\n \tif (s->ignore_submodule_arg) {\n-\t\tdiff_ignore_submodule_config =1;\n \t\thandle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg);\n \t} else {\n \t\t/*\n@@ -641,7 +639,6 @@ static void wt_status_collect_changes_index(struct wt_status *s)\n \t\t * shown any submodules she manually added (and which are\n \t\t * staged to be committed), which would be really confusing.\n \t\t */\n-\t\tdiff_ignore_submodule_config =1;\n \t\thandle_ignore_submodules_arg(&rev.diffopt, \"dirty\");\n \t}\n \n-- \n2.25.1.551.gd3318bf0d3.dirty\n\n"},{"id":"407590","messageId":"CAHjREB7xhBWHUCJ+T78i6dDGQr055sMAaYzG4gaQSr0T88cseg@mail.gmail.com","threadId":"54367","inReplyTo":"3c0bf417-b3d5-b615-c605-49a5d8be4e06@gmail.com","subject":"Re: [Outreachy] Introduction","fromName":"Sangeeta NB","fromEmail":"sangunb09@gmail.com","sentAt":"2020-10-15T10:18:11Z","receivedAt":"2020-10-15T10:18:25Z","isPatch":false,"sender":{"key":"sangunb09@gmail.com","avatar":"https://avatars.githubusercontent.com/u/64827599?v=4"},"body":"Hey,\n\n> I've got a fixup which I'll post after this which gets rid of the global\n> flag and instead uses a flag in struct diff_options.\n\nThanks for the patch, I was thinking about something on similar lines\nbut couldn't come up with anything.\n\nAlso, one thing I observed that when I add a printf statement in\nwt-status.c, something like this:\n\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -601,11 +601,11 @@ static void\nwt_status_collect_changes_worktree(struct wt_status *s)\n rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n rev.diffopt.flags.dirty_submodules = 1;\n rev.diffopt.ita_invisible_in_index = 1;\n+ printf(\"a printf statement\\n\");\n if (!s->show_untracked_files)\n\nthen git status shows output:\n\nsh-3.2$ git status -s -uno --ignore-submodules=none\na printf statement\n m submod\n\nwhich is what is expected. But when I comment out the printf statement\nit again gives no output. I couldn't understand why this is taking\nplace and how can a printf line modify the behavior of git status.\n\n> I thinking it would be worth considering if\n> some of them should instead be changed to pass --ignore-submodules=none\n> rather than changing the expected result.\n\nYa, that's a good suggestion. Would look at those tests again and see\nif I can pass the --ignore-submodules=none option.\n\nThanks\n"},{"id":"407608","messageId":"95c90e7a-8f4c-32b9-2b0e-9efce2acfb8e@gmail.com","threadId":"54367","inReplyTo":"CAHjREB7xhBWHUCJ+T78i6dDGQr055sMAaYzG4gaQSr0T88cseg@mail.gmail.com","subject":"Re: [Outreachy] Introduction","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2020-10-15T13:39:47Z","receivedAt":"2020-10-15T13:39:53Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Sangeeta\n\nOn 15/10/2020 11:18, Sangeeta NB wrote:\n> Hey,\n> \n>> I've got a fixup which I'll post after this which gets rid of the global\n>> flag and instead uses a flag in struct diff_options.\n> \n> Thanks for the patch, I was thinking about something on similar lines\n> but couldn't come up with anything.\n\nAs we store the config options in default_diff_options and then copy \nthem across at the beginning of repo_setup_diff() we can use a flag in \nstruct diff_options which is set by handle_ignore_submodule_arg() to \ntell if we need to initialize opts->flags.ignore_untracked_in_submodules \nin repo_setup_diff()\n\n> Also, one thing I observed that when I add a printf statement in\n> wt-status.c, something like this:\n> \n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -601,11 +601,11 @@ static void\n> wt_status_collect_changes_worktree(struct wt_status *s)\n> rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n> rev.diffopt.flags.dirty_submodules = 1;\n> rev.diffopt.ita_invisible_in_index = 1;\n> + printf(\"a printf statement\\n\");\n> if (!s->show_untracked_files)\n> \n> then git status shows output:\n> \n> sh-3.2$ git status -s -uno --ignore-submodules=none\n> a printf statement\n> m submod\n> \n> which is what is expected. But when I comment out the printf statement\n> it again gives no output. I couldn't understand why this is taking\n> place and how can a printf line modify the behavior of git status.\n\nAre you adding the printf and then running t3600? If so then the extra \nline of output breaks a lot of tests which in turn breaks to setup for \nthe test that was failing so there are uncommitted changes. \nUnfortunately it is hard to run a subset of tests in a lot the test \nscripts as there are implicit dependencies between the individual tests \nthem.\n\n>> I thinking it would be worth considering if\n>> some of them should instead be changed to pass --ignore-submodules=none\n>> rather than changing the expected result.\n> \n> Ya, that's a good suggestion. Would look at those tests again and see\n> if I can pass the --ignore-submodules=none option.\n\nFor the diff tests I think we want to test the new default and check \nthat --ignore-submodules=none works. I think for the other tests we \nprobably want to just add --ignore-submodules=none\n\nI'm afraid I'm still no closer to figuring out why that test in t3600 fails\n\nPhillip\n\n> Thanks\n> \n"},{"id":"407610","messageId":"CAHjREB6i7ZSSXk10OAOXcL-Av3WzesCkMKL2ES_aBRsrynT9DA@mail.gmail.com","threadId":"54367","inReplyTo":"95c90e7a-8f4c-32b9-2b0e-9efce2acfb8e@gmail.com","subject":"Re: [Outreachy] Introduction","fromName":"Sangeeta NB","fromEmail":"sangunb09@gmail.com","sentAt":"2020-10-15T13:57:59Z","receivedAt":"2020-10-15T13:58:12Z","isPatch":false,"sender":{"key":"sangunb09@gmail.com","avatar":"https://avatars.githubusercontent.com/u/64827599?v=4"},"body":"On Thu, Oct 15, 2020 at 7:09 PM Phillip Wood wrote:\nHey Phillip,\n\n> As we store the config options in default_diff_options and then copy\n> them across at the beginning of repo_setup_diff() we can use a flag in\n> struct diff_options which is set by handle_ignore_submodule_arg() to\n> tell if we need to initialize opts->flags.ignore_untracked_in_submodules\n> in repo_setup_diff()\n\nEven if we don't set a global flag it is working fine because we are\nsetting the default first, and would let the config override it. I\nhave updated the code in the PR and you can have a look at it. I have\nalso added --ignore-submodules=none in some tests to get the results\nmentioned earlier.\n\n> Are you adding the printf and then running t3600? If so then the extra\n> line of output breaks a lot of tests which in turn breaks to setup for\n> the test that was failing so there are uncommitted changes.\n> Unfortunately it is hard to run a subset of tests in a lot the test\n> scripts as there are implicit dependencies between the individual tests\n> them.\n>\nOh, okay it makes sense.\n\n>\n> I'm afraid I'm still no closer to figuring out why that test in t3600 fails\n\nWhat it is like debugging in Git? I have seen people writing debug\nstatements(print statements in between the code) to figure out how\nthings are working. But I guess we might not be able to do that. Do we\nhave to create the exact environment that is been created by that test\nto check for the code?\n"},{"id":"407611","messageId":"8c9b409b-4e44-ca8a-9075-36a7332f9765@gmail.com","threadId":"54367","inReplyTo":"CAHjREB6i7ZSSXk10OAOXcL-Av3WzesCkMKL2ES_aBRsrynT9DA@mail.gmail.com","subject":"Re: [Outreachy] Introduction","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2020-10-15T14:45:53Z","receivedAt":"2020-10-15T14:46:01Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Sangeeta\n\nOn 15/10/2020 14:57, Sangeeta NB wrote:\n> On Thu, Oct 15, 2020 at 7:09 PM Phillip Wood wrote:\n> Hey Phillip,\n> \n>> As we store the config options in default_diff_options and then copy\n>> them across at the beginning of repo_setup_diff() we can use a flag in\n>> struct diff_options which is set by handle_ignore_submodule_arg() to\n>> tell if we need to initialize opts->flags.ignore_untracked_in_submodules\n>> in repo_setup_diff()\n> \n> Even if we don't set a global flag it is working fine because we are\n> setting the default first, and would let the config override it. I\n> have updated the code in the PR and you can have a look at it. I have\n> also added --ignore-submodules=none in some tests to get the results\n> mentioned earlier.\n\nThanks, I'll have a look later\n\n>> Are you adding the printf and then running t3600? If so then the extra\n>> line of output breaks a lot of tests which in turn breaks to setup for\n>> the test that was failing so there are uncommitted changes.\n>> Unfortunately it is hard to run a subset of tests in a lot the test\n>> scripts as there are implicit dependencies between the individual tests\n>> them.\n>>\n> Oh, okay it makes sense.\n> \n>>\n>> I'm afraid I'm still no closer to figuring out why that test in t3600 fails\n\ndiff --git a/submodule.c b/submodule.c\nindex 8f6227c993..c4182be633 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1679,6 +1679,8 @@ unsigned is_submodule_modified(const char *path, \nint ignore_untracked)\n strvec_pushl(&cp.args, \"status\", \"--porcelain=2\", NULL);\n if (ignore_untracked)\n strvec_push(&cp.args, \"-uno\");\n+ else\n+ strvec_push (&cp.args, \"--ignore-submodules=none\");\n\n prepare_submodule_repo_env(&cp.env_array);\n cp.git_cmd = 1;\n\nfixes it, I'm unsure at the moment if we should be adding the extra flag \nhere or setting the appropriate option in status when -uno and \n--ignore-submodules=