On Mon, Aug 3, 2015 at 8:27 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Doug Kelly <dougk.ff7@xxxxxxxxx> writes: > >> Here's a change to prune.c that at least addresses the issue by removing >> .idx files without an associated pack, but it's by no means pretty. If anyone >> has any feedback before I turn this into a formal patch, it's more than welcome! > > I'd hesitate to see removal of a file (for that matter, a creation > too) inside a "while (de = readdir)" loop. As the original function > is about temporary files, and the new thing is not about temporary > files at all, I'd further prefer that we do not do it in the same > loop. > > I am wondering if we can add a new mode to report_pack_garbage() in > sha1_file.c to allow it to remove stale and lone ".idx". Most of > the time we are accessing packs read-only, and I do not want the > function to unconditionally remove lone ".idx", but perhaps we > can teach "prune" to set a custom report_garbage() routine and > react to a call to its custom report_garbage()? > > Perhaps that custom report_garbage() can make a list of ".idx" > files, iterate over it to pick the lone one without ".pack" and > remove them. Or the custom report_garbage() can make a list of lone > ".idx" files, if you tweak the interface to report_garbage() to > contain th seen_bits value, avoiding the need to check the existence > of ".pack" for the second time. Yeah, I didn't think this was the cleanest solution, and I wasn't even thinking about removing while inside the readdir loop, but I can see how that might be a very bad idea. In any case, thanks for the suggestions... I'll be completely blunt in saying I'm far less than well-versed in the Git internals. Looking at the implementation of report_pack_garbage(), it does look like seen_bits already has this logic, and indeed, git count-objects -v reports the files as garbage. So, I think you're right: prune would need to set report_garbage appropriately, then call count-objects to clean that up. If we wanted it to *only* care for lone idx files, we would have to string match on the message (seems fragile), but perhaps a more observant approach would be to add a custom flag to prune to clean *all* garbage in the repository, as passed to report_garbage? Probably wouldn't want to be enabled by default, but only on invocation or with careful consideration and setting an appropriate config flag. Thoughts? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html