-
-
Notifications
You must be signed in to change notification settings - Fork 770
Implements running from parent directory #298 #408
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
Conversation
Uses the absolute path to make sure we can move up. It's not smart yet, but it works.
|
Ok, this will not work on windows... I will check this later. |
| dirFragments := strings.Split(dir, string(os.PathSeparator)) | ||
| dir = "/" + strings.Join(dirFragments[1:len(dirFragments)-1], string(os.PathSeparator)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not using instead:
| dirFragments := strings.Split(dir, string(os.PathSeparator)) | |
| dir = "/" + strings.Join(dirFragments[1:len(dirFragments)-1], string(os.PathSeparator)) | |
| dir, _ := path.Split(dir) |
|
Great feature ! |
|
Yeah big +1 on this change! |
ghostsquad
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This behavior can actually be quite dangerous. I would recommend against implementation of this without further discussion of risk. Additionally, this change is not backwards compatible, right now an "error" is produced if you run task from a directory that does not contain a taskfile. As such, this feature should be hidden behind a feature flag in the CLI (with a default enabled: false).
Git CVE-2022-24765 describes the risk actually quite well:
https://github.blog/2022-04-12-git-security-vulnerability-announced/
| can be installed got `go get` given that you have a recent version of [Go][go]. | ||
|
|
||
| ```bash | ||
| go get -u github.com/go-task/task/cmd/task |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can no longer install task with go get as of 1.18.
https://tip.golang.org/doc/go1.18#introduction
This document should reflect the use of go install
| dir = "/" + strings.Join(dirFragments[1:len(dirFragments)-1], string(os.PathSeparator)) | ||
|
|
||
| } else { | ||
| found = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
technically, you don't even need found here, you can just break.
|
Closing in favor of #920 |
Uses the absolute path to make sure we can move up.
It's not smart yet, but it works.
so it could fix #289
Also resolves #395 since it was so simple to change.