Skip to content
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.

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

Include long filename support #68

Merged
merged 3 commits into from
Jul 22, 2018
Merged

Include long filename support #68

merged 3 commits into from
Jul 22, 2018

Conversation

driusan
Copy link
Owner

@driusan driusan commented Jul 22, 2018

This adds support for reading long filenames into the index, as well as fixes two other bugs preventing the last t0000-basic test from passing.

@@ -133,3 +134,18 @@ func (f File) Remove() error {
func (f File) Open() (*os.File, error) {
return os.Open(f.String())
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would filepath.Match() be more complete alternative than trying to convert wildcard patterns into regexes? https://golang.org/pkg/path/filepath/#Match

Copy link
Owner Author

Choose a reason for hiding this comment

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

Good catch. I'll try and fix it tomorrow. We'll still need a solution for ** globs, though

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right, but hopefully that and the trailing '/' for directories things can be captured as special cases.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Fixed. I amended the commits because I'm trying to keep this cleanly separated into 3 atomic commits.

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

git/lsfiles.go Outdated
@@ -2,6 +2,7 @@ package git

import (
"io/ioutil"
//"fmt"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unused import. Maybe just remove it?

Copy link
Owner Author

Choose a reason for hiding this comment

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

fixed

@@ -11,5 +11,5 @@ script:
- diff -u <(echo -n) <(gofmt -d ./)
- go test -v ./...
- chmod u+x ./official-git/run-tests.sh
- GIT_SKIP_TESTS=t0000.77 ./official-git/run-tests.sh t0000-basic.sh t0004-unwritable.sh t0007-git-var.sh t0010-racy-git.sh t0062-revision-walking.sh t0070-fundamental.sh t0081-line-buffer.sh t1009-read-tree-new-index.sh t1304-default-acl.sh t2100-update-cache-badpath.sh t4113-apply-ending.sh t4123-apply-shrink.sh t5705-clone-2gb.sh t7062-wtstatus-ignorecase.sh t7511-status-index.sh
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@@ -129,9 +129,32 @@ func UpdateIndexFromReader(c *Client, opts UpdateIndexOptions, r io.Reader) (*In
return nil, fmt.Errorf("update-index --index-info variant 1 not implemented")
case 3:
switch len(spaces[1]) {
case 20:
Copy link
Collaborator

Choose a reason for hiding this comment

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

What happened to case 20? Should it produce an error about variant 3?

Copy link
Owner Author

Choose a reason for hiding this comment

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

It was always wrong. A Sha1 is 20 bytes long, which is 40 characters when written as a hex string. It just wasn't caught before because modes are never 20 characters either.

ReadIndexEntry no longer panics if it encounters a long filename.
Add support for file globing such as `git ls-files foo*`, required
for basic git test.
Add support for the update-index --index-info variant where
the format of stdin is that of "git ls-files -s"

Fixes #55
@driusan driusan force-pushed the LongFiles branch 2 times, most recently from 85f2086 to d683417 Compare July 22, 2018 13:07
@driusan driusan merged commit 3354b02 into master Jul 22, 2018
@driusan driusan deleted the LongFiles branch July 22, 2018 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement git test suite Issues identified by the real git test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants