Shuqi Liang wrote: > Hi Victoria, > >> If a user manually modifies a SKIP_WORKTREE file, SKIP_WORKTREE will be >> removed from the file and the index expanded automatically [1]. If that >> mechanism is working properly, there would be no need to manually check the >> pathspec and expand the index. >> >> Have you tried removing the 'pathspec_needs_expanded_index()' and running >> the tests? If so, is 'diff-files' producing incorrect results? >> >> [1] https://lore.kernel.org/git/11d46a399d26c913787b704d2b7169cafc28d639.1642175983.git.gitgitgadget@xxxxxxxxx/ > > As per your suggestion, I tried removing pathspec_needs_expanded_index() > from the code, and 'diff-files pathspec expands index when necessary' > test failed. > > So, I'm thinking about keeping it to ensure everything works properly. > I'd like to know your thoughts on this. Should we keep it or explore > other options? Did the test fail because the index wasn't expanded in a case where you previously expended it to be expanded? Or because of the returned results of 'diff-files' are invalid? Only the latter represents incorrect behavior. If we're aren't expanding the index for a case that was causing index expansion before *and* the user-facing behavior is as-expected, that's the best-case scenario for a sparse index integration! Taking a step back, it's important to remember that the overarching goal of the project is not just to switch 'command_requires_full_index' to '0' everywhere, but to find all of the places where Git is working with the index and make sure that work can be done on a sparse directory. In most cases, it's possible to adapt an index-related operation to work with sparse directories (albeit with varying levels of complexity). The use of 'ensure_full_index()' is reserved for cases where it is _impossible_ to make Git perform a given action on a sparse directory - expanding the index completely eliminates the performance gains had by using a sparse index, so it should be avoided at all costs. I hope that helps! > > Thanks, > Shuqi
