"Derrick Stolee via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> From: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
>
> In preparation for a new algorithm that walks fewer trees when
> creating a pack from a set of revisions, create a method that
> takes an oidset of tree oids and marks reachable objects as
> UNINTERESTING.
>
> The current implementation uses the existing
> mark_tree_uninteresting to recursively walk the trees and blobs.
> This will walk the same number of trees as the old mechanism.
>
> There is one new assumption in this approach: we are also given
> the oids of the interesting trees. This implementation does not
> use those trees at the moment, but we will use them in a later
> rewrite of this method.
>
> Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
> ---
> revision.c | 25 +++++++++++++++++++++++++
> revision.h | 2 ++
> 2 files changed, 27 insertions(+)
>
> diff --git a/revision.c b/revision.c
> index 13e0519c02..f9eb6400f1 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -99,6 +99,31 @@ void mark_tree_uninteresting(struct repository *r, struct tree *tree)
> mark_tree_contents_uninteresting(r, tree);
> }
>
> +void mark_trees_uninteresting_sparse(struct repository *r,
> + struct oidset *set)
> +{
> + struct object_id *oid;
> + struct oidset_iter iter;
> +
> + oidset_iter_init(set, &iter);
> + while ((oid = oidset_iter_next(&iter))) {
> + struct tree *tree = lookup_tree(r, oid);
> +
> + if (!tree)
> + continue;
> +
> + if (tree->object.flags & UNINTERESTING) {
> + /*
> + * Remove the flag so the next call
> + * is not a no-op. The flag is added
> + * in mark_tree_unintersting().
> + */
> + tree->object.flags ^= UNINTERESTING;
> + mark_tree_uninteresting(r, tree);
> + }
The proposed log message claims that the method takes an oidset and
marks reachable objects as uninteresting, but the implementation
only marks those that are reachable from already uninteresting
trees. Either one of them must be wrong.
Did you mean to have this instead?
if (!tree)
continue;
/*
* Force traversal of the tree, even if it has been
* already marked as UNINTERESTING.
*/
tree->object.flags &= ~UNINTERESTING;
mark_tree_uninteresting(r, tree);
By the way, one of the bigger reasons why I have to ask, instead of
making an educated guess, is because "struct oidset *set" parameter
does not give any useful information with the variable name to the
readers. We know it is a set because its type is oidset; readers
need to know what meaning the 'set' has, what it is used for, why
the caller wants to (or decides not to) place a tree object in the
set when it calls it. None of that can be read from its name.
