lunny/git-lfs
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

git: avoid "bad object" messages when force-pushing

Browse Source 
If a user attempts to force-push over a ref and we don't currently have
the object that the reference currently points to, our pre-push hook
will fail with a message like the following:

  ref foo:: Error in git rev-list --stdin --objects --not --remotes=origin --: exit status 128 fatal: bad object 1aabb695d63c4d4afe731ab69573893390526896

This occurs because we pass the existing object name to git rev-list as
something to exclude.  Normally, this is a good idea, since it means we
can short-circuit the traversal since we already know what the other
side has.  However, in this case, it causes unwanted problems.

What we really want is for Git to just ignore an object if it's bad, and
fortunately the rev-list --ignore-missing object does exactly this, so
let's use it.  Note that we must pass it before the --stdin option since
otherwise it has no effect, so move the --stdin option to the end of the
command.
This commit is contained in:
brian m. carlson 2020-04-16 20:55:23 +00:00
parent d38724823c
commit aab142ddb9
No known key found for this signature in database
GPG Key ID: 2D0C9BC12F82B3A1
3 changed files with 46 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
git/rev_list_scanner.go
View File

@ -225,7 +225,7 @@ func NewRevListScanner(include, excluded []string, opt *ScanRefsOptions) (*RevLi
// occurred.
func revListArgs(include, exclude []string, opt *ScanRefsOptions) (io.Reader, []string, error) {
var stdin io.Reader
args := []string{"rev-list", "--stdin"}
args := []string{"rev-list"}
if !opt.CommitsOnly {
args = append(args, "--objects")
}
@ -251,6 +251,7 @@ func revListArgs(include, exclude []string, opt *ScanRefsOptions) (io.Reader, []
case ScanAllMode:
args = append(args, "--all")
case ScanRangeToRemoteMode:
args = append(args, "--ignore-missing")
if len(opt.SkippedRefs) == 0 {
args = append(args, "--not", "--remotes="+opt.Remote)
stdin = strings.NewReader(strings.Join(
@ -263,7 +264,7 @@ func revListArgs(include, exclude []string, opt *ScanRefsOptions) (io.Reader, []
default:
return nil, nil, errors.Errorf("unknown scan type: %d", opt.Mode)
}
return stdin, append(args, "--"), nil
return stdin, append(args, "--stdin", "--"), nil
}
func includeExcludeShas(include, exclude []string) []string {

24
git/rev_list_scanner_test.go
View File

@ -57,7 +57,7 @@ func TestRevListArgs(t *testing.T) {
SkipDeletedBlobs: false,
},
ExpectedStdin: fmt.Sprintf("%s\n^%s", s1, s2),
ExpectedArgs: []string{"rev-list", "--stdin", "--objects", "--do-walk", "--"},
ExpectedArgs: []string{"rev-list", "--objects", "--do-walk", "--stdin", "--"},
},
"scan refs not deleted, left and right": {
Include: []string{s1}, Exclude: []string{s2}, Opt: &ScanRefsOptions{
@ -65,7 +65,7 @@ func TestRevListArgs(t *testing.T) {
SkipDeletedBlobs: true,
},
ExpectedStdin: fmt.Sprintf("%s\n^%s", s1, s2),
ExpectedArgs: []string{"rev-list", "--stdin", "--objects", "--no-walk", "--"},
ExpectedArgs: []string{"rev-list", "--objects", "--no-walk", "--stdin", "--"},
},
"scan refs deleted, left only": {
Include: []string{s1}, Opt: &ScanRefsOptions{
@ -73,7 +73,7 @@ func TestRevListArgs(t *testing.T) {
SkipDeletedBlobs: false,
},
ExpectedStdin: s1,
ExpectedArgs: []string{"rev-list", "--stdin", "--objects", "--do-walk", "--"},
ExpectedArgs: []string{"rev-list", "--objects", "--do-walk", "--stdin", "--"},
},
"scan refs not deleted, left only": {
Include: []string{s1}, Opt: &ScanRefsOptions{
@ -81,13 +81,13 @@ func TestRevListArgs(t *testing.T) {
SkipDeletedBlobs: true,
},
ExpectedStdin: s1,
ExpectedArgs: []string{"rev-list", "--stdin", "--objects", "--no-walk", "--"},
ExpectedArgs: []string{"rev-list", "--objects", "--no-walk", "--stdin", "--"},
},
"scan all": {
Include: []string{s1}, Exclude: []string{s2}, Opt: &ScanRefsOptions{
Mode: ScanAllMode,
},
ExpectedArgs: []string{"rev-list", "--stdin", "--objects", "--all", "--"},
ExpectedArgs: []string{"rev-list", "--objects", "--all", "--stdin", "--"},
},
"scan left to remote, no skipped refs": {
Include: []string{s1}, Opt: &ScanRefsOptions{
@ -96,7 +96,7 @@ func TestRevListArgs(t *testing.T) {
SkippedRefs: []string{},
},
ExpectedStdin: s1,
ExpectedArgs: []string{"rev-list", "--stdin", "--objects", "--not", "--remotes=origin", "--"},
ExpectedArgs: []string{"rev-list", "--objects", "--ignore-missing", "--not", "--remotes=origin", "--stdin", "--"},
},
"scan left to remote, skipped refs": {
Include: []string{s1}, Exclude: []string{s2}, Opt: &ScanRefsOptions{
@ -104,7 +104,7 @@ func TestRevListArgs(t *testing.T) {
Remote: "origin",
SkippedRefs: []string{"a", "b", "c"},
},
ExpectedArgs: []string{"rev-list", "--stdin", "--objects", "--"},
ExpectedArgs: []string{"rev-list", "--objects", "--ignore-missing", "--stdin", "--"},
ExpectedStdin: s1 + "\n^" + s2 + "\na\nb\nc",
},
"scan unknown type": {
@ -119,7 +119,7 @@ func TestRevListArgs(t *testing.T) {
Order: DateRevListOrder,
},
ExpectedStdin: fmt.Sprintf("%s\n^%s", s1, s2),
ExpectedArgs: []string{"rev-list", "--stdin", "--objects", "--date-order", "--do-walk", "--"},
ExpectedArgs: []string{"rev-list", "--objects", "--date-order", "--do-walk", "--stdin", "--"},
},
"scan author date order": {
Include: []string{s1}, Exclude: []string{s2}, Opt: &ScanRefsOptions{
@ -127,7 +127,7 @@ func TestRevListArgs(t *testing.T) {
Order: AuthorDateRevListOrder,
},
ExpectedStdin: fmt.Sprintf("%s\n^%s", s1, s2),
ExpectedArgs: []string{"rev-list", "--stdin", "--objects", "--author-date-order", "--do-walk", "--"},
ExpectedArgs: []string{"rev-list", "--objects", "--author-date-order", "--do-walk", "--stdin", "--"},
},
"scan topo order": {
Include: []string{s1}, Exclude: []string{s2}, Opt: &ScanRefsOptions{
@ -135,7 +135,7 @@ func TestRevListArgs(t *testing.T) {
Order: TopoRevListOrder,
},
ExpectedStdin: fmt.Sprintf("%s\n^%s", s1, s2),
ExpectedArgs: []string{"rev-list", "--stdin", "--objects", "--topo-order", "--do-walk", "--"},
ExpectedArgs: []string{"rev-list", "--objects", "--topo-order", "--do-walk", "--stdin", "--"},
},
"scan commits only": {
Include: []string{s1}, Exclude: []string{s2}, Opt: &ScanRefsOptions{
@ -143,7 +143,7 @@ func TestRevListArgs(t *testing.T) {
CommitsOnly: true,
},
ExpectedStdin: fmt.Sprintf("%s\n^%s", s1, s2),
ExpectedArgs: []string{"rev-list", "--stdin", "--do-walk", "--"},
ExpectedArgs: []string{"rev-list", "--do-walk", "--stdin", "--"},
},
"scan reverse": {
Include: []string{s1}, Exclude: []string{s2}, Opt: &ScanRefsOptions{
@ -151,7 +151,7 @@ func TestRevListArgs(t *testing.T) {
Reverse: true,
},
ExpectedStdin: fmt.Sprintf("%s\n^%s", s1, s2),
ExpectedArgs: []string{"rev-list", "--stdin", "--objects", "--reverse", "--do-walk", "--"},
ExpectedArgs: []string{"rev-list", "--objects", "--reverse", "--do-walk", "--stdin", "--"},
},
} {
t.Run(desc, c.Assert)

31
t/t-pre-push.sh
View File

@ -1222,3 +1222,34 @@ begin_test "pre-push does not traverse Git objects server has"
! grep $contents_oid push.log
)
end_test
begin_test "pre-push with force-pushed ref"
(
set -e
reponame="pre-push-force-pushed-ref"
setup_remote_repo "$reponame"
clone_repo "$reponame" "$reponame"
git config "lfs.$(repo_endpoint "$GITSERVER" "$reponame").locksverify" false
git lfs track "*.dat"
echo "hi" > a.dat
git add .gitattributes a.dat
git commit -m "add a.dat"
git tag -a -m tagname tagname
refute_server_object "$reponame" 98ea6e4f216f2fb4b69fff9b3a44842c38686ca685f3f55dc48c5d3fb1107be4 "refs/heads/master"
git push origin master tagname
assert_server_object "$reponame" 98ea6e4f216f2fb4b69fff9b3a44842c38686ca685f3f55dc48c5d3fb1107be4 "refs/heads/master"
# We pick a different message so that we get different object IDs even if both
# commands run in the same second.
git tag -f -a -m tagname2 tagname
# Prune the old tag object.
git reflog expire --all --expire=now
git gc --prune=now
# Make sure we deal with us missing the object for the old value of the tag ref.
git push origin +tagname
)
end_test