Skip to content

change lfs.allowincompletepush default from true to false #3109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Jul 5, 2018
Merged

change lfs.allowincompletepush default from true to false #3109

merged 3 commits into from
Jul 5, 2018

Conversation

@larsxschneider
Copy link
Member

@larsxschneider larsxschneider commented Jul 5, 2018

Incomplete pushes can corrupt a repository. Therefore we should not allow them and fail hard by default.

See #3105

@larsxschneider larsxschneider requested a review from ttaylorr July 5, 2018 17:27
@ttaylorr ttaylorr mentioned this pull request Jul 5, 2018
Closed
@larsxschneider larsxschneider changed the title cahnge lfs.allowincompletepush default from true to false change lfs.allowincompletepush default from true to false Jul 5, 2018
@larsxschneider larsxschneider force-pushed the ls/check-push branch 2 times, most recently from e439315 to 4357f53 Compare July 5, 2018 18:04
ttaylorr
ttaylorr reviewed Jul 5, 2018
View reviewed changes
Copy link
Contributor

@ttaylorr ttaylorr left a comment

Choose a reason for hiding this comment

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

@larsxschneider this looks great to me.

I am glad that you added a hint, since this will break pushes from users that did not have the objects that they were trying to push, and had not explicitly configured lfs.allowincompletepush to a true-like value.

That's a breaking change, but I'm OK with it provided that we retain this message as an easy way for a user to recover from a failed push.

I left one nit for your consideration.

commands/uploader.go Outdated
}

if !c.allowMissing {
Print("hint: Your push was rejected due to missing or corrupt local objects.\n" +
Copy link
Contributor

@ttaylorr ttaylorr Jul 5, 2018

Choose a reason for hiding this comment

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

I like this message, and I think that it's good that we're only showing it once at the end of the attempted push, right before calling os.Exit.

If you don't mind, I think that we could extract this message out to a var and print it by referencing that instead of having a string constant in the code. We use this pattern in the clone and checkout commands, which have deprecation notices.

I was thinking something like:

var (
        pushMissingHint = []string{...}
)

if !c.allowMissing {
        Print(strings.Join(pushMissingHint, "\n"))
}
os.Exit(2)

Copy link
Member Author

@larsxschneider larsxschneider Jul 5, 2018

Choose a reason for hiding this comment

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

Agreed and implemented 👍

ttaylorr
ttaylorr approved these changes Jul 5, 2018
View reviewed changes
Copy link
Contributor

@ttaylorr ttaylorr left a comment

Choose a reason for hiding this comment

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

🎉

@larsxschneider larsxschneider reopened this Jul 5, 2018
@larsxschneider
test: refactor test-push-* and move tests to test-push-failures-local
4d52e08 
`test-push-missing.sh` tested two error conditions (missing and corrupt)
with one `git push` exit code check. Split the test into two tests to
ensure both errors cause a `git push` error exit.

`test-push.sh` contained tests similar to `test-push-missing.sh`.
Group them all together in the new test file.
@larsxschneider
test: rename test-push-failures to test-push-failures-remote
9f2e649
terrorobe
terrorobe reviewed Jul 5, 2018
View reviewed changes
commands/uploader.go Outdated
if !c.allowMissing {
pushMissingHint := []string{
"hint: Your push was rejected due to missing or corrupt local objects.",
"hint: Overwrite the reject with `git config lfs.allowincompletepush true`.",
Copy link
Contributor

@terrorobe terrorobe Jul 5, 2018

Choose a reason for hiding this comment

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

Wording nit: Maybe something along the lines of

hint: You can disable this check using...

Copy link
Member Author

@larsxschneider larsxschneider Jul 5, 2018

Choose a reason for hiding this comment

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

Fixed 👍
Following:

git-lfs/commands/lockverifier.go

Line 66 in f5f5731

Print("Remote %q does not support the LFS locking API. Consider disabling it with:", cfg.PushRemote())

@larsxschneider
commands/uploader: set lfs.allowincompletepush default to false
f5f5731 
Incomplete pushes can corrupt a repository. Therefore we should not
allow them and fail hard by default.

If the situation occurs, then print a hint to the user and explain how
to overwrite the hard failure using the `lfs.allowincompletepush` config.
@ttaylorr ttaylorr merged commit 5e601f8 into master Jul 5, 2018
@ttaylorr ttaylorr deleted the ls/check-push branch July 5, 2018 21:30
ttaylorr added a commit that referenced this pull request Jul 6, 2018
@ttaylorr
config/git_fetcher.go: mark 'lfs.allowincompletepush' as safe
d356482 
In config/git_fetcher.go, we have special rules to mark certain Git
configuration keys as safe or unsafe.

Particularly, we apply these rules strictly to all values fetched from
the repository-local .lfsconfig, since it is distributed to all fetchers
of a repository and provides a widely available attack-vector.
Therefore, we only allow a tiny subset of all keys via the keyIsUnsafe
function and safeKeys slice.

The 'lfs.allowincompletepush' configuration determines whether or not a
user should be allowed to push a repository that has missing LFS objects
to a remote.

Previously, it was ignored with the following message:

    $ git push repo_clone repo_orig_master
    WARNING: These unsafe lfsconfig keys were ignored:

      lfs.allowincompletepush

In [1], Lars changed the default from 'true' to 'false'. In order to
allow users or repository maintainers to retain the prior behavior,
let's let them distribute this configuration from the repository's
.lfsconfig and thusly mark it as safe.

[1]: #3109
@ttaylorr ttaylorr mentioned this pull request Jul 6, 2018
Merged
dhiwakarK pushed a commit to dhiwakarK/expert-octo-doodle that referenced this pull request Nov 3, 2022
@ttaylorr
config/git_fetcher.go: mark 'lfs.allowincompletepush' as safe
44361ae 
In config/git_fetcher.go, we have special rules to mark certain Git
configuration keys as safe or unsafe.

Particularly, we apply these rules strictly to all values fetched from
the repository-local .lfsconfig, since it is distributed to all fetchers
of a repository and provides a widely available attack-vector.
Therefore, we only allow a tiny subset of all keys via the keyIsUnsafe
function and safeKeys slice.

The 'lfs.allowincompletepush' configuration determines whether or not a
user should be allowed to push a repository that has missing LFS objects
to a remote.

Previously, it was ignored with the following message:

    $ git push repo_clone repo_orig_master
    WARNING: These unsafe lfsconfig keys were ignored:

      lfs.allowincompletepush

In [1], Lars changed the default from 'true' to 'false'. In order to
allow users or repository maintainers to retain the prior behavior,
let's let them distribute this configuration from the repository's
.lfsconfig and thusly mark it as safe.

[1]: git-lfs/git-lfs#3109


Former-commit-id: d3564823abf16517a1b40a905c8f0e968a9ed410
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Mar 28, 2025
@chrisd8088
t: align missing/bad object push exit code checks
2753452 
Our t/t-pre-push.sh and t/t-push-failures-local.sh test scripts include
a number of tests which validate the behaviour of the "git lfs pre-push"
and "git push" commands when Git LFS objects are missing or corrupt.

While these tests are largely similar in their implementation, they
vary in a number of formatting and implementation details unrelated to
the specifics of the different conditions they simulate.  These
variations are artifacts of the evolution of our test suite over time;
for example, the tests in the t/t-push-failures-local.sh script were
refactored and collected from several earlier tests in commit
4d52e08 of PR git-lfs#3109, and the original
versions of the tests in the t/t-pre-push.sh script were added
incrementally in PRs git-lfs#447, git-lfs#2199, and git-lfs#2574.

In a subsequent commit in this PR we expect to update the Git LFS
client to remove some non-functional code which attempts to recreate
missing Git LFS objects during push operations.  In many cases this
change will cause the client to report missing objects in an earlier
stage of push operations than it does now.  We also expect to reword
the error message the client will output in such cases.

Before we make these changes, we first revise the related tests in our
test suite so they are as simple and similar as possible.  This will
ensure that when we update the Git LFS client we can clearly identify
the changes that we need to make in our tests to accommodate the
client's new behaviour.

In the "pre-push with missing pointer not on server" test in the
t/t-pre-push.sh script we remove the use of the "set +e" shell option,
which is not necessary to avoid test failure.  Although we expect the
"git lfs pre-push" command to fail, its output is piped into the tee(1)
command, and when the "set -e" option is in effect the shell will exit
immediately if the last command in a pipeline returns a non-zero exit
code, but not if other commands in the pipeline return such a code.

We do, however, add a check that the "git lfs pre-push" command exits
with a non-zero code by testing the appropriate PIPESTATUS array value,
following the example of the other related tests.

These other tests include the "pre-push with missing and present pointers
(lfs.allowincompletepush true)" and "pre-push reject missing pointers
(lfs.allowincompletepush default)" tests in the t/t-pre-push.sh script,
plus the four tests in the t/t-push-failures-local.sh script.

In all these tests we adjust the error messages generated in the case
that a "git lfs pre-push" or "git push" command fails or succeeds
unexpectedly.  We rewrite these messages so they are consistent with
each other and with those found in many of our other test scripts.

Note that in the "push reject missing objects (lfs.allowincompletepush
false)" test we also correct the message reported if the "git push"
command were to succeed unexpectedly.  At present, the message states
that we expect the command to succeed, but we actually expect it to fail.

Next, in the "push reject missing objects (lfs.allowincompletepush
default)" and "push reject corrupt objects (lfs.allowincompletepush
default)" tests, we update our checks of the "git push" command's exit
code so that we confirm the command exits with a specific non-zero exit
code of 1 rather than simply checking that its exit code is not zero.
This change brings these checks into alignment with those made by the
other related tests.

Lastly, we remove and adjust some whitespace so that these tests
are all formatted in a similar manner to each other.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Mar 28, 2025
@chrisd8088
t: align missing object push test deletion steps
6dc0698 
Our t/t-pre-push.sh and t/t-push-failures-local.sh test scripts include
a number of tests which validate the behaviour of the "git lfs pre-push"
and "git push" commands when Git LFS objects are missing or corrupt.

While these tests are largely similar in their implementation, they
vary in a number of formatting and implementation details unrelated to
the specifics of the different conditions they simulate.  These
variations are artifacts of the evolution of our test suite over time;
for example, the tests in the t/t-push-failures-local.sh script were
refactored and collected from several earlier tests in commit
4d52e08 of PR git-lfs#3109, and the original
versions of the tests in the t/t-pre-push.sh script were added
incrementally in PRs git-lfs#447, git-lfs#2199, and git-lfs#2574.

In a subsequent commit in this PR we expect to update the Git LFS
client to remove some non-functional code which attempts to recreate
missing Git LFS objects during push operations.  In many cases this
change will cause the client to report missing objects in an earlier
stage of push operations than it does now.  We also expect to reword
the error message the client will output in such cases.

Before we make these changes, we first revise the related tests in our
test suite so they are as simple and similar as possible.  This will
ensure that when we update the Git LFS client we can clearly identify
the changes that we need to make in our tests to accommodate the
client's new behaviour.

In a pair of tests in t/t-pre-push.sh script, and in another pair of
tests in the t/t-push-failures-local.sh script, the tests' initial
setup code creates several Git LFS objects and then removes the object
file in the .git/lfs/objects directory hierarchy for one of them.

In each case, we can replace this code with a simple call to our
delete_local_object() test helper function, which performs the
equivalent action of removing an object's file from the internal
Git LFS storage directories.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Mar 28, 2025
@chrisd8088
t/t-push-failures-local.sh: drop extra assertions
1ec1a66 
Our t/t-push-failures-local.sh test script includes a number of tests
which validate the behaviour of the "git push" command when Git LFS
objects are missing or corrupt.

While these tests are largely similar in their implementation, they
vary in a number of formatting and implementation details unrelated to
the specifics of the different conditions they simulate.  These
variations are artifacts of the evolution of our test suite over time;
for example, the tests in the t/t-push-failures-local.sh script were
refactored and collected from several earlier tests in commit
4d52e08 of PR git-lfs#3109.

In a subsequent commit in this PR we expect to update the Git LFS
client to remove some non-functional code which attempts to recreate
missing Git LFS objects during push operations.  In many cases this
change will cause the client to report missing objects in an earlier
stage of push operations than it does now.  We also expect to reword
the error message the client will output in such cases.

Before we make these changes, we first revise the related tests in our
test suite so they are as simple and similar as possible.  This will
ensure that when we update the Git LFS client we can clearly identify
the changes that we need to make in our tests to accommodate the
client's new behaviour.

A pair of tests in the t/t-push-failures-local.sh script, specifically
the "push reject missing objects (lfs.allowincompletepush default)" and
"push reject corrupt objects (lfs.allowincompletepush default)" tests,
perform a number of additional assertions beyond those performed by
the other tests in the same script.

These extra assertions first check that the tests' initial setup code
has successfully created object files in the local Git LFS object
storage directories, and then that the delete_local_object() and
corrupt_local_object() test helper functions successfully delete or
truncate one of the object files.

However, these assertions are unnecessary, as the conditions they
check are fully validated by the remainder of the tests' actions.
For instance, we check the output of the "git push" command to confirm
that one specific object is missing or corrupt, and then at the end
of the tests we assert that only the remaining object has been
pushed to the remote server.  This final condition would not be
possible if that object had not been successfully created in the
first place.

Therefore, in order to align these two tests with the other related
tests we simply remove the unnecessary assertion statements.  We can
then also remove the setup code which initializes the variables
that store the Git LFS object sizes, because these values were only
used in the assertion calls.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Mar 28, 2025
@chrisd8088
t/t-push-failures-local.sh: align final assertions
3b35c35 
Our t/t-push-failures-local.sh test script includes a number of tests
which validate the behaviour of the "git push" command when Git LFS
objects are missing or corrupt.

While these tests are largely similar in their implementation, they
vary in a number of formatting and implementation details unrelated to
the specifics of the different conditions they simulate.  These
variations are artifacts of the evolution of our test suite over time;
for example, the tests in the t/t-push-failures-local.sh script were
refactored and collected from several earlier tests in commit
4d52e08 of PR git-lfs#3109.

In a subsequent commit in this PR we expect to update the Git LFS
client to remove some non-functional code which attempts to recreate
missing Git LFS objects during push operations.  In many cases this
change will cause the client to report missing objects in an earlier
stage of push operations than it does now.  We also expect to reword
the error message the client will output in such cases.

Before we make these changes, we first revise the related tests in our
test suite so they are as simple and similar as possible.  This will
ensure that when we update the Git LFS client we can clearly identify
the changes that we need to make in our tests to accommodate the
client's new behaviour.

A pair of tests in the t/t-push-failures-local.sh script, specifically
the "push reject missing objects (lfs.allowincompletepush default)" and
"push reject corrupt objects (lfs.allowincompletepush default)" tests,
perform their final assertions in the reverse order from those performed
in the other tests in the same script.

In order to align the code in all these tests as closely as possible,
we simply revise the final checks in the last two tests in the
t/t-push-failures-local.sh script so they follow the same pattern
as those of the other tests.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Mar 28, 2025
@chrisd8088
t/t-push-failures-local.sh: align setup steps
35bb0ab 
Our t/t-push-failures-local.sh test script includes a number of tests
which validate the behaviour of the "git push" command when Git LFS
objects are missing or corrupt.

While these tests are largely similar in their implementation, they
vary in a number of formatting and implementation details unrelated to
the specifics of the different conditions they simulate.  These
variations are artifacts of the evolution of our test suite over time;
for example, the tests in the t/t-push-failures-local.sh script were
refactored and collected from several earlier tests in commit
4d52e08 of PR git-lfs#3109.

In a subsequent commit in this PR we expect to update the Git LFS
client to remove some non-functional code which attempts to recreate
missing Git LFS objects during push operations.  In many cases this
change will cause the client to report missing objects in an earlier
stage of push operations than it does now.  We also expect to reword
the error message the client will output in such cases.

Before we make these changes, we first revise the related tests in our
test suite so they are as simple and similar as possible.  This will
ensure that when we update the Git LFS client we can clearly identify
the changes that we need to make in our tests to accommodate the
client's new behaviour.

A pair of tests in the t/t-push-failures-local.sh script, specifically
the "push reject missing objects (lfs.allowincompletepush default)" and
"push reject corrupt objects (lfs.allowincompletepush default)" tests,
perform their setup of Git LFS objects and Git commits in a different
sequence than the other tests in the same script.

In order to align the code in all these tests as closely as possible,
we simply revise the setup steps of the last two tests in the
t/t-push-failures-local.sh script so they follow the same pattern
as those of the other tests.  This change has no functional effect;
the only notable detail is that the tests now create all their Git LFS
objects in a single commit instead of using a separate commit to create
each object.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Mar 28, 2025
@chrisd8088
t: rename existing/missing/bad object push tests
06fc67c 
Our t/t-pre-push.sh and t/t-push-failures-local.sh test scripts include
a number of tests which validate the behaviour of the "git lfs pre-push"
and "git push" commands when Git LFS objects are present, missing, or
corrupt.

While these tests are largely similar in their implementation, they
vary in a number of formatting and implementation details unrelated to
the specifics of the different conditions they simulate.  These
variations are artifacts of the evolution of our test suite over time;
for example, the tests in the t/t-push-failures-local.sh script were
refactored and collected from several earlier tests in commit
4d52e08 of PR git-lfs#3109, and the original
versions of the tests in the t/t-pre-push.sh script were added
incrementally in PRs git-lfs#447, git-lfs#2199, and git-lfs#2574.

In a subsequent commit in this PR we expect to update the Git LFS
client to remove some non-functional code which attempts to recreate
missing Git LFS objects during push operations.  In many cases this
change will cause the client to report missing objects in an earlier
stage of push operations than it does now.  We also expect to reword
the error message the client will output in such cases.

Before we make these changes, we first revise the related tests in our
test suite so they are as simple and similar as possible.  This will
ensure that when we update the Git LFS client we can clearly identify
the changes that we need to make in our tests to accommodate the
client's new behaviour.

In previous commits in this PR we have revised and reformatted these
tests to increase their consistency with each other.  One additional
adjustment we make now to further increase this consistency, and to
provide greater clarity as to each test's purpose, is to rename both
the tests and test repositories they create and clone.

Note that since these tests typically only create a single missing
or corrupt object, we now use the singular word "object" rather than
"objects" in the test and repository names.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Mar 28, 2025
@chrisd8088
t: align missing/bad object push exit code checks
cb1fb2d 
Our t/t-pre-push.sh and t/t-push-failures-local.sh test scripts include
a number of tests which validate the behaviour of the "git lfs pre-push"
and "git push" commands when Git LFS objects are missing or corrupt.

While these tests are largely similar in their implementation, they
vary in a number of formatting and implementation details unrelated to
the specifics of the different conditions they simulate.  These
variations are artifacts of the evolution of our test suite over time;
for example, the tests in the t/t-push-failures-local.sh script were
refactored and collected from several earlier tests in commit
4d52e08 of PR git-lfs#3109, and the original
versions of the tests in the t/t-pre-push.sh script were added
incrementally in PRs git-lfs#447, git-lfs#2199, and git-lfs#2574.

In a subsequent commit in this PR we expect to update the Git LFS
client to remove some non-functional code which attempts to recreate
missing Git LFS objects during push operations.  In many cases this
change will cause the client to report missing objects in an earlier
stage of push operations than it does now.  We also expect to reword
the error message the client will output in such cases.

Before we make these changes, we first revise the related tests in our
test suite so they are as simple and similar as possible.  This will
ensure that when we update the Git LFS client we can clearly identify
the changes that we need to make in our tests to accommodate the
client's new behaviour.

In the "pre-push with missing pointer not on server" test in the
t/t-pre-push.sh script we remove the use of the "set +e" shell option,
which is not necessary to avoid test failure.  Although we expect the
"git lfs pre-push" command to fail, its output is piped into the tee(1)
command, and when the "set -e" option is in effect the shell will exit
immediately if the last command in a pipeline returns a non-zero exit
code, but not if other commands in the pipeline return such a code.

We do, however, add a check that the "git lfs pre-push" command exits
with a non-zero code by testing the appropriate PIPESTATUS array value,
following the example of the other related tests.

These other tests include the "pre-push with missing and present pointers
(lfs.allowincompletepush true)" and "pre-push reject missing pointers
(lfs.allowincompletepush default)" tests in the t/t-pre-push.sh script,
plus the four tests in the t/t-push-failures-local.sh script.

In all these tests we adjust the error messages generated in the case
that a "git lfs pre-push" or "git push" command fails or succeeds
unexpectedly.  We rewrite these messages so they are consistent with
each other and with those found in many of our other test scripts.

Note that in the "push reject missing objects (lfs.allowincompletepush
false)" test we also correct the message reported if the "git push"
command were to succeed unexpectedly.  At present, the message states
that we expect the command to succeed, but we actually expect it to fail.

Next, in the "push reject missing objects (lfs.allowincompletepush
default)" and "push reject corrupt objects (lfs.allowincompletepush
default)" tests, we update our checks of the "git push" command's exit
code so that we confirm the command exits with a specific non-zero exit
code of 1 rather than simply checking that its exit code is not zero.
This change brings these checks into alignment with those made by the
other related tests.

