Add ostree_repo_hash() and use it to canonicalise sets of repos

[pwithnall]

As suggested in #1179 (comment), rather than using realpath() on the paths of a set of repositories to try and eliminate duplicates, it would be better to add an ostree_repo_hash() function which can be used with ostree_repo_equal() with a GHashTable to eliminate duplicates.

ostree_repo_hash() could return its hash value as g_int_hash (repo->device) ^ g_int_hash (repo->inode), for example. If the repository is not yet open, we could either abort, or return a fixed hash value. If we do the latter, we’ll run into problems if a repo is added to a hash table, then opened. The former is a nuclear option for avoiding this.

[pwithnall]

I’ve put a patch in PR #1205 which implements ostree_repo_hash() and adds tests for it. It doesn’t port OstreeRepoFinderMount to use the new function, since it’s significantly rewritten in PR #1179, so this issue shouldn’t be closed until 1179 is in and another PR can be opened to use ostree_repo_hash() in the rewritten version of OstreeRepoFinderMount.

In the meantime, though, PR #1205 can be pushed independently of #1179.

[pwithnall]

Hah, actually, the realpath() in OstreeRepoFinderMount is more necessary than I thought at the moment: since we’re now using ostree_repo_open_at() there, the result of calling ostree_repo_get_path() is a file in /proc/self/fd, which is really not useful as the path in a URI to return to the user. The realpath() call is currently masking that problem.

@cgwalters, could we do better with ostree_repo_open_at() than this? Perhaps it could do the realpath() call internally so that it can return a more useful value for ostree_repo_get_path(). Or perhaps we could introduce a new ostree_repo_get_uri() function which returns a canonical file:// URI for the repository if possible, or NULL otherwise (if, for example, its root DFD is not addressable).

[pwithnall]

I’ve got a WIP branch here which plays around with using ostree_repo_hash() in OstreeRepoFinderMount. It works, but I can’t eliminate the realpath() call and it doesn’t simplify the code at all. I don’t think it’s worth pushing unless we can work out a decent way of dropping the existing realpath() call.

[cgwalters]

Yeah, messy. We could keep the repos (hence the fds) around rather than gathering a set of paths perhaps?

[pwithnall]

Yeah, messy. We could keep the repos (hence the fds) around rather than gathering a set of paths perhaps?

You mean passing a set of OstreeRepos further along in the code? That’s not possible, since we eventually need to return some OstreeRepoFinderResult objects, each of which contains an OstreeRemote. The remote could be local or remote, so currently has to be represented as a URI.

We could extend OstreeRemote to store an OstreeRepo (for local pulls) or a URI (for remote ones), but that would be a bigger change with impact outside the P2P code.