Handle alternates #6
Conversation
| storage := make([]storage.Storage, 2) | ||
| storage[0] = mainLoose | ||
| storage[1] = mainPacked | ||
| f, err := os.Open(path.Join(root, "info", "alternates")) |
There was a problem hiding this comment.
Do we need to keep this file open for any reason? If not, let's add a defer f.Close() after handling the error, like so:
f, err := os.Open(...)
if err != nil {
// ...
}
defer f.Close()
| } | ||
| storage = append(storage, pack) | ||
| } | ||
| return storage, nil |
There was a problem hiding this comment.
It's possible that we will have to handle an error here, too. Here's an example:
scanner := bufio.NewScanner(...)
for scanner.Scan() {
// ...
}
if err := scanner.Err(); err != nil {
return nil, err
}
ttaylorr
left a comment
ttaylorr
left a comment
There was a problem hiding this comment.
This looks good, and thanks for working on it. I left a few comments for you to take a look at. Please feel free to merge once you feel that they are resolved.
|
|
||
| scanner := bufio.NewScanner(f) | ||
| for scanner.Scan() { | ||
| storage = append(storage, newFileStorer(scanner.Text(), "")) |
There was a problem hiding this comment.
I think that this is how Git does it, though perhaps with some tighter bounds, i.e., that Git will complain specifically if a directory is not a .git/objects somewhere inside a repository. Is there anything that we need to do to ensure that we're not opening arbitrary files all over? I don't think so, since I'm pretty sure that this is how Git handles this case, too.
There was a problem hiding this comment.
I don't think there is anything. Git repositories aren't supposed to be used in untrusted contexts, so we should be able to assume that the alternates files are in fact valid and reasonably well formed. With the code the way it is, if we get a bad location, then we'll simply find zero loose objects and zero pack files in that location, and it will be a no-op.
In a future commit, we'll learn how to read alternate repositories, so teach the backend code how to deal with multiple loose and packed storage objects.
Previously, if our repository had an alternates file pointing to another repository, we would not honor it, leading to us reporting a missing object. Instead, handle these alternate repositories as simply other storage backends from which we can read. We continue to write any new objects into our repository alone.
2 participants
Previously,
gitobjdidn't handle reading and writing from alternate repositories. Teach it to do so by creating multiple storage backends for reading and continuing to write into our current repository.Note that because we have no integration tests for this repository, there are no tests for this. I have manually tested and confirmed that this series works where it did not before.