Skip to content

*: introduce package 'tlog'#2747

Merged
ttaylorr merged 2 commits intomasterfrom
make-log-top-level
Nov 22, 2017
Merged

*: introduce package 'tlog'#2747
ttaylorr merged 2 commits intomasterfrom
make-log-top-level

Conversation

@ttaylorr
Copy link
Contributor

This pull request promotes the implementation of a task-based logging system found originally in git/githistory/log to a new, top-level package tlog.

This accomplishes one of the goals that I proposed in #2743 (comment):

In fewer words, here are some things that I'd like to do:

  1. [...]
  2. Move the git/githistory/log package to package log (no longer nested in git/githistory).

However, instead of calling it log (confusing with Go's standard library package log) or gitlog (confusing with git-log(1)-related utilities), chosen here is tlog for task-log. I think that this is a good name since it:

  • Isn't easily confused with other functions.
  • Makes it a part of the name that this package is "task"-based, and therefore, provides some synchronicity guarantees.

Since this package contains a mostly-stable API, I feel comfortable promoting this package to live in the github.com/git-lfs/git-lfs-namespace. Future goals in #2743 support adding new functionality to the log package, which is something that I'd like to do after moving the code, and not before.

Tracking: #2743

/cc @git-lfs/core #2743

@ttaylorr ttaylorr added this to the v2.4.0 milestone Nov 22, 2017
Copy link
Contributor

@technoweenie technoweenie left a comment

Choose a reason for hiding this comment

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

What do you think of tasklog instead of tlog? I peeped the public API and agree it looks good:

type ListTask
    func NewListTask(msg string) *ListTask
    func (l *ListTask) Complete()
    func (l *ListTask) Entry(update string)
    func (l *ListTask) Throttled() bool
    func (l *ListTask) Updates() <-chan *Update
type Logger
    func NewLogger(sink io.Writer) *Logger
    func (l *Logger) Close()
    func (l *Logger) Enqueue(ts ...Task)
    func (l *Logger) List(msg string) *ListTask
    func (l *Logger) Percentage(msg string, total uint64) *PercentageTask
    func (l *Logger) Waiter(msg string) *WaitingTask
type PercentageTask
    func NewPercentageTask(msg string, total uint64) *PercentageTask
    func (c *PercentageTask) Count(n uint64) (new uint64)
    func (t *PercentageTask) Entry(update string)
    func (c *PercentageTask) Throttled() bool
    func (c *PercentageTask) Updates() <-chan *Update
type Task
type Update
    func (u *Update) Throttled(next time.Time) bool
type WaitingTask
    func NewWaitingTask(msg string) *WaitingTask
    func (w *WaitingTask) Complete()
    func (w *WaitingTask) Throttled() bool
    func (w *WaitingTask) Updates() <-chan *Update

@ttaylorr
Copy link
Contributor Author

ttaylorr commented Nov 22, 2017

What do you think of tasklog instead of tlog?

🤘 I like tasklog much better than tlog -- I renamed this in 45c580e.

I peeped the public API and agree it looks good:

🎉 Yay!

@ttaylorr ttaylorr merged commit d4f39ae into master Nov 22, 2017
@ttaylorr ttaylorr deleted the make-log-top-level branch November 22, 2017 22:25
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request May 26, 2023
In commit e90d54d of PR git-lfs#2329 the
"git/githistory/log" package was added, including the PercentageTask
structure and associated methods.  Then in commit
122b765 of PR git-lfs#2610 the Entry()
method was added to provide support for verbose logging in the
"git lfs migrate impot" command.  However, the name of the
receiver argument for the Entry() method differs from that of
the other methods for the PercentageTask structure, so we update
it to match now, as well as fixing a typo which remains from the
original commit in PR git-lfs#2329.

Note that the "git/githistory/log" package was later renamed to the
"tasklog" package in commits b9ab79e
and 45c580e of PR git-lfs#2747.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request May 26, 2023
In commit f100293 of PR git-lfs#2732 a local
maxInt() function was added to the "git/githistory/log" package in order
to avoid a circular dependancy on the "tools" package, which provides
a common MaxInt() implementation.  See, for reference, the discussion
in this PR comment:

git-lfs@f100293#r151551639

The "git/githistory/log" package was subsequently renamed to
the "tasklog" package in commits b9ab79e
and 45c580e of PR git-lfs#2747.

At present, though, there is no cyclic dependency which prevents
the use of the standard tools.MaxInt() function in the "tasklog"
package, so we revert to its use now.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request May 26, 2023
In commit e90d54d of PR git-lfs#2329 the
"git/githistory/log" package was added, including the PercentageTask
structure and associated methods, but unlike the WaitingTask which
was added at the same time, no Complete() method was defined for
the PercentageTask structure.

(Note that the "git/githistory/log" package was later renamed to the
"tasklog" package in commits b9ab79e
and 45c580e of PR git-lfs#2747.)

Other task types such as the ListTask and SimpleTask structures
(introduced in commit 31ffeb9 of
PR git-lfs#2335 and commit 7a760b6 of PR git-lfs#2756,
respectively) provide a Complete() method, and the Meter task type
of the "tq" package, which implemented the Task interface in commit
7c0f9e2 of PR git-lfs#2732, provides an
equivalent Finish() method.  These methods allow the caller to
explicitly close the channel returned by the Updates() method,
on which the anonymous goroutine started by the Logger.consume() method
for the task is waiting, in a "range" loop on the channel in the
Logger.logTask() method.

One key use of the PercentageTask structure is in the methods of
the Rewriter structure from the "git/githistory" package.  It is
initialized with the number of commits to be rewritten during
a "git lfs migrate" command's traversal of a Git repository's history.
As each commit is rewritten, the Count() method is called to
increment the percentage completed value.  When every commit has
been rewritten, the count reaches the expected total, and the
Count() method closes the channel, thus allowing the task receiving
updates to finish and exit its goroutine.

Under exceptional circumstances, though, the Rewrite() method of
the Rewriter structure may never finish iterating through all
the expected commits, and return in error or via a panic call.
When this happens, the goroutine waiting on the PercentageTask's
updates channel can never exit, leading to a hung process which
never fully exits.

In order to allow the Rewriter's Rewrite() method to define a
deferred function which will always be called when it returns,
under any circumstances, we add a Complete() method for the
PercentageTask structure which sets the number of completed elements
to the expected total in an atomic swap, and then closes the
updates channel if the number of completed elements was previously
less than the expected total.

We also add a test to validate this new method's behaviour, and
update the existing TestPercentageTaskCallsDoneWhenComplete()
function to also confirm that calling the new Complete() method
after the number of completed elements has reached the expected
total does not cause a second attempt to close the updates channel.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request May 26, 2023
In commit f100293 of PR git-lfs#2732 a local
maxInt() function was added to the "git/githistory/log" package in order
to avoid a circular dependancy on the "tools" package, which provides
a common MaxInt() implementation.  See, for reference, the discussion
in this PR comment:

git-lfs@f100293#r151551639

The "git/githistory/log" package was subsequently renamed to
the "tasklog" package in commits b9ab79e
and 45c580e of PR git-lfs#2747.

At present, though, there is no cyclic dependency which prevents
the use of the standard tools.MaxInt() function in the "tasklog"
package, so we revert to its use now.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request May 26, 2023
In commit e90d54d of PR git-lfs#2329 the
"git/githistory/log" package was added, including the PercentageTask
structure and associated methods, but unlike the WaitingTask which
was added at the same time, no Complete() method was defined for
the PercentageTask structure.

(Note that the "git/githistory/log" package was later renamed to the
"tasklog" package in commits b9ab79e
and 45c580e of PR git-lfs#2747.)

Other task types such as the ListTask and SimpleTask structures
(introduced in commit 31ffeb9 of
PR git-lfs#2335 and commit 7a760b6 of PR git-lfs#2756,
respectively) provide a Complete() method, and the Meter task type
of the "tq" package, which implemented the Task interface in commit
7c0f9e2 of PR git-lfs#2732, provides an
equivalent Finish() method.  These methods allow the caller to
explicitly close the channel returned by the Updates() method,
on which the anonymous goroutine started by the Logger.consume() method
for the task is waiting, in a "range" loop on the channel in the
Logger.logTask() method.

One key use of the PercentageTask structure is in the methods of
the Rewriter structure from the "git/githistory" package.  It is
initialized with the number of commits to be rewritten during
a "git lfs migrate" command's traversal of a Git repository's history.
As each commit is rewritten, the Count() method is called to
increment the percentage completed value.  When every commit has
been rewritten, the count reaches the expected total, and the
Count() method closes the channel, thus allowing the task receiving
updates to finish and exit its goroutine.

Under exceptional circumstances, though, the Rewrite() method of
the Rewriter structure may never finish iterating through all
the expected commits, and return in error or via a panic call.
When this happens, the goroutine waiting on the PercentageTask's
updates channel can never exit, leading to a hung process which
never fully exits.

In order to allow the Rewriter's Rewrite() method to define a
deferred function which will always be called when it returns,
under any circumstances, we add a Complete() method for the
PercentageTask structure which sets the number of completed elements
to the expected total in an atomic swap, and then closes the
updates channel if the number of completed elements was previously
less than the expected total.

We also add a test to validate this new method's behaviour, and
update the existing TestPercentageTaskCallsDoneWhenComplete()
function to also confirm that calling the new Complete() method
after the number of completed elements has reached the expected
total does not cause a second attempt to close the updates channel.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Feb 25, 2025
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.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Feb 25, 2025
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.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Feb 25, 2025
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.
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.

2 participants