On Tue, Jun 21, 2022 at 09:35:14AM +0200, Omar Polo wrote:
> The diff attempts to fix both issue.  I'm still not too keen on looking
> for an ENOENT thought.

Catching an error with code == GOT_ERR_ERRNO is perfectly valid.
GOT_ERR_ERRNO + ENOENT is indeed the correct error condition to check for.
Your open_blob() function eventually ends up in got_object_open_loose_fd()
which runs open(2) and converts failures to got_error_from_errno2().

On IRC I was thinking of the packed object case, in which case the
error would be GOT_ERR_NO_OBJ. But it turns out all the got_object_open()
function hide this error from you and fall back on trying to open a loose
object instead. So if the object does not exist at all, neither in packed
nor loose form, you'll only ever see the open(2) failure.

> diff 6131ff18e81056001a823f913094a92c10580cba /home/op/w/got
> blob - 9209e54ce7111b54b69980a44a48a92a333e5888
> file + lib/patch.c
> --- lib/patch.c
> +++ lib/patch.c
> @@ -643,7 +643,11 @@ apply_patch(int *overlapcnt, struct got_worktree *work
>  	/* don't run the diff3 merge on creations/deletions */
>  	if (*p->blob != '\0' && p->old != NULL && p->new != NULL) {
>  		err = open_blob(&apath, &afile, p->blob, repo);
> -		if (err && err->code != GOT_ERR_NOT_REF)
> +		/*
> +		 * ignore failures to open this blob, we might have
> +		 * parsed gibberish.
> +		 */
> +		if (err && err->code != GOT_ERR_ERRNO && errno == ENOENT)
>  			return err;
>  		else if (err == NULL)
>  			do_merge = 1;

Above condition looks wrong. Did you mean this?

		if (err && !(err->code == GOT_ERR_ERRNO && errno == ENOENT))

Which could of course also be expressed as below, if you prefer:

		if (err && (err->code != GOT_ERR_ERRNO || errno != ENOENT))