Skip to content

ensure cleaned hawser objects exist before pushing#176

Merged
technoweenie merged 2 commits intomasterfrom
ensure-cleaned-objects
Mar 19, 2015
Merged

ensure cleaned hawser objects exist before pushing#176
technoweenie merged 2 commits intomasterfrom
ensure-cleaned-objects

Conversation

@technoweenie
Copy link
Contributor

This attempts to fix the case where you can't push if the objects aren't in your .git/hawser/objects directory. Normally they should be there:

  • Objects are downloaded to .git/hawser/objects as their pointers go through the smudge filter on checkout.
  • When objects are added, they go through the clean filter which writes the files to .git/hawser/objects.

If the .git/hawser/objects directory gets into a weird state (for example, if the user manually removed some files in there), this attempts to re-clean the objects based on the git repository file path.

technoweenie added a commit that referenced this pull request Mar 19, 2015
ensure cleaned hawser objects exist before pushing
@technoweenie technoweenie merged commit 550e2a9 into master Mar 19, 2015
@technoweenie technoweenie deleted the ensure-cleaned-objects branch March 19, 2015 17:43
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Mar 28, 2025
The original version of the ensureFile() method of the uploadContext
structure in our "commands" package was first introduced in commit
5d239d6 of PR git-lfs#176 in 2015, and has
been refactored a number of times since then, but continues to be called
for each object a push operation intends to upload.

The method is documented as a function that checks whether a Git LFS
object file exists in the local .git/lfs/objects storage directories
for a given pointer, and if it does not, tries to replace the missing
object file by passing the contents of the corresponding file in the
working tree through our "clean" filter.

The description of PR git-lfs#176 explains the function's purpose as follows,
using the original pre-release name for the Git LFS project:

  If the .git/hawser/objects directory gets into a weird state
  (for example, if the user manually removed some files in there),
  this attempts to re-clean the objects based on the git repository
  file path.

The code comments preceding the ensureFile() method also describe it
in the same way, as do the notes in later PRs, such as PR git-lfs#2574, in
which the "lfs.allowIncompletePush" configuration option was introduced,
and PR git-lfs#3398, which refined the error message the Git LFS client reports
during a push operation when an object is missing locally and is also
not present on the remote server.

However, the ensureFile() method has never actually replaced missing
object files under any circumstances.  It does check whether an object
file is missing from the local storage directories, and if not, tests
whether a file exists in the current working tree at the path of the
Git LFS pointer associated with the object.  If such a file exists,
the method proceeds to run the Clean() method of the GitFilter structure
in our "lfs" package on the file's contents.

The Clean() method calculates the SHA-256 hash value of the file's
contents and creates a Pointer structure containing this hash value,
and also writes a copy of the file's data into a temporary file in
the .git/lfs/tmp directory.  It is then the responsibility of the
caller to determine whether or not this temporary file should be
moved into place in the .git/lfs/objects directory hierarchy.

The only other caller of the Clean() method, besides the ensureFile()
method, is the clean() function in the "commands" package, which
is used by multiple Git LFS commands including the "git lfs clean"
and "git lfs filter-process" plumbing commands, as well as the
"git lfs migrate import" command.

The clean() function performs several tasks after invoking the Clean()
method.  First, it checks whether the file processed by the method
was found to contain a Git LFS pointer; if so, no further action is
taken as we assume the file in the working tree has not been passed
through our "smudge" filter, and we do not want to create another
pointer which simply hashes and references the existing one.

Next, the clean() function checks whether a file already exists in the
local .git/lfs/objects storage directories at the location into which
the function would otherwise expect to move the temporary file
created by the Clean() method.  If a file does exist in this location
and has the same size as the temporary file, no further action is
taken, as we assume it contains the same contents and does not need
to be updated.

Assuming neither of these checks causes the clean() function to
return early, the function moves the temporary file created by the
Clean() method into the expected location within the .git/lfs/objects
directory hierarchy.

Unfortunately, because the ensureFile() method invokes the Clean() method
of the GitFilter structure but never performs any of the subsequent steps
taken by the clean() function, it does not ever actually recreate
a missing Git LFS object from a file found in the working tree.
This appears to have been the case at the time the ensureFile() method
was introduced in PR git-lfs#176, and has remained so ever since.

We could attempt to remedy this situation by altering the ensureFile()
method so it calls the clean() function.  To do so it would need to
simulate the conditions under which the function usually runs,
specifically within the "clean" filter context where the function is
expected to transform an input data stream into an output data stream.
We would likely use a Discard structure from the "io" package of the
standard Go library to simply discard the output from the clean()
function, as we do not need to send it back to Git in the way the
"git lfs filter-process" or "git lfs clean" commands do.

However, we would have to add logic to the clean() function guard
against the case where the file in the working tree had different
contents than those of the missing Git LFS object.  Because the
user may check out a Git reference in which a different file exists
at the same path in the working tree, or may simply modify the
file in the working tree independently, there is no guarantee that
the file we pass through the Clean() method is identical to the
one from which the missing Git LFS object was created.

The original implementation of the ensureFile() function, although it
did not fulfil its stated purpose, did include a check to verify
that the SHA hash of the working tree file, as returned by the Clean()
method, matched that of the missing object.  This check was removed in
commit 338ab40 of PR git-lfs#1812, which
would likely have introduced a serious bug, except that the ensureFile()
method never actually replaced any missing objects and so the removal
of this check had no functional impact.

While we could try to revise the ensureFile() method to operate as
was originally intended, the advantages of such a change are relatively
slim, and the disadvantages are several.  Most obviously, it requires
modifications to our clean() function to guard against the replacement
of object files with incorrect data, something the other callers of
the function do not need to be concerned about.

The fact that this is a concern at all is in turn due to the reasonable
chance that a file found in the current working tree at a given path
does not contain the identical data as that of an Git LFS object
generated from another file previously located at the same path.

As well, the fact that the ensureFile() method has never worked as
designed, despite being repeatedly refactored and enhanced over ten
years, suggests that its purpose is somewhat obscure and that the
requisite logic less intelligible than is ideal.  Users and developers
expect push operations to involve the transfer of data but not
the creation (or re-creation) of local data files, so the use of our
"clean" filter in such a context is not particularly intuitive.

For all these reasons, we just remove the ensureFile() method entirely,
which simplifies our handling of missing objects during upload transfer
operations.  Instead, we check for the presence of each object file
we intend to push in the uploadTransfer() method of our uploadContext
structure, and if a file is not found in the local storage directories,
we flag it as missing, unless the "lfs.allowIncompletePush" configuration
option is set to "true".

One consequence of this change is that when objects to be uploaded
are missing locally and are also not already present on the remote
server, and when the "lfs.allowIncompletePush" Git configuration
option is set to its default value of "false", we now always return
a consistent error message.

This occurs because, under these conditions, we now always abandon
the upload operation after the Git LFS Batch API request in which we
determine that an object which is missing locally is also not available
on the remote server.  Previously, we would sometimes continue past
this stage in the upload process, and only report a failure when we
tried to upload the object's data.  This occurred if the ensureFile()
method had found a file in the working tree at the same path as that
of the missing object (regardless of that file's contents), and thus
returned a "false" value to indicate that the object had been
recreated and was no longer missing.

Because the Git LFS client's behaviour is now more consistent when
handling missing objects during a push operation, we update several
tests in the t/t-pre-push.sh and t/t-push-failures-local.sh test
scripts to reflect this change.

In the "pre-push reject missing object" test in the t/t-pre-push.sh
script and the "push reject missing object (lfs.allowincompletepush
default)" test in the t/t-push-failures-local.sh script, we update
the error messages expected by the tests because the client now
reports an error as soon as it detects that the missing object is
not present on the remote server.

In the case of the latter test, this also means that we now expect
that no objects are uploaded to the server, because the client abandons
the upload process after the Batch API request from which it learns
that the missing object is also not present on the server, rather than
proceeding past this stage and uploading the object for which a local
copy is available.

In a previous commit in this PR we altered two other tests, in which
the "lfs.allowIncompletePush" option is set to "true", because those
tests only needed to remove a file from the local .git/lfs/objects
storage directories to simulate the condition where a Git LFS object
file is missing.  These tests did not also need to remove the Git LFS
pointer associated with the object from the repository's most recent
commit, or remove the copy of the object file's contents from the
current working tree.

We can now make the same changes to the two remaining tests that perform
these additional setup steps, namely the "pre-push reject missing object
(lfs.allowincompletepush default)" test in the t/t-pre-push.sh script
and the "push reject missing object (lfs.allowincompletepush false)"
test in the t/t-push-failures-local.sh script.  Because it is no longer
necessary to remove a working tree file in order to avoid causing
the ensureFile() method to report that the corresponding Git LFS object
is actually missing, we can simplify the setup steps in these tests
and bring them back into alignment with those of the related tests
where the "lfs.allowIncompletePush" option is set to "true".
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Mar 28, 2025
In a prior commit in this PR we simplified the Git LFS object transfer
process so that when an object to be uploaded lacks a corresponding
file in the local .git/lfs/objects storage directories, the client
does not test for the presence of a file in the Git working tree
at the path of the Git LFS pointer associated with the object.

As we described in that commit, the ensureFile() method of the
uploadContext structure in our "commands" package was documented
as a function that would try to replace a missing object file by
passing the contents of the corresponding file in the working tree
through our "clean" filter.  The original version of this method was
introduced in PR git-lfs#176 in 2015, and has been refactored a number of
times since then, but continues to be called for each object a
push operation intends to upload.

