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

[Siddharth Asthana <siddharthasthana31@gmail.com>]

On 20/04/26 13:13, Patrick Steinhardt wrote:
> On Sun, Apr 19, 2026 at 02:18:40PM +0530, 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.
> 
> Naming is a bit tough, as "print-only" sounds as if we're only printing
> them without doing anything else, but it doesn't quite convey the


The name came from Christian's original suggesttion in the issue [1], 
but agreed it's ambiguous. Phillip's --missing-only approach solve this.

[1] https://gitlab.com/gitlab-org/git/-/work_items/80#note_464005298


> relation to non-missing objects. I don't really have a better suggestion
> though -- "print-exclusively" may convey the meaning a tiny bit better,
> but still suffers kind of the same issue.
> 
>> diff --git a/builtin/rev-list.c b/builtin/rev-list.c
>> index 8f63003709..ba7e3e3919 100644
>> --- a/builtin/rev-list.c
>> +++ b/builtin/rev-list.c
>> @@ -104,14 +104,22 @@ static void missing_objects_map_entry_free(void *e)
>>   
>>   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 */
> 
> Makes me wonder whether we'll eventually also want to have
> `MA_PRINT_INFO_ONLY`.



Right - that's the strongest argument for --missing-only as a separate 
flag. Gets us that for free.


> 
>>   	MA_ALLOW_PROMISOR, /* silently allow all missing PROMISOR objects */
>>   };
>>   static enum missing_action arg_missing_action;
>>   
>> +static inline int missing_action_prints(void)
> 
> How about naming this `should_print_missing_object()` instead? That
> gives the reader a bit more context.


The function is a predicate on the mode, not on a specific object, so 
that name would be slightly misleading. But with --missing-only this 
helper might change shape anyway. WDYT?



> 
>> @@ -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)
> 
> Not a fault of your patch, but I really feel like git-rev-list(1) is
> becoming more and more tangled. The fact that we have to add this check
> to so many different sites doesn't inspire confidence that we have
> indeed catched all of them that need this check.
> 
> It would be great if this was reworked a bit to become more obvious, but
> that's probably outside the scope of this patch series.
> 
>> diff --git a/t/t6022-rev-list-missing.sh b/t/t6022-rev-list-missing.sh
>> index 08e92dd002..105560ad21 100755
>> --- a/t/t6022-rev-list-missing.sh
>> +++ b/t/t6022-rev-list-missing.sh
>> @@ -198,6 +198,32 @@ do
>>   	'
>>   done
>>   
>> +for obj in "HEAD~1" "HEAD~1^{tree}" "HEAD:1.t"
>> +do
>> +	test_expect_success "rev-list --missing=print-only with missing $obj" '
>> +		oid="$(git rev-parse $obj)" &&
>> +		path=".git/objects/$(test_oid_to_path $oid)" &&
>> +
>> +		# Capture present OIDs before hiding anything.
>> +		git rev-list --objects --no-object-names HEAD ^$obj >present.raw &&
>> +
>> +		mv "$path" "$path.hidden" &&
>> +		test_when_finished "mv $path.hidden $path" &&
>> +
>> +		git rev-list --missing=print-only --objects --no-object-names \
>> +			HEAD >actual &&
>> +
>> +		# Only the missing OID should appear, without the "?" prefix.
>> +		grep "^$oid$" actual &&
>> +
>> +		# Present objects must NOT appear in the output.
>> +		while read present_oid
>> +		do
>> +			! grep "^$present_oid$" actual || return 1
>> +		done <present.raw
> 
> How many present object IDs do we have? I'm a bit worried that we now
> execute grep(1) hundreds of times. Can we maybe do some tricks with
> comm(1) instead?


Phillip's test_cmp approach is simpler, since we hide one object, the 
output should be exactly that OID. Will use that.


Thanks,
Asthana


> 
> Thanks!
> 
> Patrick