Skip to content

Conversation

@ttaylorr
Copy link
Contributor

@ttaylorr ttaylorr commented Jun 5, 2017

This pull request teaches the *git/githistory.Rewriter type how to cull out subtrees and blobs that won't be modified by the BlobRewriteFn by first checking each tree entry's absolute path against a *filepathfilter.Filter instance.

This is required work to boost the performance of the proposed git-lfs-migrate(1) command (see discussion: #2146). Since the migrate command will take an -I and -X flag, we can precompute a *filepathfilter.Filter instance that is passed to the rewriter, which will avoid calling the BlobCallbackFn on blobs whose paths don't match the filter.

Instead of calling the BlobRewriteFn to generate a new object (and therefore, a new tree entry) we copy over the existing tree entry that we are trying to rewrite, and use the same reference to an existing object present in the object database. This ensures that performance for subtrees and blobs that don't match the given *filepathfilter.Filter is ⚡️.

To pass the filter to the NewRewriter() function, I used Dave Cheney's "option" funcs [source].


/cc @git-lfs/core

@ttaylorr ttaylorr added the review label Jun 5, 2017
@ttaylorr ttaylorr added this to the v2.2.0 milestone Jun 5, 2017
@ttaylorr ttaylorr requested a review from technoweenie June 5, 2017 18:50
@ttaylorr ttaylorr changed the base branch from git-odb-to-githistory to master June 5, 2017 19:31
@ttaylorr ttaylorr merged commit e377d7c into master Jun 5, 2017
@ttaylorr ttaylorr deleted the githistory-filter-culling branch June 5, 2017 19:31
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request May 26, 2023
In commit 3129b9a of PR git-lfs#2537 the
"githistory" package was updated so the WithLogger global option
variable now required a *log.Logger argument instead of calling
log.NewLogger().  This was done so the "git lfs migrate" commands
could pass in a previously initialized logger to which the commands
had already written log messages; that change was introduced in
PR git-lfs#2538.

In the same commit of PR git-lfs#2537, a WithLoggerTo global option function
was added to preserve the existing behaviour.  However, this option
has never been utilized, and so to reduce unnecessary complexity
we just remove it now.

Note that the use of functional options in the Git LFS project
stems from PR git-lfs#2295 and is inspired by the design proposed in several
articles from 2014; see, for reference:

https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis
https://commandcenter.blogspot.com/2014/01/self-referential-functions-and-design.html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants