git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 3/3] clean: improve performance when removing lots of directories

From
Jeff King <peff@peff.net>
Date
Apr 17, 2015, 19:00 UTC
Message-ID
<20150417190002.GB32578@peff.net>
In-Reply-To
<CAMpP7NbQ7pmcjT4pDn5dA5yvfgMgirbkoMD6ijjiFp7zmARPbA@mail.gmail.com>
On Fri, Apr 17, 2015 at 08:15:40PM +0200, erik elfström wrote:
Show 12 quoted lines
> > Doesn't this implementation get confused by modern submodule
> > checkouts and descend into and clean their working tree, though?
> > Module M with path P would have a directory P in the working tree of
> > the top-level project, and P/.git is a regular file that will fail
> > "is_git_directory()" test but records the location of the real
> > submodule repository i.e. ".git/modules/M" via the "gitdir:"
> > mechanism.
> >
> 
> Yes, there is a problem here. I've added the test below and it fails after
> my change by cleaning sub2 (sub1 is not cleaned). Are there more cases
> here that I should test for?

I wonder about the opposite case, too (finding more repos than we used to).

It looks like your patches will find bare repositories in the tree, whereas the current code does not (it only cares about ".git"). AFAIK, submodules will never exist as bare in the working tree. And I have seen repositories which embed bare repos as test cases. Admittedly this is because I work on projects that are related to git itself, but I don't see a reason to regress this case if the submodule code doesn't get any benefit.

Show 6 quoted lines
> Base on the previous discussion of the patch topic I can see 3 options
> for how to fix this:
> 
> Option 1:
>  Plug the hole in my new is_git_repository function. A quick and dirty
>  fix that passes the above test would be:

I think that makes sense. It would be nice if you could just call read_gitfile, but that function is very anxious to die on error. So the prerequisite step would probably be to refactor that into a read_gitfile_gently that returns an error code.

-Peff
PS Thank you for working on this. I have been quiet because I haven't
   had a chance to look over your patches carefully yet, but overall
   they look very promising.
Previous: erik elfströmNext: Junio C Hamano
Message 11 of 13 in “Improving performance of git clean”
  1. 0/3 Improving performance of git cleanErik Elfström, Apr 11, 2015
  2. 0/3 Improving performance of git cleanErik Elfström, Apr 11, 2015
  3. 1/3 t7300: add tests to document behavior of clean and nested gitErik Elfström, Apr 11, 2015
  4. 2/3 p7300: add performance tests for cleanErik Elfström, Apr 11, 2015
  5. Thomas GummererApr 11, 2015
  6. erik elfströmApr 12, 2015
  7. Thomas GummererApr 12, 2015
  8. 3/3 clean: improve performance when removing lots of directoriesErik Elfström, Apr 11, 2015
  9. Junio C HamanoApr 15, 2015
  10. erik elfströmApr 17, 2015
  11. Jeff KingApr 17, 2015
  12. Junio C HamanoApr 17, 2015
  13. Eric SunshineApr 15, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.