Re: [PATCH v1 1/1] rev-list: add --missing=print-only mode

[Derrick Stolee <stolee@gmail.com>]

On 4/19/26 4:48 AM, Siddharth Asthana wrote:
> When working with partial clones, it's common to want just the list of
> missing objects. The current --missing=print mode does this but mixes
> present and missing objects together, with missing ones prefixed by '?'.
> Getting only the missing OIDs requires an extra pipe:
> 
>    git rev-list --objects --all --missing=print | perl -ne 'print if s/^[?]//'
> 
> Add --missing=print-only which outputs only the missing object OIDs, one
> per line, without any prefix. This makes the above one-liner unnecessary
> and the output directly usable by downstream tools.

I'm a fan of this mode. It saves time dealing with the I/O.

>   static struct oidmap missing_objects;
>   enum missing_action {
> -	MA_ERROR = 0,    /* fail if any missing objects are encountered */
> -	MA_ALLOW_ANY,    /* silently allow ALL missing objects */
> -	MA_PRINT,        /* print ALL missing objects in special section */
> -	MA_PRINT_INFO,   /* same as MA_PRINT but also prints missing object info */
> +	MA_ERROR = 0, /* fail if any missing objects are encountered */
> +	MA_ALLOW_ANY, /* silently allow ALL missing objects */
> +	MA_PRINT, /* print ALL missing objects in special section */
> +	MA_PRINT_INFO, /* same as MA_PRINT but also prints missing object info */
> +	MA_PRINT_ONLY, /* print ONLY missing objects, without the "?" prefix */
>   	MA_ALLOW_PROMISOR, /* silently allow all missing PROMISOR objects */

I'm not a fan of this adjustment of spacing on the comments,
though I do see that MA_ALLOW_PROMISOR had already broken the
pattern. MA_PRINT_ONLY would still work with the earlier
comment alignment, though.

> -	if (line_term)
> +	if (arg_missing_action == MA_PRINT_ONLY) {
> +		printf("%s", oid_to_hex(&entry->entry.oid));
> +		putchar(line_term);

Is there a reason you didn't use a printf("%s%c") here to
put the oid and line_term together?

> +		return;
> +	} else if (line_term) {
>   		printf("?%s", oid_to_hex(&entry->entry.oid));
> -	else
> +	} else {
>   		printf("%s%cmissing=yes", oid_to_hex(&entry->entry.oid),
>   		       info_term);
> +	}



>   	if (!print_missing_info) {
>   		putchar(line_term);
> @@ -209,6 +222,7 @@ static inline void finish_object__ma(struct object *obj, const char *name)
>   
>   	case MA_PRINT:
>   	case MA_PRINT_INFO:
> +	case MA_PRINT_ONLY:
>   		add_missing_object_entry(&obj->oid, name, obj->type);
>   		return;
>   
> @@ -246,6 +260,11 @@ static void show_commit(struct commit *commit, void *data)
>   		return;
>   	}
>   
> +	if (arg_missing_action == MA_PRINT_ONLY) {
> +		finish_commit(commit);
> +		return;
> +	}
> +
>   	if (show_disk_usage)
>   		total_disk_usage += get_object_disk_usage(&commit->object);
>   
> @@ -384,6 +403,8 @@ static void show_object(struct object *obj, const char *name, void *cb_data)
>   	if (finish_object(obj, name, cb_data))
>   		return;
>   	display_progress(progress, ++progress_counter);
> +	if (arg_missing_action == MA_PRINT_ONLY)
> +		return;
>   	if (show_disk_usage)
>   		total_disk_usage += get_object_disk_usage(obj);
>   	if (info->flags & REV_LIST_QUIET)
> @@ -525,6 +546,12 @@ static inline int parse_missing_action_value(const char *value)
>   		return 1;
>   	}
>   
> +	if (!strcmp(value, "print-only")) {
> +		arg_missing_action = MA_PRINT_ONLY;
> +		fetch_if_missing = 0;
> +		return 1;
> +	}
> +
>   	if (!strcmp(value, "allow-promisor")) {
>   		arg_missing_action = MA_ALLOW_PROMISOR;
>   		fetch_if_missing = 0;
> @@ -967,8 +994,7 @@ int cmd_rev_list(int argc,
>   
>   	if (arg_print_omitted)
>   		oidset_init(&omitted_objects, DEFAULT_OIDSET_SIZE);
> -	if (arg_missing_action == MA_PRINT ||
> -	    arg_missing_action == MA_PRINT_INFO) {
> +	if (missing_action_prints()) {
>   		struct oidset_iter iter;
>   		struct object_id *oid;
>   
> @@ -994,8 +1020,7 @@ int cmd_rev_list(int argc,
>   			printf("~%s\n", oid_to_hex(oid));
>   		oidset_clear(&omitted_objects);
>   	}
> -	if (arg_missing_action == MA_PRINT ||
> -	    arg_missing_action == MA_PRINT_INFO) {
> +	if (missing_action_prints()) {
>   		struct missing_objects_map_entry *entry;
>   		struct oidmap_iter iter;
>   
> @@ -1011,7 +1036,7 @@ int cmd_rev_list(int argc,
>   
>   	stop_progress(&progress);
>   
> -	if (revs.count) {
> +	if (revs.count && arg_missing_action != MA_PRINT_ONLY) {
>   		if (revs.left_right && revs.cherry_mark)
>   			printf("%d\t%d\t%d\n", revs.count_left, revs.count_right, revs.count_same);
>   		else if (revs.left_right)
> @@ -1022,7 +1047,7 @@ int cmd_rev_list(int argc,
>   			printf("%d\n", revs.count_left + revs.count_right);
>   	}
>   
> -	if (show_disk_usage)
> +	if (show_disk_usage && arg_missing_action != MA_PRINT_ONLY)
>   		print_disk_usage(total_disk_usage);

I'm a little worried about all of these checks that need
special-casing. These seem like options that are enabled
by other options and could easily be grouped with the
print-only setting.

If there is something wrong here where these need to be
disabled, then we should update the documentation to say
how this interacts with those options. And perhaps some
warnings to say "these options are not compatible".

On that note: this patch is missing a document update.

Thanks,
-Stolee