gitignore has different behavior to git's gitignore

[shibumi]

Hi,
I don't know if you are aware of this, but the gitignore implementation is slightly different to the original gitignore implementation regarding absolute paths.

Imagine I have a repository and I have the following filepath internal/util/parse.go. In the original gitignore file I can just use parse.go to exclude the parse.go file. This is not the case for go-git's gitignore. If I just specify parse.go it will not match on an absolute path like internal/util/parse.go or /tmp/internal/util/parse.go.

You can verify this behavior via:

mkdir -p /tmp/test/internal/util
touch /tmp/test/internal/util/parse.go
cd /tmp/test/
git init
echo "parse.go" > .gitignore

If you check the status of the repository via git status you will see, that the parse.go file in /tmp/test/internal/util/parse.go is being excluded.

This is not the case for go-git. For example in our project: https://github.com/in-toto/in-toto-golang/blob/92c7c374bb0f3b9b55c54c20bc933b9be94d8477/in_toto/runlib.go#L77

We are using the gitignore matcher and we try to match on the following:

// path = "/tmp/in_toto_test_dir711641952/foo.tar.gz
// regex is: "foo.tar.gz"
match := m.Match([]string{path}, fileInfo.IsDir())

This should actually match (like in the real gitignore), and the path should get excluded.

[ZerGo0]

#107 Pretty sure that this should fix it.

[shibumi]

@ZerGo0 cool, do you plan a new release in near future with this fix?

[ZerGo0]

@ZerGo0 cool, do you plan a new release in near future with this fix?

I submitted it as a pull request, it's up to the maintainers here to merge it, it's a 1 line change.

[shibumi]

@ZerGo0 ah okay, sorry. i thought you are one of the maintainers. This is maybe off-topic, but do you know how I can apply your patch to the dependency in the go mod file? I would like to test it with my code

[ZerGo0]

That's pretty simple, navigate to your GOPATH:

The GOPATH environment variable specifies the location of your workspace. It defaults to a directory named go inside your home directory, so $HOME/go on Unix, $home/go on Plan 9, and %USERPROFILE%\go (usually C:\Users\YourName\go) on Windows, %GOPATH% might also work.

Then just navigate to %GOPATH%\src\github.com\go-git\go-git\plumbing\format\gitignore\pattern.go and then change the line as shown in the pull request: https://github.com/go-git/go-git/pull/107/files

[shibumi]

the path doesn't exist on my machine. My project is managed with go.mod.
What I have is a GOPATH/pkg/mod/github.com/go-git directory, but this one is readonly.

I tried using gohack:

module github.com/in-toto/in-toto-golang

go 1.14

require (
	github.com/go-git/go-git v4.7.0+incompatible // indirect
	github.com/go-git/go-git/v5 v5.1.0
	github.com/xanzy/go-pathspec v1.0.1
	golang.org/x/crypto v0.0.0-20200302210943-78000ba7a073
	gopkg.in/src-d/go-git.v4 v4.13.1 // indirect
)

replace github.com/go-git/go-git/v5 => /home/chris/gohack/github.com/go-git/go-git/v5

but with this change, the error persists

[shibumi]

@ZerGo0 I've successfully tested your code and I can say it doesn't work.

When entering the SimpleMatch function I have the following variables:

path = "/tmp/in_toto_test_dir858613035/foo.tar.gz"
pattern = "foo.tar.gz"

In git this would match on the file foo.tar.gz and it would get excluded. Instead it returns false and there is no match for it.

[shibumi]

@ZerGo0 I mean, what your fix is doing is actually just replacing linebreaks, so what has this to do with the issue stated here? :D

[ZerGo0]

@ZerGo0 I mean, what your fix is doing is actually just replacing linebreaks, so what has this to do with the issue stated here? :D

I know that in your test you only have 1 line, but if your .gitignore file has more than 1 line then it seems like it doesn't take into account that behind each line is a linebreak. This results in a false for the SimpleMatch function since *.log\r (\n might also be a problem, I thought I tested it, but given that it doesn't work for you it might) doesn't match test.log obviously. This is at least the case on windows, haven't tried other systems, but I imagine it's the same. I will do some testing later, but as far as I know it only takes the .gitignore file into account when you do a worktree.status() call. gitAdd and so on don't take it into account.

