Re: [PATCH 3/4] t5304: Ensure wanted files are not deleted

Doug Kelly [off-list ref] writes:

On Wed, Jan 13, 2016 at 2:55 PM, Junio C Hamano [off-list ref] wrote:

quoted

Doug Kelly [off-list ref] writes:

quoted

Subject: Re: [PATCH 3/4] t5304: Ensure wanted files are not deleted

I'd suggest s/wanted/non-garbage/.

I'm okay with this.

quoted

quoted

Explicitly test for and ensure files that may be wanted are not
deleted during a gc operation.  These include .pack without .idx
(which may be in-flight), garbage in the directory, and .keep files
the user created.

"garbage in the directory" is not well defined.  "files in the
directory that clearly are not related to packing" is probably what
you meant, but the definition of "related to packing" is still
fuzzy.  Please clarify.

This is probably a good point.  Perhaps a better way to think about it
would be by rewording the paragraph to something like this:

Explicitly test for and ensure files that may either be desired by the user
or are possibly not garbage are not deleted during a gc operation.
These include .pack files missing a corresponding .idx file (possibly due
to it being in-flight), .keep files created by the user, and other
unknown garbage in the pack directory.  These files will still be identified
by "git count-objects -v", but should not be removed automatically by
gc.  Only files we are absolutely sure are unnecessary will be removed
as a part of the gc process.

That, especially "other _unknown_ garbage", looks much better than
the original.

I'm not saying there's nothing to be said for the difference in the base
filename without extension.  Currently, the logic to remove pack garbage
doesn't look at that, though: it only considers the extension, and what
related files are found in the directory.  Whether this is good or bad, I'm
not sure.  It certainly does what I need at fairly low risk, though.

Does this help clarify the situation more?

I was shooting for making you _think_ exactly about what you wrote
in the above paragraph, i.e. what the current logic does and if it
is sensible, is overly pessimistic for some files, and/or is risky
for some other files and if so in what way, as that would help you
record the thinking behind the different treatment for files based
on various file extentions clearly in the log message.

Thanks.