Lastly, we remove and adjust some whitespace so that these tests
are all formatted in a similar manner to each other.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Mar 28, 2025
@chrisd8088
t: align missing object push test deletion steps
84742d8 
Our t/t-pre-push.sh and t/t-push-failures-local.sh test scripts include
a number of tests which validate the behaviour of the "git lfs pre-push"
and "git push" commands when Git LFS objects are missing or corrupt.

While these tests are largely similar in their implementation, they
vary in a number of formatting and implementation details unrelated to
the specifics of the different conditions they simulate.  These
variations are artifacts of the evolution of our test suite over time;
for example, the tests in the t/t-push-failures-local.sh script were
refactored and collected from several earlier tests in commit
4d52e08 of PR git-lfs#3109, and the original
versions of the tests in the t/t-pre-push.sh script were added
incrementally in PRs git-lfs#447, git-lfs#2199, and git-lfs#2574.

In a subsequent commit in this PR we expect to update the Git LFS
client to remove some non-functional code which attempts to recreate
missing Git LFS objects during push operations.  In many cases this
change will cause the client to report missing objects in an earlier
stage of push operations than it does now.  We also expect to reword
the error message the client will output in such cases.

Before we make these changes, we first revise the related tests in our
test suite so they are as simple and similar as possible.  This will
ensure that when we update the Git LFS client we can clearly identify
the changes that we need to make in our tests to accommodate the
client's new behaviour.

