On Tue, Jul 15, 2008 at 2:22 AM, Johannes Schindelin <Johannes.Schindelin@xxxxxx> wrote: > Hi, > > On Mon, 14 Jul 2008, Geoffrey Irving wrote: > >> 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. > > Funnily, (2) contradicts (1). The patch id is _different_ when you have > nr_paths > 0. At least in the general case. > > So what you propose in (1) will not work, unless you also hash the path > names (in the correct order, otherwise you'll end up with two hashes). The sha1 would include paths, of course, since it's part of diff_options. > OTOH I would be really surprised if you needed --cherry-pick with paths > and/or diff options more than once for the same commits. So the caching > does not make sense to begin with (especially since we do not have a > proper way of gc'ing it, right?). > > So I'd suggest saving diff_opts before the command line parsing, and > disable the cache when it is different _and/or_ (||) nr_paths. I'll attach the patch to the other thread. 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
