[Bug] [Eyes] Ignore pattern doesn't work for cmake-build-* when it is intended to be a folder

[tisonkun]

Search before asking

• I had searched in the issues and found no similar issues.

Apache SkyWalking Component

License Tools (apache/skywalking-eyes)

What happened

If a pattern with * is added to paths or paths-ignore when it's intended to be a folder, it doesn't work as expected. Because doublestar module only treat patterns end with / as folder.

For example, specifying cmake-build-* doesn't exclude cmake-build-debug folder.

➜  incubator-kvrocks git: ~/go/bin/license-eye -c tools/ci/licenserc.yml header check
INFO Loading configuration from file: tools/ci/licenserc.yml 
INFO Totally checked 4279 files, valid: 212, invalid: 2045, ignored: 2022, fixed: 0 
INFO GITHUB_TOKEN is not set, license-eye won't comment on the pull request 
ERROR the following files don't have a valid license header: 
cmake-build-debug/_deps/gtest-src/googletest/scripts/test/Makefile
cmake-build-debug/_deps/luajit-src/src/Makefile
...
cmake-build-debug/_deps/lz4-src/contrib/snap/snapcraft.yaml 
ERROR one or more files does not have a valid license header

What you expected to happen

Files are properly included or excluded.

How to reproduce

go install github.com/apache/skywalking-eyes/cmd/[email protected]
gh repo clone apache/incubator-kvrocks
cd incubator-kvrocks
./x.py build --ninja cmake-build-debug
~/go/bin/license-eye -c tools/ci/licenserc.yml header check

Anything else

I think these lines of code are related:

	for _, ignorePattern := range config.PathsIgnore {
		if matched, err := doublestar.Match(ignorePattern, path); matched || err != nil {
			return matched, err
		}
	}

	if stat, err := os.Stat(path); err == nil {
		for _, ignorePattern := range config.PathsIgnore {
			ignorePattern = strings.TrimRight(ignorePattern, "/")
			if strings.HasPrefix(path, ignorePattern+"/") || stat.Name() == ignorePattern {
				return true, nil
			}
		}
	}

	return false, nil

It seems that if the pattern is intended to be a folder while with *, the stat workaround has a hole. We may write:

doublestar.Match(ignorePattern + "/", path)

instead. But it may has other corner cases. I don't think of it deeply, though.

cc @kezhenxu94 @fgksgf

Are you willing to submit PR?

• Yes I am willing to submit a PR!

Code of Conduct

• I agree to follow this project's Code of Conduct

[tisonkun]

It's fair enough that we just follow how glob works and if users want to refer to a folder, use folder/ and folder/**. But the workaround that exists is a bit inconsistent with this assumption. I'm willing to learn some design purposes here.

More related code snippet for paths matches:

func glob(pattern string) (matches []string, err error) {
	if pattern == "." {
		return doublestar.Glob("./")
	}

	return doublestar.Glob(pattern)
}

[wu-sheng]

Is there any update for this?

[tisonkun]

@wu-sheng This is more like a new feature suggestion that I don't have time to implement :)

I'm OK if it's closed as workaround provided:

cmake-build-*/**

[wu-sheng]

I am going to close until someone is willing pick it back to work.

[tisonkun]

For anyone who comes here, I implement this logic for my own tool korandoru/hawkeye#65 :)