In a pair of tests in t/t-pre-push.sh script, and in another pair of
tests in the t/t-push-failures-local.sh script, the tests' initial
setup code creates several Git LFS objects and then removes the object
file in the .git/lfs/objects directory hierarchy for one of them.

In each case, we can replace this code with a simple call to our
delete_local_object() test helper function, which performs the
equivalent action of removing an object's file from the internal
Git LFS storage directories.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Mar 28, 2025
@chrisd8088
t/t-push-failures-local.sh: drop extra assertions
36f6bc8 
Our t/t-push-failures-local.sh test script includes a number of tests
which validate the behaviour of the "git push" command when Git LFS
objects are missing or corrupt.

While these tests are largely similar in their implementation, they
vary in a number of formatting and implementation details unrelated to
the specifics of the different conditions they simulate.  These
variations are artifacts of the evolution of our test suite over time;
for example, the tests in the t/t-push-failures-local.sh script were
refactored and collected from several earlier tests in commit
4d52e08 of PR git-lfs#3109.

In a subsequent commit in this PR we expect to update the Git LFS
client to remove some non-functional code which attempts to recreate
missing Git LFS objects during push operations.  In many cases this
change will cause the client to report missing objects in an earlier
stage of push operations than it does now.  We also expect to reword
the error message the client will output in such cases.

