In PR git-lfs#1955 we resolved a problem in the Read() methods of the
interfaces we use to invoke callback functions that write entries
to the progress log file defined by the GIT_LFS_PROGRESS environment
variable.  (These methods are currently located in our "tools"
package, but at the time they were defined in a package named
"progress".)

The BodyWithCallback and CallbackReader structure types in our
"tools" package both implement the Reader interface from the "io"
package of the Go standard library by passing through calls from
their Read() methods to the Read() method of another implementation
of the Reader interface.  Depending on the return values from that
call, our structures' Read() methods may then invoke a callback
function to write an entry in the progress log file.

In commit a2a7b09 of PR git-lfs#1955 we
fixed a bug in the Read() methods of our BodyWithCallback and
CallbackReader structures whereby the methods would fail to invoke
their callback functions if the underlying Read() method read a
non-zero number of bytes but also encountered an EOF condition.

The change from commit a2a7b09
now ensures the callback function is invoked under these circumstances,
but it also replaces the EOF error with the error returned by the
callback function, which will usually be nil.  As a result, the
callers of our structures' Read() methods then have to perform a
second call to determine that the EOF condition has been reached.
In this additional call, our structures' Read() methods will not
invoke their callback functions because the underlying Read() method
reads zero bytes; our methods will then return the EOF error from
the underlying Read() method call.

We will address this issue in a subsequent commit in this PR, and
make several other revisions to the Read() methods of the
BodyWithCallback and CallbackReader structures.  To ensure these
changes are effective, we will also expand the set of tests we
perform, both in our "tools" package and in our "lfs" package.

As such, we expect to make use of the EOFReader test helper structure
type added in commit 591307d of
PR git-lfs#1955, but we first need to move the definition of the structure
into a common location where it can be imported into both the
"tools" and "lfs" packages.

Our EOFReader structure was introduced to help verify that the changes
implemented in commit a2a7b09 were
effective.  As such, the structure's Read() method always returns an
EOF error as soon as possible, even if a non-zero number of bytes
were also read.

In the tests we expect to add in subsequent commits in this PR,
we will also need a test helper structure whose Read() method
consistently returns an EOF error as late as possible, meaning that
it only returns the EOF error when no bytes have also been read.

To distinguish these two test helpers, and to help clarify the
purpose of the existing EOFReader structure, we now rename it to
EagerEOFByteReader.  We also introduce a NewEagerEOFByteReader()
function which we can use to initialize a new structure, so all
of our test functions will not have to explicitly populate the
structure's fields.

As well, because we will need to use this structure (plus a
complementary DeferredEOFByteReader structure) in the test functions
from multiple packages, we create a new, common "internal/testutil"
package and move the definition of the EagerEOFByteReader and its
sole test function into this package.

Note that in future, we may begin to move some of our other, existing
packages into the "internal" directory, as we do not intend for their
APIs or ABIs to be used in third-party applications.  In PR git-lfs#4582 we
added a note to our "go.mod" file to caution developers not to rely
on our packages having stable APIs or ABIs, but a more comprehensive
approach would be to move the majority of our packages into the
"internal" directory.  However, we defer this work for the present
time as it far exceeds the scope of this PR.

In a previous commit in this PR we moved the definition of the
EOFReader structure type and its methods from our "tools" package to
a new package named "internal/testutil", and we also renamed the
structure to EagerEOFByteReader and introduced a factory function
named NewEagerEOFByteReader() to initialize an EagerEOFByteReader
structure.

In subsequent commits in this PR we expect to add new tests of
the interfaces we use to invoke callback functions that write entries
to the progress log file defined by the GIT_LFS_PROGRESS environment
variable.  In particular, we intend to expand the set of tests which
exercise the Read() methods of the BodyWithCallback and CallbackReader
structure types in our "tools" package.

Both of these structure types implement the Reader interface from the
"io" package of the Go standard library by passing through calls from
their Read() methods to the Read() method of another implementation
of the Reader interface.

The Read() method of any structure which implements the Reader
interface is permitted to return either a nil error or an EOF error
from the call which reads the final byte of data.  All subsequent
calls to the Read() method must, of course, return an EOF error.

We can use the EagerEOFByteReader structure to verify the behaviour
of the Read() methods of the BodyWithCallback and CallbackReader
structures when the Read() method they invoke returns an EOF error
at the same time as it reads the final byte of data.

However, some of our new tests will also require a way to verify the
behaviour of the Read() methods of the BodyWithCallback and
CallbackReader structures when the Read() method they invoke returns
a nil error at the same time as it reads the final byte of data.

To allow us to write these tests, we first introduce a new
DeferredEOFByteReader structure to our "internal/testutil" package,
with an accompanying NewDeferredEOFByteReader() factory function.
The Read() method of this new structure is guaranteed to always
return a nil error when it reads the last byte of data, and to
then return an EOF error from all subsequent calls.

In PR git-lfs#1955 we resolved a problem in the Read() methods of the
interfaces we use to invoke callback functions that write entries
to the progress log file defined by the GIT_LFS_PROGRESS environment
variable.  (These methods are currently located in our "tools"
package, but at the time they were defined in a package named
"progress".)

The BodyWithCallback and CallbackReader structure types in our
"tools" package both implement the Reader interface from the "io"
package of the Go standard library by passing through calls from
their Read() methods to the Read() method of another implementation
of the Reader interface.  Depending on the return values from that
call, our structures' Read() methods may then invoke a callback
function to write an entry in the progress log file.

In commit a2a7b09 of PR git-lfs#1955 we
fixed a bug in the Read() methods of our BodyWithCallback and
CallbackReader structures whereby the methods would fail to invoke
their callback functions if the underlying Read() method read a
non-zero number of bytes but also encountered an EOF condition.

In the same PR git-lfs#1955 we then added a new Go test function named
TestCopyCallbackReaderCallsCallbackUnderfilledBuffer() which verifies
that the Read() method of the CallbackReader structure invokes the
callback function after the underlying Read() method returns the
EOF error after reading a non-zero number of bytes.

However, we did not include a corresponding test of the Read() method
of the BodyWithCallback structure, so we now revise the existing test
to verify the behaviour of both the CallbackReader structure and
the BodyWithCallback structure.

