You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2022/09/04 15:53:51 UTC
[GitHub] [skywalking] tisonkun opened a new issue, #9560: [Bug] [Eyes] Ignore pattern doesn't work for `cmake-build-* ` when it is intended to be a folder
tisonkun opened a new issue, #9560:
URL: https://github.com/apache/skywalking/issues/9560
### Search before asking
- [X] I had searched in the [issues](https://github.com/apache/skywalking/issues?q=is%3Aissue) 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/license-eye@v0.4.0
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](https://github.com/apache/skywalking-eyes/blob/c27b7b3f296863003e8a10f44a6731c43c2faffa/pkg/header/config.go#L119-L134) are related:
```go
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:
```go
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
- [X] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng closed issue #9560: [Bug] [Eyes] Ignore pattern doesn't work for `cmake-build-* ` when it is intended to be a folder
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng closed issue #9560: [Bug] [Eyes] Ignore pattern doesn't work for `cmake-build-* ` when it is intended to be a folder
URL: https://github.com/apache/skywalking/issues/9560
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] tisonkun commented on issue #9560: [Bug] [Eyes] Ignore pattern doesn't work for `cmake-build-* ` when it is intended to be a folder
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on issue #9560:
URL: https://github.com/apache/skywalking/issues/9560#issuecomment-1469249446
@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-*/**
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on issue #9560: [Bug] [Eyes] Ignore pattern doesn't work for `cmake-build-* ` when it is intended to be a folder
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on issue #9560:
URL: https://github.com/apache/skywalking/issues/9560#issuecomment-1469250255
I am going to close until someone is willing pick it back to work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] tisonkun commented on issue #9560: [Bug] [Eyes] Ignore pattern doesn't work for `cmake-build-* ` when it is intended to be a folder
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on issue #9560:
URL: https://github.com/apache/skywalking/issues/9560#issuecomment-1492977566
For anyone who comes here, I implement this logic for my own tool https://github.com/korandoru/hawkeye/pull/65 :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] tisonkun commented on issue #9560: [Bug] [Eyes] Ignore pattern doesn't work for `cmake-build-* ` when it is intended to be a folder
Posted by GitBox <gi...@apache.org>.
tisonkun commented on issue #9560:
URL: https://github.com/apache/skywalking/issues/9560#issuecomment-1236370828
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on issue #9560: [Bug] [Eyes] Ignore pattern doesn't work for `cmake-build-* ` when it is intended to be a folder
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on issue #9560:
URL: https://github.com/apache/skywalking/issues/9560#issuecomment-1469247487
Is there any update for this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org