From 095550dee9f2e2bbd1426d988be1b418d0372ebc Mon Sep 17 00:00:00 2001 From: Chris Darroch Date: Sun, 23 Jan 2022 15:41:30 -0800 Subject: [PATCH 1/4] t/fixtures/migrate.sh: align test repo names We revise the names used for several test repositories created by the "git lfs migrate" tests, aligning them with their actual contents and with the majority of the other names used by the test suite. However, we do have to keep them short enough that we avoid running afoul of the Windows PATH_LEN maximum of 260 characters, including the full path to our CI runner environments. --- t/fixtures/migrate.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/t/fixtures/migrate.sh b/t/fixtures/migrate.sh index bbce1001..2ba236b9 100755 --- a/t/fixtures/migrate.sh +++ b/t/fixtures/migrate.sh @@ -28,7 +28,7 @@ assert_ref_unmoved() { setup_local_branch_with_gitattrs() { set -e - reponame="migrate-single-remote-branch-with-attrs" + reponame="migrate-single-local-branch-with-attrs" remove_and_create_local_repo "$reponame" @@ -56,7 +56,7 @@ setup_local_branch_with_gitattrs() { setup_local_branch_with_nested_gitattrs() { set -e - reponame="nested-attrs" + reponame="migrate-single-local-branch-nested-attrs" remove_and_create_local_repo "$reponame" @@ -97,7 +97,7 @@ setup_single_local_branch_untracked() { local name="${1:-a.md}" - reponame="single-local-branch-untracked" + reponame="migrate-single-local-branch-untracked" remove_and_create_local_repo "$reponame" @@ -121,7 +121,7 @@ setup_single_local_branch_untracked() { setup_single_local_branch_tracked() { set -e - reponame="migrate-single-remote-branch-with-attrs" + reponame="migrate-single-local-branch-tracked" remove_and_create_local_repo "$reponame" From 86a81660f15d18da27ccbcdb9e8e4a679a45f3b6 Mon Sep 17 00:00:00 2001 From: Chris Darroch Date: Sun, 23 Jan 2022 17:57:11 -0800 Subject: [PATCH 2/4] t/t-migrate-import.sh: simplify execute perm check We can slightly simplify some execute permission mode checks on a file to use the conventional -x shell test operator. --- t/t-migrate-import.sh | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/t/t-migrate-import.sh b/t/t-migrate-import.sh index 0a474227..400a7a72 100755 --- a/t/t-migrate-import.sh +++ b/t/t-migrate-import.sh @@ -502,7 +502,6 @@ begin_test "migrate import (above with include or exclude)" ) end_test - begin_test "migrate import (existing .gitattributes)" ( set -e @@ -580,12 +579,12 @@ begin_test "migrate import (identical contents, different permissions)" git commit -m "make file executable" # Verify we have executable permissions. - ls -la foo.dat | grep 'rwx' + [ -x foo.dat ] git lfs migrate import --everything --include="*.dat" # Verify we have executable permissions. - ls -la foo.dat | grep 'rwx' + [ -x foo.dat ] ) end_test From a7d666a7ef65bbcee63c4e8401c5483b5bb1382a Mon Sep 17 00:00:00 2001 From: Chris Darroch Date: Sun, 23 Jan 2022 17:56:50 -0800 Subject: [PATCH 3/4] t: add migrate tests for incorrect gitattrs perms We currently generate incorrect duplicate .gitattributes entries when performing an import or export migration if an existing .gitattributes file has execute file permissions. As we intend to fix this behaviour, we first add a pair of tests for it, which check for a single .gitattributes file after migration and therefore will fail until we resolve the problem. --- t/fixtures/migrate.sh | 14 +++++++++++++ t/t-migrate-export.sh | 46 +++++++++++++++++++++++++++++++++++++++++++ t/t-migrate-import.sh | 46 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+) diff --git a/t/fixtures/migrate.sh b/t/fixtures/migrate.sh index 2ba236b9..3fbd26a8 100755 --- a/t/fixtures/migrate.sh +++ b/t/fixtures/migrate.sh @@ -25,6 +25,9 @@ assert_ref_unmoved() { # refs/heads/main # # - Commit 'A' has 120, in a.txt, and a corresponding entry in .gitattributes. +# +# If "0755" is passed as an argument, the .gitattributes file is created +# with that permissions mode. setup_local_branch_with_gitattrs() { set -e @@ -40,6 +43,10 @@ setup_local_branch_with_gitattrs() { git lfs track "*.txt" git lfs track "*.other" + if [[ $1 == "0755" ]]; then + chmod +x .gitattributes + fi + git add .gitattributes git commit -m "add .gitattributes" } @@ -118,6 +125,9 @@ setup_single_local_branch_untracked() { # # - Commit 'A' has 120, in a.txt and 140 in a.md, with both files tracked as # pointers in Git LFS +# +# If "0755" is passed as an argument, the .gitattributes file is created +# with that permissions mode. setup_single_local_branch_tracked() { set -e @@ -128,6 +138,10 @@ setup_single_local_branch_tracked() { echo "*.txt filter=lfs diff=lfs merge=lfs -text" > .gitattributes echo "*.md filter=lfs diff=lfs merge=lfs -text" >> .gitattributes + if [[ $1 == "0755" ]]; then + chmod +x .gitattributes + fi + git add .gitattributes git commit -m "initial commit" diff --git a/t/t-migrate-export.sh b/t/t-migrate-export.sh index b2d80624..86aef3b2 100755 --- a/t/t-migrate-export.sh +++ b/t/t-migrate-export.sh @@ -367,6 +367,52 @@ begin_test "migrate export (include/exclude ref)" ) end_test +begin_test "migrate export (.gitattributes with different permissions)" +( + set -e + + # Windows lacks POSIX permissions. + [ "$IS_WINDOWS" -eq 1 ] && exit 0 + + setup_single_local_branch_tracked 0755 + + md_oid="$(calc_oid "$(cat a.md)")" + txt_oid="$(calc_oid "$(cat a.txt)")" + + assert_pointer "refs/heads/main" "a.txt" "$txt_oid" "120" + assert_pointer "refs/heads/main" "a.md" "$md_oid" "140" + + [ -x .gitattributes ] + + git lfs migrate export --include="*.txt" + + [ ! -x .gitattributes ] + + refute_pointer "refs/heads/main" "a.txt" + assert_pointer "refs/heads/main" "a.md" "$md_oid" "140" + + refute_local_object "$txt_oid" "120" + assert_local_object "$md_oid" "140" + + main="$(git rev-parse refs/heads/main)" + + main_attrs="$(git cat-file -p "$main:.gitattributes")" + + echo "$main_attrs" | grep -q "*.txt !text !filter !merge !diff" + + attrs_main_sha="$(git show $main:.gitattributes | git hash-object --stdin)" + md_main_sha="$(git show $main:a.md | git hash-object --stdin)" + txt_main_sha="$(git show $main:a.txt | git hash-object --stdin)" + + diff -u <(git ls-tree $main) <(cat <<-EOF +100644 blob $attrs_main_sha .gitattributes +100644 blob $md_main_sha a.md +100644 blob $txt_main_sha a.txt +EOF + ) +) +end_test + begin_test "migrate export (--object-map)" ( set -e diff --git a/t/t-migrate-import.sh b/t/t-migrate-import.sh index 400a7a72..54c5d64d 100755 --- a/t/t-migrate-import.sh +++ b/t/t-migrate-import.sh @@ -560,6 +560,52 @@ EOF) ) end_test +begin_test "migrate import (existing .gitattributes with different permissions)" +( + set -e + + # Windows lacks POSIX permissions. + [ "$IS_WINDOWS" -eq 1 ] && exit 0 + + setup_local_branch_with_gitattrs 0755 + + main="$(git rev-parse refs/heads/main)" + + txt_main_oid="$(calc_oid "$(git cat-file -p "$main:a.txt")")" + + [ -x .gitattributes ] + + git lfs migrate import --yes --include-ref=refs/heads/main --include="*.txt" + + [ ! -x .gitattributes ] + + assert_local_object "$txt_main_oid" "120" + + main="$(git rev-parse refs/heads/main)" + prev="$(git rev-parse refs/heads/main^1)" + + diff -u <(git cat-file -p $main:.gitattributes) <(cat <<-EOF +*.txt filter=lfs diff=lfs merge=lfs -text +*.other filter=lfs diff=lfs merge=lfs -text +EOF + ) + + diff -u <(git cat-file -p $prev:.gitattributes) <(cat <<-EOF +*.txt filter=lfs diff=lfs merge=lfs -text +EOF + ) + + attrs_main_sha="$(git show $main:.gitattributes | git hash-object --stdin)" + txt_main_sha="$(git show $main:a.txt | git hash-object --stdin)" + + diff -u <(git ls-tree $main) <(cat <<-EOF +100644 blob $attrs_main_sha .gitattributes +100644 blob $txt_main_sha a.txt +EOF + ) +) +end_test + begin_test "migrate import (identical contents, different permissions)" ( set -e From 133f29f763518d8443a37587515e38f8fff49a43 Mon Sep 17 00:00:00 2001 From: Chris Darroch Date: Mon, 24 Jan 2022 13:33:34 -0800 Subject: [PATCH 4/4] go.*,vendor: bump gitobj to v2.1.0 Bump gitobj to v2.1.0 to fix merges of tree entries where an existing tree entry has a different file permissions mode than the tree entry being merged with it. This should resolve the problem with import migrations reported in issue #4796 where existing .gitattributes files with the execute permission mode set resulted in duplicate .gitattributes tree entries after import, instead of a single merged entry. After updating go.mod, the specific commands used for this update were: go mod tidy && go mod vendor h/t bk2204 for spotting the simplest fix for this --- go.mod | 2 +- go.sum | 4 ++-- vendor/github.com/git-lfs/gitobj/v2/tree.go | 18 ++++++++++-------- vendor/modules.txt | 2 +- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/go.mod b/go.mod index 3bd51c6c..d4890c52 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ module github.com/git-lfs/git-lfs/v3 require ( github.com/avast/retry-go v2.4.2+incompatible github.com/dpotapov/go-spnego v0.0.0-20210315154721-298b63a54430 - github.com/git-lfs/gitobj/v2 v2.0.2 + github.com/git-lfs/gitobj/v2 v2.1.0 github.com/git-lfs/go-netrc v0.0.0-20210914205454-f0c862dd687a github.com/git-lfs/pktline v0.0.0-20210330133718-06e9096e2825 github.com/git-lfs/wildmatch/v2 v2.0.1 diff --git a/go.sum b/go.sum index 8abb768e..0b73d573 100644 --- a/go.sum +++ b/go.sum @@ -7,8 +7,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/dpotapov/go-spnego v0.0.0-20210315154721-298b63a54430 h1:oempk9HjNt6rVKyKmpdnoN7XABQv3SXLWu3pxUI7Vlk= github.com/dpotapov/go-spnego v0.0.0-20210315154721-298b63a54430/go.mod h1:AVSs/gZKt1bOd2AhkhbS7Qh56Hv7klde22yXVbwYJhc= -github.com/git-lfs/gitobj/v2 v2.0.2 h1:p8rWlhEyiSsC+4Qc+EdufySatf8sDtvJIrHAHgf7Ar8= -github.com/git-lfs/gitobj/v2 v2.0.2/go.mod h1:q6aqxl6Uu3gWsip5GEKpw+7459F97er8COmU45ncAxw= +github.com/git-lfs/gitobj/v2 v2.1.0 h1:5BUDAMga0Sv9msMXolrn6xplkiG5RaVEkOir2HSznog= +github.com/git-lfs/gitobj/v2 v2.1.0/go.mod h1:q6aqxl6Uu3gWsip5GEKpw+7459F97er8COmU45ncAxw= github.com/git-lfs/go-netrc v0.0.0-20210914205454-f0c862dd687a h1:6pskVZacdMUL93pCpMAYnMDLjH1yDFhssPYGe32sjdk= github.com/git-lfs/go-netrc v0.0.0-20210914205454-f0c862dd687a/go.mod h1:70O4NAtvWn1jW8V8V+OKrJJYcxDLTmIozfi2fmSz5SI= github.com/git-lfs/pktline v0.0.0-20210330133718-06e9096e2825 h1:riQhgheTL7tMF4d5raz9t3+IzoR1i1wqxE1kZC6dY+U= diff --git a/vendor/github.com/git-lfs/gitobj/v2/tree.go b/vendor/github.com/git-lfs/gitobj/v2/tree.go index d4996ae6..ab10708d 100644 --- a/vendor/github.com/git-lfs/gitobj/v2/tree.go +++ b/vendor/github.com/git-lfs/gitobj/v2/tree.go @@ -115,22 +115,18 @@ func (t *Tree) Encode(to io.Writer) (n int, err error) { func (t *Tree) Merge(others ...*TreeEntry) *Tree { unseen := make(map[string]*TreeEntry) - // Build a cache of name+filemode to *TreeEntry. + // Build a cache of name to *TreeEntry. for _, other := range others { - key := fmt.Sprintf("%s\x00%o", other.Name, other.Filemode) - - unseen[key] = other + unseen[other.Name] = other } // Map the existing entries ("t.Entries") into a new set by either // copying an existing entry, or replacing it with a new one. entries := make([]*TreeEntry, 0, len(t.Entries)) for _, entry := range t.Entries { - key := fmt.Sprintf("%s\x00%o", entry.Name, entry.Filemode) - - if other, ok := unseen[key]; ok { + if other, ok := unseen[entry.Name]; ok { entries = append(entries, other) - delete(unseen, key) + delete(unseen, entry.Name) } else { oid := make([]byte, len(entry.Oid)) copy(oid, entry.Oid) @@ -234,6 +230,12 @@ func (e *TreeEntry) Type() ObjectType { } } +// IsLink returns true if the given TreeEntry is a blob which represents a +// symbolic link (i.e., with a filemode of 0120000. +func (e *TreeEntry) IsLink() bool { + return e.Filemode & sIFMT == sIFLNK +} + // SubtreeOrder is an implementation of sort.Interface that sorts a set of // `*TreeEntry`'s according to "subtree" order. This ordering is required to // write trees in a correct, readable format to the Git object database. diff --git a/vendor/modules.txt b/vendor/modules.txt index d7264eb1..f330680f 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -8,7 +8,7 @@ github.com/avast/retry-go github.com/davecgh/go-spew/spew # github.com/dpotapov/go-spnego v0.0.0-20210315154721-298b63a54430 github.com/dpotapov/go-spnego -# github.com/git-lfs/gitobj/v2 v2.0.2 +# github.com/git-lfs/gitobj/v2 v2.1.0 github.com/git-lfs/gitobj/v2 github.com/git-lfs/gitobj/v2/errors github.com/git-lfs/gitobj/v2/pack