On Sun, Jul 13, 2008 at 10:11 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Here are the topics that have been cooking. Commits prefixed > with '-' are only in 'pu' while commits prefixed with '+' are > in 'next'. > > <snip> > > * gi/cherry-cache (Sat Jul 12 20:14:51 2008 -0700) 1 commit > - cherry: cache patch-ids to avoid repeating work > > This does not seem to pass tests even on its own. The problem (beyond the basic problem of me not having tried running the tests) is that the current caching code isn't taking into account the changing values of diff_options. t6007 computes a patch-id for a commit with one value of options.paths, and then tries to compute a _different_ patch-id for the same commit using a different value of options.paths. Here are a few different ways of fixing this: 1. Modify commit_patch_id in patch-ids.c to compute a sha1 of the diff_options structure and xor it with the commit sha1 to get a truly unique hash of the input. This means the optimization can be safely applied for all patch-id computations regardless of the diff_options. I can add a diff_options_sha1 function in diff.[ch] to compute the checksum. 2. Restrict commit_patch_id in patch-ids.c to apply the optimization only if options.nr_paths is zero, and perhaps a few other conditions. This is rather fragile, since it would mean that the cache would break if someone decided to change the default diff options. 3. Add a flag in struct patch_ids defaulting to false which turns the caching on or off, and manually set the flag to true in cmd_cherry. I'd lean towards (1), but wanted to check before writing the code to make sure that it's reasonable to treat diff_options as stable enough that computing a sha1 hash of it makes sense. According to "git help patch-id", it is only "reasonable stable", which is sufficient as long as we're confident that whenever the diff format changes, the diff_options_sha1 function will be updated to reflect that change. As an aside: is it correct that as long as the change is in pu, I should be submitting complete (nonincremental) patches whenever I fix bugs? Thanks, Geoffrey -- 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
