RFC: separate object lookup error for promisor packs

[rcoup]

There's currently no support in libgit2 for partial clones (#5564). This attempts to start support them by distinguishing object-lookup faults between ENOTFOUND (the requested oid is bad or the repo is corrupt) from a new EMISSING error (indicating the oid is referenced from a promisor pack, and therefore should be available remotely).

1. Detect a promisor packfile by looking for a .packfile sidecar and track it in an attribute of git_pack_file
2. When an object is referenced from a promisor packfile (eg. tree_entry → blob), raise EMISSING instead of ENOTFOUND if the object isn't present anywhere in the ODB.

To be clear, there's no support for fetching missing objects from a promisor remote.

The code doesn't actually do (2) properly when there are multiple ODB backends in place, but hopefully it conveys the basic idea.

Conceptually, EMISSING could also be used for things like shallow clones where parent commits aren't available locally, etc.

[rcoup]

@ethomson @tiennou @pks-t sorry to nag, possible to get some feedback here? Happy to think through a plan for future steps if you think it's important to consider atm.

[olsen232]

Hi github maintainers, I'm willing to do some work to help improve libgit2's partial clone support. What can I do to help this along?

[ethomson]

Sorry about the delay, @rcoup; I haven't had a lot of bandwidth in 2020. I think that the direction that this is going is largely correct.

The code doesn't actually do (2) properly when there are multiple ODB backends in place, but hopefully it conveys the basic idea.

Is the problem here that we're short circuiting the lookup on the first promisor pack, and a subsequent odb backend may have the object? (In other words, we erroneously report EMISSING even though we have the object?) Or is there a deeper underlying problem that I'm not seeing yet?

Conceptually, EMISSING could also be used for things like shallow clones where parent commits aren't available locally, etc.

I wonder if that's what we would want to do. I agree that they're similar (object exists on the remote and doesn't exist locally) but I wonder if they should be distinct error codes so that a caller can distinguish them more easily. 🤔

[ethomson]

In line 1181 we ensure that path_len is at least strlen(".idx"). If it were strlen(".idx"), here we would set root_len to (size_t)-1.

I think that it's unlikely that there's a call path that would call us with just .idx as the path but some sort of guard seems like it should be in order.

[rcoup]

Is the problem here that we're short circuiting the lookup on the first promisor pack, and a subsequent odb backend may have the object? (In other words, we erroneously report EMISSING even though we have the object?)

Yes. Presumably there might be some other complexities wrt multiple ODB backends.

Conceptually, EMISSING could also be used for things like shallow clones where parent commits aren't available locally, etc.

I wonder if that's what we would want to do. I agree that they're similar (object exists on the remote and doesn't exist locally) but I wonder if they should be distinct error codes so that a caller can distinguish them more easily. 🤔

Yes, maybe EPROMISED & EGRAFT (or ESHALLOW_GRAFT?) to make it clearer?

[olsen232]

FYI I picked up this PR and started to address the review comments - however I ran into a major obstacle - git_object_lookup doesn't actually have the necessary context to know if an object is non-existent, missing-without-good-reason (ie corrupt), or promised. In order to say that an object is promised, we should ensure the following:

1. The object is not in the ODB.
2. The object is referenced by an object that is in the ODB.
3. The object that is in the ODB is in a packfile, and, that packfile is a promisor packfile.
   Since git_object_lookup doesn't have any context except the OID that we're looking for, we can't make the distinction at this layer without changing the API.

This PR doesn't currently do those things and lacks the context to do so without changing the libgit2 API. I think this PR promotes ENOTFOUND to EPROMISED basically every time if there is a promisor packfile involved, which is too eager (unless we define the error code as meaning "this object might be promised", as opposed to "is promised"). I think this PR is probably best simply closed - there are two other PRs that I will try to write instead:

• Separate error code eg EMISSING for when an object lookup by path fails due to missing object (not due to an invalid path). For instance, if I run git_tree_entry_bypath(&entry, root, "ab/cd/ef") and "ab/cd/ef" is not a valid path - perhaps ab has no child cd - then I get ENOTFOUND, as I should. But if I run git_tree_entry_bypath(&entry, root, "ab/cd/ef")` and "ab/cd/ef" is valid, as far as can be tested, but an object is missing, I should get this new error instead. (Eg if ab does have a child cd, but that child is nowhere to be found.

• Separate error code eg EPROMISED for when an object is EMISSING (as in the last PR) AND the packfile where we would expect to find the object is a promisor packfile. I would salvage some of this PR to check if the promisor file exists.

Limitations - I can only add these codes to the path API eg git_tree_entry_bypath, git_tree_entry_byname, git_object_lookup_bypath. git itself also has this limitation eg if you run git cat-file $SOME_OBJECT_ID it doesn't have enough context to know if the object is non-existent or promised, so it has to search all promisor backends for it.

[rcoup]

Closed in favour of #5993 and follow-ups