Before we make these changes, we first revise the related tests in our
test suite so they are as simple and similar as possible.  This will
ensure that when we update the Git LFS client we can clearly identify
the changes that we need to make in our tests to accommodate the
client's new behaviour.

A pair of tests in the t/t-push-failures-local.sh script, specifically
the "push reject missing objects (lfs.allowincompletepush default)" and
"push reject corrupt objects (lfs.allowincompletepush default)" tests,
perform a number of additional assertions beyond those performed by
the other tests in the same script.

These extra assertions first check that the tests' initial setup code
has successfully created object files in the local Git LFS object
storage directories, and then that the delete_local_object() and
corrupt_local_object() test helper functions successfully delete or
truncate one of the object files.

However, these assertions are unnecessary, as the conditions they
check are fully validated by the remainder of the tests' actions.
For instance, we check the output of the "git push" command to confirm
that one specific object is missing or corrupt, and then at the end
of the tests we assert that only the remaining object has been
pushed to the remote server.  This final condition would not be
possible if that object had not been successfully created in the
first place.

Therefore, in order to align these two tests with the other related
tests we simply remove the unnecessary assertion statements.  We can
then also remove the setup code which initializes the variables
that store the Git LFS object sizes, because these values were only
used in the assertion calls.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Mar 28, 2025
@chrisd8088
t/t-push-failures-local.sh: align final assertions
127dbbf 
Our t/t-push-failures-local.sh test script includes a number of tests
which validate the behaviour of the "git push" command when Git LFS
objects are missing or corrupt.

While these tests are largely similar in their implementation, they
vary in a number of formatting and implementation details unrelated to
the specifics of the different conditions they simulate.  These
variations are artifacts of the evolution of our test suite over time;
for example, the tests in the t/t-push-failures-local.sh script were
refactored and collected from several earlier tests in commit
4d52e08 of PR git-lfs#3109.

In a subsequent commit in this PR we expect to update the Git LFS
client to remove some non-functional code which attempts to recreate
missing Git LFS objects during push operations.  In many cases this
change will cause the client to report missing objects in an earlier
stage of push operations than it does now.  We also expect to reword
the error message the client will output in such cases.

Before we make these changes, we first revise the related tests in our
test suite so they are as simple and similar as possible.  This will
ensure that when we update the Git LFS client we can clearly identify
the changes that we need to make in our tests to accommodate the
client's new behaviour.

A pair of tests in the t/t-push-failures-local.sh script, specifically
the "push reject missing objects (lfs.allowincompletepush default)" and
"push reject corrupt objects (lfs.allowincompletepush default)" tests,
perform their final assertions in the reverse order from those performed
in the other tests in the same script.

In order to align the code in all these tests as closely as possible,
we simply revise the final checks in the last two tests in the
t/t-push-failures-local.sh script so they follow the same pattern
as those of the other tests.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Mar 28, 2025
@chrisd8088
t/t-push-failures-local.sh: align setup steps
c8abfb4 
Our t/t-push-failures-local.sh test script includes a number of tests
which validate the behaviour of the "git push" command when Git LFS
objects are missing or corrupt.

While these tests are largely similar in their implementation, they
vary in a number of formatting and implementation details unrelated to
the specifics of the different conditions they simulate.  These
variations are artifacts of the evolution of our test suite over time;
for example, the tests in the t/t-push-failures-local.sh script were
refactored and collected from several earlier tests in commit
4d52e08 of PR git-lfs#3109.

In a subsequent commit in this PR we expect to update the Git LFS
client to remove some non-functional code which attempts to recreate
missing Git LFS objects during push operations.  In many cases this
change will cause the client to report missing objects in an earlier
stage of push operations than it does now.  We also expect to reword
the error message the client will output in such cases.

Before we make these changes, we first revise the related tests in our
test suite so they are as simple and similar as possible.  This will
ensure that when we update the Git LFS client we can clearly identify
the changes that we need to make in our tests to accommodate the
client's new behaviour.

A pair of tests in the t/t-push-failures-local.sh script, specifically
the "push reject missing objects (lfs.allowincompletepush default)" and
"push reject corrupt objects (lfs.allowincompletepush default)" tests,
perform their setup of Git LFS objects and Git commits in a different
sequence than the other tests in the same script.

In order to align the code in all these tests as closely as possible,
we simply revise the setup steps of the last two tests in the
t/t-push-failures-local.sh script so they follow the same pattern
as those of the other tests.  This change has no functional effect;
the only notable detail is that the tests now create all their Git LFS
objects in a single commit instead of using a separate commit to create
each object.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Mar 28, 2025
@chrisd8088
t: rename existing/missing/bad object push tests
b7c72c1 
Our t/t-pre-push.sh and t/t-push-failures-local.sh test scripts include
a number of tests which validate the behaviour of the "git lfs pre-push"
and "git push" commands when Git LFS objects are present, missing, or
corrupt.

While these tests are largely similar in their implementation, they
vary in a number of formatting and implementation details unrelated to
the specifics of the different conditions they simulate.  These
variations are artifacts of the evolution of our test suite over time;
for example, the tests in the t/t-push-failures-local.sh script were
refactored and collected from several earlier tests in commit
4d52e08 of PR git-lfs#3109, and the original
versions of the tests in the t/t-pre-push.sh script were added
incrementally in PRs git-lfs#447, git-lfs#2199, and git-lfs#2574.

In a subsequent commit in this PR we expect to update the Git LFS
client to remove some non-functional code which attempts to recreate
missing Git LFS objects during push operations.  In many cases this
change will cause the client to report missing objects in an earlier
stage of push operations than it does now.  We also expect to reword
the error message the client will output in such cases.

Before we make these changes, we first revise the related tests in our
test suite so they are as simple and similar as possible.  This will
ensure that when we update the Git LFS client we can clearly identify
the changes that we need to make in our tests to accommodate the
client's new behaviour.

In previous commits in this PR we have revised and reformatted these
tests to increase their consistency with each other.  One additional
adjustment we make now to further increase this consistency, and to
provide greater clarity as to each test's purpose, is to rename both
the tests and test repositories they create and clone.

Note that since these tests typically only create a single missing
or corrupt object, we now use the singular word "object" rather than
"objects" in the test and repository names.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Apr 3, 2025
@chrisd8088
t: align missing/bad object push test setup steps
a6f7920 
Our t/t-pre-push.sh and t/t-push-failures-local.sh test scripts include
a number of tests which validate the behaviour of the "git lfs pre-push"
and "git push" commands when Git LFS objects are missing or corrupt.

While these tests are largely similar in their implementation, they
vary in a number of formatting and implementation details unrelated to
the specifics of the different conditions they simulate.  These
variations are artifacts of the evolution of our test suite over time;
for example, the tests in the t/t-push-failures-local.sh script were
refactored and collected from several earlier tests in commit
4d52e08 of PR git-lfs#3109, and the original
versions of the tests in the t/t-pre-push.sh script were added
incrementally in PRs git-lfs#447, git-lfs#2199, and git-lfs#2574.

In a subsequent commit in this PR we expect to update the Git LFS
client to remove some non-functional code which attempts to recreate
missing Git LFS objects during push operations.  In many cases this
change will cause the client to report missing objects in an earlier
stage of push operations than it does now.  We also expect to reword
the error message the client will output in such cases.

Before we make these changes, we first revise the related tests in our
test suite so they are as simple and similar as possible.  This will
ensure that when we update the Git LFS client we can clearly identify
the changes that we need to make in our tests to accommodate the
client's new behaviour.

A pair of tests in the t/t-push-failures-local.sh script, specifically
the "push reject missing objects (lfs.allowincompletepush default)" and
"push reject corrupt objects (lfs.allowincompletepush default)" tests,
perform their setup of Git LFS objects and Git commits in a different
sequence than the other tests in the same script.

In order to align the code in all these tests as closely as possible,
we simply revise the setup steps of the last two tests in the
t/t-push-failures-local.sh script so they follow the same pattern
as those of the other tests.  This change has no functional effect;
the only notable detail is that the tests now create all their Git LFS
objects in a single commit instead of using a separate commit to create
each object.

As well, we reorder the lists of files we pass to the "git add" command
in the first two tests in the t/t-push-failures-local.sh script so
they now follow the same pattern as those of the other tests in both
that script and in the t/t-pre-push.sh script.

We also adjust the commit message used when creating Git LFS objects
in a pair of tests in the t/t-pre-push.sh script, specifically the
"pre-push with missing and present pointers (lfs.allowincompletepush true)"
and "pre-push reject missing pointers (lfs.allowincompletepush default)"
tests, so they are equivalent to those used in the related tests in
the t/t-push-failures-local.sh script.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Apr 3, 2025
@chrisd8088
t: rename existing/missing/bad object push tests
adbf459 
Our t/t-pre-push.sh and t/t-push-failures-local.sh test scripts include
a number of tests which validate the behaviour of the "git lfs pre-push"
and "git push" commands when Git LFS objects are present, missing, or
corrupt.

While these tests are largely similar in their implementation, they
vary in a number of formatting and implementation details unrelated to
the specifics of the different conditions they simulate.  These
variations are artifacts of the evolution of our test suite over time;
for example, the tests in the t/t-push-failures-local.sh script were
refactored and collected from several earlier tests in commit
4d52e08 of PR git-lfs#3109, and the original
versions of the tests in the t/t-pre-push.sh script were added
incrementally in PRs git-lfs#447, git-lfs#2199, and git-lfs#2574.

In a subsequent commit in this PR we expect to update the Git LFS
client to remove some non-functional code which attempts to recreate
missing Git LFS objects during push operations.  In many cases this
change will cause the client to report missing objects in an earlier
stage of push operations than it does now.  We also expect to reword
the error message the client will output in such cases.

Before we make these changes, we first revise the related tests in our
test suite so they are as simple and similar as possible.  This will
ensure that when we update the Git LFS client we can clearly identify
the changes that we need to make in our tests to accommodate the
client's new behaviour.

In previous commits in this PR we have revised and reformatted these
tests to increase their consistency with each other.  One additional
adjustment we make now to further increase this consistency, and to
provide greater clarity as to each test's purpose, is to rename both
the tests and test repositories they create and clone.

Note that since these tests typically only create a single missing
or corrupt object, we now use the singular word "object" rather than
"objects" in the test and repository names.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Apr 3, 2025
@chrisd8088
t,tq: make missing object push errors consistent
6479308 
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
@chrisd8088
t,tq: make missing object push errors consistent
1567990 
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
@chrisd8088
t: align missing/bad object push exit code checks
8b1ea95 
Our t/t-pre-push.sh and t/t-push-failures-local.sh test scripts include
a number of tests which validate the behaviour of the "git lfs pre-push"
and "git push" commands when Git LFS objects are missing or corrupt.

While these tests are largely similar in their implementation, they
vary in a number of formatting and implementation details unrelated to
the specifics of the different conditions they simulate.  These
variations are artifacts of the evolution of our test suite over time;
for example, the tests in the t/t-push-failures-local.sh script were
refactored and collected from several earlier tests in commit
4d52e08 of PR git-lfs#3109, and the original
versions of the tests in the t/t-pre-push.sh script were added
incrementally in PRs git-lfs#447, git-lfs#2199, and git-lfs#2574.

In a subsequent commit in this PR we expect to update the Git LFS
client to remove some non-functional code which attempts to recreate
missing Git LFS objects during push operations.  In many cases this
change will cause the client to report missing objects in an earlier
stage of push operations than it does now.  We also expect to reword
the error message the client will output in such cases.

Before we make these changes, we first revise the related tests in our
test suite so they are as simple and similar as possible.  This will
ensure that when we update the Git LFS client we can clearly identify
the changes that we need to make in our tests to accommodate the
client's new behaviour.

In the "pre-push with missing pointer not on server" test in the
t/t-pre-push.sh script we remove the use of the "set +e" shell option,
which is not necessary to avoid test failure.  Although we expect the
"git lfs pre-push" command to fail, its output is piped into the tee(1)
command, and when the "set -e" option is in effect the shell will exit
immediately if the last command in a pipeline returns a non-zero exit
code, but not if other commands in the pipeline return such a code.

We do, however, add a check that the "git lfs pre-push" command exits
with a non-zero code by testing the appropriate PIPESTATUS array value,
following the example of the other related tests.

These other tests include the "pre-push with missing and present pointers
(lfs.allowincompletepush true)" and "pre-push reject missing pointers
(lfs.allowincompletepush default)" tests in the t/t-pre-push.sh script,
plus the four tests in the t/t-push-failures-local.sh script.

In all these tests we adjust the error messages generated in the case
that a "git lfs pre-push" or "git push" command fails or succeeds
unexpectedly.  We rewrite these messages so they are consistent with
each other and with those found in many of our other test scripts.

Note that in the "push reject missing objects (lfs.allowincompletepush
false)" test we also correct the message reported if the "git push"
command were to succeed unexpectedly.  At present, the message states
that we expect the command to succeed, but we actually expect it to fail.

Next, in the "push reject missing objects (lfs.allowincompletepush
default)" and "push reject corrupt objects (lfs.allowincompletepush
default)" tests, we update our checks of the "git push" command's exit
code so that we confirm the command exits with a specific non-zero exit
code of 1 rather than simply checking that its exit code is not zero.
This change brings these checks into alignment with those made by the
other related tests.

Lastly, we remove and adjust some whitespace so that these tests
are all formatted in a similar manner to each other.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request May 22, 2025
@chrisd8088
t: align missing object push test deletion steps
2204507 
Our t/t-pre-push.sh and t/t-push-failures-local.sh test scripts include
a number of tests which validate the behaviour of the "git lfs pre-push"
and "git push" commands when Git LFS objects are missing or corrupt.

While these tests are largely similar in their implementation, they
vary in a number of formatting and implementation details unrelated to
the specifics of the different conditions they simulate.  These
variations are artifacts of the evolution of our test suite over time;
for example, the tests in the t/t-push-failures-local.sh script were
refactored and collected from several earlier tests in commit
4d52e08 of PR git-lfs#3109, and the original
versions of the tests in the t/t-pre-push.sh script were added
incrementally in PRs git-lfs#447, git-lfs#2199, and git-lfs#2574.

In a subsequent commit in this PR we expect to update the Git LFS
client to remove some non-functional code which attempts to recreate
missing Git LFS objects during push operations.  In many cases this
change will cause the client to report missing objects in an earlier
stage of push operations than it does now.  We also expect to reword
the error message the client will output in such cases.

Before we make these changes, we first revise the related tests in our
test suite so they are as simple and similar as possible.  This will
ensure that when we update the Git LFS client we can clearly identify
the changes that we need to make in our tests to accommodate the
client's new behaviour.

In a pair of tests in t/t-pre-push.sh script, and in another pair of
tests in the t/t-push-failures-local.sh script, the tests' initial
setup code creates several Git LFS objects and then removes the object
file in the .git/lfs/objects directory hierarchy for one of them.

In each case, we can replace this code with a simple call to our
delete_local_object() test helper function, which performs the
equivalent action of removing an object's file from the internal
Git LFS storage directories.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request May 22, 2025
@chrisd8088
t/t-push-failures-local.sh: drop extra assertions
bad3bd9 
Our t/t-push-failures-local.sh test script includes a number of tests
which validate the behaviour of the "git push" command when Git LFS
objects are missing or corrupt.

While these tests are largely similar in their implementation, they
vary in a number of formatting and implementation details unrelated to
the specifics of the different conditions they simulate.  These
variations are artifacts of the evolution of our test suite over time;
for example, the tests in the t/t-push-failures-local.sh script were
refactored and collected from several earlier tests in commit
4d52e08 of PR git-lfs#3109.

In a subsequent commit in this PR we expect to update the Git LFS
client to remove some non-functional code which attempts to recreate
missing Git LFS objects during push operations.  In many cases this
change will cause the client to report missing objects in an earlier
stage of push operations than it does now.  We also expect to reword
the error message the client will output in such cases.

Before we make these changes, we first revise the related tests in our
test suite so they are as simple and similar as possible.  This will
ensure that when we update the Git LFS client we can clearly identify
the changes that we need to make in our tests to accommodate the
client's new behaviour.

A pair of tests in the t/t-push-failures-local.sh script, specifically
the "push reject missing objects (lfs.allowincompletepush default)" and
"push reject corrupt objects (lfs.allowincompletepush default)" tests,
perform a number of additional assertions beyond those performed by
the other tests in the same script.

These extra assertions first check that the tests' initial setup code
has successfully created object files in the local Git LFS object
storage directories, and then that the delete_local_object() and
corrupt_local_object() test helper functions successfully delete or
truncate one of the object files.

However, these assertions are unnecessary, as the conditions they
check are fully validated by the remainder of the tests' actions.
For instance, we check the output of the "git push" command to confirm
that one specific object is missing or corrupt, and then at the end
of the tests we assert that only the remaining object has been
pushed to the remote server.  This final condition would not be
possible if that object had not been successfully created in the
first place.

Therefore, in order to align these two tests with the other related
tests we simply remove the unnecessary assertion statements.  We can
then also remove the setup code which initializes the variables
that store the Git LFS object sizes, because these values were only
used in the assertion calls.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request May 22, 2025
@chrisd8088
t/t-push-failures-local.sh: align final assertions
2e292e9 
Our t/t-push-failures-local.sh test script includes a number of tests
which validate the behaviour of the "git push" command when Git LFS
objects are missing or corrupt.

While these tests are largely similar in their implementation, they
vary in a number of formatting and implementation details unrelated to
the specifics of the different conditions they simulate.  These
variations are artifacts of the evolution of our test suite over time;
for example, the tests in the t/t-push-failures-local.sh script were
refactored and collected from several earlier tests in commit
4d52e08 of PR git-lfs#3109.

In a subsequent commit in this PR we expect to update the Git LFS
client to remove some non-functional code which attempts to recreate
missing Git LFS objects during push operations.  In many cases this
change will cause the client to report missing objects in an earlier
stage of push operations than it does now.  We also expect to reword
the error message the client will output in such cases.

Before we make these changes, we first revise the related tests in our
test suite so they are as simple and similar as possible.  This will
ensure that when we update the Git LFS client we can clearly identify
the changes that we need to make in our tests to accommodate the
client's new behaviour.

A pair of tests in the t/t-push-failures-local.sh script, specifically
the "push reject missing objects (lfs.allowincompletepush default)" and
"push reject corrupt objects (lfs.allowincompletepush default)" tests,
perform their final assertions in the reverse order from those performed
in the other tests in the same script.

In order to align the code in all these tests as closely as possible,
we simply revise the final checks in the last two tests in the
t/t-push-failures-local.sh script so they follow the same pattern
as those of the other tests.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request May 22, 2025
@chrisd8088
t: align missing/bad object push test setup steps
c767ed7 
Our t/t-pre-push.sh and t/t-push-failures-local.sh test scripts include
a number of tests which validate the behaviour of the "git lfs pre-push"
and "git push" commands when Git LFS objects are missing or corrupt.

While these tests are largely similar in their implementation, they
vary in a number of formatting and implementation details unrelated to
the specifics of the different conditions they simulate.  These
variations are artifacts of the evolution of our test suite over time;
for example, the tests in the t/t-push-failures-local.sh script were
refactored and collected from several earlier tests in commit
4d52e08 of PR git-lfs#3109, and the original
versions of the tests in the t/t-pre-push.sh script were added
incrementally in PRs git-lfs#447, git-lfs#2199, and git-lfs#2574.

In a subsequent commit in this PR we expect to update the Git LFS
client to remove some non-functional code which attempts to recreate
missing Git LFS objects during push operations.  In many cases this
change will cause the client to report missing objects in an earlier
stage of push operations than it does now.  We also expect to reword
the error message the client will output in such cases.

Before we make these changes, we first revise the related tests in our
test suite so they are as simple and similar as possible.  This will
ensure that when we update the Git LFS client we can clearly identify
the changes that we need to make in our tests to accommodate the
client's new behaviour.

A pair of tests in the t/t-push-failures-local.sh script, specifically
the "push reject missing objects (lfs.allowincompletepush default)" and
"push reject corrupt objects (lfs.allowincompletepush default)" tests,
perform their setup of Git LFS objects and Git commits in a different
sequence than the other tests in the same script.

In order to align the code in all these tests as closely as possible,
we simply revise the setup steps of the last two tests in the
t/t-push-failures-local.sh script so they follow the same pattern
as those of the other tests.  This change has no functional effect;
the only notable detail is that the tests now create all their Git LFS
objects in a single commit instead of using a separate commit to create
each object.

As well, we reorder the lists of files we pass to the "git add" command
in the first two tests in the t/t-push-failures-local.sh script so
they now follow the same pattern as those of the other tests in both
that script and in the t/t-pre-push.sh script.

We also adjust the commit message used when creating Git LFS objects
in a pair of tests in the t/t-pre-push.sh script, specifically the
"pre-push with missing and present pointers (lfs.allowincompletepush true)"
and "pre-push reject missing pointers (lfs.allowincompletepush default)"
tests, so they are equivalent to those used in the related tests in
the t/t-push-failures-local.sh script.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request May 22, 2025
@chrisd8088
t: rename existing/missing/bad object push tests
0af7e2c 
Our t/t-pre-push.sh and t/t-push-failures-local.sh test scripts include
a number of tests which validate the behaviour of the "git lfs pre-push"
and "git push" commands when Git LFS objects are present, missing, or
corrupt.

While these tests are largely similar in their implementation, they
vary in a number of formatting and implementation details unrelated to
the specifics of the different conditions they simulate.  These
variations are artifacts of the evolution of our test suite over time;
for example, the tests in the t/t-push-failures-local.sh script were
refactored and collected from several earlier tests in commit
4d52e08 of PR git-lfs#3109, and the original
versions of the tests in the t/t-pre-push.sh script were added
incrementally in PRs git-lfs#447, git-lfs#2199, and git-lfs#2574.

In a subsequent commit in this PR we expect to update the Git LFS
client to remove some non-functional code which attempts to recreate
missing Git LFS objects during push operations.  In many cases this
change will cause the client to report missing objects in an earlier
stage of push operations than it does now.  We also expect to reword
the error message the client will output in such cases.

Before we make these changes, we first revise the related tests in our
test suite so they are as simple and similar as possible.  This will
ensure that when we update the Git LFS client we can clearly identify
the changes that we need to make in our tests to accommodate the
client's new behaviour.

In previous commits in this PR we have revised and reformatted these
tests to increase their consistency with each other.  One additional
adjustment we make now to further increase this consistency, and to
provide greater clarity as to each test's purpose, is to rename both
the tests and test repositories they create and clone.

Note that since these tests typically only create a single missing
or corrupt object, we now use the singular word "object" rather than
"objects" in the test and repository names.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request May 22, 2025
@chrisd8088
t,tq: make missing object push errors consistent
563de1c 
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@terrorobe terrorobe terrorobe left review comments

@ttaylorr ttaylorr ttaylorr approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

discussion

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@larsxschneider @terrorobe @ttaylorr