However, as we also described in our prior commit in this PR, the
ensureFile() method has never actually replaced missing object files
under any circumstances, because it has never moved the temporary file
created by the "clean" filter into the appropriate location in the
local Git LFS storage directories.

We have therefore removed this method entirely, which simplifies
our handling of missing objects during upload transfer operations.

One consequence of this change is that when objects to be uploaded
are missing locally and are also not already present on the remote
server, and when the "lfs.allowIncompletePush" Git configuration
option is set to its default value of "false", we now always return
a consistent error message.  (Under these conditions we also now
always abandon the upload operation after the Git LFS Batch API
request in which we determine that a locally-missing object is not
available on the remote server, rather than waiting until we try
to upload the object, as we previously did in some cases.)

The error message we now always return in these cases was originally
introduced into the ensureFile() method in commit
fea77e1 of PR git-lfs#3398.  This change was
made so that when an upload operation failed due to a missing object,
the error message provided to users clearly explained the problem and
how to try to resolve it.

Since commit 1412d6e of PR git-lfs#3634
this message has been output by the enqueueAndCollectRetriesFor() method
of the TransferQueue structure in our "tq" package in commit, rather
than by the ensureFile() method in the "commands" package, but the
purpose of the message remains the same.

This message was drafted with the expectation that the ensureFile()
method would try to recreate a missing object file using the potential
source file for that object from the current Git working tree.  As such,
the message is phrased to state that the Git LFS client is "Unable to
find source for object <oid>".

However, as noted above and in our prior commit in this PR, the client
has never actually attempted to recreate objects from possible source
files in the working tree, at least not during upload operations.

Therefore we now adjust this error message to simply state that the
client is "Unable to find object <oid>", and revise the relevant tests
in our test suite to expect this new, shorter version of the message.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Mar 28, 2025
The original version of the ensureFile() method of the uploadContext
structure in our "commands" package was first introduced in commit
5d239d6 of PR git-lfs#176 in 2015, and has
been refactored a number of times since then, but continues to be called
for each object a push operation intends to upload.

The method is documented as a function that checks whether a Git LFS
object file exists in the local .git/lfs/objects storage directories
for a given pointer, and if it does not, tries to replace the missing
object file by passing the contents of the corresponding file in the
working tree through our "clean" filter.

The description of PR git-lfs#176 explains the function's purpose as follows,
using the original pre-release name for the Git LFS project:

  If the .git/hawser/objects directory gets into a weird state
  (for example, if the user manually removed some files in there),
  this attempts to re-clean the objects based on the git repository
  file path.

The code comments preceding the ensureFile() method also describe it
in the same way, as do the notes in later PRs, such as PR git-lfs#2574, in
which the "lfs.allowIncompletePush" configuration option was introduced,
and PR git-lfs#3398, which refined the error message the Git LFS client reports
during a push operation when an object is missing locally and is also
not present on the remote server.

However, the ensureFile() method has never actually replaced missing
object files under any circumstances.  It does check whether an object
file is missing from the local storage directories, and if not, tests
whether a file exists in the current working tree at the path of the
Git LFS pointer associated with the object.  If such a file exists,
the method proceeds to run the Clean() method of the GitFilter structure
in our "lfs" package on the file's contents.

The Clean() method calculates the SHA-256 hash value of the file's
contents and creates a Pointer structure containing this hash value,
and also writes a copy of the file's data into a temporary file in
the .git/lfs/tmp directory.  It is then the responsibility of the
caller to determine whether or not this temporary file should be
moved into place in the .git/lfs/objects directory hierarchy.

The only other caller of the Clean() method, besides the ensureFile()
method, is the clean() function in the "commands" package, which
is used by multiple Git LFS commands including the "git lfs clean"
and "git lfs filter-process" plumbing commands, as well as the
"git lfs migrate import" command.

The clean() function performs several tasks after invoking the Clean()
method.  First, it checks whether the file processed by the method
was found to contain a Git LFS pointer; if so, no further action is
taken as we assume the file in the working tree has not been passed
through our "smudge" filter, and we do not want to create another
pointer which simply hashes and references the existing one.

Next, the clean() function checks whether a file already exists in the
local .git/lfs/objects storage directories at the location into which
the function would otherwise expect to move the temporary file
created by the Clean() method.  If a file does exist in this location
and has the same size as the temporary file, no further action is
taken, as we assume it contains the same contents and does not need
to be updated.

Assuming neither of these checks causes the clean() function to
return early, the function moves the temporary file created by the
Clean() method into the expected location within the .git/lfs/objects
directory hierarchy.

Unfortunately, because the ensureFile() method invokes the Clean() method
of the GitFilter structure but never performs any of the subsequent steps
taken by the clean() function, it does not ever actually recreate
a missing Git LFS object from a file found in the working tree.
This appears to have been the case at the time the ensureFile() method
was introduced in PR git-lfs#176, and has remained so ever since.

We could attempt to remedy this situation by altering the ensureFile()
method so it calls the clean() function.  To do so it would need to
simulate the conditions under which the function usually runs,
specifically within the "clean" filter context where the function is
expected to transform an input data stream into an output data stream.
We would likely use a Discard structure from the "io" package of the
standard Go library to simply discard the output from the clean()
function, as we do not need to send it back to Git in the way the
"git lfs filter-process" or "git lfs clean" commands do.

However, we would have to add logic to the clean() function guard
against the case where the file in the working tree had different
contents than those of the missing Git LFS object.  Because the
user may check out a Git reference in which a different file exists
at the same path in the working tree, or may simply modify the
file in the working tree independently, there is no guarantee that
the file we pass through the Clean() method is identical to the
one from which the missing Git LFS object was created.

The original implementation of the ensureFile() function, although it
did not fulfil its stated purpose, did include a check to verify
that the SHA hash of the working tree file, as returned by the Clean()
method, matched that of the missing object.  This check was removed in
commit 338ab40 of PR git-lfs#1812, which
would likely have introduced a serious bug, except that the ensureFile()
method never actually replaced any missing objects and so the removal
of this check had no functional impact.

While we could try to revise the ensureFile() method to operate as
was originally intended, the advantages of such a change are relatively
slim, and the disadvantages are several.  Most obviously, it requires
modifications to our clean() function to guard against the replacement
of object files with incorrect data, something the other callers of
the function do not need to be concerned about.

The fact that this is a concern at all is in turn due to the reasonable
chance that a file found in the current working tree at a given path
does not contain the identical data as that of an Git LFS object
generated from another file previously located at the same path.

As well, the fact that the ensureFile() method has never worked as
designed, despite being repeatedly refactored and enhanced over ten
years, suggests that its purpose is somewhat obscure and that the
requisite logic less intelligible than is ideal.  Users and developers
expect push operations to involve the transfer of data but not
the creation (or re-creation) of local data files, so the use of our
"clean" filter in such a context is not particularly intuitive.

For all these reasons, we just remove the ensureFile() method entirely,
which simplifies our handling of missing objects during upload transfer
operations.  Instead, we check for the presence of each object file
we intend to push in the uploadTransfer() method of our uploadContext
structure, and if a file is not found in the local storage directories,
we flag it as missing, unless the "lfs.allowIncompletePush" configuration
option is set to "true".

One consequence of this change is that when objects to be uploaded
are missing locally and are also not already present on the remote
server, and when the "lfs.allowIncompletePush" Git configuration
option is set to its default value of "false", we now always return
a consistent error message.

This occurs because, under these conditions, we now always abandon
the upload operation after the Git LFS Batch API request in which we
determine that an object which is missing locally is also not available
on the remote server.  Previously, we would sometimes continue past
this stage in the upload process, and only report a failure when we
tried to upload the object's data.  This occurred if the ensureFile()
method had found a file in the working tree at the same path as that
of the missing object (regardless of that file's contents), and thus
returned a "false" value to indicate that the object had been
recreated and was no longer missing.

Because the Git LFS client's behaviour is now more consistent when
handling missing objects during a push operation, we update several
tests in the t/t-pre-push.sh and t/t-push-failures-local.sh test
scripts to reflect this change.

In the "pre-push reject missing object" test in the t/t-pre-push.sh
script and the "push reject missing object (lfs.allowincompletepush
default)" test in the t/t-push-failures-local.sh script, we update
the error messages expected by the tests because the client now
reports an error as soon as it detects that the missing object is
not present on the remote server.

In the case of the latter test, this also means that we now expect
that no objects are uploaded to the server, because the client abandons
the upload process after the Batch API request from which it learns
that the missing object is also not present on the server, rather than
proceeding past this stage and uploading the object for which a local
copy is available.

In a previous commit in this PR we altered two other tests, in which
the "lfs.allowIncompletePush" option is set to "true", because those
tests only needed to remove a file from the local .git/lfs/objects
storage directories to simulate the condition where a Git LFS object
file is missing.  These tests did not also need to remove the Git LFS
pointer associated with the object from the repository's most recent
commit, or remove the copy of the object file's contents from the
current working tree.

We can now make the same changes to the two remaining tests that perform
these additional setup steps, namely the "pre-push reject missing object
(lfs.allowincompletepush default)" test in the t/t-pre-push.sh script
and the "push reject missing object (lfs.allowincompletepush false)"
test in the t/t-push-failures-local.sh script.  Because it is no longer
necessary to remove a working tree file in order to avoid causing
the ensureFile() method to report that the corresponding Git LFS object
is actually missing, we can simplify the setup steps in these tests
and bring them back into alignment with those of the related tests
where the "lfs.allowIncompletePush" option is set to "true".
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Mar 28, 2025
In a prior commit in this PR we simplified the Git LFS object transfer
process so that when an object to be uploaded lacks a corresponding
file in the local .git/lfs/objects storage directories, the client
does not test for the presence of a file in the Git working tree
at the path of the Git LFS pointer associated with the object.

As we described in that commit, the ensureFile() method of the
uploadContext structure in our "commands" package was documented
as a function that would try to replace a missing object file by
passing the contents of the corresponding file in the working tree
through our "clean" filter.  The original version of this method was
introduced in PR git-lfs#176 in 2015, and has been refactored a number of
times since then, but continues to be called for each object a
push operation intends to upload.

However, as we also described in our prior commit in this PR, the
ensureFile() method has never actually replaced missing object files
under any circumstances, because it has never moved the temporary file
created by the "clean" filter into the appropriate location in the
local Git LFS storage directories.

We have therefore removed this method entirely, which simplifies
our handling of missing objects during upload transfer operations.

One consequence of this change is that when objects to be uploaded
are missing locally and are also not already present on the remote
server, and when the "lfs.allowIncompletePush" Git configuration
option is set to its default value of "false", we now always return
a consistent error message.  (Under these conditions we also now
always abandon the upload operation after the Git LFS Batch API
request in which we determine that a locally-missing object is not
available on the remote server, rather than waiting until we try
to upload the object, as we previously did in some cases.)

The error message we now always return in these cases was originally
introduced into the ensureFile() method in commit
fea77e1 of PR git-lfs#3398.  This change was
made so that when an upload operation failed due to a missing object,
the error message provided to users clearly explained the problem and
how to try to resolve it.

Since commit 1412d6e of PR git-lfs#3634
this message has been output by the enqueueAndCollectRetriesFor() method
of the TransferQueue structure in our "tq" package in commit, rather
than by the ensureFile() method in the "commands" package, but the
purpose of the message remains the same.

This message was drafted with the expectation that the ensureFile()
method would try to recreate a missing object file using the potential
source file for that object from the current Git working tree.  As such,
the message is phrased to state that the Git LFS client is "Unable to
find source for object <oid>".

However, as noted above and in our prior commit in this PR, the client
has never actually attempted to recreate objects from possible source
files in the working tree, at least not during upload operations.

Therefore we now adjust this error message to simply state that the
client is "Unable to find object <oid>", and revise the relevant tests
in our test suite to expect this new, shorter version of the message.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Apr 3, 2025
Since commit 1412d6e of PR git-lfs#3634,
during push operations the Git LFS client has sometimes avoided
reporting an error when an object to be pushed is missing locally
if the remote server reports that it has a copy already.

To implement this feature, a new Missing element was added to the
Transfer and objectTuple structures in our "tq" package, and the
Add() method of the TransferQueue structure in that package was
updated to accept an additional "missing" flag, which the method
uses to set the Missing element of the objectTuple structure it
creates and sends to the "incoming" channel.  Batches of objects to be
pushed are then gathered from this channel by the collectBatches()
method of the TransferQueue structure.

As batches of objectTuple structures are collected, they are passed
to the enqueueAndCollectRetriesFor() method, which converts them
to Transfer structures using the ToTransfers() method and then passes
them to the Batch() function, which is defined in our tq/api.go
source file.  This function initializes a batchRequest structure
which contains the set of Transfer structures as its Objects element,
and then passes those to the Batch() method specific to the current
batch transfer adapter's structure.  These Batch() methods return
a BatchResponse structure, which the Batch() function then returns
to the enqueueAndCollectRetriesFor() method.  The BatchResponse
structure also contains an Objects element which is another set
of Transfer structures that represent the per-object metadata
received from the remote server.

After the enqueueAndCollectRetriesFor() method receives a BatchResponse
during a push operation, if any of the Transfer structures in that
response define an upload action to be performed, this implies that
the remote server does not have a copy of those objects.

