This works great as long as the repository doesn't actually specify any extensions. If someone has preciousObjects activated, libgit2 itself would be fine since we never delete objects, but I think we should be exposing this to the caller as well as they may want to run rampant on the odb themselves.

This is still rather future-looking, but the issue is bound to become larger as extensions come up.

Not that this should stop this from getting merged, but this is bound to have API implications.

This works great as long as the repository doesn't actually specify any extensions. If someone has preciousObjects activated, libgit2 itself would be fine since we never delete objects, but I think we should be exposing this to the caller as well as they may want to run rampant on the odb themselves.

We can add a getter on the repository to get the available extensions. Can you confirm that libgit2 does not delete objects in any way?

Can you confirm that libgit2 does not delete objects in any way?

We don't even have a method for that in the odb interface :)

Hah, makes sense. Gonna add handling for the preciousObjects flag then.

We can add a getter on the repository to get the available extensions.

Extensions aren't really "available", they're things you must support in order to operate on the repository, which is why I'm worried about how to expose it. If there's an extension that relates to how a user of libgit2 must behave but where libgit2 doesn't have anything in particular to do, there wouldn't be any reason for libgit2 to refuse opening the repository, but a caller which is not aware of extensions would continue regardless, because they don't know that they must check them.

What I'm worried about is enabling a user of libgit2 to open the repository even though they shouldn't. I would like to make the caller agree to supporting the particular extensions while opening the repository, instead of relying on them looking at it afterwards, which a programmer in a hurry/under pressure isn't going to do while they're whipping up the prototype, and then that prototype gets to production and breaks some v1-repo.

But this is probably better left for an issue to discuss the dangers/possibilities of this rather than this PR.

Something that would be relevant is an entry in the changelog (remember those rules you put? ;)

Something that would be relevant is an entry in the changelog (remember those rules you put? ;)

@carlosmn Pffff... That's a very cheap shot. 😜

@carlosmn Yeah, I share your concern about an application not respecting preciousObjects. And that can easily happen if you even have an existing application which touches the objects, and upgrade to a libgit2 that behind-the-scenes says "I'll just claim we are preciousObjects-compliant on behalf of the app".

The conservative API to expose it is that the application says to libgit2 beforehand "these are the extensions I'm willing to comply with". And then libgit2 barfs if the application hasn't agreed to it (just like old pre-preciousObjects versions of git would barf on the same repo).

But there's a flip side, too. For something like a new ref storage format, we'd expect libgit2 to handle that completely and transparently from the application's perspective. Sure, an app could try skipping libgit2 and writing directly into .git/refs, but at some point we have to assume sanity on the part of the app.

So I think it really comes down to a per-extension interpretation: is this something libgit2 can control, or is it something where we reasonably expect the app to go around our back?

For preciousObjects, that answer is a little less clear, but I think I'd be willing to say that apps deleting from .git/objects without using libgit2 are crazy. If you do eventually add an object-pruning API to libgit2, then the preciousObjects behavior can be enforced by libgit2 at the level of those API calls, and the app doesn't have to know about the extension (beyond handling the failure of its request to prune).

Sorry to ramble, but I think we do need to know whether to claim preciousObjects compatibility here or not. And by the logic above, I think it is probably OK, but we should not add new extensions without examining each one.

When we implement the tdb/lmdb/berkley/whatever refs backend which git-core is getting, we will handle it for the user. But that only really works if the user doesn't switch the backend to their own. This is presumably an unlikely scenario since such a user wouldn't have bothered with the newer refs backend.

But more likely would be a user who has their own odb backend. We can vouch for libgit2 not deleting objects, but if we've vouched for the user and they swap odb backends, they might just delete objects so in this case we would want the user to specify preciousObjects even though most of the time, there would be no need.

Maybe having the user pass a list would be good enough most of the time, but for users which replace backends, we'd want to have them agree to the extensions which libgit2 would normally take care of and it's probably not practicable to have libgit2 know which extensions belong to which backend, as new ones might crop up which we don't know about.

But more likely would be a user who has their own odb backend.

Hmm, yeah, I didn't think people would do stuff that wild. But I guess they do.

