Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support fetching entire history of specific refs #3849

Merged
merged 5 commits into from
Oct 15, 2019
Merged

Support fetching entire history of specific refs #3849

merged 5 commits into from
Oct 15, 2019

Conversation

pluehne
Copy link
Contributor

@pluehne pluehne commented Oct 8, 2019

Motivation

I’m currently working on an automation that synchronizes Git repositories between remotes, some of which use Git LFS. Compared to git lfs fetch --all, I’d like to fetch all objects referenced in the entire commit history but, and here comes the difference, not for all refs but only a specific set of refs.

For this reason, I’d like extend git lfs fetch to support downloading all objects referenced by any commit reachable from a user-defined set of refs.

Design

I suggest allowing git lfs fetch to accept a combination of the --all option and a list of refs specified as positional command-line arguments. This would then instruct Git LFS to download all objects referenced by any commit reachable from any of the specified refs (but no other objects).

For example, the call

$ git lfs fetch --all origin master develop v1.0.0

would download all objects referenced by any commit in the master and develop branches as well as the v1.0.0 tag—not just the tips of those branches and tags. However, objects stemming from a sandbox branch or a v0.1.0 tag would not be downloaded if they aren’t contained in master, develop, or v1.0.0.

In contrast,

$ git lfs fetch origin master develop v1.0.0

only downloads objects referenced by the tips of master, develop, and v1.0.0 but none of the objects from earlier commits.

Implementation

I based the implementation off the preexisting functions used when providing the --all option and when providing just a list of refs without --all. I’ve added tests covering the new usage of the --all option to show that indeed only objects referenced by commits reachable from the requested refs (but none aside from those) will be fetched.

@pluehne
Copy link
Contributor Author

pluehne commented Oct 8, 2019

As I now realized, I believe that my suggested change to git lfs fetch is in line with the current behavior of git lfs push. According to the documentation of git lfs push:

--all: This pushes all objects to the remote that are referenced by any commit reachable from the refs provided as arguments. If no refs are provided, then all refs are pushed.
https://github.com/git-lfs/git-lfs/blob/f92bd3b/docs/man/git-lfs-push.1.ronn

@bk2204
Copy link
Member

bk2204 commented Oct 9, 2019

Hey,

This seems to be a nice improvement, and I agree that it mirrors git lfs push. I'd definitely like to see some integration tests for this and documentation, as you've pointed out, but I think the basic idea is sound and the code looks reasonably sane.

@pluehne
Copy link
Contributor Author

pluehne commented Oct 9, 2019

@bk2204: Thanks a lot, this is exactly the kind of feedback I was looking for before proceeding with integration tests and documentation. I’ll make this pull request ready for review, or I’ll let you know if I have questions.

This adds support for combining the --all option with refs specified as
positional command-line arguments. This will download all objects
referenced by any commit reachable from any of the specified refs.

The purpose of this is to be able to perform full backups, migrations,
or mirroring operations of only a specific set of refs.
This adds test covering the usage of git lfs fetch with the --all option
in combination with a list of refs specified as positional arguments,
both for regular and bare repositories.
@pluehne pluehne marked this pull request as ready for review October 13, 2019 00:27
@pluehne
Copy link
Contributor Author

pluehne commented Oct 13, 2019

@bk2204: I’ve updated the documentation to match the new behavior of git lfs fetch, and I have added a few tests covering the combination of --all with a list of refs.

I wouldn’t mind if someone had a look at my changes now 🙂. Thanks!

Copy link
Member

@bk2204 bk2204 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great; I think there's just one or two documentation issues to fix.

docs/man/git-lfs-fetch.1.ronn Outdated Show resolved Hide resolved
docs/man/git-lfs-fetch.1.ronn Outdated Show resolved Hide resolved
@pluehne
Copy link
Contributor Author

pluehne commented Oct 14, 2019

@bk2204: Thanks a lot for your feedback, I’ve fixed the two documentation issues that you spotted.

Copy link
Member

@bk2204 bk2204 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. I'll wait for this to pass on Windows, and then merge.

@bk2204 bk2204 merged commit 05f9104 into git-lfs:master Oct 15, 2019
@pluehne pluehne deleted the patrick/git-lfs-fetch-all-with-refs branch October 16, 2019 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants