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

Extended globbing support #225

Closed
Wicketd opened this issue Jun 29, 2019 · 23 comments · Fixed by #1324
Closed

Extended globbing support #225

Wicketd opened this issue Jun 29, 2019 · 23 comments · Fixed by #1324
Labels
type: enhancement A change to an existing feature or functionality. type: feature A new feature or functionality.

Comments

@Wicketd
Copy link

Wicketd commented Jun 29, 2019

Task version OS
master macOS Mojave 10.14.5

Using extglob patterns in sources entries makes Task panic. I tried using this to exclude some files from being picked up.


Given the following Taskfile.yml:

version: '2'

tasks:
  foo:
    cmds:
      - printf %s Test
    sources:
      - ./!(vendor)/**/*.go
    method: checksum

Task panics after running task foo:

panic: unhandled word part: *syntax.ExtGlob

goroutine 1 [running]:
github.com/go-task/task/vendor/mvdan.cc/sh/expand.(*Config).wordFields(0xc00019e140, 0xc00015a540, 0x2, 0x2, 0x1, 0x1, 0x203000, 0x0, 0xc0001a0010)
    /Users/thierry/go/src/github.com/go-task/task/vendor/mvdan.cc/sh/expand/expand.go:546 +0x1292
github.com/go-task/task/vendor/mvdan.cc/sh/expand.Fields(0xc00019e140, 0xc0001a0010, 0x1, 0x1, 0x0, 0x0, 0x1, 0x0, 0xc000194020)
    /Users/thierry/go/src/github.com/go-task/task/vendor/mvdan.cc/sh/expand/expand.go:323 +0x19a
github.com/go-task/task/vendor/mvdan.cc/sh/shell.Fields(0xc0001940a0, 0x11, 0x0, 0x1, 0x13852c8, 0x2, 0xffffffffffffffff, 0xc0001940a0)
    /Users/thierry/go/src/github.com/go-task/task/vendor/mvdan.cc/sh/shell/expand.go:62 +0x162
github.com/go-task/task/internal/execext.Expand(0xc0001940a0, 0x11, 0x2, 0xc0001940a0, 0x11, 0x10e1548)
    /Users/thierry/go/src/github.com/go-task/task/internal/execext/exec.go:78 +0x8e
github.com/go-task/task/internal/status.glob(0x0, 0x0, 0xc000146010, 0x1, 0x1, 0x21, 0x0, 0x2341460, 0x0, 0x0)
    /Users/thierry/go/src/github.com/go-task/task/internal/status/glob.go:17 +0xca
github.com/go-task/task/internal/status.(*Checksum).IsUpToDate(0xc000144280, 0xc00015c000, 0x13f5520, 0xc000144280)
    /Users/thierry/go/src/github.com/go-task/task/internal/status/checksum.go:30 +0x10b
github.com/go-task/task.(*Executor).isTaskUpToDate(0xc0000b34a0, 0x13f9a20, 0xc000144000, 0xc00015c000, 0x1, 0x0, 0x0)
    /Users/thierry/go/src/github.com/go-task/task/status.go:40 +0x7f
github.com/go-task/task.(*Executor).RunTask(0xc0000b34a0, 0x13f9a20, 0xc000144000, 0x7ffeefbff38d, 0x3, 0x0, 0xc0000a25a0, 0x13f5d20)
    /Users/thierry/go/src/github.com/go-task/task/task.go:218 +0x60f
github.com/go-task/task.(*Executor).Run(0xc0000b34a0, 0x13f9a20, 0xc000144000, 0xc000090ca0, 0x1, 0x1, 0x0, 0x1394802)
    /Users/thierry/go/src/github.com/go-task/task/task.go:79 +0x139
main.main()
    /Users/thierry/go/src/github.com/go-task/task/cmd/task/task.go:141 +0xa8d
@andreynering andreynering added the type: bug Something not working as intended. label Jun 30, 2019
@andreynering
Copy link
Member

Hi @Yottum,

I think this just isn't supported by https://github.com/mattn/go-zglob.

Can you check if you glob use cases work on Gosh?

You can do that by installing Gosh and running:

gosh -c "echo ./!(vendor)/**/*.go"

Or by running it in Taskfile like this:

tasks:
  glob:
    - echo ./!(vendor)/**/*.go

If it works, we could maybe replace zglob with Gosh for globbing in Task.

@andreynering
Copy link
Member

Ping @Yottum

@Wicketd
Copy link
Author

Wicketd commented Jul 14, 2019

Woops, sorry! Had totally forgotten I opened this issue. Thanks for replying.

No avail, sadly:

$ gosh -c 'echo ./!(vendor)/**/*.go'
panic: unhandled word part: *syntax.ExtGlob

goroutine 1 [running]:
mvdan.cc/sh/v3/expand.(*Config).wordFields(0xc0000f2000, 0xc0000bc100, 0x3, 0x4, 0xc0000c4028, 0x4, 0x0, 0x0, 0x0)
	/Users/thierry/go/pkg/mod/mvdan.cc/sh/v3@v3.0.0-alpha2/expand/expand.go:584 +0x1292
mvdan.cc/sh/v3/expand.Fields(0xc0000f2000, 0xc0000daab0, 0x2, 0x4, 0x20300000000000, 0x163ffff, 0x400, 0x1447900, 0x1115b38)
	/Users/thierry/go/pkg/mod/mvdan.cc/sh/v3@v3.0.0-alpha2/expand/expand.go:373 +0x15b
mvdan.cc/sh/v3/interp.(*Runner).fields(0xc0000dc000, 0xc0000daab0, 0x2, 0x4, 0x163ffff, 0x400, 0x20300000000000)
	/Users/thierry/go/pkg/mod/mvdan.cc/sh/v3@v3.0.0-alpha2/interp/interp.go:131 +0x5a
mvdan.cc/sh/v3/interp.(*Runner).cmd(0xc0000dc000, 0x11a3a60, 0xc0000c4000, 0x11a31a0, 0xc0000daa80)
	/Users/thierry/go/pkg/mod/mvdan.cc/sh/v3@v3.0.0-alpha2/interp/interp.go:782 +0x1114
mvdan.cc/sh/v3/interp.(*Runner).stmtSync(0xc0000dc000, 0x11a3a60, 0xc0000c4000, 0xc0000e2000)
	/Users/thierry/go/pkg/mod/mvdan.cc/sh/v3@v3.0.0-alpha2/interp/interp.go:721 +0x2d9
mvdan.cc/sh/v3/interp.(*Runner).stmt(0xc0000dc000, 0x11a3a60, 0xc0000c4000, 0xc0000e2000)
	/Users/thierry/go/pkg/mod/mvdan.cc/sh/v3@v3.0.0-alpha2/interp/interp.go:702 +0x1ce
mvdan.cc/sh/v3/interp.(*Runner).stmts(0xc0000dc000, 0x11a3a60, 0xc0000c4000, 0xc0000ee000, 0x1, 0x4)
	/Users/thierry/go/pkg/mod/mvdan.cc/sh/v3@v3.0.0-alpha2/interp/interp.go:1037 +0x56
mvdan.cc/sh/v3/interp.(*Runner).Run(0xc0000dc000, 0x11a3a60, 0xc0000c4000, 0x11a1e20, 0xc0000bc0c0, 0xc0000bc0c0, 0x0)
	/Users/thierry/go/pkg/mod/mvdan.cc/sh/v3@v3.0.0-alpha2/interp/interp.go:650 +0x172
main.run(0x11a1920, 0xc0000de060, 0x0, 0x0, 0x0, 0xc000044f28)
	/Users/thierry/go/pkg/mod/mvdan.cc/sh/v3@v3.0.0-alpha2/cmd/gosh/main.go:76 +0xd8
main.runAll(0xc0000b00c0, 0xc0000c0010)
	/Users/thierry/go/pkg/mod/mvdan.cc/sh/v3@v3.0.0-alpha2/cmd/gosh/main.go:44 +0x200
main.main()
	/Users/thierry/go/pkg/mod/mvdan.cc/sh/v3@v3.0.0-alpha2/cmd/gosh/main.go:30 +0x7c

@leefernandes
Copy link

I'm not seeing many go implementations that support negation syntax.

https://github.com/gobwas/glob doesn't directly implement filesystem search.

https://godoc.org/gopkg.in/godo.v2/glob appears to support negation, but is a bit stale.

@andreynering
Copy link
Member

I just opened an issue on the glob lib we currently use: mattn/go-zglob#26.

I wouldn't expect that to be implemented soon, though.

@andreynering andreynering added type: enhancement A change to an existing feature or functionality. type: feature A new feature or functionality. and removed type: bug Something not working as intended. labels Sep 7, 2019
@leefernandes
Copy link

also, perhaps another approach is an explicit exclusion glob similar to CompileDaemon

https://github.com/githubnemo/CompileDaemon/blob/master/daemon.go#L45

@andreynering
Copy link
Member

also, perhaps another approach is an explicit exclusion glob similar to CompileDaemon

https://github.com/githubnemo/CompileDaemon/blob/master/daemon.go#L45

This also sounds like a good idea

@lunny
Copy link

lunny commented Mar 20, 2020

See #309

@marco-m
Copy link
Contributor

marco-m commented Apr 1, 2021

I just stumbled upon https://github.com/bmatcuk/doublestar that supports negation of character classes.

@marco-m
Copy link
Contributor

marco-m commented Aug 13, 2021

Update: mattn/go-zglob#26 (Add glob negation support) has just been implemented!

@Porges
Copy link
Contributor

Porges commented Aug 17, 2021

@andreynering could we get a new release with updated zglob? I’d love to use this.

@pd93
Copy link
Member

pd93 commented Apr 6, 2022

Dug into this a little bit as I also wanted to exclude the vendor directory from a pattern. However, it looks like Taskfile is already using a version of zglob that supports negating globs.

The stacktrace I get (which is the same as OP) is actually pointing to an error caused by mvdan/sh. It looks like this issue is what we need for this to work in Taskfile.