OK, brought the PR up to speed and added support for preciousObjects (which we've decided is one of the extensions we can understand and do nothing about).

I've also added changelog entries to please our new benevolent dictators. lulz

I'm less worried about custom ODB backends and whether they have some notion of deleting objects because we don't actually have an API that would drive them to do so. My concern is just app code in general. Maybe some user of libgit2 has built their own git gc like mechanism around us (unlikely, yes, but not impossible).

If they were to upgrade to this new version then they would (unwittingly) be violating the preciousObjects contract.

This particular extension has the implication of pretty serious damage if misunderstood, so I want to be very clear to end-users about it. Let me ask some questions:

Should we forcefully break the repository_open API so that users must agree to their understanding of this extension? (My opinion: no, this is probably not necessary as there is probably no app that has built gc around libgit2 that would unwittingly get broken.)

Should we offer an API to query configured extensions so that if somebody had built gc that they could easy query this? (My opinion: no, we can't build an API that is much more valuable than reading the config.)

One thing I do think that we should do, though, is be more forceful in our CHANGELOG message. Right now we indicate that we support preciousObjects but it would be nice to tell end-users what that means to them. Eg:

If your application deletes objects directory (repacking pack files or deleting loose objects directly) then you must query for the preciousObjects extension in a repository yourself.

Sorry I'm coming late to the party - those questions were more for myself to better understand this issue and for people who didn't weigh in (above). I think that everybody who's commented thus far has made their opinions clear and it seems like we're all on the same page.

Anyway, I think this is the right start and we can deal with future extensions on a case-by-case basis.

Should we offer an API to query configured extensions so that if somebody had built gc that they could easy query this?

This looks free and immediately useful. Why not do it?

This looks free and immediately useful. Why not do it?

It looks like the extensions are bools now? So extension.preciousObjects = true... My concern is that this will get refined in the next extension and the API we built here is bullshit. Maybe we assume boolean but the next extension offers extension.foobar = sometimes.

But if this is something that would be useful immediately then let's just put it in.

It looks like the extensions are bools now?

They could be anything, really (e.g. extensions.refdb = sqlite) but there's value on a simple getter to find out whether an extension is set at all, so the user can query more about its changes in behaviour, I believe.

Yeah, the extensions can be any config value. They may even take progressive examination to know whether you support them or not. For example, the parsing process for a ref backend will probably be something like:

1. Notice that we have extensions.refBackend. Old versions of git say "I don't know about" and barf. New versions look at the value.
2. If the value is "foo", and we do not support the "foo" backend, then barf. Otherwise, we can proceed.

We can't build the code for (2) until the extension is defined; all we can do generically is step 1.

Note that as a side effect, setting extensions.preciousObjects = false will cause old versions of git to barf (even though we, who know what the extension does, know that it is harmless). This is a good thing, though; if you do not know the extension, you must barf, and not guess at what the extension's config means.

Maybe some user of libgit2 has built their own git gc like mechanism around us (unlikely, yes, but not impossible).

I know if at least one build system that does this... so you're not far off the mark here.

It seems we might need to take a step back on preciousObjects. @vmg if you don't have this extension in repos you work with, maybe we can remove that bit of the PR and leave it as just loading version 1 without any extensions.

How so? What has changed re: preciousObjects?

I think he means the discussion above. I can certainly live with libgit2 not claiming preciousObjects by default. But I am planning to start experimenting with repositories using it, so it would be nice if we have some way for an app to tell libgit2 that it's OK.

I think the most sensible API may be to just provide a callback function to the library, and libgit2 will call it for each extensions.* config key it does not understand. Then the app can parse or make a note of it as appropriate, and return a value from the callback to say whether it's OK to continue opening the repo.

It's not so much that it's changed, but it does look like even preciousObjects, the one extension that exists, is more about the application's behaviour than libgit2 by itself.

I was also thinking about callbacks, but I don't think we should limit them to extensions libgit2 doesn't understand. Even for preciousObjects both libgit2 and the application need to be aware of it.

It's OK to show all extensions to the application, but I don't think you necessarily want to respect the application callback's response in all cases; only when you do not trust that libgit2 can handle the extension by itself. So for preciousObjects, we require that the app consent, but for other extensions, we may want to handle them transparently in libgit2. Having the app say "I don't know what that extension is" will just be a hassle. We can cross that bridge when we come to it, though.

Right, for something like extensions.refdb = lmdb, libgit2 can handle it, but for unknown ones and for those which require the app to also be aware, we would have to trust that they tell the truth when saying that it's ok.

This PR has been idle for over a year; I'm going to close this. I think that the issues that were raised were / are important and that we will look to this in the future for inspiration, but I don't think that we want to merge this as-is. Thanks again!