Skip to content

Conversation

@chrisd8088
Copy link
Member

Three of the Git LFS commands, namely the git lfs fetch, git lfs prune, and git lfs migrate commands, 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 migrate command'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 migrate command in particular.

$ git lfs prune
prune: 2 local objects, 0 retained, done.                                       
prune: Deleting objects: 100% (2/2), done.

$ git lfs fetch
fetch: Fetching reference refs/heads/main
Downloading LFS objects: 100% (2/2), 108 B | 0 B/s 

$ git lfs migrate import --verbose --include="*.bin"
migrate: Sorting commits: ..., done.                                            
migrate: commit 33dc05eafb8435c90404dc2ebdbdd7897ce3d1d9: foo.bin               
migrate: commit 01228a55ec03011a64168400dc143689c3d41237: bar.bin
migrate: Rewriting commits: 100% (2/2), done.
  main	01228a55ec03011a64168400dc143689c3d41237 -> 241ac413649fa10a6c9460233a2aad7261bc54f5
migrate: Updating refs: ..., done.                                              
migrate: checkout: ..., done. 

With the revisions in this PR, the output from the same commands now appears as follows.

$ git lfs prune
2 local objects, 0 retained, done.                                       
Deleting objects: 100% (2/2), done.

$ git lfs fetch
Fetching reference refs/heads/main
Downloading LFS objects: 100% (2/2), 108 B | 0 B/s 

$ git lfs migrate import --verbose --include="*.bin"
Fetching remote refs: ..., done.                                                
Sorting commits: ..., done.                                                     
  commit 33dc05eafb8435c90404dc2ebdbdd7897ce3d1d9: foo.bin                      
  commit 01228a55ec03011a64168400dc143689c3d41237: bar.bin
Rewriting commits: 100% (2/2), done.
  main	01228a55ec03011a64168400dc143689c3d41237 -> 241ac413649fa10a6c9460233a2aad7261bc54f5
Updating refs: ..., done.                                                       
Checkout: ..., done.

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 --porcelain or --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 githistory package to avoid exporting unnecessary structure elements or methods outside of the package, and we align the name of the tasklog.Logger element 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.

@chrisd8088 chrisd8088 added the migration Related to `git lfs migrate` label Feb 25, 2025
@chrisd8088 chrisd8088 requested a review from a team as a code owner February 25, 2025 10:09
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.
Copy link
Member

@larsxschneider larsxschneider left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you 🙇

@chrisd8088 chrisd8088 merged commit 79dfe3b into git-lfs:main Mar 5, 2025
19 of 20 checks passed
@chrisd8088 chrisd8088 deleted the drop-command-prefixes branch March 5, 2025 00:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

migration Related to `git lfs migrate`

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants