Re: [PATCH] diff: don't read index when --no-index is given
From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:59:25
Thomas Gummerer wrote:
git diff --no-index ... currently reads the index, during setup, when calling gitmodules_config(). In the usual case this gives us some performance drawbacks,
Makes sense.
but it's especially annoying if there is a broken index file.
Is this really a normal case? It makes sense that as a side-effect it is easier to use "git diff --no-index" as a general-purpose tool while investigating a broken repo, but I would have thought that quickly learning a repo is broken is a good thing in any case. A little more information about the context where this came up would be helpful, I guess. [...]
quoted hunk ↗ jump to hunk
--- a/builtin/diff.c +++ b/builtin/diff.c
[...]
quoted hunk ↗ jump to hunk
@@ -282,9 +282,18 @@ int cmd_diff(int argc, const char **argv, const char *prefix) * * Other cases are errors. */ + for (i = 1; i < argc; i++) { + if (!strcmp(argv[i], "--")) + break; + if (!strcmp(argv[i], "--no-index")) { + no_index = 1; + break; + }
setup_revisions() uses the same logic that doesn't handle options that take arguments in their "unstuck" form (e.g., "--word-diff-regex --"), so this is probably not a regression, though I haven't checked. :) [...]
prefix = setup_git_directory_gently(&nongit); - gitmodules_config(); + if (!no_index) + gitmodules_config();
Perhaps we can improve performance and behavior by skipping the setup_git_directory_gently() call, too? That would help with the repairing-broken-repository case by working even if .git/config or .git/HEAD is broken, and I think it is more intuitive that the repository-local configuration is ignored by "git diff --no-index". It would also help with performance by avoiding some filesystem access. [...]
+test_expect_success 'git diff --no-index with broken index' ' + cd repo && + echo broken >.git/index && + test_expect_code 0 git diff --no-index a ../non/git/a
Clever. I wouldn't use "test_expect_code 0", since that's already implied by including the "git diff --no-index" call in the && chain. Thanks and hope that helps, Jonathan