Edit: These issues are probably related to each other, I haven't done a lot of debugging since I wanted to get some own stuff working first and this did the job just fine for me. All patterns should probably be filtered right when they get saved in the variable instead of each pattern match function.

[shibumi]

@ZerGo0 ah thanks for your explanation. As for my use case: I don't have a gitignore file I pass the pattern directly as string. So there should be no newline or \n character.

The problem is that go-git uses a simple filepath.Match call and filepath.Match is not matching correctly, because it was never intended to copy gitignore "regex". filepath.Match uses shell patterns for matching.

So actually to match parse.go on the string internal/util/parse.go there is more work to do. Either via injecting asterisks into the pattern or via implementing an own algorithm that mimics the behavior of a real gitignore file.

[mrpotes]

I've also hit this issue. Before I found this issue, I wrote a test to check whether something wasn't working right (having checked the compatibility matrix which says that gitignore is compatible). Here it is, in case it's useful in a PR:

package main

import (
	"io"
	"os"
	"testing"
	"time"

	"github.com/go-git/go-git/v5"
	"github.com/go-git/go-git/v5/plumbing/object"
	"github.com/stretchr/testify/assert"
)

func TestGitignore(t *testing.T) {
	tests := []struct {
		name            string
		ignoreContent   string
		expectedChanges int
	}{
		{"ignored without path", "ignored.file", 0},
		{"ignored with path", "/ignored.file", 0},
		{"not ignored", "", 1},
	}
	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			assert.NoError(t, os.RemoveAll("/tmp/go-git-test-repo"))
			r, err := git.PlainInit("/tmp/go-git-test-repo", false)
			assert.NoError(t, err)
			w, err := r.Worktree()
			assert.NoError(t, err)
			writeFile(t, w, ".gitignore", tt.ignoreContent+"\n")
			_, err = w.Add(".gitignore")
			assert.NoError(t, err)
			_, err = w.Commit(".gitignore", &git.CommitOptions{Author: author()})
			assert.NoError(t, err)

			writeFile(t, w, "ignored.file", "ignore me\n")
			_, err = w.Add(".")
			assert.NoError(t, err)
			_, err = w.Commit("should have no files", &git.CommitOptions{Author: author()})
			assert.NoError(t, err)
			h, err := r.Head()
			assert.NoError(t, err)
			c, err := r.CommitObject(h.Hash())
			assert.NoError(t, err)
			s, err := c.Stats()
			assert.NoError(t, err)
			assert.Len(t, s, tt.expectedChanges)
		})
	}
}

func writeFile(t *testing.T, w *git.Worktree, name, content string) {
	f, err := w.Filesystem.Create(name)
	assert.NoError(t, err)
	_, err = io.WriteString(f, content)
	assert.NoError(t, err)
	err = f.Close()
	assert.NoError(t, err)
}

func author() *object.Signature {
	return &object.Signature{
		Name:  "Git Author",
		Email: "author@example.com",
		When:  time.Now(),
	}
}

[mrpotes]

I've tested the above with the latest master, it still doesn't work, although AddWithOptions(&AddOptions{All:true}) now behaves correctly. I'm not sure if my test case is related to the original issue, so maybe I should create a separate issue for it?

[shibumi]

Short note: If you are just looking for a gitignore implementation that behaves exactly like the git one, have a look on my go-pathspec package: https://github.com/shibumi/go-pathspec

It's a fork of the old go-pathspec package. I am cleaning it up right now and it fulfills my desired test cases

[circle-shubham]

We have also run into this issue, wondering if we have workaround already?

[AriehSchneier]

@pjbgf not sure when it was fixed, but I'm pretty sure this works correctly now

[pjbgf]

@AriehSchneier thanks for the heads-up. I ran the test provided by @mrpotes above and it worked fine, so I will be closing this issue.