Automatically create shared-cache directory if necessary #206
Conversation
jeffhostetler
requested review from
derrickstolee,
dscho,
garimasi514 and
wilbaker
| * | ||
| * See GIT_ALTERNATE_OBJECT_DIRECTORIES for another example | ||
| * of this kind of usage. | ||
| */ |
There was a problem hiding this comment.
I think this change is a good one, I'm having trouble thinking of a scenario where we'd have gvfs.sharedCache set and not want git to treat it as an alternate.
Force this directory into the odb-list as an in-memory alternate.
With this change does it still makes sense for us to put gvfs.sharedCache in the alternates file when cloning? Or can/should we rely on git always treating the shared cache location as an alternate?
cc: @derrickstolee for his thoughts as well
There was a problem hiding this comment.
While this change makes it not completely necessary to have the shared cache in the alternates file, it is still important to put it there, if only for interoperability with other tools or Git versions.
There was a problem hiding this comment.
Yes, the proper thing to do is to put it into .git/objects/info/alternates during our setup, so that everything
in the Git ecosystem understands that it exists. This is a backstop to try to keep us from falling back to
.git/objects (and to force the creation of the directory).
I debated updating the .git/objects/info/alternates on disk to completely fix the mis-match, but
chickened out.
There was a problem hiding this comment.
I updated the comment block to indicate that the don't want to rely on this behavior.
| * alt_odb_usable() nulls this if it cannot create the | ||
| * directory on disk, so fallback to the previous choice | ||
| * if this one failed. | ||
| */ |
There was a problem hiding this comment.
I'm having some trouble following how the code manages the lifetime of gvfs_shared_cache_pathname.
It looks like alt_odb_usable will not free gvfs_shared_cache_pathname before setting it to NULL. Should it be freed first? (I'm wondering if we might be leaking the memory returned by strbuf_detach)
There was a problem hiding this comment.
yeah, i was ok with leaking it. i originally created the variable as a const (there were times when it was just set to something from argv during early iterations, rather than privately copying it). And i wasn't sure it was worth the bother to refactor that.
I'll take another look and see if i can tidy it up a bit.
There was a problem hiding this comment.
I converted the global var to a strbuf which better lets me manage the underlying string (and simplified a few things too).
We will leak the final value of the string at exit() but lots of things do that.
Remove unnecessary includes from gvfs-helper-client.c. An earlier WIP draft of this commit included additional functionality that was removed before final publication. In particular, it included code to do a custom object traversal and the additional header files were necessary. When I removed that functionality, I forgot to remove the additional headers. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
The config variable `gvfs.sharedCache` contains the pathname to an alternate <odb> that will be used by `gvfs-helper` to store dynamically-fetched missing objects. If this directory does not exist on disk, `prepare_alt_odb()` omits this directory from the in-memory list of alternates. This causes `git` commands (and `gvfs-helper` in particular) to fall-back to `.git/objects` for storage of these objects. This disables the shared-cache and leads to poorer performance. Teach `alt_obj_usable()` and `prepare_alt_odb()`, match up the directory named in `gvfs.sharedCache` with an entry in `.git/objects/info/alternates` and force-create the `<odb>` root directory (and the associated `<odb>/pack` directory) if necessary. If the value of `gvfs.sharedCache` refers to a directory that is NOT listed as an alternate, create an in-memory alternate entry in the odb-list. (This is similar to how GIT_ALTERNATE_OBJECT_DIRECTORIES works.) This work happens the first time that `prepare_alt_odb()` is called. Furthermore, teach the `--shared-cache=<odb>` command line option in `gvfs-helper` (which is runs after the first call to `prepare_alt_odb()`) to override the inherited shared-cache (and again, create the ODB directory if necessary). Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
jeffhostetler
force-pushed
the
users/jeffhost/auto-create-shared-cache
branch
from
09a9ff7
to
205244f
Compare
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
…or clarity Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Add trace2 message for CURL and HTTP errors. Fix typo reporting network error code back to gvfs-helper-client. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Fix parsing of the "loose <odb>" response from `gvfs-helper` and use the actually parsed OID when updating the loose oid cache. Previously, an uninitialized "struct oid" was used to update the cache. This did not cause any corruption, but could cause extra fetches for objects visited multiple times. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
|
This patch series now also includes a couple of bug fixes. I also included trace2 messages for network errors. |
See microsoft/git#206 and microsoft/git#207 for the Git updates. Un-ignore a functional test now that Git is doing the right thing. Resolves #147
Teach git to create the shared-cache directory named in
gvfs.sharedCacheif it does not exist on disk. Additionally, create it as an in-memory alternate
if it not listed in
.git/objects/info/alternates.Most of this code runs in
git_default_config()andprepare_alt_odb(), so allGit commands should properly handle shared-caches and
gvfs.sharedCache-- whether the directory is listed as an alternate or not.
As an additional safety net, the code in
gvfs-helper-client.cpasses a shared-cacheargument to
gvfs-helper. This letsgvfs-helperuse exactly the shared-cachedirectory expected by the client.
This latter step may not be necessary, since
git.execlients spawn the helper asgit.exe gvfs-helper(rather thangit-gvfs-helper.exe) and this form allowsgit.exe -c gvfs.sharedcache=/path/to/shared/cache gvfs-helper ....