-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Remove command name prefixes from progress messages #5995
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
Merged
Merged
Conversation
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
In PR git-lfs#2757 the output of the "git lfs prune" command was revised to use the methods of the Logger structure of the "tasklog" package to output progress messages, replacing use of the Spinner structure from the older "progress" package. The Spinner structure and its methods were then removed in PR git-lfs#2759, and the "progress" package was removed entirely in PR git-lfs#2762. As part of this change, the "git lfs prune" command's progress messages were reformatted so as to always begin with the Git LFS subcommand name, for example, "prune: 8 local objects, 9 retained, done." Two other Git LFS commands follow the same design, the "git lfs fetch" and "git lfs migrate" commands, but no others. Git commands also do not prefix their progress messages with the name of the subcommand (although messages from a Git remote may be reported with the prefix "remote:"). To help bring all our commands' progress messages into alignment, we simply remove the "prune:" prefix from the messages output by the "git lfs prune" command. We expect to revise the progress messages from the "git lfs fetch" and "git lfs migrate" commands in the same way in subsequent commits of this PR.
In PR git-lfs#2758 the output of the "git lfs fetch" command was revised to use the methods of the Logger structure of the "tasklog" package to output progress messages, replacing use of the Spinner structure from the older "progress" package. The Spinner structure and its methods were then removed in PR git-lfs#2759, and the "progress" package was removed entirely in PR git-lfs#2762. As part of this change, the "git lfs fetch" command's progress messages were reformatted so as to always begin with the Git LFS subcommand name, for example, "fetch: Fetching reference refs/heads/main". Two other Git LFS commands follow the same design, the "git lfs prune" and "git lfs migrate" commands, but no others. Git commands also do not prefix their progress messages with the name of the subcommand (although messages from a Git remote may be reported with the prefix "remote:"). To help bring all our commands' progress messages into alignment, we simply remove the "fetch:" prefix from the messages output by the "git lfs fetch" command. We have similarly altered the progress messages of the "git lfs prune" command in a previous commit in this PR, and we expect to revise the progress messages from the "git lfs migrate" command in the same way in a subsequent commit in this PR.
In PR git-lfs#2329 the output of the "git lfs migrate" command was revised to use the methods of the Logger structure of the "log" package to output progress messages. This package was originally located within the "git/githistory" source directory, but was later moved and renamed to the "tlog" package and then to the current "tasklog" package in PR git-lfs#2747. As part of the changes in PR git-lfs#2329, the methods of the Rewriter structure in the "githistory" package were updated to use the methods of the Logger and PercentageTask types in the "log" package to generate progress messages which began with the Git LFS subcommand name, for example, "migrate: Rewriting commits". Since that PR, multiple other informational and error messages output by the "git lfs migrate" command have been added, and while most begin with the "migrate:" prefix, not all do. For instance, running the "git lfs migrate import --verbose" command outputs commit SHAs and file paths with the prefix, but Git references and the mapping of their old and new SHAs without the prefix. Two other Git LFS commands, the "git lfs prune" and "git lfs fetch" commands, follow the same design and output their subcommand names as prefixes to their progress messages, but no other commands do this. Git commands also do not prefix their progress messages with the name of the subcommand (although messages from a Git remote may be reported with the prefix "remote:"). To help bring all our commands' progress messages into alignment, we simply remove the "migrate:" prefix from the messages output by the "git lfs migrate" command. We have similarly altered the progress messages of the "git lfs prune" and "git lfs fetch" commands in previous commits in this PR.
When the --verbose option is supplied to the "git lfs migrate export" or "git lfs migrate import" command, additional output messages are generated listing the file paths of all migrated files and the original commit SHAs which introduced those files. This option was first introduced for the "git lfs migrate import" command in PR git-lfs#2610. The rewriteBlob() method of the Rewriter structure in the "githistory" package outputs the pairs of original commits SHAs and file paths on individual lines by adding entries to a PercentageTask structure from the "tasklog" package. Each line is formatted without any leading whitespace. During a subsequent step in the commands' progress, the updateOneRef() method of the refUpdater structure in the "githistory" package outputs pairs of Git reference names and the original and rewritten commit SHAs associated with them. Unlike the list of commits and file paths, these lines are formatted with an indentation of two space characters, which sets them apart visually from the main progress messages output by the command. To help clarify the principal steps of the migration process and make the output of the "git lfs migrate" subcommands more readable, we add indentation to the lines with the pairs of commit SHAs and file paths, matching that of the lines with the Git references and old and new commit SHAs. We then update the "migrate export (--verbose)" test in our t/t-migrate-export.sh test script to validate the format of the lines with commit SHAs output by the "git lfs migrate export" command. Similarly, we update two tests in our t/t-migrate-import.sh test script to do the same for the "git lfs migrate import" command. Note that in all of these tests we use the tee(1) command to capture the output of the "git lfs migrate" command under test into a log file, and then use a grep(1) command to check for the formatted lines in the log file. We avoid piping the output of the "git lfs migrate" command directly into the grep(1) command because we prefer to use that command's -q option, which causes it to exit as soon as it reads a matching line. This in turn would cause the "git lfs migrate" command to exit immediately when it receives a SIGPIPE signal trying to write to the pipe, which would leave the migration incomplete. Further, we need to pass the --yes option to the "git lfs migrate" command in these tests because it will now otherwise pause and wait for confirmation to proceed when it detects the untracked "migrate.log" file being written into the current Git working tree. Finally, we also update the final progress message output by the "git lfs migrate export" and "git lfs migrate import" subcommands to begin with an uppercase letter. It now reads "Checkout" rather than "checkout", which better aligns with the format of all the preceding messages such as "Sorting commits" and "Rewriting commits".
The refUpdater structure and related methods were introduced to the "githistory" package in PR git-lfs#2340, as part of the series of PRs that implemented the "git lfs migrate" command. The structure is not exported outside of the package, as it is only used by the Rewrite() method of the Rewriter structure in the same package. (The Rewriter structure and methods are the principal external interface of the "githistory" package.) While the structure itself is not exported, most of its elements are exported, as is its UpdateRefs() method. However, none of these are referenced outside of the "githistory" package, so we rename them now to clarify that they are only used within the package.
In the "githistory" package, the Rewriter and refUpdater structures both contain elements that are pointers to Logger structures from the "tasklog" package. Moreover, the refUpdater structure's element of this type is initialized from the value of the corresponding element of the Rewriter structure. However, these elements have different names, so we rename the Logger element of the Rewriter structure of "logger", which brings the two structure's element names into closer alignment, and also avoids the use of the abbreviated name "l", since the Logger element is used in multiple disparate methods and a more descriptive name is helpful in such cases.
001c2a4 to
c79822f
Compare
This was referenced Feb 28, 2025
larsxschneider
approved these changes
Mar 3, 2025
Member
larsxschneider
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you 🙇
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.
Three of the Git LFS commands, namely the
git lfs fetch,git lfs prune, andgit lfs migratecommands, all output informational messages prefixed by their Git LFS subcommand names.This behaviour differs from all of our other commands, and from the messages output by Git commands, so we revise these three Git LFS commands now to remove these message prefixes, and make some other minor adjustments to the format of the
git lfs migratecommand's output to improve its readability.As an example, our existing output from these commands looks like the following. Note the inconsistency in some of the lines output by the
git lfs migratecommand in particular.With the revisions in this PR, the output from the same commands now appears as follows.
Note that while we try to guarantee stable and machine-readable output formats from some of our commands, those require the use of options such as
--porcelainor--json. Users should not depend on the exact format of our other informational and error messages.In addition to these changes, we also update several of the structures and methods in our
githistorypackage to avoid exporting unnecessary structure elements or methods outside of the package, and we align the name of thetasklog.Loggerelement in two structures in that package.This PR will be most easily reviewed on a commit-by-commit basis, as each commit includes a detailed description of its changes.