Jay Soffian <jaysoffian@xxxxxxxxx> writes:
> This area of git is still black magic to me. My best guess is
> something like this:
>
> diff --git a/tree-diff.c b/tree-diff.c
> index b3cc2e4753..6fd84eb2bb 100644
> --- a/tree-diff.c
> +++ b/tree-diff.c
> @@ -280,6 +282,19 @@ int diff_tree_sha1(const unsigned char *old,
> const unsigned char *new, const cha
> die("unable to read destination tree (%s)", sha1_to_hex(new));
> init_tree_desc(&t1, tree1, size1);
> init_tree_desc(&t2, tree2, size2);
> +
> + if (is_bare_repository()) {
> + struct unpack_trees_options unpack_opts;
> + memset(&unpack_opts, 0, sizeof(unpack_opts));
> + unpack_opts.index_only = 1;
> + unpack_opts.head_idx = -1;
> + unpack_opts.src_index = &the_index;
> + unpack_opts.dst_index = &the_index;
> + unpack_opts.fn = oneway_merge;
> + if (unpack_trees(1, DIFF_OPT_TST(opt, REVERSE_DIFF) ? &t1 : &t2,
> &unpack_opts) == 0)
> + git_attr_set_direction(GIT_ATTR_INDEX, &the_index);
> + }
This is hooking at too low a level in the callchain. diff_tree_sha1() is
meant to be a general purpose "I have two tree-ish objects and I want the
comparison machinery to work on them" library function [*1*].
- One of the more important uses is the history simplification done
during revision traversal by checking if the subtrees and the blobs
have the same SHA-1, and we should not pay penalty of reading the index
for each and every tree here.
- The caller may be using the index for its own purposes, and your use of
"the_index" here will break them.
If you want to allow use of in-tree attributes in _all_ callers of
diff_tree_sha1(), then the right approach is to add an instance of "struct
index_state" to "struct diff_options", have the caller _explicitly_ ask
for use of in-tree attributes by setting a bit somewhere in "struct
diff_options", and read the tree into that separate index_state using
tree.c::read_tree(). I however doubt it is worth it.
I would think it makes more sense to add a codeblock like that at the
beginning of builtin/diff.c::builtin_diff_tree() when a new command option
asks for it. In that codepath, you _know_ that we are not using the index
at all, and reading the index there will not interfere with other uses of
the index in the program.
[Footnote]
*1* which means that it is not a good justification to say "no current
caller is broken by this change". We need to make the library usable
for future callers.
--
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