Re: [PATCH v4 02/11] commit-graph: return with errors during write

[Derrick Stolee <stolee@gmail.com>]

On 5/13/2019 7:04 AM, Derrick Stolee wrote:
> On 5/12/2019 11:13 PM, Junio C Hamano wrote:
>> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>
>>> diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
>>> @@ -188,14 +187,14 @@ static int graph_write(int argc, const char **argv)
>>>  		UNLEAK(buf);
>>>  	}
>>>  
>>> -	write_commit_graph(opts.obj_dir,
>>> -			   pack_indexes,
>>> -			   commit_hex,
>>> -			   opts.append,
>>> -			   1);
>>> +	result = write_commit_graph(opts.obj_dir,
>>> +				    pack_indexes,
>>> +				    commit_hex,
>>> +				    opts.append,
>>> +				    1);
>>>  
>>>  	UNLEAK(lines);
>>> -	return 0;
>>> +	return result;
>>>  }
>>
>> What were the error values this function used to return?  I am
>> wondering if the callers of this function are prepraed to see the
>> returned values from write_commit_graph() this function stores in
>> 'result' (which presumably are small negative value like our usual
>> internal API convention)?
> 
> The only caller is cmd_commit_graph() and it is in this snippet:
> 
>         if (argc > 0) {
>                 if (!strcmp(argv[0], "read"))
>                         return graph_read(argc, argv);
>                 if (!strcmp(argv[0], "verify"))
>                         return graph_verify(argc, argv);
>                 if (!strcmp(argv[0], "write"))
>                         return graph_write(argc, argv);
>         }
> 
> So these return values are passed directly to the result of the
> builtin. If that is against convention (passing an error code from
> the library to the result of the builtin) then I can modify.

And I see from your other feedback (upon re-reading) that you prefer
translating a negative error value from the library into a "1" here
for the builtin.

As I prepare my next version, I'll have write_commit_graph() return -1
for all errors and have graph_write() translate that to a 1. But I'll
wait to see if you want more specific error codes from write_commit_graph().

-Stolee