Allow reporting of LFS pointers separately in migrate info command#4436
Merged
chrisd8088 merged 6 commits intogit-lfs:mainfrom May 3, 2021
Merged
Allow reporting of LFS pointers separately in migrate info command#4436chrisd8088 merged 6 commits intogit-lfs:mainfrom
chrisd8088 merged 6 commits intogit-lfs:mainfrom
Conversation
e628dea to
9b12027
Compare
sami555555
approved these changes
Apr 13, 2021
399afc1 to
c09081c
Compare
The last test in this file has been accidentally skipped since it was added in commit c8c29f5 in PR git-lfs#2558, so we just ensure it runs by removing the early exit call. We also change one use of "wc -l" to "wc -c" to ensure we are indeed checking that the command outputs nothing; otherwise, if it happened to output one line, the "echo -n" would remove the trailing newline and "wc -l" would report zero.
At present the summary of file types and sizes output by the "migrate info" command is sorted only by the byte count totals. When two entries have the same counts, though, the order in which they are output is not defined, and may vary. This will pose a problem for some tests we expect to add in subsequent commits. Therefore we handle the case where two entries have identical byte counts by further comparing the file types and returning whichever sorts first lexigraphically, according to Go's internal string sort order.
At present, the "git lfs migrate info" command reports the total sizes, numbers, and file extensions for blobs which have already been converted to LFS pointers, and it uses the size of the pointer blobs themselves instead of the size of object data referenced by the pointers. The result is a confusing output which does not distinguish these types, has misleading size totals, and generates output listings even for repositories fully migrated to LFS. We therefore add a new --pointers option to the "migrate info" command so that a summary of all valid LFS pointers found in the repository traversal may be either skipped entirely or reported separately. To support this option we add an lfs.DecodePointerFromBlob() function which parallels the existing lfs.DecodePointerFromFile() function. We also refactor some of the entry lookup logic in the BlobFn callback function used in the migrateInfoCommand() function into a new findEntryByExtension() utility function.
We add a number of tests of the functionality of the new --pointers option to the "migrate info" command in order to confirm it works as expected in all modes, and both with and without Git LFS pointers in the test repositories. We also add tests of repositories where some file extensions are tracked in .gitattributes files and the corresponding blobs should be Git LFS pointers, but are not yet. In these cases, we do want to report these file extensions in summary data of the output listing. In order to ensure our tests succeed on Windows, we alter several fixture setup functions to create the .gitattributes files using "echo" instead of "git lfs track". This keeps the line endings consistent across platforms, and therefore the .gitattributes file sizes in the "migrate info" summaries.
We update the manual page for the "git lfs migrate" command to describe the use and purpose of the new --pointers option of the "info" command mode.
5b0876d to
47e10cd
Compare
bk2204
approved these changes
May 3, 2021
Member
bk2204
left a comment
There was a problem hiding this comment.
This looks good.
One note: since we're doing a 3.0 release, you can break backwards compatibility if you want and think that would be more intuitive.
Use the new "follow" mode by default when running the "git lfs migrate info" command, since we can break with backwards compatibility for a Git LFS 3.0 release.
Member
Author
|
Alright, thanks! I've flipped the sense of the default around, then. (Hopefully this work will get me closer to the |
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
May 17, 2021
We split out the existing tests of various values of the "git lfs migrate info" --pointers option, added in PR git-lfs#4436, in order to treat each value in a separate test. This has the small advantage that if support for a particular option value should be broken unexpectedly in the future, it should result in a specific test failure instead of a more generic one.
pcal43
pushed a commit
to pcal43/git-lfs-hack
that referenced
this pull request
Jul 22, 2021
We split out the existing tests of various values of the "git lfs migrate info" --pointers option, added in PR git-lfs#4436, in order to treat each value in a separate test. This has the small advantage that if support for a particular option value should be broken unexpectedly in the future, it should result in a specific test failure instead of a more generic one.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The output of the
git lfs migrate infocommand, when the repository history contains existing Git LFS pointers, treats those pointers as regular files and counts only the size of the pointer, not the referenced object. This can result in misleading output for repositories where there is a potential use case for thegit lfs migrate import --fixupcommand and option, among others.This PR adds a
--pointersoption to thegit lfs migrate infocommand, which accepts one of three values,ignore,follow, orno-follow(the default). The latter is the default only because it ensures we maintain backwards compatibility with the output of the command in previous Git LFS versions.The
--pointers=ignoreoption simply skips any existing Git LFS pointers when calculating the totals for the summary, while thefollowalternative reports (on a separate line) the total sizes and numbers of Git LFS objects in the traversed Git history. For example, on a small test repo:We also adjust the sorting logic for the summary output to make it fully deterministic; previously, if two entries for different file extensions had the same totals, they might appear in any order, but now we sort them using Go's default string comparison ordering.
In the
t/t-migrate-info.shtest suite, we remove a spurious earlyexit 0which has been in place for some time, meaning the last test was always skipped. We then add additional tests of the new--pointersoption and its various settings, with and without Git LFS objects in the test repositories.We also add, for good measure, a pair of tests which check the behaviour of the
git lfs migrate infocommand when there are files in the history which should be Git LFS objects according to the corresponding.gitattributesfiles but are not. This is another type of situation wheregit lfs migrate import --fixupmight be used, so we want to ensure theinfosubcommand produces correct output for it as well.