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). 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. Ciao, Dscho -- 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