(Note that in a future PR, we may eliminate one of these two similar
structures, at which point this test will only need to check the
behaviour of that common structure's methods.)

Since the BodyWithCallback structure implements the ReadSeekCloser
interface from the "io" package, we need to add simple Seek()
and Close() methods for the EagerEOFByteReader structure in our
"internal/testutil" package.  Fortunately, the Seek() method only
needs to exist for the purposes of our test, which allows us to just
define a stub version of the method.  Otherwise, we would likely need
to mirror the complete implementation from the Seek() method of the
Reader structure in the Go standard library's "bytes" package:

  https://github.com/golang/go/blob/go1.26.4/src/bytes/reader.go#L115-L134

To alter the TestCopyCallbackReaderCallsCallbackUnderfilledBuffer()
function to check the behaviour of both the CallbackReader and
BodyWithCallback structures, we first create one of each using the
same initial settings.  We then iterate over the pair of these
structures and perform identical checks on each one, including an
additional check that after EOF has been reached and a final callback
has been made, no further callback occurs on a subsequent invocation
of the Read() method when no bytes are available to be read.

We also simplify the test in several respects.  First, because our
structures' Read() methods invoke their callback functions
synchronously within a single goroutine, there is no need for us
to use the "sync/atomic" package from the Go standard library to
increment the count of the number of times the callbacks occur.

Second, we revise the test to avoid the use of hard-coded constant
values in favour of local variables whose values are dependent on
the size of the byte slice read by the structures' Read() methods.

Third, where possible we check "int" values using just the Equal()
function from the "github.com/stretchr/testify/assert" package, as
we can avoid the use of the EqualValues() function except when the
integer types of the expected and actual values differ.

And fourth, to clarify the purpose of the test and its expanded scope,
we rename it to TestBothCallbackReadersInvokeCallbackOnEagerEOF().

In commit 589dffb of PR git-lfs#1958 we
added two other functions to test the Read() method of the
BodyWithCallback structure.  One of these test functions, the
TestBodyCallbackReaderUpdatesOffsetOnSeek() function, only pertains
to the specific behaviour of the BodyWithCallback structure's
Read() method.

However, the TestBodyCallbackReaderCountsReads() test function checks
that the BodyWithCallback structure's Read() method correctly updates
its internal count of the number of bytes read, which is an operation
also performed by the CallbackReader structure's Read() method.  We
therefore revise this function in a similar manner to we did for the
TestCopyCallbackReaderCallsCallbackUnderfilledBuffer() function so
that it now checks that both structures' Read() methods correctly
update the internal counts of the number of bytes read.

We likewise rename the function to TestBothCallbackReadersCountReads()
and replace the use of hard-coded constants with local variables
wherever possible.  We also alter the test so that it performs
multiple calls to each structure's Read() method and checks the
internal count after each call, rather than just performing a single
call to the Read() method which consumes all the available data.

Finally, we take the opportunity to also amend the existing
TestBodyCallbackReaderUpdatesOffsetOnSeek() function so that it
uses the same buffer initialization steps as the other two test
functions, avoids hard-coded constants in favour of local
variables, and checks that the results from the Read() method
match our expectations after the read position has been modified
by one or more Seek() method calls.

In a subsequent commit in this PR we expect to revise how the Read()
methods of the BodyWithCallback and CallbackReader structures in our
"tools" package behave when the underlying Read() method reads a
non-zero number of bytes but also encounters an EOF condition.

At present, under these conditions our structures' Read() methods
always replace the EOF error returned by the underlying Read()
method with the "error" value returned by the callback function,
even if that value is nil.

Before we adjust our structures' Read() methods to preserve the EOF
condition from the underlying resource when a callback function does
not encounter an error, we first add a pair of new Go test functions
to verify that other types of error conditions are handled
appropriately by these methods.

Our new TestBothCallbackReadersPreferCallbackErrorOverEOF() test
function simulates a callback function which returns a non-nil
"error" value, and checks that the Read() methods of our
BodyWithCallback and CallbackReader structures always return that
value if they perform the callback.

This test uses the EagerEOFByteReader structure we added to our
"internal/testutil" package in a prior commit in this PR to ensure
that the underlying Read() method for both the BodyWithCallback
and CallbackReader structures will return an EOF error as soon as
the final bytes of a byte slice are read.  The test then confirms
that a callback is performed in this case and that the error
returned by the callback function is preferred over the EOF error.

As well, the test checks that once all the bytes of the byte slice
have been read, if another call to the Read() method of either the
BodyWithCallback or CallbackReader structures is made, no callback
will be performed and the EOF error from the underlying Read()
method will be returned.

Our new TestBothCallbackReadersSkipCallbackAfterReadError() test
function, by comparison, simulates an underlying Read() method that
encounters an error condition other than the EOF condition, and
checks that the Read() methods of our BodyWithCallback and
CallbackReader structures do not perform a callback and just
return the "error" value from the underlying Read() method.

This test requires the use of a new ErrReader structure which we
add to our "internal/testutil" package.  We model this structure's
Read() method on that of the ErrReader structure from the
"testing/iotest" package of the Go standard library, but also
define no-op Seek() and Close() methods so that the structure's
methods implement the complete ReadSeekCloser interface from the
"io" package.  We require a full implementation of this interface
so that we can use our new ErrReader structure as the underlying
resource of a BodyWithCallback structure.

In commit 589dffb of PR git-lfs#1958 we
introduced Seek() and ResetProgress() methods for the BodyWithCallback
structure, which at the time was defined in a "progress" package
and is now located in our "tools" package.

While the Seek() method implements the expected behaviour of the
Seeker interface from the Go standard library's "io" package, the
ResetProgress() method simply invokes the callback function registered
with our BodyWithCallback structure and passes the current number of
bytes read so far as a negative value in the function's final
"readSinceLast" parameter.  We then call the ResetProgress() method
when an object upload transfer needs to be retried so that we can
adjust our progress meter's count of the bytes that have been
transferred successfully.

In PR git-lfs#1958 we added the TestBodyCallbackReaderUpdatesOffsetOnSeek()
test function to verify the behaviour of the BodyWithCallback
structure's new Seek() method, but we did not also create a test
for the ResetProgress() method at the same time.

Because we expect to refactor the BodyWithCallback structure and its
methods in a subsequent commit in this PR, we now add a new
TestBodyCallbackReaderResetsProgress() test function which verifies
that the ResetProgress() method works as expected.  In particular,
it invokes the ResetProgress() method and confirms that the
registered callback function receives a "readSinceLast" parameter
whose value is the negation of the number of bytes read so far
from the underlying source buffer.

In commit d7d9fd7 of PR git-lfs#4042 we
introduced the "closingFileReader" structure to our "tools" package,
which wraps a File structure from the Go standard library's "os"
package but replaces its Close() method with a no-op stub method.

In the same commit we also added the NewFileBody() and
NewFileBodyWithCallback() factory functions, the latter of which
calls the former and then passes the "closingFileReader" structure
it instantiates to the NewBodyWithCallback() function.  We then
call the NewFileBodyWithCallback() function in both our SSH and
HTTP upload transfer adapters in instances where we expect the
underlying File structure's Close() method to be called later
than the Close() method of the "closingFileReader" structure.

In the case of our HTTP upload transfer adapter in particular,
when the upload request for an individual object is redirected to
another URL, the Go standard library's "net/http" package will
invoke the Close() method on the Body field of the current Request
structure after the initial request is complete.  However, we do
not want this call to cause the underlying file resource to be
closed as well, so we need to provide a stub Close() method which
takes no action when it is called by the HTTP transport mechanism.

To verify that the changes in PR git-lfs#4042 were effective, in commit
d7d9fd7 we also implemented a new
"redirect upload" test in our "t/t-upload-redirect.sh" shell test
script, which checks that when the Git LFS client receives an HTTP
redirection in response to an object upload request, the client is
able to make a complete and successful object transfer to the new
URL provided in the initial response.

However, we did not also define a Go test function which exercises
the "closingFileReader" structure and related factory functions.
Because we expect to rename this structure and the NewFileBody()
function in a subsequent commit in this PR, we now add a new
TestFileBodyCallbackReaderIgnoresClose() test function which verifies
that the structure's Close() method works as expected.  In particular,
it invokes the Close() method and confirms that the underlying file
resource is not closed, and that additional read operations on the
file resource will still succeed.

In PR git-lfs#1454 we added the RetriableReader structure to our "tools"
package, and then in commit a9bbe0e
of PR git-lfs#1482 we introduced several Go test functions to exercise the
structure's methods under various conditions.  We defined these
functions in our "tools/iotools_test.go" source file and set the
Go package name in the file to "tools_test" rather than just "tools".

One consequence of this difference is that we then need to import
our own "tools" package into the "tools/iotools_test.go" source file
even though it resides in the same directory as the rest of our
"tools" package's source files.

Previously, in commit 1ef2d43
of PR git-lfs#1265, we moved the definition of the CopyWithCallback()
function from our "lfs" package to our "tools" package, and also
moved the corresponding TestCopyWithCallback() test function at
the same time.  These functions were previously defined in the
"util.go" and "util_test.go" source files, respectively, in our
"lfs" package.

However, while we now define both functions in our "tools" package,
the CopyWithCallback() function is located in a source file named
"iotools.go" but the TestCopyWithCallback() function is still located
in a source file named "util_test.go".

In a subsequent commit in this PR we intend to move the
TestCopyWithCallback() function into the "tools/iotools_test.go"
source file, to align with the location of the corresponding
CopyWithCallback() function in the "tools/iotools.go" file.

As well, in other future commits in this PR we expect to migrate
the "closingByteReader" structure and its related methods from the
"tools/copycallback.go" source file to the "tools/iotools.go" file,
and to add a new test function to the "tools/iotools_test.go" source
file which exercises the structure's Close() method.

At present, any test functions in the "tools/iotools_test.go"
source file must explicitly reference the structures and methods
they test using the "tools." prefix, since the source file
specifies a distinct "tools_test" Go package name.

To simplify the changes we expect to make in subsequent commits in
this PR, we therefore first change the package name in the
"tools/iotools_test.go" source file to just "tools", and update
the existing test functions for the RetriableReader structure to
remove the "tools." prefix from their references to the
NewRetriableReader() function.

In commit 1ef2d43 of PR git-lfs#1265 we
moved the definition of the CopyWithCallback() function from our
"lfs" package to our "tools" package, and at the same time we also
moved the corresponding TestCopyWithCallback() test function.  These
functions were previously defined in the "util.go" and "util_test.go"
source files, respectively, in our "lfs" package.

However, while we now define both functions in our "tools" package,
the CopyWithCallback() function is located in a source file named
"iotools.go" but the TestCopyWithCallback() function is still located
in a source file named "util_test.go".

In subsequent commits in this PR we expect to slightly modify the
CopyWithCallback() function.  We also intend to refactor the
CallbackReader structure and its methods, on whose implementation
the CopyWithCallback() function depends.

Before we make these changes, though, we first update the
TestCopyWithCallback() function and move it to the "iotools_test.go"
source file to correspond more closely with the location of the
CopyWithCallback() function.  We make several minor adjustments to
the TestCopyWithCallback() function so that it now follows the same
design as the tests of the CallbackReader structure and its methods,
which we revised in previous commits in this PR.

In particular, we replace a number of hard-coded constants with local
variables, change variable names to match those from the related tests
in the "tools/copycallback_test.go" source file, and make use of the
EqualValues() function from the "github.com/stretchr/testify/assert"
package to compare integer values of different bit lengths.

In commit e04316f of PR git-lfs#1839 we
added the initial version of what is now the BodyWithCallback
structure in our "tools" package, and to verify the behaviour of
its methods we included a new TestBodyWithCallback() Go test function
in our "lfs" package which copied the design of the existing
TestReadWithCallback() test function.

Both functions perform the same set of checks, with the only
substantive difference being that one exercises the methods of a
BodyWithCallback structure while the other exercises the methods
of a CallbackReader structure, which are also defined in our
"tools" package.

We expect to refactor both of these structures and their methods in
subsequent commits in this PR, so in previous commits in this PR
we have already updated the relevant tests in the "tools" package.
We now begin to do the same for the tests in the "lfs" package,
beginning with the TestBodyWithCallback() and TestReadWithCallback()
functions.

Following the pattern we have used for several test functions in our
"tools/copycallback_test.go" source file, we combine the two functions
into a single function named TestBothCallbackReadersWithCallback(),
which now exercises the methods of both the BodyWithCallback and
the CallbackReader structures.

(Note that in a future PR, we may eliminate one of these two similar
structures, at which point this test will only need to check the
behaviour of that common structure's methods.)

We also update our single new TestBothCallbackReadersWithCallback()
function to follow the same design as the related tests in the
"tools/copycallback_test.go" and "tools/iotools_test.go" source files.

In particular, we replace a number of hard-coded constants with
local variables, change variable names to match those of the other
tests, and make use of the EqualValues() function from the
"github.com/stretchr/testify/assert" package to compare integer
values of different bit lengths.

In PR git-lfs#5504 we modified the CopyCallbackFile() method of the
GitFilter structure in our "lfs" package so that the callback
function it creates only outputs a progress log message when
at least 200 milliseconds have elapsed since the last message
was written, unless the number of bytes read has now met or
exceeded the total size of the given Git LFS object.  (The
minimum time allowed between messages is specified by the
DefaultLoggingThrottle constant from our "tasklog" package.)

In commit 00f1e95 of PR git-lfs#5504 we then
also introduced the TestCopyCallbackFileThrottle() test function to
verify that the callback function created by the CopyCallbackFile()
method obeyed the new limits on how often it should write to the
progress log.

At present, the callback function checks whether the number of
bytes read has met or exceeded the total size of the given object,
and if it has, unconditionally writes a message to the progress
log, on the assumption that no further callbacks will occur because
the final bytes of the object have now been read.

However, this assumes the total size of the object is known and
correct when the callback is utilized by the CopyWithCallback()
function of our "tools" package.

While this remains true at present for most of the situations in
which we pass a callback function to the CopyWithCallback() function,
these will be more limited in number after we make the change to our
"clean" filter commands proposed in PR git-lfs#6011.

PR git-lfs#6011 resolves a bug in our "git lfs filter-progress" and
"git lfs clean" commands whereby they can pass an incorrect
"totalSize" parameter to the CopyWithCallback() function.  The only
solution for this problem is for the commands to simply not attempt
to determine the total size of an object.

Absent other changes, the result of this bug fix would be that
even if a progress log file is specified by the GIT_LFS_PROGRESS
environment variable, no messages would be written to the log file
during "clean" filter operations.  This would occur because the
copyToTemp() method of the GitFilter structure in our "lfs" package
would find a negative value in its "fileSize" parameter and then
set its "cb" callback function parameter to nil.

We intend to address this issue in PR git-lfs#6011, but before we do so,
in subsequent commits in this PR we will revise the callback
function created by the CopyCallbackFile() method to handle the
case where it is passed a negative value in its "total" parameter,
which indicates that the total size of the object is unknown.

Before we make those revisions to the CopyCallbackFile() method,
though, we first expand our test suite to exercise the callback
function created by the method under a wider range of conditions.

Specifically, we now refactor the TestCopyCallbackFileThrottle()
function so we will be able to easily expand it to simulate a
number of different test cases.  We define these test cases using
a new copyCallbackFileThrottleTestCase structure and an associated
copyCallbackFileThrottleTestMode enumerated value.

At present, the only test case we specify is one where the initial
total size of an object is both known and correct, but in future
commits in this PR we will add other test cases, including one
where the total size of an object is unknown and is therefore
represented by a negative value.

We alter the TestCopyCallbackFileThrottle() function itself to
iterate over one or more test cases and check each one using both
the EagerEOFByteReader and the DeferredEOFByteReader structures from
our "tools" package.  We use these structures' Read() methods to
simulate conditions where the method returns either an EOF error or
a nil error, respectively, at the same time as the final byte of an
input source is read.

For each test case and underlying implementation of the Reader
interface from the Go standard library's "io" package, the
TestCopyCallbackFileThrottle() function now calls the setup()
method of the copyCallbackFileThrottleTestCase structure, which
initializes the progress log file, mock time clock, callback
function, and input data source for a CallbackReader structure
from our "tools" package.

The test function then runs the Assert() method of the
copyCallbackFileThrottleTestCase structure to perform a series of
calls to the Read() method of the CallbackReader structure after
simulated periods of time have elapsed, and checks that the final
contents of the progress log file match the expected results for
the given test case.

In later commits in this PR we will expand the set of test cases run
by the TestCopyCallbackFileThrottle() function as we revise the
callback function created by the CopyCallbackFile() method and the
Read() methods of the BodyWithCallback and CallbackReader structures
in our "tools" package.

Our "tools" package defines two structures which perform similar
purposes, the BodyWithCallback and CallbackReader structures, whose
Read() methods are identical and invoke a callback function after
first calling the Read() method of an underlying resource.

The only significant difference between the two structures is that
the BodyWithCallback structure's methods implement the ReadSeekCloser
interface from the Go standard library's "io" package, while the
CallbackReader structure's methods only implement the Reader
interface from the "io" package.

In a prior commit in this PR we updated a number of the Go test
functions in both our "tools" and "lfs" packages to invoke these
structures' Read() methods and verify the results under various
conditions.

While our test functions can initialize a new BodyWithCallback
structure that reads from an underlying byte slice using a factory
function, they have to directly initialize a new CallbackReader
structure by setting its fields' values explicitly.  So too does
the CopyWithCallback() function in our "tools" package.

In a subsequent commit in this PR we intend to make substantial
revisions to the Read() methods of both the BodyWithCallback and
the CallbackReader structures, and to avoid duplication of this
code will introduce a new internal structure shared by both of
these structures.

As this change would otherwise require us to modify a number of
our test functions to reflect the new internal design and fields
of the CallbackReader structure, we first add a NewCallbackReader()
factory function which abstracts the initialization of a
CallbackReader structure.

We then update the CopyWithCallback() function to make use of
the NewCallbackReader() function, and do the same for all of the
test functions which require that they initialize a CallbackReader
structure.

We define a "closingByteReader" structure with a no-op Close() method
in several of our packages, including the "tools", "lfsapi", and
"lfshttp" packages.  In each case, the purpose of the structure is
to allow us to use the structure returned by the NewReader() function
of the Go standard library's "bytes" package in contexts that require
a structure that implements the ReadSeekCloser interface from the
standard library's "io" package.

We introduced the first of these structures to our "lfsapi" package
in commit 6659b9d of PR git-lfs#1839.
In a subsequent commit in that PR we then added a NewByteBody()
factory function which initializes a "closingByteReader" structure
by passing the byte slice provided as a parameter to the "bytes"
package's NewReader() function.  Next, in the same PR we replicated
the structure, its Close() method, and the NewByteBody() function
into what was at that time called our "progress" package.

In PR git-lfs#2749 we later moved the "progress" package's contents to
our "tools" package, and then in PR git-lfs#3244 we replicated the
structure, its Close() method, and the NewByteBody() function a
third time when we refactored portions of the "lfsapi" package
into our "lfshttp" package.

In our "tools" package we also provide a NewByteBodyWithCallback()
factory function which calls the NewByteBody() function and uses
the "closingByteReader" structure it returns as the "body"
parameter in a call to the NewBodyWithCallback() function.
The result is that a new BodyWithCallback structure is initialized
with the byte slice passed to the NewByteBodyWithCallback()
function as its underlying source of data.

This NewByteBodyWithCallback() function is used exclusively in
various test functions, both in our "tools" package and our "lfs"
package.  Ideally, though, we should not define functions or methods
in our non-test source files that are intended for use only in test
functions.

In prior commits in this PR we updated all of the test functions
which use the NewByteBodyWithCallback() function, and revised a
number of them so that they now perform their checks against the
methods of both a BodyWithCallback structure and the methods of a
CallbackReader structure.

To simplify the initialization of the CallbackReader structures, in
another prior commit in this PR we introduced a NewCallbackReader()
factory function, which operates in a similar manner to the
NewBodyWithCallback() function except that it returns a CallbackReader
structure instead of a BodyWithCallback structure.  We did not,
though, introduce a NewByteCallbackReader() function, as it is
relatively straightforward for our tests to simply pass the return
value from a call to the "bytes" package's NewReader() function to
our NewCallbackReader() function.

Given the marginal utility of the NewByteBodyWithCallback() function
and the fact that it is only used in our test functions, we remove
it now.  This also helps align the structure initialization steps
performed by our test functions, since they now just call the
NewBodyWithCallback() and NewCallbackReader() functions, which
initialize their respective structures in an equivalent manner.

To bring these initialization steps into even closer alignment, we
also rename the "closingByteReader" structure in the "tools" package
to ClosingByteReader, and rename its factory function from
NewByteBody() to NewClosingByteReader().

These changes will allow us to make use of the structure in our
"lfsapi" and "lfshttp" packages, replacing the identical definitions
of the "closingByteReader" structure and the related functions and
methods in those packages.  We defer this refactoring step to a
future PR, however, as it is outside of this PR's scope.

Finally, we move the definition of the ClosingByteReader structure,
its Close() method, and the NewClosingByteReader() function to
our "tools/iotools.go" source file, and add a new test function
named TestClosingByteReaderNopClose() to the "tools/iotools_test.go"
source file.  This test function simply verifies that the Close()
method of the ClosingByteReader structure performs no action, as we
expect that it should not do so.

In commit d7d9fd7 of PR git-lfs#4042 we
introduced the "closingFileReader" structure to our "tools" package,
which wraps a File structure from the Go standard library's "os"
package but replaces its Close() method with a no-op stub method.
In the same commit we also added the NewFileBody() factory function,
which initializes a new "closingFileReader" structure.

The "closingFileReader" structure and the NewFileBody() function were
modelled on the "closingByteReader" structure and NewByteBody()
function which were already present in our "tools/copycallback.go"
source file at the time when PR git-lfs#4042 was developed.  However, the
two structures and their methods serve different purposes.

The "closingByteReader" structure provides a no-op Close() method
so that we can use the structure returned by the NewReader() function
of the Go standard library's "bytes" package in contexts that require
a structure that implements the ReadSeekCloser interface from the
standard library's "io" package.  Since the underlying byte slice
cannot be closed, the Close() method we provide is necessarily just
a no-op.

By contrast, the "closingFileReader" structure's underlying resource
is a File structure from the Go standard library's "os" package,
which does provide a Close() method of its own.  The reason we
substitute this method with a no-op Close() method is so that we
can use the "closingFileReader" structure in instances where we
expect the underlying File structure's Close() method to be called
later than the Close() method of the "closingFileReader" structure.

In the case of our HTTP upload transfer adapter in particular,
when the upload request for an individual object is redirected to
another URL, the Go standard library's "net/http" package will
invoke the Close() method on the Body field of the current Request
structure after the initial request is complete.  However, we do
not want this call to cause the underlying file resource to be
closed as well, so we need to provide a stub Close() method which
takes no action when it is called by the HTTP transport mechanism.

Thus while the "closingByteReader" structure provided a way to
"close" a byte slice (although this operation is not actually
possible), the "closingFileReader" structure provides a way to
avoid "closing" a file resource.

In a prior commit in this PR we renamed the "closingByteReader"
structure to ClosingByteReader in order to export it for later use
in several of our packages, and we also renamed the NewByteBody()
function to NewClosingByteReader().

For greater clarity as to the purpose of the "closingFileReader"
structure, we now rename it to "nopClosingFile", which more clearly
indicates that the structure just wraps a File structure and provides
a no-op Close() method.  For consistency, we also rename the
NewFileBody() function to newNopClosingFile().  As there is only
one caller of this function, the NewFileBodyWithCallback() function,
this contains the sole reference we need to update to reflect the
revised newNopClosingFile() name.

Fortunately, we already have a test function which demonstrates that
these changes have not introduced a regression.  In a previous commit
in this PR we introduced the TestFileBodyCallbackReaderIgnoresClose()
function, which verifies that when the Close() method is called on
the structure returned by the NewFileBodyWithCallback() function,
it does not actually close the underlying file resource.

Our "tools" package defines two structures which perform similar
purposes, the BodyWithCallback and CallbackReader structures, whose
Read() methods are identical and invoke a callback function after
first calling the Read() method of an underlying resource.

The only significant difference between the two structures is that
the BodyWithCallback structure's methods implement the ReadSeekCloser
interface from the Go standard library's "io" package, while the
CallbackReader structure's methods only implement the Reader
interface from the "io" package.

We introduced the first version of our CallbackReader structure in
commit d7a787d of PR git-lfs#97, along
with a Read() method quite similar to the one currently defined
for the structure.

Later, in commit e04316f of PR git-lfs#1839,
we added the initial version of what is now the BodyWithCallback
structure, at which time we simply copied the Read() method of the
CallbackReader structure to create an identical method for the
BodyWithCallback structure.

The implementation of the two Read() methods is identical, so when we
fix a bug in one, we have to make the same change in the other also.
For instance, in commit a2a7b09
of PR git-lfs#1955 we revised both methods to address a problem whereby
they would fail to invoke their callback functions if the underlying
Read() method read a non-zero number of bytes but also encountered
an EOF condition.

In subsequent commits in this PR we intend to alter the design
of the Read() methods of the BodyWithCallback and CallbackReader
structures, and then to make further changes to handle instances
where the total size of a Git LFS object is initially unknown.

As we would prefer to avoid making these revisions in two places,
and to avoid future problems where we might accidentally update
only one of the two Read() methods, we begin by refactoring both
the methods and the BodyWithCallback and CallbackReader structures
so they share a common internal implementation.

Specifically, we introduce a "callbackState" structure whose fields
represent the current state for a given callback function, and then
we embed this structure in the BodyWithCallback and CallbackReader
structures.  These structures now only have one unique field each
to store their underlying data source.

We also define a read() method for the "callbackState" structure,
with an identical design to the existing Read() methods of the
BodyWithCallback and CallbackReader structures except that it
accepts a parameter of the Reader type from the "io" package.
We then revise the Read() methods of the BodyWithCallback and
CallbackReader structures to simply invoke this new read() method
and pass their respective data sources as its Reader type parameter.

Note that in a future PR we may further refactor the implementation
of the BodyWithCallback and CallbackReader structures to replace
them entirely with a single common structure.  However, as this
change would affect a number of our packages, it is outside the
scope of this PR, so for the time being we just revise the existing
structures to share a new, common internal structure and its read()
method.

In a previous commit in this PR we refactored the BodyWithCallback
and CallbackReader structures in our "tools" package so that they
now share a common internal implementation in the form of an internal
"callbackState" structure and its read() method.  The Read() methods
of both the BodyWithCallback and CallbackReader structures now just
invoke the "callbackState" structure's read() method, whereas they
were previously defined separately but with an identical design.

In subsequent commits in this PR we expect to revise the new, common
read() method to handle the condition where a Git LFS object's total
size is initially unknown.  Before we make this change, though, we
first refactor the existing read() method so that it will be easier
to modify and understand.

Rather than execute the callback within a nested pair of conditional
blocks, we introduce a new doCallbackIfRequired() method which checks
the same conditions (albeit expressed differently) and then executes
the callback if it should be performed, and otherwise just returns
the "error" value that was passed by the caller.  The read() method
can then simply invoke the doCallbackIfRequired() method and pass
the values returned by the underlying Read() method.

We also move the actual execution of the callback function into a
doCallback() method, which takes no action if the callback function
is not defined.  While this duplicates one of the conditions also
checked by the doCallbackIfRequired() method, it ensures that it is
no longer possible that the ResetProgress() method might cause a
panic by executing an undefined callback function.

In a previous commit in this PR we refactored the BodyWithCallback
and CallbackReader structures in our "tools" package so that they
now share a common internal implementation in the form of an internal
"callbackState" structure and its read() method.

We derived the design of the read() method from that of the Read()
methods of the BodyWithCallback and CallbackReader structures as
they were previously defined.  This design dates back to the
original introduction of the CallbackReader structure in commit
d7a787d of PR git-lfs#97.

One notable feature of the current implementation of both our new
read() method as well as that of its predecessors is that when
the registered callback function is invoked, the methods always
use the function's "error" return value as their own return value.

As a consequence, if the underlying resource's Read() method reads
a non-zero number of bytes and also encounters an EOF condition,
our read() method will perform a callback and then substitute the
callback function's return value for the EOF error.  In the normal
case where no error occurs during the callback, this will force
the caller to make another invocation of our read() method, at
which point no bytes will be read and so no callback will be
performed, and the EOF condition will be reported to the caller.

While this design is acceptable, because the Reader interface of
the Go standard library's "io" package allows Read() methods to
only return the EOF condition when no bytes are available to be
read, it is slightly less efficient than it could be.

We can improve this situation by returning an EOF error from the
underlying resource's Read() method unless our callback function
encountered an error condition.

To implement this change, we first revise the doCallbackIfRequired()
method of the "callbackState" structure so that the method returns a
nil "error" value when no callback should be performed, and otherwise
returns the "error" value from the callback function.  Next, we
update the read() method so that it saves the return value from the
doCallbackIfRequired() method into a local "callbackErr" variable,
and then returns the "error" value from the Read() method unless the
value of the "callbackErr" variable is non-nil, in which case the
read() method returns that value instead.

Finally, we update two of our existing Go test functions to reflect
the new behaviour of the Read() methods of the BodyWithCallback and
CallbackReader structures.  These tests are sufficient to demonstrate
that our changes to the "callbackState" structure's read() and
doCallbackIfRequired() methods in this commit mean that the read()
method now preserves an EOF error from its underlying resource if
the callback function is invoked and does not encounter an error.

The TestBothCallbackReadersInvokeCallbackOnEagerEOF() function in our
"tools" package and the TestCopyCallbackFileThrottle() function in
our "lfs" package both make use of the EagerEOFByteReader structure
we added to our "internal/testutil" package in a prior commit in
this PR.  This structure's Read() method always returns an EOF error
when it reads the final bytes from a given byte slice.  Previously,
these two test functions expected that when an EagerEOFByteReader
structure was the underlying resource of a BodyWithCallback or
CallbackReader structure, that structure's Read() method would return
a nil "error" value when it read the final bytes of a slice.

In subsequent commits in this PR we expect to make several revisions
to the anonymous callback function created by the CopyCallbackFile()
method of the GitFilter structure in our "lfs" package.  In
particular, we intend to alter the function to more appropriately
handle the case where its "total" parameter is negative, which
indicates that the total size of the current object is unknown.

At present, when the callback function is passed a negative value in
its "total" parameter, it simply writes that value to the progress
log as the denominator of the fraction representing the amount of
the object's data processed so far.

As well, if the callback function is passed a negative value in
its "total" parameter, it will always write to the progress log
regardless of how much time has elapsed since the last message was
written.

In PR git-lfs#5504 we modified the CopyCallbackFile() method of the
GitFilter structure in our "lfs" package so that the callback
function it creates only outputs a progress log message when
at least 200 milliseconds have elapsed since the last message
was written, unless the number of bytes read has now met or
exceeded the total size of the given Git LFS object.  (The
minimum time allowed between messages is specified by the
DefaultLoggingThrottle constant from our "tasklog" package.)

The callback function checks whether the number of bytes read has
met or exceeded the total size of the given object, and if it has,
unconditionally writes a message to the progress log, on the
assumption that no further callbacks will occur because the final
bytes of the object have now been read.

However, this assumes the total size of the object is known and
correct when the callback is utilized by the CopyWithCallback()
function of our "tools" package.  When the "total" parameter is
negative, though, the number of bytes read so far is always larger
than the total size, so a message is always written.

While it is currently rare for the value in the callback function's
"total" parameter to be negative, we expect it to become more common
in the future as a result of the bug fix to our "clean" filter
commands implemented by PR git-lfs#6011.

Before we address this issue, we first refactor the callback function
created by the CopyCallbackFile() method to make the logic in the
function easier to modify and understand.  In particular, we separate
the multiple parts of the function's single conditional expression
into distinct steps.  We also revise the function so that it returns
immediately when the condition checked by one of these individual
steps is indicates that no log message should be written.

In PR git-lfs#5504 we modified the CopyCallbackFile() method of the
GitFilter structure in our "lfs" package so that the callback
function it creates only outputs a progress log message when
at least 200 milliseconds have elapsed since the last message
was written, unless the number of bytes read has now met or
exceeded the total size of the given Git LFS object.  (The
minimum time allowed between messages is specified by the
DefaultLoggingThrottle constant from our "tasklog" package.)

The callback function checks whether the number of bytes read has
met or exceeded the total size of the given object, and if it has,
unconditionally writes a message to the progress log, on the
assumption that no further callbacks will occur because the final
bytes of the object have now been read.

However, this assumes the total size of the object is known and
correct when the callback is utilized by the CopyWithCallback()
function of our "tools" package.  When the "total" parameter is
negative, though, the number of bytes read so far is always larger
than the total size, so a message is always written.

While it is currently rare for the value in the callback function's
"total" parameter to be negative, we expect it to become more common
in the future as a result of the bug fix to our "clean" filter
commands implemented by PR git-lfs#6011.

PR git-lfs#6011 resolves a bug in our "git lfs filter-progress" and
"git lfs clean" commands whereby they can pass an incorrect
"totalSize" parameter to the CopyWithCallback() function.  The only
solution for this problem is for the commands to simply not attempt
to determine the total size of an object.

Absent other changes, the result of this bug fix would be that
even if a progress log file is specified by the GIT_LFS_PROGRESS
environment variable, no messages would be written to the log file
during "clean" filter operations.  This would occur because the
copyToTemp() method of the GitFilter structure in our "lfs" package
would find a negative value in its "fileSize" parameter and then
set its "cb" callback function parameter to nil.

We intend to address this issue in PR git-lfs#6011, but the consequence
when we do so will be that the callback function created by the
CopyCallbackFile() method will receive a negative value in its
"total" parameter during "clean" filter operations if a progress log
is configured with the GIT_LFS_PROGRESS environment variable.

At present, the callback function may already receive a negative
value in its "total" parameter, but only under a relatively unusual
combination of circumstances.  First, of course, a progress log must
be enabled with the GIT_LFS_PROGRESS environment variable.  Second,
the legacy "git lfs smudge" command must be used during a "smudge"
filter operation rather than the "git lfs filter-process" command;
this may occur if a version of Git older than v2.11.0 is in use.
Third, the "git lfs smudge" command must download an object that is
not available locally from a server which compresses the content
for transit and therefore sends a Content-Encoding header but no
Content-Length header.

Under these conditions, the lack of a Content-Length header means
the size of the HTTP response body will be represented with a value
of -1.  When our HTTP download adapter passes this negative value to
the CopyWithCallback() function, it will be used to initialize a new
CallbackReader structure from our "tools" package, and ultimately
passed to the callback function created by the CopyCallbackFile()
method.  Note that the callback function itself is passed from the
Smudge() method of the GitFilter structure to the same structure's
downloadFile() method, which creates a new transfer queue using
the WithProgressCallback() option function.

By contrast, when the "git lfs filter-process" command is in use,
it creates a transfer queue without a defined callback function.
As a result, objects that not are available locally are downloaded
and written to local storage without generating any progress log
messages.  When the Smudge() method is later invoked (because Git
re-sends a "smudge" command to our long-running filter process),
it runs the GitFilter structure's readLocalFile() method instead of
the downloadFile() method.  While the readLocalFile() method does
invoke the CopyWithCallback() function and passes a callback
function to it, at this point the size of the object can be
determined from the file in local storage, so the callback function
receives a non-negative value in its "total" parameter.

However, as described above, when the "git lfs smudge" command is
in use and a content-encoded download occurs, the callback function
created by the CopyCallbackFile() method will receive a negative
value in its "total" parameter, which means the function will write
to the progress log regardless of the amount of time that has
elapsed since the previous message was written.

Further, we expect the changes in PR git-lfs#6011 to result in a wider
range of conditions under which the callback function will receive
a negative value in its "total" parameter.

We therefore now revise the callback function created by the
CopyCallbackFile() method so that it checks whether the value of
the "total" parameter is negative, and if so will not write a
progress log message unless a sufficient period of time has
elapsed since the previous log message was written.

To verify that our change is effective, we define a second test
case for the TestCopyCallbackFileThrottle() test function that
we refactored in a prior commit in this PR.  Our new test case
is named "initial total unknown" and simulates multiple invocations
of the callback function where the "total" parameter has a negative
value each time.  In this test case, we expect that the modifications
from this commit should ensure that the callback function only writes
one log message, after the test advances the mock time clock by the
full duration specified in the DefaultLoggingThrottle constant from
our "tasklog" package.

Note that the log message expected by our new "initial total unknown"
test case contains a "-1" in the denominator of the fraction
representing the amount of source data read so far.  We will refine
this field in a subsequent commit in this PR so that negative
"total" values are no longer reported in a literal manner.

In PR git-lfs#5504 we modified the CopyCallbackFile() method of the
GitFilter structure in our "lfs" package so that the callback
function it created only output a progress log message when
at least 200 milliseconds had elapsed since the last message
was written, unless the number of bytes read had met or exceeded
the total size of the given Git LFS object.  (The minimum time
allowed between messages was specified by the DefaultLoggingThrottle
constant from our "tasklog" package.)

This design depended on the assumption that the total size of the
given object was known and correct when the callback was utilized
by the CopyWithCallback() function of our "tools" package.  When
the "total" parameter was negative, though, a message would always
be written because the number of bytes read so far would always be
larger than the total size.

In a previous commit in this PR we adjusted the callback function
so that it now checks whether the value of the "total" parameter
is negative, and if so will not write a progress log message unless
a sufficient period of time has elapsed since the previous log
message was written.

However, one consequence of this adjustment is that when the "total"
parameter is negative, it is not guaranteed that a log message will
be written when the last callback is made after the final bytes of
the given object's data are read.  Further, if an object's data is
fully processed in less than 200 milliseconds, no progress log
message will be written for the object at all.

While it is currently rare for the value in the callback function's
"total" parameter to be negative, we expect it to become more common
in the future as a result of the bug fix to our "clean" filter
commands implemented by PR git-lfs#6011.

PR git-lfs#6011 resolves a bug in our "git lfs filter-progress" and
"git lfs clean" commands whereby they can pass an incorrect
"totalSize" parameter to the CopyWithCallback() function.  The only
solution for this problem is for the commands to simply not attempt
to determine the total size of an object.

Absent other changes, the result of this bug fix would be that
even if a progress log file is specified by the GIT_LFS_PROGRESS
environment variable, no messages would be written to the log file
during "clean" filter operations.  This would occur because the
copyToTemp() method of the GitFilter structure in our "lfs" package
would find a negative value in its "fileSize" parameter and then
set its "cb" callback function parameter to nil.

We intend to address this issue in PR git-lfs#6011, but the consequence
when we do so will be that the callback function created by the
CopyCallbackFile() method will receive a negative value in its
"total" parameter during "clean" filter operations if a progress log
is configured with the GIT_LFS_PROGRESS environment variable.

At present, the callback function may already receive a negative
value in its "total" parameter, but only under a relatively unusual
combination of circumstances.  First, of course, a progress log must
be enabled with the GIT_LFS_PROGRESS environment variable.  Second,
the legacy "git lfs smudge" command must be used during a "smudge"
filter operation rather than the "git lfs filter-process" command;
this may occur if a version of Git older than v2.11.0 is in use.
Third, the "git lfs smudge" command must download an object that is
not available locally from a server which compresses the content
for transit and therefore sends a Content-Encoding header but no
Content-Length header.

Under these conditions, the lack of a Content-Length header means
the size of the HTTP response body will be represented with a value
of -1.  When our HTTP download adapter passes this negative value to
the CopyWithCallback() function, it will be used to initialize a new
CallbackReader structure from our "tools" package, and ultimately
passed to the callback function created by the CopyCallbackFile()
method.  Note that the callback function itself is passed from the
Smudge() method of the GitFilter structure to the same structure's
downloadFile() method, which creates a new transfer queue using
the WithProgressCallback() option function.

By contrast, when the "git lfs filter-process" command is in use,
it creates a transfer queue without a defined callback function.
As a result, objects that are not available locally are downloaded
and written to local storage without generating any progress log
messages.  When the Smudge() method is later invoked (because Git
re-sends a "smudge" command to our long-running filter process),
it runs the GitFilter structure's readLocalFile() method instead of
the downloadFile() method.  While the readLocalFile() method does
invoke the CopyWithCallback() function and passes a callback
function to it, at this point the size of the object can be
determined from the file in local storage, so the callback function
receives a non-negative value in its "total" parameter.

However, as described above, when the "git lfs smudge" command is
in use and a content-encoded download occurs, the callback function
created by the CopyCallbackFile() method will receive a negative
value in its "total" parameter, which means the function will write
to the progress log regardless of the amount of time that has
elapsed since the previous message was written.

Further, we expect the changes in PR git-lfs#6011 to result in a wider
range of conditions under which the callback function will receive
a negative value in its "total" parameter.

We therefore now enhance both the callback function and the internal
implementation of the Read() method of the BodyWithCallback and
CallbackReader structures in our "tools" package so that a progress
log message will always be written when the final bytes of a given
object are read.

First, we update the doCallbackIfRequired() method of the
"callbackState" structure in our "tools" package so that it checks
if an EOF condition has been encountered, and if so, checks whether
the amount of data read so far exceeds the total size that was
provided initially for the given object.  Should both of these
conditions apply, the method resets the "totalSize" field of the
"callbackState" structure to the final value of the "readSize"
field, and then runs the doCallback() method to invoke the callback
function.

As a result of this change, a callback will be performed the first
time the EOF condition is encountered while processing an object
whose total size was initially unknown, and when this callback is
made, the callback function's "total" parameter will not have a
negative value but will have the same value as the function's
"written" parameter.

Next, we update the CopyCallbackFile() method of the GitFilter
structure in our "lfs" package so that the callback function the
method creates now checks whether the "total" parameter passed to
the function has a different value than it did the previous time
the function was invoked.  If this is the case, and if the current
value of the "total" is also equal to the non-negative value of the
"written" parameter, then the function writes a message to the
progress log regardless of the amount of time that has elapsed since
the previous message was written.

This change has the key consequence that if the callback function
is invoked one or more times with a negative value for its "total"
parameter, and is invoked a final time with matching non-negative
values for its "written" and "total" parameters, it will always
write a progress log message at that time.

When this change is combined with the revisions we have made to the
"callbackState" structure's doCallbackIfRequired() method in this
commit, the result is that if an object's total size is initially
unknown, when an EOF condition is encountered while reading the
object's data, then a final progress log message is guaranteed to
be written with the total size of the object represented as equal
to the actual number of bytes that were read.

To verify this new behaviour, we add or update a number of our tests,
starting with the TestCopyCallbackFileThrottle() test function in
our "lfs" package.  Most significantly, we update this function's
"initial total unknown" test case so that it now expects a final
log message in which the total size of the object is not negative
but matches the total number of bytes read.

Note that the first log message expected by our new "initial total
unknown" test case still contains a "-1" in the denominator of the
fraction representing the amount of source data read so far.  We will
refine this field in a subsequent commit in this PR so that negative
"total" parameter values are no longer reported in a literal manner.

We also add two new test cases for the TestCopyCallbackFileThrottle()
function, in order to simulate situations where the total size of
an object is initially either overestimated or underestimated.  In
both cases, a final callback should be made in which the "total"
parameter value has been adjusted to match the actual number of
bytes that have been read when the EOF condition is reached.

To simulate these additional test cases, we expand the setup() and
Assert() methods of the "copyCallbackFileThrottleTestCase" structure
we introduced to our "lfs/util_test.go" source file in a previous
commit in this PR.

For the particular test case where the total size of an object is
underestimated, the Assert() method now performs three extra calls to
the Read() method of the CallbackReader structure, with a simulated
delay between the first two of these calls.  The first of these calls
should never result in a log message being written, while the second
should produce a log message in which the number of bytes read so far
exceeds the total size of the object.  For both new test cases, when
the EOF condition is reached, a callback should be made which produces
a log message with the object's total size adjusted in the final log
message to match the actual number of bytes read.

To independently verify the new behaviour of the "callbackState"
structure's doCallbackIfRequired() method, we add a test function to
our "tools" package which also simulates situations in which the
total size of the object is either unknown, overestimated, or
underestimated.

The TestBothCallbackReadersInvokeCallbackOnEOFWhenTotalSizeIncorrect()
function confirms that the Read() methods of both the BodyWithCallback
and CallbackReader structures perform a final callback when the EOF
condition is reached and pass the callback function an updated value
for the total size of the object which matches the actual number of
bytes read.  Note that in a prior commit in this PR we refactored the
Read() methods of both of the BodyWithCallback and CallbackReader
structures so that these methods now just invoke the read() method of
a "callbackState" structure, which in turn executes the
doCallbackIfRequired() method, so we do not need to provide a test
of the doCallbackIfRequired() method itself.

In addition to these changed or new Go test functions, we also expand
our "t/t-progress.sh" shell script with two new tests, one of which
exercises the "git lfs smudge" command under the specific set of
conditions we described above.

Our new "progress log: unknown initial size (smudge)" shell test
forces the use of the "git lfs smudge" command by a "git clone"
command and specified a progress log file using the GIT_LFS_PROGRESS
environment variable.  As well, in the repository cloned by the
"git clone" command, the test establishes a Git LFS object with
the special key string "storage-download-encoding-gzip" as its
contents, which causes our "lfstest-gitserver" test utility program
to return the object's contents in an HTTP response with a
gzip-encoded body and without a Content-Length header.

The test then checks that the progress log file contains an entry
with the full size of the object reported in both the numerator and
denominator of the field which represents the amount of data
downloaded for the object.  Previously, all progress log entries
generated by the "git lfs smudge" command while downloading a
content-encoded object would have a "-1" in the field's denominator.

Note that any intermediate entries preceding the final log entry
will still have a negative value in this field, but we will address
that issue in a subsequent commit in this PR.

(Note also that we use the same "storage-download-encoding-gzip"
special object content key for a more complete test of the Git LFS
client's handling of gzip-encoded responses in the "batch storage
HTTP download with gzip encoding" test in our
"t/t-batch-storage-encoding.sh" shell script.)

The other new shell test in our "t/t-progress.sh" script is named
"progress log: unknown initial size (fetch/pull)", and operates in
a similar manner to the "progress log: unknown initial size (smudge)"
test except that it exercises the "git lfs fetch" and "git lfs pull"
commands rather than the "git lfs smudge" command.

As noted in the following comment on PR git-lfs#6011, our "git lfs fetch"
and "git lfs pull" commands may write progress log entries with a "-1"
value in the denominator of the field which represents the amount of
data downloaded for the object, if the HTTP response to an object
download request is content-encoded and lacks a Content-Length header.

  git-lfs#6011 (comment)

In these commands, a transfer queue is created using the
WithProgress() option function rather than the WithProgressCallback()
option function, so the "cb" field of the TransferQueue structure
from our "tq" package is nil.  However, both commands pass a Meter
structure from our "tq" package to the WithProgress() function,
so the TransferQueue structure's "m" field is non-nil.

As a result, when the TransferQueue structure's ensureAdapterBegun()
method is executed, the callback function it creates will execute
only the TransferBytes() method of the Meter structure, which in
turn does not immediately return without taking any action because
its "m" receiver parameter is non-nil.  By comparison, when the
WithProgressCallback() option function is used instead of the
WithProgress() option function, this same callback function runs
the TransferBytes() method, which returns immediately, and then
runs the callback function specified by the non-nil "cb" field.

The callback function created by the ensureAdapterBegun() method
is then passed to the Begin() method of the transfer adapter, and
ultimately passed to the CopyWithCallback() function of our "tools"
package by the "basicDownloadAdapter" structure's download() method.
(Note that this is also true for our "git lfs filter-process" and
"git lfs smudge" commands, not just for the "git lfs fetch"
and "git lfs pull" commands.)

As described above regarding the "git lfs smudge" command, the
CopyWithCallback() function initializes a new CallbackReader structure
and so the callback function passed to the CopyWithCallback() function
will ultimately be invoked as object data is read from the HTTP
response body.  With the "git lfs fetch" and "git lfs pull" commands,
though, this callback function only runs the TransferBytes() method
of the Meter structure, and that method does not return immediately
but runs the logBytes() method of the same structure.

For historical reasons which date back to the introduction of the
logBytes() method in commit 710862b
of PR git-lfs#476, this method writes a line to the progress log file with
the identical format as the lines generated by the callback function
created by the CopyCallbackFile() method of the GitFilter structure.

However, unlike that anonymous callback function, the logBytes()
method does not limit its output based on the amount of time that
has elapsed since it was previously invoked.  When we revised the
CopyCallbackFile() method in PR git-lfs#5504 to implement a mechanism
which throttled the frequency of its output to the progress log,
we did not also update the logBytes() method to do the same.

While we likely should refactor the logBytes() method and the
CopyCallbackFile() method's callback function to share a common
implementation, that change falls outside of the scope of this PR,
so we defer it to a future PR.

What this PR does accomplish, though, is to ensure that regardless
of their source, when progress log messages are written for an object
whose total size is initially unknown, a final callback will be made
after the EOF condition is encountered, and this will generate a
progress log message in which the total size has been adjusted to
match the actual number of bytes read.

For this reason, our "progress log: unknown initial size (fetch/pull)"
shell test exercises the "git lfs fetch" and "git lfs pull" commands
with a progress log file specified in the GIT_LFS_PROGRESS environment
variable and an HTTP object download response that lacks a
Content-Length header.  Like our other new shell test, this test
also uses the "storage-download-encoding-gzip" special object
content key to cause the "lfstest-gitserver" utility to gzip-encode
its response to an object download request.

The test then confirms that the progress logs generated by the
"git lfs fetch" and "git lfs pull" commands contain an entry with
the full size of the object reported in both the numerator and
denominator of the field which represents the amount of data
downloaded for the object.  Previously, all progress log entries
generated by these commands while downloading a content-encoded
object would have a "-1" in the field's denominator.

Note that any intermediate entries preceding the final log entry
will still have a negative value in this field, but we will address
that issue in a subsequent commit in this PR.

As described in a previous commit in this PR, several of our Git LFS
commands may write progress log messages with a negative value in
the denominator of the fraction representing the amount of data
downloaded so far for an individual object.  Specifically, the
"git lfs fetch", "git lfs pull", and "git lfs smudge" commands
will write such log messages when a progress log is enabled with
the GIT_LFS_PROGRESS environment variable and the response to an
HTTP object download request is content-encoded and therefore lacks
a Content-Length header.

As described in a previous commit in this PR, for historical reasons
progress log messages are written either by the anonymous callback
function created by the CopyCallbackFile() method of the GitFilter
structure in our "lfs" package, or by the logBytes() method of the
Meter structure in our "tq" package.  However, the format of the
messages is identical regardless of which function or method
generates them.

When the logBytes() method was first introduced in commit
710862b of PR git-lfs#476, it duplicated
the format of the progress log messages generated by the callback
function created by the CopyCallbackFile() method.  The current
progress log message format in that method dates from commit
36fa23d of PR git-lfs#100, but the original
version of the method and its callback function themselves were added
in commit fd41906 of PR git-lfs#97.

While we likely should refactor the logBytes() method and the
CopyCallbackFile() method's callback function to share a common
implementation, that change falls outside of the scope of this PR,
so we defer it to a future PR.

However, one of the changes we made in a prior commit in this PR
now ensures that if an object's total size is initially unknown,
when an EOF condition is encountered while reading the object's
data, then a final progress log message is guaranteed to be written
with the total size of the object represented as equal to the
actual number of bytes that were read.  Further, this final
progress log entry will be output regardless of whether the
CopyCallbackFile() method's callback function or the logBytes()
method is being used to write messages to the log file.

While this change affects the content of the last progress log
entry written for a given object, until the EOF condition is
reached, intermediate log messages may be generated which still
have a negative value in the denominator of the fraction
representing the amount of the object's data processed so far.

These negative values occur because the length of the data to be
read is initially unknown.  At present, the only circumstance in
which this is possible is when the HTTP response to an object
download request lacks a Content-Length header, as will be the
case when the response body is gzip-encoded, as we noted in the
following comment on PR git-lfs#6011:

  git-lfs#6011 (comment)

PR git-lfs#6011 resolves a bug in our "git lfs filter-progress" and
"git lfs clean" commands whereby they can pass an incorrect
"totalSize" parameter to the CopyWithCallback() function.  The only
solution for this problem is for the commands to simply not attempt
to determine the total size of an object.  As a consequence, we
expect that our "git lfs filter-progress" and "git lfs clean" commands
will never know in advance the length of the object data they are
reading from Git, and it will become a more common occurrence for
the CopyCallbackFile() method's callback function and the logBytes()
method to receive a "total" parameter with a negative value.

We therefore now refine the format of the progress log when the
total size of an object is unknown by writing a series of question
mark characters ("?") in place of the denominator of the fraction
which represents the amount of the object's data processed so far.
We adjust the number of these question mark characters to match
the length of the numerator, e.g., "123/???" and "12345/?????".

To avoid implementing the necessary string formatting logic in
both the CopyCallbackFile() method's callback function and the
logBytes() method, we introduce a new FormatFraction() function
to our "tools" package and then substitute its output in place
of the appropriate fraction field in the log messages.

We then add a TestFormatFraction() Go test function which verifies
that the FormatFraction() function generates the expected output
under a range of conditions, including when the number of bytes
read so far exceeds the value provided as the total size of an
object, and when the total size of an object is unknown and should
be represented using various numbers of question mark characters.

Finally, we update the comments in several of the tests we added
to the "t/t-progress.sh" shell script in a prior commit of this PR,
and we revise the "initial total unknown" test case for our
TestCopyCallbackFileThrottle() test function.  In that test case,
we update the expected contents of the progress log because the
intermediate entry which precedes the final entry should now have
a series of question mark characters rather than a "-1" value in
the denominator of the fraction which represents the amount of the
object's data processed so far.