Re: [PATCH] rev-list: print missing object type with --missing=print-type
- From
Christian Couder <christian.couder@gmail.com>
- Date
- Jan 8, 2025, 10:08 UTC
- Message-ID
- <CAP8UFD1Q0C22TH7V7K9QZAJmLFDp+DPus1uB1O0oiAvJnWQgig@mail.gmail.com>
- In-Reply-To
- <20250108034012.211043-1-jltobler@gmail.com>
On Wed, Jan 8, 2025 at 4:43 AM Justin Tobler <jltobler@gmail.com> wrote:
Show 5 quoted lines
> > Handling of missing objects encounted by git-rev-list(1) can be > configured with the `--missing=<action>` option and specifying the > desired action. Of the available missing actions, none provide a way to > print additional information about the missing object such as its type.
What kind of additional information could we also print except for the type?
Show 13 quoted lines
> Add a new missing action called `print-type`. Similar to `print`, this > action prints a list of missing objects but also includes the object > type if available in the form: `?<oid> [type]`. > > Signed-off-by: Justin Tobler <jltobler@gmail.com> > --- > Instead of adding an additional missing action type for the explicit > purpose of printing type info, I also considered a separate option > (something like `--object-types`, or maybe a `--missing-format`) that > could be used in combination with the `--missing=print` option to > achieve the same result. The main intent is for the missing object type > and I wasn't sure if type info would be useful in the general case so I > opted to choose the former approach.
If there are many other kinds of information we could also print, then maybe a `--missing-format=<format>` option would make more sense rather than adding `print-X`, `print-Y`, `print-X-Y`, etc. Otherwise, yeah, I would say that `print-type` makes sense.
Show 21 quoted lines
> @@ -103,6 +109,8 @@ static off_t get_object_disk_usage(struct object *obj)
>
> static inline void finish_object__ma(struct object *obj)
> {
> + struct missing_objects_map_entry *entry, *old;
> +
> /*
> * Whether or not we try to dynamically fetch missing objects
> * from the server, we currently DO NOT have the object. We
> @@ -119,7 +127,12 @@ static inline void finish_object__ma(struct object *obj)
> return;
>
> case MA_PRINT:
> - oidset_insert(&missing_objects, &obj->oid);
> + case MA_PRINT_TYPE:
> + CALLOC_ARRAY(entry, 1);
> + entry->entry.oid = obj->oid;
> + entry->type = obj->type;
> + old = oidmap_put(&missing_objects, entry);
> + free(old);
> return;Maybe a function like:
static void add_missing_object_entry(struct object_id *oid, unsigned int type)
{
struct missing_objects_map_entry *entry, *old; CALLOC_ARRAY(entry, 1);
entry->entry.oid = *oid;
entry->type = type;
old = oidmap_put(&missing_objects, entry);
free(old);
}and then:
case MA_PRINT:
case MA_PRINT_TYPE:
add_missing_object_entry(&obj->oid, obj->type);
return;could help keep finish_object__ma() (which is inlined) short and also avoid some code duplication in the code below.
Show 41 quoted lines
> case MA_ALLOW_PROMISOR:
> @@ -414,6 +427,12 @@ static inline int parse_missing_action_value(const char *value)
> return 1;
> }
>
> + if (!strcmp(value, "print-type")) {
> + arg_missing_action = MA_PRINT_TYPE;
> + fetch_if_missing = 0;
> + return 1;
> + }
> +
> if (!strcmp(value, "allow-promisor")) {
> arg_missing_action = MA_ALLOW_PROMISOR;
> fetch_if_missing = 0;
> @@ -781,10 +800,26 @@ int cmd_rev_list(int argc,
>
> if (arg_print_omitted)
> oidset_init(&omitted_objects, DEFAULT_OIDSET_SIZE);
> - if (arg_missing_action == MA_PRINT) {
> - oidset_init(&missing_objects, DEFAULT_OIDSET_SIZE);
> - /* Add missing tips */
> - oidset_insert_from_set(&missing_objects, &revs.missing_commits);
> + if (arg_missing_action == MA_PRINT ||
> + arg_missing_action == MA_PRINT_TYPE) {
> + struct oidset_iter iter;
> + struct object_id *oid;
> +
> + oidmap_init(&missing_objects, DEFAULT_OIDSET_SIZE);
> + oidset_iter_init(&revs.missing_commits, &iter);
> +
> + /*
> + * Revisions pointing to missing objects lack the context
> + * required to determine object type.
> + */
> + while ((oid = oidset_iter_next(&iter))) {
> + struct missing_objects_map_entry *entry;
> +
> + CALLOC_ARRAY(entry, 1);
> + oidcpy(&entry->entry.oid, oid);
> + oidmap_put(&missing_objects, entry);
> + }Using the function suggested above this could be:
/*
* Revisions pointing to missing objects lack the context
* required to determine object type.
*/
while ((oid = oidset_iter_next(&iter)))
add_missing_object_entry(oid, 0);Show 36 quoted lines
> +
> oidset_clear(&revs.missing_commits);
> }
>
> @@ -801,12 +836,27 @@ int cmd_rev_list(int argc,
> oidset_clear(&omitted_objects);
> }
> if (arg_missing_action == MA_PRINT) {
> - struct oidset_iter iter;
> - struct object_id *oid;
> - oidset_iter_init(&missing_objects, &iter);
> - while ((oid = oidset_iter_next(&iter)))
> - printf("?%s\n", oid_to_hex(oid));
> - oidset_clear(&missing_objects);
> + struct missing_objects_map_entry *entry;
> + struct oidmap_iter iter;
> +
> + oidmap_iter_init(&missing_objects, &iter);
> +
> + while ((entry = oidmap_iter_next(&iter)))
> + printf("?%s\n", oid_to_hex(&entry->entry.oid));
> +
> + oidmap_free(&missing_objects, true);
> + }
> + if (arg_missing_action == MA_PRINT_TYPE) {
> + struct missing_objects_map_entry *entry;
> + struct oidmap_iter iter;
> +
> + oidmap_iter_init(&missing_objects, &iter);
> +
> + while ((entry = oidmap_iter_next(&iter)))
> + printf("?%s %s\n", oid_to_hex(&entry->entry.oid),
> + entry->type ? type_name(entry->type) : "");
> +
> + oidmap_free(&missing_objects, true);
> }Maybe a function like:
static void print_missing_objects(unsigned int with_type)
{
struct missing_objects_map_entry *entry;
struct oidmap_iter iter;oidmap_iter_init(&missing_objects, &iter);
while ((entry = oidmap_iter_next(&iter)))
if (with_type && entry->type)
printf("?%s %s\n", oid_to_hex(&entry->entry.oid),
type_name(entry->type));
else
printf("?%s\n", oid_to_hex(&entry->entry.oid));oidmap_free(&missing_objects, true); }
could avoid some code duplication. It could also make the output a bit cleaner as when there is no type, there would be no space after the oid in the output.
Thanks.