Junio C Hamano <[email protected]> writes:
> Karthik Nayak <[email protected]> writes:
>
>> Remove the error reporting variable to make the code easier to port
>> over to using ref-filter APIs.
>>
>> This also removes the error from being displayed. As branch.c will use
>> ref-filter APIs in the following patches, the error checking becomes
>> redundant with the error reporting system found in the ref-filter
>> (ref-filter.c:1336).
>
> Hmm, do you mean these lines by the above reference?
>
> if (filter->merge_commit || filter->with_commit) {
> commit = lookup_commit_reference_gently(oid->hash, 1);
> if (!commit)
> return 0;
Note: the test becomes
if (filter->merge_commit || filter->with_commit || filter->verbose) {
When the code starts using ref-filter, so the condition of the if
becomes the same as it is here. Not related to your concern, but I was
worried about "verbose" being used on one side but not the other, and
it's actually OK.
> That is "silently return ignoring it if it is not a commit", i.e. I
> do not think that deserves to be called error REPORTING system.
>
> Do you really understand what the error message you are removing is
> trying to diagnose? A branch ref must not point at a blob or any
> non-commit object, and if we find such a branch ref, we report it as
> error.
More precisely: if we find such a branch ref and we're used with an
option that requires us to lookup the commit, then we report it as an
error.
To be sure, I tried:
echo ee0f5eeeae36cd1b5a346a1e2ae5c8cb841cd5da > .git/refs/heads/broken
where the sha1 is the one of a blob.
$ git branch
broken
* master
$ git branch -v
error: branch 'broken' does not point at a commit
* master 5cc76d7 foo
error: some refs could not be read
After the series, I get:
$ git branch
broken
* master
$ git branch -v
* master 5cc76d7 foo
So I agree with Junio that the commit message is not sufficient: there
is a behavioral change. I'm OK with it, but the commit message shouldn't
claim that there isn't.
Porting to ref-filter drops the commit before we get an opportunity to
complain, so we stop complaining because it's not worth the trouble.
BTW, this looks like an fsck bug:
$ git fsck --strict
Checking object directories: 100% (256/256), done.
error: refs/heads/broken: not a commit
$ echo $?
0
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
