On Tue, May 17, 2016 at 5:22 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Felipe Contreras <felipe.contreras@xxxxxxxxx> writes: > >> Certain lines of the marks file might be corrupted (or the objects >> missing due to a garbage collection), but that's no reason to truncate >> the file and essentially destroy the rest of it. > > Hmm, so the issue is: > > - we use die_nicely() that calls dump_marks() after writing a crash > report as our die_routine(). > > - when dump_marks() is called, and export_marks_file names an > existing file, it tries to write marks in it. If we let it go > through, we would end up writing a new marks file based on an > incomplete set of marks we have only half-read in the earlier > step, which is bad, as the resulting one is incomplete, and the > original one that this replaced may have been a good one. > > Is that what this change addresses? Yes. As I said; the marks file gets truncated. > I am just wondering if a solution to preserve both files is more > desirable. > > This change looks a bit over-eager to discard the dump die_nicely() > is trying to create in one scenario, and a bit less careful at the > same time in another scenario. > > - Even if we are reading from somewhere, export_marks_file can > point at a completely new file that is different from > import_marks file, in which case, we are not really losing any > information by freshly creating a new marks file, no? Right, we are not losing any information, but we are not gaining much either: it's a truncated version of the import marks. > - Even if we did not read from any existing marks file, if we are > given export_marks_file that names an existing file, wouldn't we > want to avoid corrupting it with a dump from this aborted run? If we don't run from an existing marks file, this patch has no effect. -- Felipe Contreras -- 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