As one of the changes we made in PR git-lfs#3634, we introduced a step into
the enqueueAndCollectRetriesFor() method which halts the push operation
if the server's response indicates that the server lacks a copy of an
object, and if the "missing" value passed to the Add() method for that
object was set to "true".  (Initially, this step also decremented the
count of the number of objects waiting to be transferred, but this
created the potential for stalled push operations, and so another
approach to handling an early exit from the batch transfer process was
implemented in commit eb83fcd of
PR git-lfs#3800.)

Also in PR git-lfs#3634, several methods of the uploadContext structure in
our "commands" package were revised to set the "missing" value for
each object before calling the Add() method of the TransferQueue
structure.  Specifically, the ensureFile() method of the uploadContext
structure first checks for the presence of the object data file in
the .git/lfs/objects local storage directories.  If that does not
exist, then the method looks for a file in the working tree at the
path associated with the Git LFS pointer that corresponds to the
object.  If that file also does not exist, and if the
"lfs.allowIncompletePush" Git configuration option is set to "false",
then the method returns "true", and this value is ultimately used
for the "missing" argument in the call to the TransferQueue's
Add() method for the object.

Note that the file in the working tree may have any content, or be
entirely empty; its simple presence is enough to change the value
returned by the ensureFile() method, given the other conditions
described above.  We expect to revise this unintuitive behaviour in
a subsequent commit in this PR.

Before we make that change, however, we first adjust two aspects
of the implementation from PR git-lfs#3634 so as to simplify our handling
of missing objects during push operations.  We made one of these
adjustments in the previous commit in this PR, and we make the
other in this commit.

As noted above, the enqueueAndCollectRetriesFor() method halts a
push operation if the server's response indicates that the server lacks
a copy of an object, and if the "missing" value passed to the Add()
method for that object was set to "true".  When this occurs, the
enqueueAndCollectRetriesFor() method outputs an error message that
is distinct from the message which would otherwise be reported later,
when the partitionTransfers() method of the TransferQueue rechecks
whether individual objects' data files are present in the local
storage directories.

The message reported by the enqueueAndCollectRetriesFor() method
when it abandons a push operation states that the client was
"Unable to find source for object".  This message was originally
introduced in commit fea77e1 of
PR git-lfs#3398, and was generated by the ensureFile() method of the
uploadContext structure in our "commands" package, when that method
was unable to locate either an object file in the local storage
directories, or a corresponding file in the working tree at the path
of the object's Git LFS pointer.

As such, the wording of the message alludes to the expectation that
the ensureFile() method will try to recreate a missing object file
from a file in the working tree, i.e., from the object's "source".
This is how the method is described in the code comments that
precede it, and how it was intended to operate since it was first
added in PR git-lfs#176.

However, the ensureFile() has never actually recreated object files
in this way, due to an oversight in its implementation, and given
the challenges posed by the likelihood that files in the current
working tree do not correspond exactly to the source of missing Git LFS
object files, we expect to simply remove the ensureFile() method in
a subsequent commit in this PR.

In PR git-lfs#3634 the enqueueAndCollectRetriesFor() method of the TransferQueue
structure was revised to halt push operations if the server reports that
it requires the upload of an object for which the "missing" value
provided to the TransferQueue's Add() method was set to "true".  The
error message reported in such a case was copied from the message
formerly output by the ensureFile() method of the uploadContext
structure, and that method was altered so it no longer generated this
error message.

This error message is not the only one reported by the Git LFS client
when an object file is missing during a push operation, though, because
it is only output when the ensureFile() method does not find a file
(with any content) in the working at the path associated with the
object's pointer.  When a file does exist in the working tree, but
the actual object file in the Git LFS local storage directories is
missing, the push operation proceeds past the checks in the
enqueueAndCollectRetriesFor() method and continues to the point where
that method calls the addToAdapter() method of the TransferQueue
structure.  That method in turn calls the partitionTransfers() method
to determine which of the current batch of objects to be uploaded
have local object files present and which do not.

If the partitionTransfers() method finds that an object file is
missing for a given object, it creates a new MalformedObjectError
with the "missing" element of that error structure set to "true".
The method then instantiates a TransferResult structure for the object
and sets the Error element of the TransferResult to the new
MalformedObjectError.  Finally, the method returns this TransferResult
along with the other TransferResult structures it creates for all the
other objects in the batch.

The addToAdapter() method passes these TransferResult structures
individually to the handleTransferResult() method, which checks whether
the Error element is defined for the given TransferResult.  If an
error was encountered, and is one which indicates the object transfer
should not be retried, then the handleTransferResult() method sends
the error to the TransferQueue's "errorc" channel.  For errors of the
MalformedObjectError type, this is always the case.

During a push operation, the CollectErrors() method of the uploadContext
structure in the "commands" package receives these errors from the
channel, and if they are errors of the MalformedObjectError type and
have a "true" values in their "missing" element, the object's ID and
the filename associated with the object's pointer are recorded in the
"missing" map of the uploadContext structure.

When the ReportErrors() method of the uploadContext structure is then
run, it iterates over the keys and values of the "missing" map and
outputs an error message containing both the object ID and the
associated filename of the object's pointer, along with a "(missing)"
prefix.  As well, a leading error message is output, whose exact text
depends on the value of the "lfs.allowIncompletePush" configuration
option, and if this option is set to its default value of "false",
several trailing error messages are output which provide a hint as to
how to set that option if the user wants to allow a subsequent push
operation with missing objects to proceed to completion as best it can.

These per-object error messages were first defined in commit
9be11e8 of PR git-lfs#2082, and the trailing
hints were added in commit f5f5731 of
PR git-lfs#3109, at which time the "lfs.allowIncompletePush" option's default
values was changed to "false".

As mentioned above, in a subsequent commit in this PR we expect to
remove the ensureFile() method of the uploadContext structure.  We
still expect to perform a check for missing object files in the
uploadTransfer() method, though, as this will typically find any
missing object files at the start of a push operation, and thereby
allow the enqueueAndCollectRetriesFor() method of the TransferQueue
structure to halt the operation as soon as one of those objects is
found to be required by the remote server.

For the time being, we will leave the secondary check for missing
object files in place in the partitionTransfers() method of the
TransferQueue structure, as this method also tests the size of the
object file and reports those with unexpected sizes as corrupt.

Nevertheless, we would like to make our error messages as consistent
as possible when handling missing object files.  Therefore we revise
the enqueueAndCollectRetriesFor() method so it no longer returns the
"Unable to find source for object" error message, but instead returns a
new MalformedObjectError with a "true" value for its "missing" element.

One advantage of this change is that we remove the somewhat stale
wording of the previous message, which reflected the assumption that
the ensureFile() method of the uploadContext structure would attempt
to recreate missing object files from "source" files in the working
tree, even though the method has never actually done so.

Another advantage is that by returning a MalformedObjectError, the
existing logic of the CollectErrors() and ReportErrors() methods of the
uploadContext structure will handle the error exactly as if it had been
generated by the TransferQueue's partitionTransfers() method, and will
output both the same leading error message and trailing hint messages
as in that case.

As a result, we also adjust several tests in our t/t-pre-push.sh
and t/t-push-failures-local.sh scripts to expect the error messages
output by the ReportErrors() method instead of the message previously
generated by the enqueueAndCollectRetriesFor() method.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Apr 3, 2025
The original version of the ensureFile() method of the uploadContext
structure in our "commands" package was first introduced in commit
5d239d6 of PR git-lfs#176 in 2015, and has
been refactored a number of times since then, but continues to be called
for each object a push operation intends to upload.

The method is documented as a function that checks whether a Git LFS
object file exists in the local .git/lfs/objects storage directories
for a given pointer, and if it does not, tries to replace the missing
object file by passing the contents of the corresponding file in the
working tree through our "clean" filter.

The description of PR git-lfs#176 explains the function's purpose as follows,
using the original pre-release name for the Git LFS project:

  If the .git/hawser/objects directory gets into a weird state
  (for example, if the user manually removed some files in there),
  this attempts to re-clean the objects based on the git repository
  file path.

The code comments preceding the ensureFile() method also describe it
in the same way, as do the notes in later PRs, such as PR git-lfs#2574, in
which the "lfs.allowIncompletePush" configuration option was introduced,
and PR git-lfs#3398, which refined the error message the Git LFS client reports
during a push operation when an object is missing locally and is also
not present on the remote server.

However, the ensureFile() method has never actually replaced missing
object files under any circumstances.  It does check whether an object
file is missing from the local storage directories, and if not, tests
whether a file exists in the current working tree at the path of the
Git LFS pointer associated with the object.  If such a file exists,
the method proceeds to run the Clean() method of the GitFilter structure
in our "lfs" package on the file's contents.

The Clean() method calculates the SHA-256 hash value of the file's
contents and creates a Pointer structure containing this hash value,
and also writes a copy of the file's data into a temporary file in
the .git/lfs/tmp directory.  It is then the responsibility of the
caller to determine whether or not this temporary file should be
moved into place in the .git/lfs/objects directory hierarchy.

The only other caller of the Clean() method, besides the ensureFile()
method, is the clean() function in the "commands" package, which
is used by multiple Git LFS commands including the "git lfs clean"
and "git lfs filter-process" plumbing commands, as well as the
"git lfs migrate import" command.

The clean() function performs several tasks after invoking the Clean()
method.  First, it checks whether the file processed by the method
was found to contain a Git LFS pointer; if so, no further action is
taken as we assume the file in the working tree has not been passed
through our "smudge" filter, and we do not want to create another
pointer which simply hashes and references the existing one.

Next, the clean() function checks whether a file already exists in the
local .git/lfs/objects storage directories at the location into which
the function would otherwise expect to move the temporary file
created by the Clean() method.  If a file does exist in this location
and has the same size as the temporary file, no further action is
taken, as we assume it contains the same contents and does not need
to be updated.

Assuming neither of these checks causes the clean() function to
return early, the function moves the temporary file created by the
Clean() method into the expected location within the .git/lfs/objects
directory hierarchy.

Unfortunately, because the ensureFile() method invokes the Clean() method
of the GitFilter structure but never performs any of the subsequent steps
taken by the clean() function, it never recreates a missing Git LFS object
from a file found in the working tree.  This appears to have been the
case at the time the ensureFile() method was introduced in PR git-lfs#176,
and has remained so ever since.

We could attempt to remedy this situation by altering the ensureFile()
method so it calls the clean() function.  To do so it would need to
simulate the conditions under which the function usually runs,
specifically within the "clean" filter context where the function is
expected to transform an input data stream into an output data stream.
We would likely use a Discard structure from the "io" package of the
standard Go library to simply discard the output from the clean()
function, as we do not need to send it back to Git in the way the
"git lfs filter-process" or "git lfs clean" commands do.

However, we would have to add logic to the clean() function to guard
against the case where the file in the working tree had different
contents than those of the missing Git LFS object.  Because the
user may check out a Git reference in which a different file exists
at the same path in the working tree, or may simply modify the
file in the working tree independently, there is no guarantee that
the file we pass through the Clean() method is identical to the
one from which the missing Git LFS object was created.

The original implementation of the ensureFile() function, although it
did not fulfil its stated purpose, did include a check to verify
that the SHA hash of the working tree file, as returned by the Clean()
method, matched that of the missing object.  This check was removed in
commit 338ab40 of PR git-lfs#1812, which
would likely have introduced a serious bug, except that the ensureFile()
method never actually replaced any missing objects and so the removal
of this check had no functional impact.

While we could try to revise the ensureFile() method to operate as
was originally intended, the advantages of such a change are relatively
slim, and the disadvantages are several.  Most obviously, it requires
modifications to our clean() function to guard against the replacement
of object files with incorrect data, something the other callers of
the function do not need to be concerned about.

That this is a concern at all is in turn due to the reasonable
chance that a file found in the current working tree at a given path
does not contain the identical data as that of an Git LFS object
generated from another file previously located at the same path.

As well, the fact that the ensureFile() method has never worked as
designed, despite being repeatedly refactored and enhanced over ten
years, suggests that its purpose is somewhat obscure and that the
requisite logic less intelligible than is ideal.  Users and developers
expect push operations to involve the transfer of data but not
the creation (or re-creation) of local data files, so the use of our
"clean" filter in such a context is not particularly intuitive.

For all these reasons, we just remove the ensureFile() method entirely,
which simplifies our handling of missing objects during upload transfer
operations.  Instead, we check for the presence of each object file
we intend to push in the uploadTransfer() method of our uploadContext
structure, and if a file is not found in the local storage directories,
we flag it as missing, unless the "lfs.allowIncompletePush" configuration
option is set to "true".  We also use the IsNotExist() function from
the "os" package in the Go standard library to ascertain whether an
object file is missing, or if some other type of error prevents us
from reading its state.  This mirrors the more detailed checks performed
on each object file during a push operation by the partitionTransfers()
method of the TransferQueue structure in the "tq" package.

One consequence of this change is that when an object to be uploaded
is missing locally and is also not already present on the remote
server, and when the "lfs.allowIncompletePush" Git configuration
option is set to its default value of "false", we now always abandon
a push operation after the remote server indicates that it expects
the client to upload the object.

This means that in the "push reject missing object
(lfs.allowincompletepush default)*" tests in our
t/t-push-failures-local.sh test script, we should now expect
to find a trace log message output by the client stating that the
push operation's batch queue has been stopped because an object
is missing on both the local system and the remote server.

We added this trace log message in a prior commit in this PR, and were
able to insert checks for it in several other tests in our test suite,
but only because those tests either did not create a file in the working
tree at all, as in the case of the "pre-push reject missing object"
test in our t/t-pre-push.sh script, or removed the file in the working
tree that corresponded to the object file they removed from the
.git/lfs/objects storage directories.

As a result of the changes in this commit, we can also now simplify
the three tests that performed this extra setup step, where they
used to remove the file in the working tree which corresponded to
the object file they removed from the local Git LFS storage directories.
This step is no longer necessary to cause the client to abandon the
push operation after the server indicates that it requires an upload
of an object the client has determined is missing from the local system.
Therefore we can remove these extra setup steps from both of the
"push reject missing object (lfs.allowincompletepush false)*" tests
in the t/t-push-failures-local.sh script, and from the "pre-push
reject missing object (lfs.allowincompletepush default)" test in the
t/t-pre-push.sh script.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Apr 3, 2025
The original version of the ensureFile() method of the uploadContext
structure in our "commands" package was first introduced in commit
5d239d6 of PR git-lfs#176 in 2015, and has
been refactored a number of times since then, but continues to be called
for each object a push operation intends to upload.

The method is documented as a function that checks whether a Git LFS
object file exists in the local .git/lfs/objects storage directories
for a given pointer, and if it does not, tries to replace the missing
object file by passing the contents of the corresponding file in the
working tree through our "clean" filter.

The description of PR git-lfs#176 explains the function's purpose as follows,
using the original pre-release name for the Git LFS project:

  If the .git/hawser/objects directory gets into a weird state
  (for example, if the user manually removed some files in there),
  this attempts to re-clean the objects based on the git repository
  file path.

The code comments preceding the ensureFile() method also describe it
in the same way, as do the notes in later PRs, such as PR git-lfs#2574, in
which the "lfs.allowIncompletePush" configuration option was introduced,
and PR git-lfs#3398, which refined the error message the Git LFS client reports
during a push operation when an object is missing locally and is also
not present on the remote server.

However, the ensureFile() method has never actually replaced missing
object files under any circumstances.  It does check whether an object
file is missing from the local storage directories, and if not, tests
whether a file exists in the current working tree at the path of the
Git LFS pointer associated with the object.  If such a file exists,
the method proceeds to run the Clean() method of the GitFilter structure
in our "lfs" package on the file's contents.

The Clean() method calculates the SHA-256 hash value of the file's
contents and creates a Pointer structure containing this hash value,
and also writes a copy of the file's data into a temporary file in
the .git/lfs/tmp directory.  It is then the responsibility of the
caller to determine whether or not this temporary file should be
moved into place in the .git/lfs/objects directory hierarchy.

The only other caller of the Clean() method, besides the ensureFile()
method, is the clean() function in the "commands" package, which
is used by multiple Git LFS commands including the "git lfs clean"
and "git lfs filter-process" plumbing commands, as well as the
"git lfs migrate import" command.

The clean() function performs several tasks after invoking the Clean()
method.  First, it checks whether the file processed by the method
was found to contain a Git LFS pointer; if so, no further action is
taken as we assume the file in the working tree has not been passed
through our "smudge" filter, and we do not want to create another
pointer which simply hashes and references the existing one.

Next, the clean() function checks whether a file already exists in the
local .git/lfs/objects storage directories at the location into which
the function would otherwise expect to move the temporary file
created by the Clean() method.  If a file does exist in this location
and has the same size as the temporary file, no further action is
taken, as we assume it contains the same contents and does not need
to be updated.

Assuming neither of these checks causes the clean() function to
return early, the function moves the temporary file created by the
Clean() method into the expected location within the .git/lfs/objects
directory hierarchy.

Unfortunately, because the ensureFile() method invokes the Clean() method
of the GitFilter structure but never performs any of the subsequent steps
taken by the clean() function, it never recreates a missing Git LFS object
from a file found in the working tree.  This appears to have been the
case at the time the ensureFile() method was introduced in PR git-lfs#176,
and has remained so ever since.

We could attempt to remedy this situation by altering the ensureFile()
method so it calls the clean() function.  To do so it would need to
simulate the conditions under which the function usually runs,
specifically within the "clean" filter context where the function is
expected to transform an input data stream into an output data stream.
We would likely use a Discard structure from the "io" package of the
standard Go library to simply discard the output from the clean()
function, as we do not need to send it back to Git in the way the
"git lfs filter-process" or "git lfs clean" commands do.

However, we would have to add logic to the clean() function to guard
against the case where the file in the working tree had different
contents than those of the missing Git LFS object.  Because the
user may check out a Git reference in which a different file exists
at the same path in the working tree, or may simply modify the
file in the working tree independently, there is no guarantee that
the file we pass through the Clean() method is identical to the
one from which the missing Git LFS object was created.

The original implementation of the ensureFile() function, although it
did not fulfil its stated purpose, did include a check to verify
that the SHA hash of the working tree file, as returned by the Clean()
method, matched that of the missing object.  This check was removed in
commit 338ab40 of PR git-lfs#1812, which
would likely have introduced a serious bug, except that the ensureFile()
method never actually replaced any missing objects and so the removal
of this check had no functional impact.

While we could try to revise the ensureFile() method to operate as
was originally intended, the advantages of such a change are relatively
slim, and the disadvantages are several.  Most obviously, it requires
modifications to our clean() function to guard against the replacement
of object files with incorrect data, something the other callers of
the function do not need to be concerned about.

That this is a concern at all is in turn due to the reasonable
chance that a file found in the current working tree at a given path
does not contain the identical data as that of an Git LFS object
generated from another file previously located at the same path.

As well, the fact that the ensureFile() method has never worked as
designed, despite being repeatedly refactored and enhanced over ten
years, suggests that its purpose is somewhat obscure and that the
requisite logic is less intelligible than would be ideal.  Users and
developers expect push operations to involve the transfer of data but
not the creation (or re-creation) of local data files, so the use of
some of our "clean" filter code in such a context is not particularly
intuitive.

For all these reasons, we just remove the ensureFile() method entirely,
which simplifies our handling of missing objects during upload transfer
operations.  Instead, we check for the presence of each object file
we intend to push in the uploadTransfer() method of our uploadContext
structure, and if a file is not found in the local storage directories,
we flag it as missing, unless the "lfs.allowIncompletePush" configuration
option is set to "true".  We also use the IsNotExist() function from
the "os" package in the Go standard library to ascertain whether an
object file is missing, or if some other type of error prevents us
from reading its state.  This mirrors the more detailed checks performed
on each object file during a push operation by the partitionTransfers()
method of the TransferQueue structure in the "tq" package.

One consequence of this change is that when an object to be uploaded
is missing locally and is also not already present on the remote
server, and when the "lfs.allowIncompletePush" Git configuration
option is set to its default value of "false", we now always abandon
a push operation after the remote server indicates that it expects
the client to upload the object.

This means that in the "push reject missing object
(lfs.allowincompletepush default)*" tests in our
t/t-push-failures-local.sh test script, we should now expect
to find a trace log message output by the client stating that the
push operation's batch queue has been stopped because an object
is missing on both the local system and the remote server.

We added this trace log message in a prior commit in this PR, and were
able to insert checks for it in several other tests in our test suite,
but only because those tests either did not create a file in the working
tree at all, as in the case of the "pre-push reject missing object"
test in our t/t-pre-push.sh script, or removed the file in the working
tree that corresponded to the object file they removed from the
.git/lfs/objects storage directories.

As a result of the changes in this commit, we can also now simplify
the three tests that performed this extra setup step, where they
used to remove the file in the working tree which corresponded to
the object file they removed from the local Git LFS storage directories.
This step is no longer necessary to cause the client to abandon the
push operation after the server indicates that it requires an upload
of an object the client has determined is missing from the local system.
Therefore we can remove these extra setup steps from both of the
"push reject missing object (lfs.allowincompletepush false)*" tests
in the t/t-push-failures-local.sh script, and from the "pre-push
reject missing object (lfs.allowincompletepush default)" test in the
t/t-pre-push.sh script.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Apr 3, 2025
Since commit 1412d6e of PR git-lfs#3634,
during push operations the Git LFS client has sometimes avoided
reporting an error when an object to be pushed is missing locally
if the remote server reports that it has a copy already.

To implement this feature, a new Missing element was added to the
Transfer and objectTuple structures in our "tq" package, and the
Add() method of the TransferQueue structure in that package was
updated to accept an additional "missing" flag, which the method
uses to set the Missing element of the objectTuple structure it
creates and sends to the "incoming" channel.  Batches of objects to be
pushed are then gathered from this channel by the collectBatches()
method of the TransferQueue structure.

As batches of objectTuple structures are collected, they are passed
to the enqueueAndCollectRetriesFor() method, which converts them
to Transfer structures using the ToTransfers() method and then passes
them to the Batch() function, which is defined in our tq/api.go
source file.  This function initializes a batchRequest structure
which contains the set of Transfer structures as its Objects element,
and then passes those to the Batch() method specific to the current
batch transfer adapter's structure.  These Batch() methods return
a BatchResponse structure, which the Batch() function then returns
to the enqueueAndCollectRetriesFor() method.  The BatchResponse
structure also contains an Objects element which is another set
of Transfer structures that represent the per-object metadata
received from the remote server.

After the enqueueAndCollectRetriesFor() method receives a BatchResponse
during a push operation, if any of the Transfer structures in that
response define an upload action to be performed, this implies that
the remote server does not have a copy of those objects.

As one of the changes we made in PR git-lfs#3634, we introduced a step into
the enqueueAndCollectRetriesFor() method which halts the push operation
if the server's response indicates that the server lacks a copy of an
object, and if the "missing" value passed to the Add() method for that
object was set to "true".  (Initially, this step also decremented the
count of the number of objects waiting to be transferred, but this
created the potential for stalled push operations, and so another
approach to handling an early exit from the batch transfer process was
implemented in commit eb83fcd of
PR git-lfs#3800.)

Also in PR git-lfs#3634, several methods of the uploadContext structure in
our "commands" package were revised to set the "missing" value for
each object before calling the Add() method of the TransferQueue
structure.  Specifically, the ensureFile() method of the uploadContext
structure first checks for the presence of the object data file in
the .git/lfs/objects local storage directories.  If that does not
exist, then the method looks for a file in the working tree at the
path associated with the Git LFS pointer that corresponds to the
object.  If that file also does not exist, and if the
"lfs.allowIncompletePush" Git configuration option is set to "false",
then the method returns "true", and this value is ultimately used
for the "missing" argument in the call to the TransferQueue's
Add() method for the object.

Note that the file in the working tree may have any content, or be
entirely empty; its simple presence is enough to change the value
returned by the ensureFile() method, given the other conditions
described above.  We expect to revise this unintuitive behaviour in
a subsequent commit in this PR.

Before we make that change, however, we first adjust two aspects
of the implementation from PR git-lfs#3634 so as to simplify our handling
of missing objects during push operations.  We made one of these
adjustments in the previous commit in this PR, and we make the
other in this commit.

As noted above, the enqueueAndCollectRetriesFor() method halts a
push operation if the server's response indicates that the server lacks
a copy of an object, and if the "missing" value passed to the Add()
method for that object was set to "true".  When this occurs, the
enqueueAndCollectRetriesFor() method outputs an error message that
is distinct from the message which would otherwise be reported later,
when the partitionTransfers() method of the TransferQueue rechecks
whether individual objects' data files are present in the local
storage directories.

The message reported by the enqueueAndCollectRetriesFor() method
when it abandons a push operation states that the client was
"Unable to find source for object".  This message was originally
introduced in commit fea77e1 of
PR git-lfs#3398, and was generated by the ensureFile() method of the
uploadContext structure in our "commands" package, when that method
was unable to locate either an object file in the local storage
directories, or a corresponding file in the working tree at the path
of the object's Git LFS pointer.

As such, the wording of the message alludes to the expectation that
the ensureFile() method will try to recreate a missing object file
from a file in the working tree, i.e., from the object's "source".
This is how the method is described in the code comments that
precede it, and how it was intended to operate since it was first
added in PR git-lfs#176.

However, the ensureFile() has never actually recreated object files
in this way, due to an oversight in its implementation, and given
the challenges posed by the likelihood that files in the current
working tree do not correspond exactly to the source of missing Git LFS
object files, we expect to simply remove the ensureFile() method in
a subsequent commit in this PR.

In PR git-lfs#3634 the enqueueAndCollectRetriesFor() method of the TransferQueue
structure was revised to halt push operations if the server reports that
it requires the upload of an object for which the "missing" value
provided to the TransferQueue's Add() method was set to "true".  The
error message reported in such a case was copied from the message
formerly output by the ensureFile() method of the uploadContext
structure, and that method was altered so it no longer generated this
error message.

This error message is not the only one reported by the Git LFS client
when an object file is missing during a push operation, though, because
it is only output when the ensureFile() method does not find a file
(with any content) in the working at the path associated with the
object's pointer.  When a file does exist in the working tree, but
the actual object file in the Git LFS local storage directories is
missing, the push operation proceeds past the checks in the
enqueueAndCollectRetriesFor() method and continues to the point where
that method calls the addToAdapter() method of the TransferQueue
structure.  That method in turn calls the partitionTransfers() method
to determine which of the current batch of objects to be uploaded
have local object files present and which do not.

If the partitionTransfers() method finds that an object file is
missing for a given object, it creates a new MalformedObjectError
with the "missing" element of that error structure set to "true".
The method then instantiates a TransferResult structure for the object
and sets the Error element of the TransferResult to the new
MalformedObjectError.  Finally, the method returns this TransferResult
along with the other TransferResult structures it creates for all the
other objects in the batch.

The addToAdapter() method passes these TransferResult structures
individually to the handleTransferResult() method, which checks whether
the Error element is defined for the given TransferResult.  If an
error was encountered, and is one which indicates the object transfer
should not be retried, then the handleTransferResult() method sends
the error to the TransferQueue's "errorc" channel.  For errors of the
MalformedObjectError type, this is always the case.

During a push operation, the CollectErrors() method of the uploadContext
structure in the "commands" package receives these errors from the
channel, and if they are errors of the MalformedObjectError type and
have a "true" values in their "missing" element, the object's ID and
the filename associated with the object's pointer are recorded in the
"missing" map of the uploadContext structure.

When the ReportErrors() method of the uploadContext structure is then
run, it iterates over the keys and values of the "missing" map and
outputs an error message containing both the object ID and the
associated filename of the object's pointer, along with a "(missing)"
prefix.  As well, a leading error message is output, whose exact text
depends on the value of the "lfs.allowIncompletePush" configuration
option, and if this option is set to its default value of "false",
several trailing error messages are output which provide a hint as to
how to set that option if the user wants to allow a subsequent push
operation with missing objects to proceed to completion as best it can.

These per-object error messages were first defined in commit
9be11e8 of PR git-lfs#2082, and the trailing
hints were added in commit f5f5731 of
PR git-lfs#3109, at which time the "lfs.allowIncompletePush" option's default
values was changed to "false".

As mentioned above, in a subsequent commit in this PR we expect to
remove the ensureFile() method of the uploadContext structure.  We
still expect to perform a check for missing object files in the
uploadTransfer() method, though, as this will typically find any
missing object files at the start of a push operation, and thereby
allow the enqueueAndCollectRetriesFor() method of the TransferQueue
structure to halt the operation as soon as one of those objects is
found to be required by the remote server.

For the time being, we will leave the secondary check for missing
object files in place in the partitionTransfers() method of the
TransferQueue structure, as this method also tests the size of the
object file and reports those with unexpected sizes as corrupt.

Nevertheless, we would like to make our error messages as consistent
as possible when handling missing object files.  Therefore we revise
the enqueueAndCollectRetriesFor() method so it no longer returns the
"Unable to find source for object" error message, but instead returns a
new MalformedObjectError with a "true" value for its "missing" element.

One advantage of this change is that we remove the somewhat stale
wording of the previous message, which reflected the assumption that
the ensureFile() method of the uploadContext structure would attempt
to recreate missing object files from "source" files in the working
tree, even though the method has never actually done so.

Another advantage is that by returning a MalformedObjectError, the
existing logic of the CollectErrors() and ReportErrors() methods of the
uploadContext structure will handle the error exactly as if it had been
generated by the TransferQueue's partitionTransfers() method, and will
output both the same leading error message and trailing hint messages
as in that case.

As a result, we also adjust several tests in our t/t-pre-push.sh
and t/t-push-failures-local.sh scripts to expect the error messages
output by the ReportErrors() method instead of the message previously
generated by the enqueueAndCollectRetriesFor() method.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Apr 3, 2025
The original version of the ensureFile() method of the uploadContext
structure in our "commands" package was first introduced in commit
5d239d6 of PR git-lfs#176 in 2015, and has
been refactored a number of times since then, but continues to be called
for each object a push operation intends to upload.

The method is documented as a function that checks whether a Git LFS
object file exists in the local .git/lfs/objects storage directories
for a given pointer, and if it does not, tries to replace the missing
object file by passing the contents of the corresponding file in the
working tree through our "clean" filter.

The description of PR git-lfs#176 explains the function's purpose as follows,
using the original pre-release name for the Git LFS project:

  If the .git/hawser/objects directory gets into a weird state
  (for example, if the user manually removed some files in there),
  this attempts to re-clean the objects based on the git repository
  file path.

The code comments preceding the ensureFile() method also describe it
in the same way, as do the notes in later PRs, such as PR git-lfs#2574, in
which the "lfs.allowIncompletePush" configuration option was introduced,
and PR git-lfs#3398, which refined the error message the Git LFS client reports
during a push operation when an object is missing locally and is also
not present on the remote server.

However, the ensureFile() method has never actually replaced missing
object files under any circumstances.  It does check whether an object
file is missing from the local storage directories, and if not, tests
whether a file exists in the current working tree at the path of the
Git LFS pointer associated with the object.  If such a file exists,
the method proceeds to run the Clean() method of the GitFilter structure
in our "lfs" package on the file's contents.

The Clean() method calculates the SHA-256 hash value of the file's
contents and creates a Pointer structure containing this hash value,
and also writes a copy of the file's data into a temporary file in
the .git/lfs/tmp directory.  It is then the responsibility of the
caller to determine whether or not this temporary file should be
moved into place in the .git/lfs/objects directory hierarchy.

The only other caller of the Clean() method, besides the ensureFile()
method, is the clean() function in the "commands" package, which
is used by multiple Git LFS commands including the "git lfs clean"
and "git lfs filter-process" plumbing commands, as well as the
"git lfs migrate import" command.

The clean() function performs several tasks after invoking the Clean()
method.  First, it checks whether the file processed by the method
was found to contain a Git LFS pointer; if so, no further action is
taken as we assume the file in the working tree has not been passed
through our "smudge" filter, and we do not want to create another
pointer which simply hashes and references the existing one.

Next, the clean() function checks whether a file already exists in the
local .git/lfs/objects storage directories at the location into which
the function would otherwise expect to move the temporary file
created by the Clean() method.  If a file does exist in this location
and has the same size as the temporary file, no further action is
taken, as we assume it contains the same contents and does not need
to be updated.

Assuming neither of these checks causes the clean() function to
return early, the function moves the temporary file created by the
Clean() method into the expected location within the .git/lfs/objects
directory hierarchy.

Unfortunately, because the ensureFile() method invokes the Clean() method
of the GitFilter structure but never performs any of the subsequent steps
taken by the clean() function, it never recreates a missing Git LFS object
from a file found in the working tree.  This appears to have been the
case at the time the ensureFile() method was introduced in PR git-lfs#176,
and has remained so ever since.

We could attempt to remedy this situation by altering the ensureFile()
method so it calls the clean() function.  To do so it would need to
simulate the conditions under which the function usually runs,
specifically within the "clean" filter context where the function is
expected to transform an input data stream into an output data stream.
We would likely use a Discard structure from the "io" package of the
standard Go library to simply discard the output from the clean()
function, as we do not need to send it back to Git in the way the
"git lfs filter-process" or "git lfs clean" commands do.

However, we would have to add logic to the clean() function to guard
against the case where the file in the working tree had different
contents than those of the missing Git LFS object.  Because the
user may check out a Git reference in which a different file exists
at the same path in the working tree, or may simply modify the
file in the working tree independently, there is no guarantee that
the file we pass through the Clean() method is identical to the
one from which the missing Git LFS object was created.

The original implementation of the ensureFile() function, although it
did not fulfil its stated purpose, did include a check to verify
that the SHA hash of the working tree file, as returned by the Clean()
method, matched that of the missing object.  This check was removed in
commit 338ab40 of PR git-lfs#1812, which
would likely have introduced a serious bug, except that the ensureFile()
method never actually replaced any missing objects and so the removal
of this check had no functional impact.

While we could try to revise the ensureFile() method to operate as
was originally intended, the advantages of such a change are relatively
slim, and the disadvantages are several.  Most obviously, it requires
modifications to our clean() function to guard against the replacement
of object files with incorrect data, something the other callers of
the function do not need to be concerned about.

That this is a concern at all is in turn due to the reasonable
chance that a file found in the current working tree at a given path
does not contain the identical data as that of an Git LFS object
generated from another file previously located at the same path.

As well, the fact that the ensureFile() method has never worked as
designed, despite being repeatedly refactored and enhanced over ten
years, suggests that its purpose is somewhat obscure and that the
requisite logic is less intelligible than would be ideal.  Users and
developers expect push operations to involve the transfer of data but
not the creation (or re-creation) of local data files, so the use of
some of our "clean" filter code in such a context is not particularly
intuitive.

For all these reasons, we just remove the ensureFile() method entirely,
which simplifies our handling of missing objects during upload transfer
operations.  Instead, we check for the presence of each object file
we intend to push in the uploadTransfer() method of our uploadContext
structure, and if a file is not found in the local storage directories,
we flag it as missing, unless the "lfs.allowIncompletePush" configuration
option is set to "true".  We also use the IsNotExist() function from
the "os" package in the Go standard library to ascertain whether an
object file is missing, or if some other type of error prevents us
from reading its state.  This mirrors the more detailed checks performed
on each object file during a push operation by the partitionTransfers()
method of the TransferQueue structure in the "tq" package.

One consequence of this change is that when an object to be uploaded
is missing locally and is also not already present on the remote
server, and when the "lfs.allowIncompletePush" Git configuration
option is set to its default value of "false", we now always abandon
a push operation after the remote server indicates that it expects
the client to upload the object.

This means that in the "push reject missing object
(lfs.allowincompletepush default)*" tests in our
t/t-push-failures-local.sh test script, we should now expect
to find a trace log message output by the client stating that the
push operation's batch queue has been stopped because an object
is missing on both the local system and the remote server.

We added this trace log message in a prior commit in this PR, and were
able to insert checks for it in several other tests in our test suite,
but only because those tests either did not create a file in the working
tree at all, as in the case of the "pre-push reject missing object"
test in our t/t-pre-push.sh script, or removed the file in the working
tree that corresponded to the object file they removed from the
.git/lfs/objects storage directories.

As a result of the changes in this commit, we can also now simplify
the three tests that performed this extra setup step, where they
used to remove the file in the working tree which corresponded to
the object file they removed from the local Git LFS storage directories.
This step is no longer necessary to cause the client to abandon the
push operation after the server indicates that it requires an upload
of an object the client has determined is missing from the local system.
Therefore we can remove these extra setup steps from both of the
"push reject missing object (lfs.allowincompletepush false)*" tests
in the t/t-push-failures-local.sh script, and from the "pre-push
reject missing object (lfs.allowincompletepush default)" test in the
t/t-pre-push.sh script.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request May 22, 2025
Since commit 1412d6e of PR git-lfs#3634,
during push operations the Git LFS client has sometimes avoided
reporting an error when an object to be pushed is missing locally
if the remote server reports that it has a copy already.

To implement this feature, a new Missing element was added to the
Transfer and objectTuple structures in our "tq" package, and the
Add() method of the TransferQueue structure in that package was
updated to accept an additional "missing" flag, which the method
uses to set the Missing element of the objectTuple structure it
creates and sends to the "incoming" channel.  Batches of objects to be
pushed are then gathered from this channel by the collectBatches()
method of the TransferQueue structure.

As batches of objectTuple structures are collected, they are passed
to the enqueueAndCollectRetriesFor() method, which converts them
to Transfer structures using the ToTransfers() method and then passes
them to the Batch() function, which is defined in our tq/api.go
source file.  This function initializes a batchRequest structure
which contains the set of Transfer structures as its Objects element,
and then passes those to the Batch() method specific to the current
batch transfer adapter's structure.  These Batch() methods return
a BatchResponse structure, which the Batch() function then returns
to the enqueueAndCollectRetriesFor() method.  The BatchResponse
structure also contains an Objects element which is another set
of Transfer structures that represent the per-object metadata
received from the remote server.

After the enqueueAndCollectRetriesFor() method receives a BatchResponse
during a push operation, if any of the Transfer structures in that
response define an upload action to be performed, this implies that
the remote server does not have a copy of those objects.

As one of the changes we made in PR git-lfs#3634, we introduced a step into
the enqueueAndCollectRetriesFor() method which halts the push operation
if the server's response indicates that the server lacks a copy of an
object, and if the "missing" value passed to the Add() method for that
object was set to "true".  (Initially, this step also decremented the
count of the number of objects waiting to be transferred, but this
created the potential for stalled push operations, and so another
approach to handling an early exit from the batch transfer process was
implemented in commit eb83fcd of
PR git-lfs#3800.)

Also in PR git-lfs#3634, several methods of the uploadContext structure in
our "commands" package were revised to set the "missing" value for
each object before calling the Add() method of the TransferQueue
structure.  Specifically, the ensureFile() method of the uploadContext
structure first checks for the presence of the object data file in
the .git/lfs/objects local storage directories.  If that does not
exist, then the method looks for a file in the working tree at the
path associated with the Git LFS pointer that corresponds to the
object.  If that file also does not exist, and if the
"lfs.allowIncompletePush" Git configuration option is set to "false",
then the method returns "true", and this value is ultimately used
for the "missing" argument in the call to the TransferQueue's
Add() method for the object.

Note that the file in the working tree may have any content, or be
entirely empty; its simple presence is enough to change the value
returned by the ensureFile() method, given the other conditions
described above.  We expect to revise this unintuitive behaviour in
a subsequent commit in this PR.

Before we make that change, however, we first adjust two aspects
of the implementation from PR git-lfs#3634 so as to simplify our handling
of missing objects during push operations.  We made one of these
adjustments in the previous commit in this PR, and we make the
other in this commit.

As noted above, the enqueueAndCollectRetriesFor() method halts a
push operation if the server's response indicates that the server lacks
a copy of an object, and if the "missing" value passed to the Add()
method for that object was set to "true".  When this occurs, the
enqueueAndCollectRetriesFor() method outputs an error message that
is distinct from the message which would otherwise be reported later,
when the partitionTransfers() method of the TransferQueue rechecks
whether individual objects' data files are present in the local
storage directories.

The message reported by the enqueueAndCollectRetriesFor() method
when it abandons a push operation states that the client was
"Unable to find source for object".  This message was originally
introduced in commit fea77e1 of
PR git-lfs#3398, and was generated by the ensureFile() method of the
uploadContext structure in our "commands" package, when that method
was unable to locate either an object file in the local storage
directories, or a corresponding file in the working tree at the path
of the object's Git LFS pointer.

As such, the wording of the message alludes to the expectation that
the ensureFile() method will try to recreate a missing object file
from a file in the working tree, i.e., from the object's "source".
This is how the method is described in the code comments that
precede it, and how it was intended to operate since it was first
added in PR git-lfs#176.

However, the ensureFile() has never actually recreated object files
in this way, due to an oversight in its implementation, and given
the challenges posed by the likelihood that files in the current
working tree do not correspond exactly to the source of missing Git LFS
object files, we expect to simply remove the ensureFile() method in
a subsequent commit in this PR.

In PR git-lfs#3634 the enqueueAndCollectRetriesFor() method of the TransferQueue
structure was revised to halt push operations if the server reports that
it requires the upload of an object for which the "missing" value
provided to the TransferQueue's Add() method was set to "true".  The
error message reported in such a case was copied from the message
formerly output by the ensureFile() method of the uploadContext
structure, and that method was altered so it no longer generated this
error message.

This error message is not the only one reported by the Git LFS client
when an object file is missing during a push operation, though, because
it is only output when the ensureFile() method does not find a file
(with any content) in the working at the path associated with the
object's pointer.  When a file does exist in the working tree, but
the actual object file in the Git LFS local storage directories is
missing, the push operation proceeds past the checks in the
enqueueAndCollectRetriesFor() method and continues to the point where
that method calls the addToAdapter() method of the TransferQueue
structure.  That method in turn calls the partitionTransfers() method
to determine which of the current batch of objects to be uploaded
have local object files present and which do not.

If the partitionTransfers() method finds that an object file is
missing for a given object, it creates a new MalformedObjectError
with the "missing" element of that error structure set to "true".
The method then instantiates a TransferResult structure for the object
and sets the Error element of the TransferResult to the new
MalformedObjectError.  Finally, the method returns this TransferResult
along with the other TransferResult structures it creates for all the
other objects in the batch.

The addToAdapter() method passes these TransferResult structures
individually to the handleTransferResult() method, which checks whether
the Error element is defined for the given TransferResult.  If an
error was encountered, and is one which indicates the object transfer
should not be retried, then the handleTransferResult() method sends
the error to the TransferQueue's "errorc" channel.  For errors of the
MalformedObjectError type, this is always the case.

During a push operation, the CollectErrors() method of the uploadContext
structure in the "commands" package receives these errors from the
channel, and if they are errors of the MalformedObjectError type and
have a "true" values in their "missing" element, the object's ID and
the filename associated with the object's pointer are recorded in the
"missing" map of the uploadContext structure.

When the ReportErrors() method of the uploadContext structure is then
run, it iterates over the keys and values of the "missing" map and
outputs an error message containing both the object ID and the
associated filename of the object's pointer, along with a "(missing)"
prefix.  As well, a leading error message is output, whose exact text
depends on the value of the "lfs.allowIncompletePush" configuration
option, and if this option is set to its default value of "false",
several trailing error messages are output which provide a hint as to
how to set that option if the user wants to allow a subsequent push
operation with missing objects to proceed to completion as best it can.

These per-object error messages were first defined in commit
9be11e8 of PR git-lfs#2082, and the trailing
hints were added in commit f5f5731 of
PR git-lfs#3109, at which time the "lfs.allowIncompletePush" option's default
values was changed to "false".

As mentioned above, in a subsequent commit in this PR we expect to
remove the ensureFile() method of the uploadContext structure.  We
still expect to perform a check for missing object files in the
uploadTransfer() method, though, as this will typically find any
missing object files at the start of a push operation, and thereby
allow the enqueueAndCollectRetriesFor() method of the TransferQueue
structure to halt the operation as soon as one of those objects is
found to be required by the remote server.

For the time being, we will leave the secondary check for missing
object files in place in the partitionTransfers() method of the
TransferQueue structure, as this method also tests the size of the
object file and reports those with unexpected sizes as corrupt.

Nevertheless, we would like to make our error messages as consistent
as possible when handling missing object files.  Therefore we revise
the enqueueAndCollectRetriesFor() method so it no longer returns the
"Unable to find source for object" error message, but instead returns a
new MalformedObjectError with a "true" value for its "missing" element.

One advantage of this change is that we remove the somewhat stale
wording of the previous message, which reflected the assumption that
the ensureFile() method of the uploadContext structure would attempt
to recreate missing object files from "source" files in the working
tree, even though the method has never actually done so.

Another advantage is that by returning a MalformedObjectError, the
existing logic of the CollectErrors() and ReportErrors() methods of the
uploadContext structure will handle the error exactly as if it had been
generated by the TransferQueue's partitionTransfers() method, and will
output both the same leading error message and trailing hint messages
as in that case.

As a result, we also adjust several tests in our t/t-pre-push.sh
and t/t-push-failures-local.sh scripts to expect the error messages
output by the ReportErrors() method instead of the message previously
generated by the enqueueAndCollectRetriesFor() method.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request May 22, 2025
The original version of the ensureFile() method of the uploadContext
structure in our "commands" package was first introduced in commit
5d239d6 of PR git-lfs#176 in 2015, and has
been refactored a number of times since then, but continues to be called
for each object a push operation intends to upload.

The method is documented as a function that checks whether a Git LFS
object file exists in the local .git/lfs/objects storage directories
for a given pointer, and if it does not, tries to replace the missing
object file by passing the contents of the corresponding file in the
working tree through our "clean" filter.

The description of PR git-lfs#176 explains the function's purpose as follows,
using the original pre-release name for the Git LFS project:

  If the .git/hawser/objects directory gets into a weird state
  (for example, if the user manually removed some files in there),
  this attempts to re-clean the objects based on the git repository
  file path.

The code comments preceding the ensureFile() method also describe it
in the same way, as do the notes in later PRs, such as PR git-lfs#2574, in
which the "lfs.allowIncompletePush" configuration option was introduced,
and PR git-lfs#3398, which refined the error message the Git LFS client reports
during a push operation when an object is missing locally and is also
not present on the remote server.

However, the ensureFile() method has never actually replaced missing
object files under any circumstances.  It does check whether an object
file is missing from the local storage directories, and if not, tests
whether a file exists in the current working tree at the path of the
Git LFS pointer associated with the object.  If such a file exists,
the method proceeds to run the Clean() method of the GitFilter structure
in our "lfs" package on the file's contents.

The Clean() method calculates the SHA-256 hash value of the file's
contents and creates a Pointer structure containing this hash value,
and also writes a copy of the file's data into a temporary file in
the .git/lfs/tmp directory.  It is then the responsibility of the
caller to determine whether or not this temporary file should be
moved into place in the .git/lfs/objects directory hierarchy.

The only other caller of the Clean() method, besides the ensureFile()
method, is the clean() function in the "commands" package, which
is used by multiple Git LFS commands including the "git lfs clean"
and "git lfs filter-process" plumbing commands, as well as the
"git lfs migrate import" command.

The clean() function performs several tasks after invoking the Clean()
method.  First, it checks whether the file processed by the method
was found to contain a Git LFS pointer; if so, no further action is
taken as we assume the file in the working tree has not been passed
through our "smudge" filter, and we do not want to create another
pointer which simply hashes and references the existing one.

Next, the clean() function checks whether a file already exists in the
local .git/lfs/objects storage directories at the location into which
the function would otherwise expect to move the temporary file
created by the Clean() method.  If a file does exist in this location
and has the same size as the temporary file, no further action is
taken, as we assume it contains the same contents and does not need
to be updated.

Assuming neither of these checks causes the clean() function to
return early, the function moves the temporary file created by the
Clean() method into the expected location within the .git/lfs/objects
directory hierarchy.

Unfortunately, because the ensureFile() method invokes the Clean() method
of the GitFilter structure but never performs any of the subsequent steps
taken by the clean() function, it never recreates a missing Git LFS object
from a file found in the working tree.  This appears to have been the
case at the time the ensureFile() method was introduced in PR git-lfs#176,
and has remained so ever since.

We could attempt to remedy this situation by altering the ensureFile()
method so it calls the clean() function.  To do so it would need to
simulate the conditions under which the function usually runs,
specifically within the "clean" filter context where the function is
expected to transform an input data stream into an output data stream.
We would likely use a Discard structure from the "io" package of the
standard Go library to simply discard the output from the clean()
function, as we do not need to send it back to Git in the way the
"git lfs filter-process" or "git lfs clean" commands do.

However, we would have to add logic to the clean() function to guard
against the case where the file in the working tree had different
contents than those of the missing Git LFS object.  Because the
user may check out a Git reference in which a different file exists
at the same path in the working tree, or may simply modify the
file in the working tree independently, there is no guarantee that
the file we pass through the Clean() method is identical to the
one from which the missing Git LFS object was created.

The original implementation of the ensureFile() function, although it
did not fulfil its stated purpose, did include a check to verify
that the SHA hash of the working tree file, as returned by the Clean()
method, matched that of the missing object.  This check was removed in
commit 338ab40 of PR git-lfs#1812, which
would likely have introduced a serious bug, except that the ensureFile()
method never actually replaced any missing objects and so the removal
of this check had no functional impact.

While we could try to revise the ensureFile() method to operate as
was originally intended, the advantages of such a change are relatively
slim, and the disadvantages are several.  Most obviously, it requires
modifications to our clean() function to guard against the replacement
of object files with incorrect data, something the other callers of
the function do not need to be concerned about.

That this is a concern at all is in turn due to the reasonable
chance that a file found in the current working tree at a given path
does not contain the identical data as that of an Git LFS object
generated from another file previously located at the same path.

As well, the fact that the ensureFile() method has never worked as
designed, despite being repeatedly refactored and enhanced over ten
years, suggests that its purpose is somewhat obscure and that the
requisite logic is less intelligible than would be ideal.  Users and
developers expect push operations to involve the transfer of data but
not the creation (or re-creation) of local data files, so the use of
some of our "clean" filter code in such a context is not particularly
intuitive.

For all these reasons, we just remove the ensureFile() method entirely,
which simplifies our handling of missing objects during upload transfer
operations.  Instead, we check for the presence of each object file
we intend to push in the uploadTransfer() method of our uploadContext
structure, and if a file is not found in the local storage directories,
we flag it as missing, unless the "lfs.allowIncompletePush" configuration
option is set to "true".  We also use the IsNotExist() function from
the "os" package in the Go standard library to ascertain whether an
object file is missing, or if some other type of error prevents us
from reading its state.  This mirrors the more detailed checks performed
on each object file during a push operation by the partitionTransfers()
method of the TransferQueue structure in the "tq" package.

One consequence of this change is that when an object to be uploaded
is missing locally and is also not already present on the remote
server, and when the "lfs.allowIncompletePush" Git configuration
option is set to its default value of "false", we now always abandon
a push operation after the remote server indicates that it expects
the client to upload the object.

This means that in the "push reject missing object
(lfs.allowincompletepush default)*" tests in our
t/t-push-failures-local.sh test script, we should now expect
to find a trace log message output by the client stating that the
push operation's batch queue has been stopped because an object
is missing on both the local system and the remote server.

We added this trace log message in a prior commit in this PR, and were
able to insert checks for it in several other tests in our test suite,
but only because those tests either did not create a file in the working
tree at all, as in the case of the "pre-push reject missing object"
test in our t/t-pre-push.sh script, or removed the file in the working
tree that corresponded to the object file they removed from the
.git/lfs/objects storage directories.

As a result of the changes in this commit, we can also now simplify
the three tests that performed this extra setup step, where they
used to remove the file in the working tree which corresponded to
the object file they removed from the local Git LFS storage directories.
This step is no longer necessary to cause the client to abandon the
push operation after the server indicates that it requires an upload
of an object the client has determined is missing from the local system.
Therefore we can remove these extra setup steps from both of the
"push reject missing object (lfs.allowincompletepush false)*" tests
in the t/t-push-failures-local.sh script, and from the "pre-push
reject missing object (lfs.allowincompletepush default)" test in the
t/t-pre-push.sh script.
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.

1 participant