@ghostsquad
Copy link
Contributor

I'm working on this in the V4 Discussion #694 by turning the sources to act more or less like an ignore file, such that every item in the list is "combined" and "and'd" together. This would allow you to write:

/**/*.go
!/vendor

@ghostsquad
Copy link
Contributor

Dug into this a little bit as I also wanted to exclude the vendor directory from a pattern. However, it looks like Taskfile is already using a version of zglob that supports negating globs.

The stacktrace I get (which is the same as OP) is actually pointing to an error caused by mvdan/sh. It looks like this issue is what we need for this to work in Taskfile.

I'm a bit curious how sh comes into play for sources in task. I haven't yet looked at the code path for this yet. This is one thing that I definitely would not shell out to handle. This can be handled natively in Go.

@ghostsquad
Copy link
Contributor

I'm working on this in the V4 Discussion #694 by turning the sources to act more or less like an ignore file, such that every item in the list is "combined" and "and'd" together. This would allow you to write:

/**/*.go
!/vendor

Oh, I wanted to add one more thing, that I was also thinking about adding support to automatically read in a .gitignore file, and use that as an baseline for what files are applicable for matching. This would be an inline feature flag that can be disabled.

@kirb
Copy link

kirb commented Nov 14, 2022

FYI the negation feature just got tagged: https://github.com/mattn/go-zglob/releases/tag/v0.0.4

@aminya
Copy link
Contributor

aminya commented Nov 16, 2022

#924

the globbing library that task uses fails to check some patterns.

The following checks the files inside the dev folder

"{dev/**/*,something}"

But this doesn't!

"{something,dev/**/*}"

Or for example, ** doesn't check all the files of the current project.

@aminya
Copy link
Contributor

aminya commented Jan 2, 2023

Another issue with the go-zglob library is that it is very slow. It still tries to read the excluded directory using the fastwalk function.

    sources:
      ["node_modules/**/*", "**/*.js"]

I have done some profiling on my fork, and here is the result. As you see, it reads the whole node_modules directory, which takes 70 seconds.

image

image

@krystian-panek-vmltech
Copy link

I really need this negation support in my beloved tool "task" ;)

I have some generated sources under some directory that usually holds only regular sources and I need to exclude them without listing all dirs separately as sources to cover that. I know that it is ugly but currently, the only way to make build caching work. To do it elegantly I need to have it implemented in Task for sure. Unfortunately, I am unable to reconfigure the build to move these generated sources somewhere else.

Could we somehow push this feature forward? May I help somehow?

@thenbe
Copy link

thenbe commented Aug 1, 2023

One suggestion would be to support a shell command that produces the necessary watched paths. Then, users can use a tool like fd to implement whatever watched logic they desire.

tasks:
  foo:
    sources_command: "fd -lp --exclude='**/*.cache.*' --glob '**/packages/{site,backend}/**/*.{go,js,ts,json}'"

I'm using fd as an example because it:

  • supports deterministic sort
  • very fast
  • powerful matching capabilities (globs, regex, negation, etc)
  • very actively maintained (to be fair towards go-zglob, there are no outlying bug reports. But it only has a single code commit/new feature during the past few years)

@krystian-panek-vmltech
Copy link

krystian-panek-vmltech commented Sep 7, 2023

@thenbe I like the idea because currently as far as I investigated there is no nice Go library to support exclusions with the glob patterns that we use currently (without introducing backward incompatible change). Your extension to the task tool will be backward-compatible. However a little bit annoying then will be a need to provide fd binary for each host machine manually. If the task tool could embed fd somehow as well then that other problem will be gone.

I am using the task tool intensively and adopting it in my company however the lack of exclusions in sources is really annoying. Instead of having a single source pattern with one negation, I need to list many paths separately (to avoid treat generated files as sources) which is very error-prone as these path structures may change/are under active development.

If I could somehow help in pushing that topic forward, please let me know. I could contribute but at least I need to be sure which kind of improvements will be accepted and merged into the tool.

@andreynering, @pd93 maybe?

@pd93
Copy link
Member

pd93 commented Sep 7, 2023

@krystian-panek-wttech I think it's very unlikely that Task will ever embed another binary. If you need the functionality of fd, then you should add a precondition to you Taskfile that checks if it is installed and ask users to go and install it. Also fd is written in Rust and so there is no way for us to import its code either.

With that said, this issue has been bothering me for a long time. zglob supposedly added support for negation a while ago, but I've never been able to get it to work. Also, the syntax for it isn't great. I've opened a PR instead that will add native support for negating patterns in Task. This seems like a better solution to me. Please feel free to check it out and give it a try. Feedback always welcome.

@krystian-panek-vmltech
Copy link

krystian-panek-vmltech commented Sep 7, 2023

I will give it a try and let you know.

the implementation looks good enough / promising. thank you very much for your time! ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A change to an existing feature or functionality. type: feature A new feature or functionality.
Projects
None yet
Development

Successfully merging a pull request may close this issue.